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

[Luca Boccassi]

On Sat, 2017-09-02 at 13:55 +0100, Luca Boccassi wrote:
> 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 at 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.

Thanks Ferruh, this works in making the build reproducible with the
current CI as an alternative with a fixed build path.

But on the other hand, it will cause the compiler to always embed the
custom build directory in the path to the header in the DWARF .debug
files, rather than the "repository" path:

readelf --debug-dump=line /tmp/69c02db5050024b530a14555b49528c419419f.debug

<cut>
 The Directory Table (offset 0x1b):
  1     dpdk/lib/librte_eal/linuxapp/eal
  2     /usr/include/x86_64-linux-gnu/bits
  3     dpdk/lib/librte_eal/common
  4     dpdk/debian/build/shared-root/include
  5     dpdk/debian/build/shared-root/include/generic
  6     /usr/lib/gcc/x86_64-linux-gnu/7/include
  7     /usr/include/x86_64-linux-gnu/sys
  8     /usr/include

 The File Name Table (offset 0x15a):
  Entry Dir     Time    Size    Name
  1     1       0       0       eal.c
  2     2       0       0       stdio2.h
  3     3       0       0       eal_filesystem.h
  4     2       0       0       fcntl2.h
  5     4       0       0       rte_atomic.h
  6     5       0       0       rte_atomic.h
  7     2       0       0       string3.h
  8     4       0       0       rte_cycles.h
  9     4       0       0       rte_random.h
  10    6       0       0       xmmintrin.h
  11    4       0       0       rte_eal_memconfig.h
  12    4       0       0       rte_lcore.h

<cut>

note 4 and 5 items in the directory tables, they point to debian/build-
shared-root which is the output directory we use in the package build. 

This means that 2 builds that are otherwise exactly the same (same
machine type, same build dependencies) will not be binary identical due
to that difference.

On one hand it's a much smaller and nicer patch and there's the Meson
work which will make both obsolete, on the other hand it's not very
reproducibl-y. What do you think?

-- 
Kind regards,
Luca Boccassi