doc: clarify implicit filtering in transfer rules

  As per existing documentation, attribute "transfer", quote, "complements
the behavior of some pattern items such as RTE_FLOW_ITEM_TYPE_PHY_PORT
and is meaningless without them". That effectively confronts the idea of
implicit filtering imposed by port_id argument passed by flow create API.

This bit of documentation is vague, and having no implicit filtering is
unfriendly to applications which insert flow rules on specific ports
based on the source port IDs of the (not offloaded) incoming packets.

In order to address the problem, document the existence of the implicit
filtering. Use the term "weak" for this filtering as it implies the
possibility to override it by including explicit traffic source criteria
in the flow pattern (PORT_ID, PHY_PORT and the likes).

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
The topic was briefly discussed in mail thread [1].

I'm not sure if the patch should have "Fixes:" tag. If it is really
behaviour intended from the very beginning, it should be backported
and corresponding fixes in drivers should be backported as well.

[1] https://patches.dpdk.org/project/dpdk/patch/20210601111420.5549-1-ivan.malov@oktetlabs.ru/

 doc/guides/prog_guide/rte_flow.rst   | 17 ++++++++++++++---
 doc/guides/rel_notes/deprecation.rst |  5 -----
 2 files changed, 14 insertions(+), 8 deletions(-)
  

Hi Andrew,

PSB

Thanks,
Ori

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
> Sent: Wednesday, September 1, 2021 6:11 PM
> 
> As per existing documentation, attribute "transfer", quote, "complements
> the behavior of some pattern items such as
> RTE_FLOW_ITEM_TYPE_PHY_PORT and is meaningless without them". That
> effectively confronts the idea of implicit filtering imposed by port_id
> argument passed by flow create API.
> 
> This bit of documentation is vague, and having no implicit filtering is
> unfriendly to applications which insert flow rules on specific ports based on
> the source port IDs of the (not offloaded) incoming packets.
> 
> In order to address the problem, document the existence of the implicit
> filtering. Use the term "weak" for this filtering as it implies the possibility to
> override it by including explicit traffic source criteria in the flow pattern
> (PORT_ID, PHY_PORT and the likes).
> 
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
> The topic was briefly discussed in mail thread [1].
> 
> I'm not sure if the patch should have "Fixes:" tag. If it is really behaviour
> intended from the very beginning, it should be backported and
> corresponding fixes in drivers should be backported as well.
> 
> [1] https://patches.dpdk.org/project/dpdk/patch/20210601111420.5549-1-
> ivan.malov@oktetlabs.ru/
> 
>  doc/guides/prog_guide/rte_flow.rst   | 17 ++++++++++++++---
>  doc/guides/rel_notes/deprecation.rst |  5 -----
>  2 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 2b42d5ec8c..af54939418 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -171,13 +171,24 @@ When supported, this effectively enables an
> application to reroute traffic  not necessarily intended for it (e.g. coming from
> or addressed to different  physical ports, VFs or applications) at the device
> level.
> 
> -It complements the behavior of some pattern items such as `Item:
> PHY_PORT`_ -and is meaningless without them.
> -
>  When transferring flow rules, **ingress** and **egress** attributes
>  (`Attribute: Traffic direction`_) keep their original meaning, as if  processing
> traffic emitted or received by the application.

I know this is original code but what do we mean application?
You assume that the application is the switch?
Or the application is some DPDK application running on the PF?

> 
> +DPDK port used to create transfer rule is important since it implicitly
> +adds filtering by it (similar to `Item: PORT_ID` with ``spec.id`` equal
> +to the port ID and exact match mask) if no other items which specify
> +source are present in the rule pattern (e.g. `Item: PHY_PORT`, `Item:
> +VF` or explicit `Item: PORT_ID`). It means that by default ingress
> +rules apply to traffic which comes from associated upstream switch
> +port, i.e. physical network port for PF DPDK port, VF for VF
> +representor. Egress rules transfer traffic transmitted via
> +corresponding DPDK port, i.e. PF DPDK port or VF representor itself.
> +
To make sure I understand the direction should be defined as follows:
Traffic from  ---> to
Wire --> VF ==> ingress direction.
VF  --> Wire ==> ingress direction.
VF1 --> VF2 ==> ingress direction.
VF 1--> VF2 representor ==> ingress.
VF representor --> wire ==> egress.
VF representor --> VF ==> egress



