Re: [dpdk-dev] [PATCH] doc: clarify implicit filtering in transfer rules

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

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