Re: [PATCH 1/7] member: implement main API

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 8/24/2017 10:55 AM, Luca Boccassi wrote:
> On Thu, 2017-08-24 at 10:35 +0100, Ferruh Yigit wrote:
>> On 8/22/2017 11:02 AM, Luca Boccassi wrote:
>>> On Mon, 2017-08-21 at 17:19 -0700, Yipeng Wang wrote:
>>>> Membership library is an extension and generalization of a
>>>> traditional
>>>> filter (for example Bloom Filter) structure. In general, the
>>>> Membership
>>>> library is a data structure that provides a "set-summary" and
>>>> responds
>>>> to set-membership queries of whether a certain element belongs to
>>>> a
>>>> set(s). A membership test for an element will return the set this
>>>> element
>>>> belongs to or not-found if the element is never inserted into the
>>>> set-summary.
>>>>
>>>> The results of the membership test is not 100% accurate. Certain
>>>> false positive or false negative probability could exist.
>>>> However,
>>>> comparing to a "full-blown" complete list of elements, a "set-
>>>> summary"
>>>> is memory efficient and fast on lookup.
>>>>
>>>> This patch add the main API definition.
>>>>
>>>> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
>>>> ---
>>>>  lib/Makefile                             |   2 +
>>>>  lib/librte_eal/common/eal_common_log.c   |   1 +
>>>>  lib/librte_eal/common/include/rte_log.h  |   1 +
>>>>  lib/librte_member/Makefile               |  48 +++
>>>>  lib/librte_member/rte_member.c           | 357
>>>> +++++++++++++++++++++
>>>>  lib/librte_member/rte_member.h           | 518
>>>> +++++++++++++++++++++++++++++++
>>>>  lib/librte_member/rte_member_version.map |  15 +
>>>>  7 files changed, 942 insertions(+)
>>>>  create mode 100644 lib/librte_member/Makefile
>>>>  create mode 100644 lib/librte_member/rte_member.c
>>>>  create mode 100644 lib/librte_member/rte_member.h
>>>>  create mode 100644 lib/librte_member/rte_member_version.map
>>>>
>>>> diff --git a/lib/librte_member/Makefile
>>>> b/lib/librte_member/Makefile
>>>> new file mode 100644
>>>> index 0000000..997c825
>>>> --- /dev/null
>>>> +++ b/lib/librte_member/Makefile
>>>> @@ -0,0 +1,48 @@
>>>> +#   BSD LICENSE
>>>> +#
>>>> +#   Copyright(c) 2017 Intel Corporation. All rights reserved.
>>>> +#   All rights reserved.
>>>> +#
>>>> +#   Redistribution and use in source and binary forms, with or
>>>> without
>>>> +#   modification, are permitted provided that the following
>>>> conditions
>>>> +#   are met:
>>>> +#
>>>> +#     * Redistributions of source code must retain the above
>>>> copyright
>>>> +#       notice, this list of conditions and the following
>>>> disclaimer.
>>>> +#     * Redistributions in binary form must reproduce the above
>>>> copyright
>>>> +#       notice, this list of conditions and the following
>>>> disclaimer
>>>> in
>>>> +#       the documentation and/or other materials provided with
>>>> the
>>>> +#       distribution.
>>>> +#     * Neither the name of Intel Corporation nor the names of
>>>> its
>>>> +#       contributors may be used to endorse or promote products
>>>> derived
>>>> +#       from this software without specific prior written
>>>> permission.
>>>> +#
>>>> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
>>>> CONTRIBUTORS
>>>> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING,
>>>> BUT
>>>> NOT
>>>> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
>>>> FITNESS FOR
>>>> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
>>>> COPYRIGHT
>>>> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>>>> INCIDENTAL,
>>>> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
>>>> NOT
>>>> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
>>>> LOSS OF
>>>> USE,
>>>> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
>>>> AND
>>>> ON ANY
>>>> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
>>>> OR
>>>> TORT
>>>> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
>>>> OF
>>>> THE USE
>>>> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>>>> DAMAGE.
>>>> +
>>>> +include $(RTE_SDK)/mk/rte.vars.mk
>>>> +
>>>> +# library name
>>>> +LIB = librte_member.a
>>>> +
>>>> +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
>>>> +
>>>
>>> This breaks reproducibility as the output directory will be
>>> included
>>> before the source directory, causing a race - please do something
>>> like:
>>>
>>> CFLAGS := -I$(SRCDIR) $(CFLAGS)
>>> CFLAGS += $(WERROR_FLAGS) -O3
>>
>> Can we remove "-I$(SRCDIR)" completely by first installing headers
>> and
>> later compiling objects, all using $(RTE_OUT) only?
>>
>> Do you think can this work?
> 
> I'm not sure, it might - but given Bruce's effort to port to Meson I'm
> not sure it's worth spending a lot of time doing big refactoring of the
> existing build system

Not big refactoring, following seems worked for me, if you would like to
test:

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index 13115d146..643da47da 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -59,14 +59,19 @@ endif


 _BUILD = $(LIB)
-_INSTALL = $(INSTALL-FILES-y) $(SYMLINK-FILES-y) $(RTE_OUTPUT)/lib/$(LIB)
+PREINSTALL = $(SYMLINK-FILES-y)
+_INSTALL = $(INSTALL-FILES-y) $(RTE_OUTPUT)/lib/$(LIB)
 _CLEAN = doclean

 .PHONY: all
 all: install

 .PHONY: install
+ifeq ($(SYMLINK-FILES-y),)
 install: build _postinstall
+else
+install: _preinstall build _postinstall
+endif

 _postinstall: build


> 
>>>> +EXPORT_MAP := rte_member_version.map
>>>> +
>>>> +LIBABIVER := 1
>>>> +
>>>> +# all source are stored in SRCS-y
>>>> +SRCS-$(CONFIG_RTE_LIBRTE_MEMBER) +=  rte_member.c
>>>> +# install includes
>>>> +SYMLINK-$(CONFIG_RTE_LIBRTE_MEMBER)-include := rte_member.h
>>>> +
>>>> +include $(RTE_SDK)/mk/rte.lib.mk
>>
>>
>