> +It is still possible to apply transfer rule on a traffic originating
> +from any switch port using wildcard mask in corresponding pattern item
> +if underlying PMD supports it.
> +
>  Pattern item
>  ~~~~~~~~~~~~
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 76a4abfd6b..f1d290a911 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -134,11 +134,6 @@ Deprecation Notices
>    traffic direction to the represented entity or ethdev port itself
>    in DPDK 21.11.
> 
> -* ethdev: Flow API documentation is unclear if ethdev port used to create
> -  a flow rule adds any implicit match criteria in the case of transfer rules.
> -  The semantics will be clarified in DPDK 21.11 and it will require fixes in
> -  drivers and applications which interpret it in a different way.
> -
>  * ethdev: The flow API matching pattern structures, ``struct
> rte_flow_item_*``,
>    should start with relevant protocol header.
>    Some matching pattern structures implements this by duplicating protocol
> header
> --
> 2.30.2
  

Hi Ori,

sorry, long answer below.

Andrew.

On 9/1/21 7:28 PM, Ori Kam wrote:
> Hi Andrew,
> 
> PSB
> 
> Thanks,
> Ori
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
>> Sent: Wednesday, September 1, 2021 6:11 PM
>>
>> As per existing documentation, attribute "transfer", quote, "complements
>> the behavior of some pattern items such as
>> RTE_FLOW_ITEM_TYPE_PHY_PORT and is meaningless without them". That
>> effectively confronts the idea of implicit filtering imposed by port_id
>> argument passed by flow create API.
>>
>> This bit of documentation is vague, and having no implicit filtering is
>> unfriendly to applications which insert flow rules on specific ports based on
>> the source port IDs of the (not offloaded) incoming packets.
>>
>> In order to address the problem, document the existence of the implicit
>> filtering. Use the term "weak" for this filtering as it implies the possibility to
>> override it by including explicit traffic source criteria in the flow pattern
>> (PORT_ID, PHY_PORT and the likes).
>>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> ---
>> The topic was briefly discussed in mail thread [1].
>>
>> I'm not sure if the patch should have "Fixes:" tag. If it is really behaviour
>> intended from the very beginning, it should be backported and
>> corresponding fixes in drivers should be backported as well.
>>
>> [1] https://patches.dpdk.org/project/dpdk/patch/20210601111420.5549-1-
>> ivan.malov@oktetlabs.ru/
>>
>>  doc/guides/prog_guide/rte_flow.rst   | 17 ++++++++++++++---
>>  doc/guides/rel_notes/deprecation.rst |  5 -----
>>  2 files changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>> b/doc/guides/prog_guide/rte_flow.rst
>> index 2b42d5ec8c..af54939418 100644
>> --- a/doc/guides/prog_guide/rte_flow.rst
>> +++ b/doc/guides/prog_guide/rte_flow.rst
>> @@ -171,13 +171,24 @@ When supported, this effectively enables an
>> application to reroute traffic  not necessarily intended for it (e.g. coming from
>> or addressed to different  physical ports, VFs or applications) at the device
>> level.
>>
>> -It complements the behavior of some pattern items such as `Item:
>> PHY_PORT`_ -and is meaningless without them.
>> -
>>  When transferring flow rules, **ingress** and **egress** attributes
>>  (`Attribute: Traffic direction`_) keep their original meaning, as if  processing
>> traffic emitted or received by the application.
> 
> I know this is original code but what do we mean application?
> You assume that the application is the switch?
> Or the application is some DPDK application running on the PF?

