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

[Luca Boccassi <luca.boccassi@gmail.com>]

On Thu, 2017-08-24 at 11:32 +0100, Ferruh Yigit wrote:
> 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

Easier than I would have thought :-)

I'll give that patch a spin through the repro build grinder next week,
thanks.

-- 
Kind regards,
Luca Boccassi