Hi Sunil,

I'll respin the patch set based on these documentation comments with your Ack.

Thanks,
Cian

> -----Original Message-----
> From: dev <[email protected]> On Behalf Of Pai G, Sunil
> Sent: Tuesday 11 October 2022 11:52
> To: Amber, Kumar <[email protected]>; [email protected]
> Cc: [email protected]; Amber, Kumar <[email protected]>; 
> [email protected]
> Subject: Re: [ovs-dev] [PATCH v6 6/9] dpif-mfex: Modify set/get MFEX commands 
> to include inner.
> 
> Hi Amber,
> 
> One comment below , rest looks good to me.
> 
> > -----Original Message-----
> > From: dev <[email protected]> On Behalf Of Kumar Amber
> > Sent: Thursday, October 6, 2022 3:54 PM
> > To: [email protected]
> > Cc: [email protected]; [email protected]; Amber, Kumar
> > <[email protected]>
> > Subject: [ovs-dev] [PATCH v6 6/9] dpif-mfex: Modify set/get MFEX commands
> > to include inner.
> >
> > The set command is modified to allow the user to select different
> > implementations for processing inner packets.
> > Also, the get command is modified to indicate both inner and outer MFEX
> > implementation in use.
> >
> > $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 study 1024 -recirc
> >
> > Signed-off-by: Kumar Amber <[email protected]>
> > Signed-off-by: Cian Ferriter <[email protected]>
> > Co-authored-by: Cian Ferriter <[email protected]>
> > ---
> >  Documentation/topics/dpdk/bridge.rst | 24 ++++++++++++++++++------
> >  lib/dpif-netdev-private-extract.c    | 23 ++++++++++++++++++++++-
> >  lib/dpif-netdev-private-extract.h    |  6 +++++-
> >  lib/dpif-netdev-private-thread.h     |  3 +++
> >  lib/dpif-netdev.c                    | 23 +++++++++++++++++++----
> >  5 files changed, 67 insertions(+), 12 deletions(-)
> >
> > diff --git a/Documentation/topics/dpdk/bridge.rst
> > b/Documentation/topics/dpdk/bridge.rst
> > index 354f1ced1..dbebea624 100644
> > --- a/Documentation/topics/dpdk/bridge.rst
> > +++ b/Documentation/topics/dpdk/bridge.rst
> > @@ -293,13 +293,21 @@ command also shows whether the CPU supports each
> > implementation::
> >  An implementation can be selected manually by the following command::
> >
> >      $ ovs-appctl dpif-netdev/miniflow-parser-set [-pmd core_id] name \
> > -      [study_cnt]
> > +      [study_cnt] [-recirc]
> >
> > -The above command has two optional parameters: ``study_cnt`` and
> > ``core_id``.
> > -The ``core_id`` sets a particular packet parsing function to a specific -
> > PMD thread on the core.  The third parameter ``study_cnt``, which is
> > specific -to ``study`` and ignored by other implementations, means how
> > many packets -are needed to choose the best implementation.
> > +The above command has three optional parameters: ``study_cnt``,
> > +``core_id`` and ``-recirc``. The ``core_id`` sets a particular packet
> > +parsing function to a specific PMD thread on the core.  The third
> > +parameter ``study_cnt``, which is specific to ``study`` and ignored by
> > +other implementations, means how many packets are needed to choose the
> > +best implementation.
> 
> 
> > The fourth parameter ``-recirc`` indicates to MFEX
> > +to use optimized MFEX inner for processing tunneled inner packets. The
> > +optional ``-recirc`` parameter gives flexibility to set different
> > +optimized MFEX function on inner and outer, when set to study.t
> 
> I think we can improve this documentation a bit, how about the following:
> The optional ``-recirc`` parameter allows OVS to set the optimized MFEX 
> function for inner packet
> processing.
> 
> For example:
> - ``name``               : sets the outer implementation to ``name``, inner 
> defaults to scalar.
> - ``name``  + ``recirc`` : sets both inner and outer implementations to 
> ``name``.
> - ``study`` + ``recirc`` : sets inner as well as outer implementations 
> separately based on the traffic
> pattern.
> 
> 

Makes sense, this explains it better. I'll make the change and build the docs 
to see how it looks, then respin a new version with your Ack.

> > +
> > +    $ ovs-appctl dpif-netdev/miniflow-parser-set study -recirc
> >
> 
> 
> >  Also user can select the ``study`` implementation which studies the
> > traffic for  a specific number of packets by applying all available
> > implementations of @@ -322,6 +330,10 @@ following command::
> >
> >      $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 scalar
> >
> > +``study`` can be selected with packet count and explicit PMD selection
> > +along with the ``recirc`` by following command::
> > +
> > +    $ ovs-appctl dpif-netdev/miniflow-parser-set -pmd 3 study 1024
> > + -recirc
> >
> 
> Other than that, the changes look OK to me,
> 
> Acked-by: Sunil Pai G <[email protected]>
> 
> Thanks and regards
> Sunil
> 
> <snipped>
> _______________________________________________
> 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

Reply via email to