From: Bruce Richardson <bruce.richardson@intel.com>
To: Leyi Rong <leyi.rong@intel.com>
Cc: <ferruh.yigit@xilinx.com>, <david.marchand@redhat.com>,
<suanmingm@nvidia.com>, <yipeng1.wang@intel.com>,
<zaoxingliu@gmail.com>, <sameh.gobriel@intel.com>, <dev@dpdk.org>
Subject: Re: [PATCH v4 1/2] member: implement NitroSketch mode
Date: Thu, 15 Sep 2022 15:12:52 +0100 [thread overview]
Message-ID: <YyMy5O1tcZcn/C8z@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20220915130342.1497512-2-leyi.rong@intel.com>
On Thu, Sep 15, 2022 at 09:03:41PM +0800, Leyi Rong 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>
> ---
Couple of comments below on the meson.build file.
/Bruce
> 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 | 70 +++
> lib/member/rte_member_sketch_avx512.h | 35 ++
> 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..3df0fbc7a6 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')
At this point, with 4 entries I'd suggest reworking this to a vertical
list.
sources = files(
'rte_member.c',
....
)
> headers = files('rte_member.h')
> deps += ['hash']
> +includes += include_directories('../hash', '../ring')
> +
Use dependencies rather than include-paths. Having a dependency on hash
means that your code will be compiled with the hash include paths already
present. Similarly you can add "ring" to the deps variable and drop this
line.
> +# compile AVX512 version if:
> +# we are building 64-bit binary AND binutils can generate proper code
I think this comment is no longer necessary. The condition itself is clear.
The following comment can be kept though as it explains what is being done
at a higher level than the 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')
> + sketch_avx512_tmp = static_library('sketch_avx512_tmp',
> + 'rte_member_sketch_avx512.c',
> + include_directories: includes,
> + dependencies: static_rte_eal,
> + c_args: cflags +
> + ['-mavx512f', '-mavx512dq', '-mavx512ifma'])
> + objs += sketch_avx512_tmp.extract_objects('rte_member_sketch_avx512.c')
> + cflags += ['-DCC_AVX512_SUPPORT']
Logic looks correct here now, but beware of your indentation. You are
mixing tabs and spaces. Meson files should have spaces only, no tabs.
> + endif
> +endif
next prev parent reply other threads:[~2022-09-15 14:13 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-10 7:45 [PATCH 0/2] introduce NitroSketch Mode into membership library Leyi Rong
2022-08-10 7:45 ` [PATCH 1/2] member: implement NitroSketch mode Leyi Rong
2022-08-16 8:59 ` Ferruh Yigit
2022-08-26 2:32 ` Rong, Leyi
2022-08-10 7:45 ` [PATCH 2/2] test/member: add functional and perf tests for sketch Leyi Rong
2022-08-11 2:33 ` Suanming Mou
2022-08-26 2:26 ` Rong, Leyi
2022-08-31 6:46 ` [PATCH v2 0/2] introduce NitroSketch Mode into membership library Leyi Rong
2022-08-31 6:46 ` [PATCH v2 1/2] member: implement NitroSketch mode Leyi Rong
2022-09-13 14:56 ` David Marchand
2022-09-14 9:42 ` Rong, Leyi
2022-08-31 6:46 ` [PATCH v2 2/2] test/member: add functional and perf tests for sketch Leyi Rong
2022-09-15 2:14 ` [PATCH v3 0/2] introduce NitroSketch Mode into membership library Leyi Rong
2022-09-15 2:14 ` [PATCH v3 1/2] member: implement NitroSketch mode Leyi Rong
2022-09-15 7:47 ` David Marchand
2022-09-15 13:02 ` Rong, Leyi
2022-09-15 2:14 ` [PATCH v3 2/2] test/member: add functional and perf tests for sketch Leyi Rong
2022-09-15 7:30 ` Jiang, YuX
2022-09-15 7:41 ` David Marchand
2022-09-15 13:03 ` [PATCH v4 0/2] introduce NitroSketch Mode into membership library Leyi Rong
2022-09-15 13:03 ` [PATCH v4 1/2] member: implement NitroSketch mode Leyi Rong
2022-09-15 14:12 ` Bruce Richardson [this message]
2022-09-16 2:30 ` Rong, Leyi
2022-09-15 13:03 ` [PATCH v4 2/2] test/member: add functional and perf tests for sketch Leyi Rong
2022-09-16 3:03 ` [PATCH v5 0/2] introduce NitroSketch Mode into membership library Leyi Rong
2022-09-16 3:03 ` [PATCH v5 1/2] member: implement NitroSketch mode Leyi Rong
2022-09-22 3:00 ` Jiang, YuX
2022-10-03 12:37 ` Thomas Monjalon
2022-10-08 3:36 ` Rong, Leyi
2022-09-16 3:03 ` [PATCH v5 2/2] test/member: add functional and perf tests for sketch Leyi Rong
2022-10-09 23:13 ` [PATCH v5 0/2] introduce NitroSketch Mode into membership library Thomas Monjalon
2022-10-10 2:36 ` Rong, Leyi
2022-10-10 7:30 ` Thomas Monjalon
2022-10-10 8:28 ` Rong, Leyi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YyMy5O1tcZcn/C8z@bricha3-MOBL.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@xilinx.com \
--cc=leyi.rong@intel.com \
--cc=sameh.gobriel@intel.com \
--cc=suanmingm@nvidia.com \
--cc=yipeng1.wang@intel.com \
--cc=zaoxingliu@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).