> From: Ferriter, Cian <[email protected]> > Sent: Thursday, June 10, 2021 3:47 PM > To: Stokes, Ian <[email protected]>; [email protected]; Van > Haaren, Harry <[email protected]> > Cc: [email protected] > Subject: RE: [ovs-dev] [v12 09/16] docs/dpdk/bridge: Add dpif performance > section. > > Hi Ian, > > Thanks for the review. My responses are inline. > > > -----Original Message----- > > From: Stokes, Ian <[email protected]> > > Sent: Wednesday 2 June 2021 19:41 > > To: Ferriter, Cian <[email protected]>; [email protected]; Van > Haaren, Harry <[email protected]> > > Cc: [email protected] > > Subject: RE: [ovs-dev] [v12 09/16] docs/dpdk/bridge: Add dpif performance > section. > > > > > This section details how two new commands can be used to list and select > > > the different dpif implementations. It also details how a non default > > > dpif implementation can be tested with the OVS unit test suite. > > > > > > Add NEWS updates for the dpif-netdev.c refactor and the new dpif > > > implementations/commands. > > > > > > Signed-off-by: Cian Ferriter <[email protected]> > > > > Thanks for the patch Cian, some comments inline below. > > > > One thing to flag is that for reviewing this is fine as a commit on it's own > focusing on updating the documentation. > > > > But I think it would be better to split this patch among the previous > > patches > and update the documentation as features are added, > > code refactored etc. in the code base. > > > > That way the documentation is an accurate reflection of the codebase at that > particular commit for each change. Does this make > > sense? > > > > That makes sense. I'll fix this for the next version. I'll add the > documentation > relevant to each commit in that commit. >
Thanks. > > > > > > --- > > > > > > v8: > > > - Merge NEWS file items into one Userspace Datapath: heading > > > --- > > > Documentation/topics/dpdk/bridge.rst | 37 > > > ++++++++++++++++++++++++++++ > > > NEWS | 4 +++ > > > 2 files changed, 41 insertions(+) > > > > > > diff --git a/Documentation/topics/dpdk/bridge.rst > > > b/Documentation/topics/dpdk/bridge.rst > > > index 526d5c959..ca90d7bdb 100644 > > > --- a/Documentation/topics/dpdk/bridge.rst > > > +++ b/Documentation/topics/dpdk/bridge.rst > > > @@ -214,3 +214,40 @@ implementation :: > > > > > > Compile OVS in debug mode to have `ovs_assert` statements error out if > > > there is a mis-match in the DPCLS lookup implementation. > > > + > > > +Datapath Interface Performance > > > +------------------------------ > > > + > > > +The datapath interface (DPIF) or dp_netdev_input() is responsible for > > > taking > > > +packets through the major components of the userspace datapath; such as > > > +miniflow_extract, EMC, SMC and DPCLS lookups, and a lot of the > > > performance > > > +stats associated with the datapath. > > > + > > > +Just like with the SIMD DPCLS work above, SIMD can be applied to the DPIF > > > > Minor, would replace "work" with "feature". > > > > I'll fix this in the next version. > > > > to > > > +improve performance. > > > + > > > +OVS provides multiple implementations of the DPIF. These can be listed > > > with the > > > +following command :: > > > + > > > + $ ovs-appctl dpif-netdev/dpif-get > > > + Available DPIF implementations: > > > + dpif_scalar > > > + dpif_avx512 > > > + > > > +By default, dpif_scalar is used. The DPIF implementation can be selected > > > by > > > +name :: > > > + > > > + $ ovs-appctl dpif-netdev/dpif-set dpif_avx512 > > > + DPIF implementation set to dpif_avx512. > > > + > > > + $ ovs-appctl dpif-netdev/dpif-set dpif_scalar > > > + DPIF implementation set to dpif_scalar. > > > + > > > > Maybe this is later in the series but have you given consideration to > > updating > the man pages for these dpif-netdev commands also? > > > > I'm happy to add to man pages. I'm just wondering which man pages the > information should go into. Do you mean the " lib/dpif-netdev-unixctl.man" > file. > If so, I'll add some info there. I think so. I think you'll find examples for other commands used by the dpif-netdev in there such as pmd stats and pmd to queue summary. As it's a user command I would think it would go along with them? > > > > +Running Unit Tests with AVX512 DPIF > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > + > > > +Since the AVX512 DPIF is disabled by default, a compile time option is > > > +available in order to test it with the OVS unit test suite. When > > > building with > > > +a CPU that supports AVX512, use the following configure option :: > > > + > > > + $ ./configure --enable-dpif-default-avx512 > > > > Would it be useful to provide the log output the user would expect to see > during configuration to confirm AVX512 DPIF is in use? > > > > Good idea. I'll add that in the next version. > > > > diff --git a/NEWS b/NEWS > > > index 3eab5f4fa..5e847a95e 100644 > > > --- a/NEWS > > > +++ b/NEWS > > > @@ -7,6 +7,10 @@ Post-v2.15.0 > > > cases, e.g if all PMD threads are running on the same NUMA node. > > > * Add a partial HWOL PMD statistic counting hits similar to existing > > > * EMC/SMC/DPCLS stats. > > > + * Refactor lib/dpif-netdev.c to multiple header files. > > > + * Add avx512 implementation of dpif which can process non > > > recirculated > > > + packets. It supports partial HWOL, EMC, SMC and DPCLS lookups. > > > + * Add commands to get and set the dpif implementations. > > > > As flagged above these news entries could be split into the previous patches > that made those changes. > > > > I'll fix this in the next version. Thanks, look forward to the next revision. Regards Ian > > > Thanks > > Ian > > > - ovs-ctl: > > > * New option '--no-record-hostname' to disable hostname > > > configuration > > > in ovsdb on startup. > > > -- > > > 2.31.1 > > > > > > _______________________________________________ > > > dev mailing list > > > [email protected] > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