I think that "the application" is always a DPDK application
which transmits egress traffic (using Tx burst) and would
receive ingress traffic (using Rx burst). It could a switch
(e.g. OvS+DPDK) or any other DPDK application.
Egress rules are applied to traffic emitted by the application.
Ingress rules are applied to traffic which would be received
by the application by default.

By inserting ingress flow rule application says:
I know that to do with such traffic, apply transformations
and redirect to other consumer (i.e. make it egress), or
other application entry point (DPDK port), or keep current destination
port as is.

By inserting egress flow rule application says:
I'm sending a traffic and should do some transformations,
but would like to offload it to HW. Changing destination is
unlikely in this case, but still possible.

I think it is important that *ingress* in the case of
*transfer* may be defined in terms of default rules only
since transfer flow rules may change destination of the
future packets and it

>>
>> +DPDK port used to create transfer rule is important since it implicitly
>> +adds filtering by it (similar to `Item: PORT_ID` with ``spec.id`` equal
>> +to the port ID and exact match mask) if no other items which specify
>> +source are present in the rule pattern (e.g. `Item: PHY_PORT`, `Item:
>> +VF` or explicit `Item: PORT_ID`). It means that by default ingress
>> +rules apply to traffic which comes from associated upstream switch
>> +port, i.e. physical network port for PF DPDK port, VF for VF
>> +representor. Egress rules transfer traffic transmitted via
>> +corresponding DPDK port, i.e. PF DPDK port or VF representor itself.
>> +
> To make sure I understand the direction should be defined as follows:
> Traffic from  ---> to
> Wire --> VF ==> ingress direction.

If wire (physical port) representor is bound to
DPDK application, yes, it is ingress regardless
VF binding.

If wire (physical port) representor is not bound
to DPDK application, but VF is bound, yes, it is
an ingress as well.

In both cases the traffic would be received by
the DPDK application.

If neither wire nor VF is bound to DPDK application,
such traffic simply does not exists for it. The
application simply knows nothing about wire and VF.

> VF  --> Wire ==> ingress direction.

If VF is bound to DPDK application, it is definitely
*egress*, not *ingress*.
If the VF representor is bound to DPDK application (i.e.
traffic would be received by the DPDK application by default)
yes, it is an *ingress*.
Otherwise, similar to above it is not applicable.

> VF1 --> VF2 ==> ingress direction.

It depends on these VFs itself or its representors
binding to DPDK application.

> VF 1--> VF2 representor ==> ingress.
> VF representor --> wire ==> egress.
> VF representor --> VF ==> egress

Any representor is a DPDK entity. So, traffic to be received
by a representor is an ingress. Traffic transmitted via
representor is an egress.

>> +It is still possible to apply transfer rule on a traffic originating
>> +from any switch port using wildcard mask in corresponding pattern item
>> +if underlying PMD supports it.
>> +
>>  Pattern item
>>  ~~~~~~~~~~~~
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst
>> b/doc/guides/rel_notes/deprecation.rst
>> index 76a4abfd6b..f1d290a911 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -134,11 +134,6 @@ Deprecation Notices
>>    traffic direction to the represented entity or ethdev port itself
>>    in DPDK 21.11.
>>
>> -* ethdev: Flow API documentation is unclear if ethdev port used to create
>> -  a flow rule adds any implicit match criteria in the case of transfer rules.
>> -  The semantics will be clarified in DPDK 21.11 and it will require fixes in
>> -  drivers and applications which interpret it in a different way.
>> -
>>  * ethdev: The flow API matching pattern structures, ``struct
>> rte_flow_item_*``,
>>    should start with relevant protocol header.
>>    Some matching pattern structures implements this by duplicating protocol
>> header
>> --
>> 2.30.2
  

Hi,

Thanks you for your answer it looks to me we are on the same page but I'm not sure,
also I think the result of this discusstion should be part of the commit log and documentation.
Since I think this issue is not clear to many people.

PSB some questions. (sorry for the long mail but I want to make sure we underdand each other)

Thanks, Ori


> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Thursday, September 2, 2021 10:00 AM
> 
> Hi Ori,
> 
> sorry, long answer below.
> 
> Andrew.
> 
> On 9/1/21 7:28 PM, Ori Kam wrote:
> > Hi Andrew,
> >
> > PSB
> >
> > Thanks,
> > Ori
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
> >> Sent: Wednesday, September 1, 2021 6:11 PM
> >>
> >> As per existing documentation, attribute "transfer", quote,
> >> "complements the behavior of some pattern items such as
> >> RTE_FLOW_ITEM_TYPE_PHY_PORT and is meaningless without them".
> That
> >> effectively confronts the idea of implicit filtering imposed by
> >> port_id argument passed by flow create API.
> >>
> >> This bit of documentation is vague, and having no implicit filtering
> >> is unfriendly to applications which insert flow rules on specific
> >> ports based on the source port IDs of the (not offloaded) incoming
> packets.
> >>
> >> In order to address the problem, document the existence of the
> >> implicit filtering. Use the term "weak" for this filtering as it
> >> implies the possibility to override it by including explicit traffic
> >> source criteria in the flow pattern (PORT_ID, PHY_PORT and the likes).
> >>
> >> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> ---
> >> The topic was briefly discussed in mail thread [1].
> >>
> >> I'm not sure if the patch should have "Fixes:" tag. If it is really
> >> behaviour intended from the very beginning, it should be backported
> >> and corresponding fixes in drivers should be backported as well.
> >>
> >> [1]
> >> https://patches.dpdk.org/project/dpdk/patch/20210601111420.5549-1-
> >> ivan.malov@oktetlabs.ru/
> >>
> >>  doc/guides/prog_guide/rte_flow.rst   | 17 ++++++++++++++---
> >>  doc/guides/rel_notes/deprecation.rst |  5 -----
> >>  2 files changed, 14 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/doc/guides/prog_guide/rte_flow.rst
> >> b/doc/guides/prog_guide/rte_flow.rst
> >> index 2b42d5ec8c..af54939418 100644
> >> --- a/doc/guides/prog_guide/rte_flow.rst
> >> +++ b/doc/guides/prog_guide/rte_flow.rst
> >> @@ -171,13 +171,24 @@ When supported, this effectively enables an
> >> application to reroute traffic  not necessarily intended for it (e.g.
> >> coming from or addressed to different  physical ports, VFs or
> >> applications) at the device level.
> >>
> >> -It complements the behavior of some pattern items such as `Item:
> >> PHY_PORT`_ -and is meaningless without them.
> >> -
> >>  When transferring flow rules, **ingress** and **egress** attributes
> >>  (`Attribute: Traffic direction`_) keep their original meaning, as if
> >> processing traffic emitted or received by the application.
> >
> > I know this is original code but what do we mean application?
> > You assume that the application is the switch?
> > Or the application is some DPDK application running on the PF?
> 
> I think that "the application" is always a DPDK application which transmits
> egress traffic (using Tx burst) and would receive ingress traffic (using Rx
> burst). It could a switch (e.g. OvS+DPDK) or any other DPDK application.
> Egress rules are applied to traffic emitted by the application.
> Ingress rules are applied to traffic which would be received by the application
> by default.
> 
I like this line " Egress rules are applied to traffic emitted by the application.
Ingress rules are applied to traffic which would be received by the application
by default." I think it should be part of the documentation.

But this raise some issue that I think you touched later in this thread,
Egress rule is applyed to the traffic sent be the DPDK application using TX burst
but after sending this traffic it may jump to other groups that are getting also traffic
from different VMs so the second rule for the traffic is it ingress or egress?

For example a simple flow that encaps all traffic from all ports (DPDK + other VMs) can I create
just one flow with ingress pattern or do I need to create two one with ingress to catch all VMs
and one egress to catch the DPDK port?


