Re: [dpdk-dev] [PATCH v2 12/15] ethdev: update behavior of VF/PF in flow API

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

On Sat, Apr 07, 2018 at 12:41:17PM +0300, Andrew Rybchenko wrote:
> On 04/06/2018 04:25 PM, Adrien Mazarguil wrote:
> > Contrary to all other pattern items, these are inconsistently documented as
> > affecting traffic instead of simply matching its origin, without provision
> > for the latter.
> > 
> > This commit clarifies documentation and updates PMDs since the original
> > behavior now has to be explicitly requested using the new transfer
> > attribute.
> > 
> > It breaks ABI compatibility for the following public functions:
> > 
> > - rte_flow_create()
> > - rte_flow_validate()
> > 
> > Impacted PMDs are bnxt and i40e, for which the VF pattern item is now only
> > supported when a transfer attribute is also present.
> > 
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Cc: Somnath Kotur <somnath.kotur@broadcom.com>
> > Cc: Beilei Xing <beilei.xing@intel.com>
> > Cc: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> >   app/test-pmd/cmdline_flow.c                 | 12 +++---
> >   doc/guides/prog_guide/rte_flow.rst          | 36 +++++++++---------
> >   doc/guides/testpmd_app_ug/testpmd_funcs.rst | 12 +++---
> >   drivers/net/bnxt/bnxt_filter.c              | 22 ++++++-----
> >   drivers/net/i40e/i40e_flow.c                | 23 +++++++-----
> >   lib/librte_ether/rte_flow.h                 | 47 ++++++++++--------------
> >   6 files changed, 77 insertions(+), 75 deletions(-)
> 
> <...>
> 
> > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> > index 735ce6323..beedc713b 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -518,15 +518,12 @@ Usage example, matching non-TCPv4 packets only:
> >   Item: ``PF``
> >   ^^^^^^^^^^^^
> > -Matches packets addressed to the physical function of the device.
> > +Matches traffic originating from (ingress) or going to (egress) the physical
> > +function of the current device.
> 
> Not sure that I understand above. It looks like ingress and egress are
> misplaced.
> There many similar cases below.

In this API, "ingress" and "egress" are always defined as relative to the
application creating the flow rule. In that sense they are respectively
synonyms for "from" and "to".

I agree they are not properly defined in this document, in fact ingress and
egress were clarified (with diagrams and all) in the RFC submitted prior to
this patch [1].

I will update the "Attribute: Traffic direction" section in my next update.

[1] See "Traffic direction" in
    http://dpdk.org/ml/archives/dev/2018-March/092513.html

-- 
Adrien Mazarguil
6WIND