RE: [PATCH v3 1/2] member: implement NitroSketch mode

["Rong, Leyi" <leyi.rong@intel.com>]

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, September 15, 2022 3:47 PM
> To: Rong, Leyi <leyi.rong@intel.com>
> Cc: ferruh.yigit@xilinx.com; suanmingm@nvidia.com; Wang, Yipeng1
> <yipeng1.wang@intel.com>; zaoxingliu@gmail.com; Gobriel, Sameh
> <sameh.gobriel@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH v3 1/2] member: implement NitroSketch mode
> 
> On Thu, Sep 15, 2022 at 4:15 AM Leyi Rong <leyi.rong@intel.com> wrote:
> >
> > Sketching algorithm provide high-fidelity approximate measurements and
> > appears as a promising alternative to traditional approaches such as
> > packet sampling.
> >
> > NitroSketch [1] is a software sketching framework that optimizes
> > performance, provides accuracy guarantees, and supports a variety of
> > sketches.
> >
> > This commit adds a new data structure called sketch into membership
> > library. This new data structure is an efficient way to profile the
> > traffic for heavy hitters. Also use min-heap structure to maintain the
> > top-k flow keys.
> >
> > [1] Zaoxing Liu, Ran Ben-Basat, Gil Einziger, Yaron Kassner, Vladimir
> > Braverman, Roy Friedman, Vyas Sekar, "NitroSketch: Robust and General
> > Sketch-based Monitoring in Software Switches", in ACM SIGCOMM 2019.
> > https://dl.acm.org/doi/pdf/10.1145/3341302.3342076
> >
> > Signed-off-by: Alan Liu <zaoxingliu@gmail.com>
> > Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> > Signed-off-by: Leyi Rong <leyi.rong@intel.com>
> > ---
> >  lib/member/meson.build                |  42 +-
> >  lib/member/rte_member.c               |  75 ++++
> >  lib/member/rte_member.h               | 154 ++++++-
> >  lib/member/rte_member_heap.h          | 424 ++++++++++++++++++
> >  lib/member/rte_member_sketch.c        | 594 ++++++++++++++++++++++++++
> >  lib/member/rte_member_sketch.h        |  97 +++++
> >  lib/member/rte_member_sketch_avx512.c |  69 +++
> > lib/member/rte_member_sketch_avx512.h |  36 ++
> >  lib/member/rte_xxh64_avx512.h         | 117 +++++
> >  lib/member/version.map                |   9 +
> >  10 files changed, 1613 insertions(+), 4 deletions(-)  create mode
> > 100644 lib/member/rte_member_heap.h  create mode 100644
> > lib/member/rte_member_sketch.c  create mode 100644
> > lib/member/rte_member_sketch.h  create mode 100644
> > lib/member/rte_member_sketch_avx512.c
> >  create mode 100644 lib/member/rte_member_sketch_avx512.h
> >  create mode 100644 lib/member/rte_xxh64_avx512.h
> >
> > diff --git a/lib/member/meson.build b/lib/member/meson.build index
> > e06fddc240..8de0c09a6a 100644
> > --- a/lib/member/meson.build
> > +++ b/lib/member/meson.build
> > @@ -7,6 +7,46 @@ if is_windows
> >      subdir_done()
> >  endif
> >
> > -sources = files('rte_member.c', 'rte_member_ht.c',
> > 'rte_member_vbf.c')
> > +sources = files('rte_member.c', 'rte_member_ht.c',
> > +'rte_member_vbf.c', 'rte_member_sketch.c')
> >  headers = files('rte_member.h')
> >  deps += ['hash']
> > +includes += include_directories('../hash', '../ring')
> > +
> > +# compile AVX512 version if:
> > +# we are building 64-bit binary AND binutils can generate proper code
> > +if dpdk_conf.has('RTE_ARCH_X86_64') and binutils_ok
> > +    # compile AVX512 version if either:
> > +    # a. we have AVX512 supported in minimum instruction set
> > +    #    baseline
> > +    # b. it's not minimum instruction set, but supported by
> > +    #    compiler
> > +    #
> > +    # in former case, just add avx512 C file to files list
> > +    # in latter case, compile c file to static lib, using correct
> > +    # compiler flags, and then have the .o file from static lib
> > +    # linked into main lib.
> > +
> > +    #check if all required flags already enabled
> > +    sketch_avx512_flags = ['__AVX512F__', '__AVX512DQ__',
> > + '__AVX512IFMA__']
> > +
> > +    sketch_avx512_on = true
> > +    foreach f:sketch_avx512_flags
> > +        if cc.get_define(f, args: machine_args) == ''
> > +            sketch_avx512_on = false
> > +        endif
> > +    endforeach
> > +
> > +    if sketch_avx512_on == true
> > +       cflags += ['-DCC_AVX512_SUPPORT']
> > +       sources += files('rte_member_sketch_avx512.c')
> > +    elif cc.has_multi_arguments('-mavx512f', '-mavx512dq', '-mavx512ifma')
> > +       cflags += ['-DCC_AVX512_SUPPORT']
> > +       cflags += ['-mavx512f', '-mavx512dq', '-mavx512ifma']
> 
> No.
> Again, you can't push AVX512 flags to the variable cflags.
> 
> On my laptop, running with Fedora 36:
> 
> $ DPDK_TEST=member_autotest ./build-gcc/app/test/dpdk-test --no-huge -m
> 2048
> EAL: Detected CPU lcores: 12
> EAL: Detected NUMA nodes: 1
> EAL: Detected shared linkage of DPDK
> EAL: Multi-process socket /run/user/1000/dpdk/rte/mp_socket
> EAL: Selected IOVA mode 'VA'
> APP: HPET is not enabled, using TSC as default timer
> RTE>>member_autotest
> Expected error section begin...
> rte_member_create_vbf(): Membership vBF create with invalid parameters
> rte_member_create_vbf(): Membership vBF create with invalid parameters
> rte_member_create_vbf(): Membership vBF create with invalid parameters
> rte_member_create_ht(): Membership HT create with invalid parameters
> rte_member_create_ht(): Membership HT create with invalid parameters
> rte_member_create_ht(): Membership HT create with invalid parameters
> Expected error section end...
> rte_member_create_ht(): Hash table based filter created, the table has
> 65536 entries, 4096 buckets
> rte_member_create(): Creating a setsummary table with mode 0
> rte_member_create_ht(): Hash table based filter created, the table has
> 65536 entries, 4096 buckets
> rte_member_create(): Creating a setsummary table with mode 0
> rte_member_create_ht(): Hash table based filter created, the table has
> 65536 entries, 4096 buckets
> rte_member_create(): Creating a setsummary table with mode 0 Illegal
> instruction (core dumped)
> 
> The reason is the same as I reported earlier, and this can be reproduced like this:
> 
> $ touch lib/member/rte_member_sketch*
> $ ninja -C build-gcc -vv
> ninja: Entering directory `build-gcc'
> [1/9] ccache gcc -Ilib/member/libsketch_avx512_tmp.a.p -Ilib/member -
> I../lib/member -Ilib/hash -I../lib/hash -Ilib/ring -I../lib/ring -I.
> -I.. -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -
> I../lib/eal/linux/include -Ilib/eal/x86/include -I../lib/eal/x86/include -
> Ilib/eal/common -I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -
> I../lib/kvargs -Ilib/metrics -I../lib/metrics -Ilib/telemetry -I../lib/telemetry -
> fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -
> Wextra -Werror -O3 -g -include rte_config.h -Wcast-qual -Wdeprecated -
> Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -
> Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -
> Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-
> packed-member -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-
> zero-length-bounds -D_GNU_SOURCE -fPIC -march=native -
> DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-
> truncation -DCC_AVX512_SUPPORT -mavx512f -mavx512dq -mavx512ifma -MD
> -MQ lib/member/libsketch_avx512_tmp.a.p/rte_member_sketch_avx512.c.o -
> MF lib/member/libsketch_avx512_tmp.a.p/rte_member_sketch_avx512.c.o.d -o
> lib/member/libsketch_avx512_tmp.a.p/rte_member_sketch_avx512.c.o -
> c ../lib/member/rte_member_sketch_avx512.c
> [2/9] ccache gcc -Ilib/librte_member.a.p -Ilib -I../lib -Ilib/hash -I../lib/hash -
> Ilib/ring -I../lib/ring -Ilib/member -I../lib/member -I. -I.. -Iconfig -I../config -
> Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include
> -Ilib/eal/x86/include -I../lib/eal/x86/include -Ilib/eal/common -
> I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/metrics -
> I../lib/metrics -Ilib/telemetry -I../lib/telemetry -Ilib/net -I../lib/net -Ilib/mbuf -
> I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/rcu -I../lib/rcu -fdiagnostics-
> color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -
> O3 -g -include rte_config.h -Wcast-qual -Wdeprecated -Wformat -Wformat-
> nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -
> Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -
> Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member
> -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-zero-length-
> bounds -D_GNU_SOURCE -fPIC -march=native -DALLOW_EXPERIMENTAL_API -
> DALLOW_INTERNAL_API -Wno-format-truncation -DCC_AVX512_SUPPORT -
> mavx512f -mavx512dq -mavx512ifma -
> DRTE_LOG_DEFAULT_LOGTYPE=lib.member -MD -MQ
> lib/librte_member.a.p/member_rte_member.c.o -MF
> lib/librte_member.a.p/member_rte_member.c.o.d -o
> lib/librte_member.a.p/member_rte_member.c.o -
> c ../lib/member/rte_member.c
> 
> Here, the rte_member.c file is compiled with "-mavx512f -mavx512dq -
> mavx512ifma".
> The compiler might insert AVX512 instruction in this generic code.
> 
> [3/9] ccache gcc -Ilib/librte_member.a.p -Ilib -I../lib -Ilib/hash -I../lib/hash -
> Ilib/ring -I../lib/ring -Ilib/member -I../lib/member -I. -I.. -Iconfig -I../config -
> Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include
> -Ilib/eal/x86/include -I../lib/eal/x86/include -Ilib/eal/common -
> I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/metrics -
> I../lib/metrics -Ilib/telemetry -I../lib/telemetry -Ilib/net -I../lib/net -Ilib/mbuf -
> I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/rcu -I../lib/rcu -fdiagnostics-
> color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -
> O3 -g -include rte_config.h -Wcast-qual -Wdeprecated -Wformat -Wformat-
> nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -
> Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -
> Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member
> -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-zero-length-
> bounds -D_GNU_SOURCE -fPIC -march=native -DALLOW_EXPERIMENTAL_API -
> DALLOW_INTERNAL_API -Wno-format-truncation -DCC_AVX512_SUPPORT -
> mavx512f -mavx512dq -mavx512ifma -
> DRTE_LOG_DEFAULT_LOGTYPE=lib.member -MD -MQ
> lib/librte_member.a.p/member_rte_member_sketch.c.o -MF
> lib/librte_member.a.p/member_rte_member_sketch.c.o.d -o
> lib/librte_member.a.p/member_rte_member_sketch.c.o -
> c ../lib/member/rte_member_sketch.c
> 
> Idem.
> 
> [4/9] rm -f lib/member/libsketch_avx512_tmp.a && gcc-ar csrDT
> lib/member/libsketch_avx512_tmp.a
> lib/member/libsketch_avx512_tmp.a.p/rte_member_sketch_avx512.c.o
> [5/9] rm -f lib/librte_member.a && gcc-ar csrD lib/librte_member.a
> lib/librte_member.a.p/member_rte_member.c.o
> lib/librte_member.a.p/member_rte_member_ht.c.o
> lib/librte_member.a.p/member_rte_member_vbf.c.o
> lib/librte_member.a.p/member_rte_member_sketch.c.o
> lib/member/libsketch_avx512_tmp.a.p/rte_member_sketch_avx512.c.o
> [6/9] /usr/bin/meson --internal exe --capture lib/member.sym_chk --
> /home/dmarchan/git/pub/dpdk.org/buildtools/check-symbols.sh
> /home/dmarchan/git/pub/dpdk.org/lib/member/version.map
> lib/librte_member.a
> [7/9] gcc  -o lib/librte_member.so.23.0
> lib/member/libsketch_avx512_tmp.a.p/rte_member_sketch_avx512.c.o
> lib/librte_member.a.p/member_rte_member.c.o
> lib/librte_member.a.p/member_rte_member_ht.c.o
> lib/librte_member.a.p/member_rte_member_vbf.c.o
> lib/librte_member.a.p/member_rte_member_sketch.c.o -Wl,--as-needed -Wl,--
> no-undefined -Wl,-O1 -shared -fPIC -Wl,--start-group
> -Wl,-soname,librte_member.so.23 -Wl,--no-as-needed -pthread -lm -ldl -lnuma -
> lfdt -larchive '-Wl,-rpath,$ORIGIN/'
> -Wl,-rpath-link,/home/dmarchan/git/pub/dpdk.org/build-gcc/lib
> lib/librte_eal.so.23.0 lib/librte_kvargs.so.23.0
> lib/librte_telemetry.so.23.0 lib/librte_hash.so.23.0
> lib/librte_net.so.23.0 lib/librte_mbuf.so.23.0
> lib/librte_mempool.so.23.0 lib/librte_ring.so.23.0
> lib/librte_rcu.so.23.0
> -Wl,--version-
> script=/home/dmarchan/git/pub/dpdk.org/lib/member/version.map
> /usr/lib64/libbsd.so -Wl,--end-group
> [8/9] /usr/bin/meson --internal symbolextractor
> /home/dmarchan/git/pub/dpdk.org/build-gcc lib/librte_member.so.23.0
> lib/librte_member.so.23.0
> lib/librte_member.so.23.0.p/librte_member.so.23.0.symbols
> 

Catch it, will fix in v4.

> 
> 
> > +       sketch_avx512_tmp = static_library('sketch_avx512_tmp',
> > +           'rte_member_sketch_avx512.c',
> > +           include_directories: includes,
> > +           dependencies: static_rte_eal,
> > +           c_args: cflags)
> > +       objs +=
> sketch_avx512_tmp.extract_objects('rte_member_sketch_avx512.c')
> > +    endif
> > +endif
> 
> 
> --
> David Marchand