diff mbox series

[ovs-dev,v12,09/16] docs/dpdk/bridge: Add dpif performance section.

Message ID 20210517140434.59555-10-cian.ferriter@intel.com
State Changes Requested
Headers show
Series DPIF Framework + Optimizations | expand

Commit Message

Ferriter, Cian May 17, 2021, 2:04 p.m. UTC
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 <cian.ferriter@intel.com>

---

v8:
- Merge NEWS file items into one Userspace Datapath: heading
---
 Documentation/topics/dpdk/bridge.rst | 37 ++++++++++++++++++++++++++++
 NEWS                                 |  4 +++
 2 files changed, 41 insertions(+)

Comments

Stokes, Ian June 2, 2021, 6:40 p.m. UTC | #1
> 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 <cian.ferriter@intel.com>

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?

> 
> ---
> 
> 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".

> 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?

> +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?

> 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.

Thanks
Ian
>     - ovs-ctl:
>       * New option '--no-record-hostname' to disable hostname configuration
>         in ovsdb on startup.
> --
> 2.31.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ferriter, Cian June 10, 2021, 2:46 p.m. UTC | #2
Hi Ian,

Thanks for the review. My responses are inline.

> -----Original Message-----
> From: Stokes, Ian <ian.stokes@intel.com>
> Sent: Wednesday 2 June 2021 19:41
> To: Ferriter, Cian <cian.ferriter@intel.com>; ovs-dev@openvswitch.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: i.maximets@ovn.org
> 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 <cian.ferriter@intel.com>
> 
> 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.

> >
> > ---
> >
> > 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.

> > +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
> Ian
> >     - ovs-ctl:
> >       * New option '--no-record-hostname' to disable hostname configuration
> >         in ovsdb on startup.
> > --
> > 2.31.1
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Stokes, Ian June 16, 2021, 11:39 a.m. UTC | #3
> From: Ferriter, Cian <cian.ferriter@intel.com>
> Sent: Thursday, June 10, 2021 3:47 PM
> To: Stokes, Ian <ian.stokes@intel.com>; ovs-dev@openvswitch.org; Van
> Haaren, Harry <harry.van.haaren@intel.com>
> Cc: i.maximets@ovn.org
> 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 <ian.stokes@intel.com>
> > Sent: Wednesday 2 June 2021 19:41
> > To: Ferriter, Cian <cian.ferriter@intel.com>; ovs-dev@openvswitch.org; Van
> Haaren, Harry <harry.van.haaren@intel.com>
> > Cc: i.maximets@ovn.org
> > 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 <cian.ferriter@intel.com>
> >
> > 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
> > > dev@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

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 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.
+
+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
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.
    - ovs-ctl:
      * New option '--no-record-hostname' to disable hostname configuration
        in ovsdb on startup.