> By inserting ingress flow rule application says:
> I know that to do with such traffic, apply transformations and redirect to
> other consumer (i.e. make it egress), or other application entry point (DPDK
> port), or keep current destination port as is.
> 
> By inserting egress flow rule application says:
> I'm sending a traffic and should do some transformations, but would like to
> offload it to HW. Changing destination is unlikely in this case, but still possible.
> 
> I think it is important that *ingress* in the case of
> *transfer* may be defined in terms of default rules only since transfer flow
> rules may change destination of the future packets and it
> 
How would you define default rule?

> >>
> >> +DPDK port used to create transfer rule is important since it
> >> +implicitly adds filtering by it (similar to `Item: PORT_ID` with
> >> +``spec.id`` equal to the port ID and exact match mask) if no other
> >> +items which specify source are present in the rule pattern (e.g. `Item:
> PHY_PORT`, `Item:
> >> +VF` or explicit `Item: PORT_ID`). It means that by default ingress
> >> +rules apply to traffic which comes from associated upstream switch
> >> +port, i.e. physical network port for PF DPDK port, VF for VF
> >> +representor. Egress rules transfer traffic transmitted via
> >> +corresponding DPDK port, i.e. PF DPDK port or VF representor itself.
> >> +
> > To make sure I understand the direction should be defined as follows:
> > Traffic from  ---> to
> > Wire --> VF ==> ingress direction.
> 
> If wire (physical port) representor is bound to DPDK application, yes, it is
> ingress regardless VF binding.
> 
By the word bound you mean that it has a port for it? 
Or that the traffic should be routed to some DPDK port?

> If wire (physical port) representor is not bound to DPDK application, but VF is
> bound, yes, it is an ingress as well.
> 
> In both cases the traffic would be received by the DPDK application.
> 
> If neither wire nor VF is bound to DPDK application, such traffic simply does
> not exists for it. The application simply knows nothing about wire and VF.
> 
> > VF  --> Wire ==> ingress direction.
> 
> If VF is bound to DPDK application, it is definitely *egress*, not *ingress*.
> If the VF representor is bound to DPDK application (i.e.
> traffic would be received by the DPDK application by default) yes, it is an
> *ingress*.
> Otherwise, similar to above it is not applicable.
> 
> > VF1 --> VF2 ==> ingress direction.
> 
> It depends on these VFs itself or its representors binding to DPDK
> application.
> 
> > VF 1--> VF2 representor ==> ingress.
> > VF representor --> wire ==> egress.
> > VF representor --> VF ==> egress
> 
> Any representor is a DPDK entity. So, traffic to be received by a representor
> is an ingress. Traffic transmitted via representor is an egress.
> 
I assume that like said above the line should say 
traffic to be received by a representor by default
is an ingress. Traffic transmitted via representor is an egress.
Since the real target is not the represenor but since there are no rules
it will be routed to the representor. Right?

> >> +It is still possible to apply transfer rule on a traffic originating
> >> +from any switch port using wildcard mask in corresponding pattern
> >> +item if underlying PMD supports it.
> >> +
> >>  Pattern item
> >>  ~~~~~~~~~~~~
> >>
> >> diff --git a/doc/guides/rel_notes/deprecation.rst
> >> b/doc/guides/rel_notes/deprecation.rst
> >> index 76a4abfd6b..f1d290a911 100644
> >> --- a/doc/guides/rel_notes/deprecation.rst
> >> +++ b/doc/guides/rel_notes/deprecation.rst
> >> @@ -134,11 +134,6 @@ Deprecation Notices
> >>    traffic direction to the represented entity or ethdev port itself
> >>    in DPDK 21.11.
> >>
> >> -* ethdev: Flow API documentation is unclear if ethdev port used to
> >> create
> >> -  a flow rule adds any implicit match criteria in the case of transfer rules.
> >> -  The semantics will be clarified in DPDK 21.11 and it will require
> >> fixes in
> >> -  drivers and applications which interpret it in a different way.
> >> -
> >>  * ethdev: The flow API matching pattern structures, ``struct
> >> rte_flow_item_*``,
> >>    should start with relevant protocol header.
> >>    Some matching pattern structures implements this by duplicating
> >> protocol header
> >> --
> >> 2.30.2