[dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Andrew Rybchenko]

By its very name, action PORT_ID means that packets hit an ethdev with the
given DPDK port ID. At least the current comments don't state the opposite.
That said, since port representors had been adopted, applications like OvS
have been misusing the action. They misread its purpose as sending packets
to the opposite end of the "wire" plugged to the given ethdev, for example,
redirecting packets to the VF itself rather than to its representor ethdev.
Another example: OvS relies on this action with the admin PF's ethdev port
ID specified in it in order to send offloaded packets to the physical port.

Since there might be applications which use this action in its valid sense,
one can't just change the documentation to greenlight the opposite meaning.

The documentation must be clarified and rte_flow_action_port_id structure
should be extended to support both meanings.

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d9c0e65921..6e6413c89f 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -158,3 +158,8 @@ Deprecation Notices
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
+
+* ethdev: Definition of the flow API action PORT_ID is ambiguous and needs
+  clarification. Structure rte_flow_action_port_id will be extended to
+  specify traffic direction to represented entity or ethdev port itself in
+  DPDK 21.11.
-- 
2.30.2

[dpdk-dev] [PATCH 2/2] ethdev: announce clarification of implicit filter by port

[Andrew Rybchenko]

Transfer flow rules may be applied to traffic entering switch from
many sources. There are flow API pattern items which allow to specify
ingress port match criteria explicitly, but it is not documented
if ethdev port used to create flow rule adds any implicit match
criteria and how it coexists with explicit criterias.

These aspects should be documented and drivers and applications
which misuse it must be fixed.

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 6e6413c89f..4d174c8952 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -163,3 +163,8 @@ Deprecation Notices
   clarification. Structure rte_flow_action_port_id will be extended to
   specify traffic direction to 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.
-- 
2.30.2

Re: [dpdk-dev] [PATCH 2/2] ethdev: announce clarification of implicit filter by port

[Ori Kam]

Hi Andrew,

Acked-by: Ori Kam <orika@nvidia.com>

Best,
Ori

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Sunday, August 1, 2021 1:22 PM

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Eli Britstein]

On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
> External email: Use caution opening links or attachments
>
>
> By its very name, action PORT_ID means that packets hit an ethdev with the
> given DPDK port ID. At least the current comments don't state the opposite.
> That said, since port representors had been adopted, applications like OvS
> have been misusing the action. They misread its purpose as sending packets
> to the opposite end of the "wire" plugged to the given ethdev, for example,
> redirecting packets to the VF itself rather than to its representor ethdev.
> Another example: OvS relies on this action with the admin PF's ethdev port
> ID specified in it in order to send offloaded packets to the physical port.
>
> Since there might be applications which use this action in its valid sense,
> one can't just change the documentation to greenlight the opposite meaning.
>
> The documentation must be clarified and rte_flow_action_port_id structure
> should be extended to support both meanings.

I think the only clarification needed is that PORT_ID acts as if 
rte_eth_tx_burst is called with the specified port-id.

Regarding representors, it's not different. When using TX on a 
representor port, the packets appear as RX on its represented port.

Please elaborate if there is a use case for the PORT_ID~ in which the 
app can get the packets using rte_eth_rx_burst on the specified port-id.

>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> ---
>   doc/guides/rel_notes/deprecation.rst | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index d9c0e65921..6e6413c89f 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -158,3 +158,8 @@ Deprecation Notices
>   * security: The functions ``rte_security_set_pkt_metadata`` and
>     ``rte_security_get_userdata`` will be made inline functions and additional
>     flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
> +
> +* ethdev: Definition of the flow API action PORT_ID is ambiguous and needs
> +  clarification. Structure rte_flow_action_port_id will be extended to
> +  specify traffic direction to represented entity or ethdev port itself in
> +  DPDK 21.11.
> --
> 2.30.2
>

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Andrew Rybchenko]

On 8/1/21 1:57 PM, Eli Britstein wrote:
> 
> On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> By its very name, action PORT_ID means that packets hit an ethdev with 
>> the
>> given DPDK port ID. At least the current comments don't state the 
>> opposite.
>> That said, since port representors had been adopted, applications like 
>> OvS
>> have been misusing the action. They misread its purpose as sending 
>> packets
>> to the opposite end of the "wire" plugged to the given ethdev, for 
>> example,
>> redirecting packets to the VF itself rather than to its representor 
>> ethdev.
>> Another example: OvS relies on this action with the admin PF's ethdev 
>> port
>> ID specified in it in order to send offloaded packets to the physical 
>> port.
>>
>> Since there might be applications which use this action in its valid 
>> sense,
>> one can't just change the documentation to greenlight the opposite 
>> meaning.
>>
>> The documentation must be clarified and rte_flow_action_port_id structure
>> should be extended to support both meanings.
> 
> I think the only clarification needed is that PORT_ID acts as if 
> rte_eth_tx_burst is called with the specified port-id.

Sorry, but I still think that it is opposite meaning to the current
documentation which says "Directs matching traffic to a given DPDK port 
ID." Since it happens on switching level (transfer rule) "to a given
DPDK port" means that it will be received on a given DPDK port.

Anyway, the goal of the deprecation notice is to highlight that it must
be fixed and ensure that we can choose right decision even if it
breaks API/ABI.

> Regarding representors, it's not different. When using TX on a 
> representor port, the packets appear as RX on its represented port.
> 
> Please elaborate if there is a use case for the PORT_ID~ in which the 
> app can get the packets using rte_eth_rx_burst on the specified port-id.

Multi-home host with a NIC with two physical ports and two PFs used
by DPDK app with layer 3 (IP addresses). Different cores used to handle
traffic from different ports plus routing in DPDK app. If traffic to
port #0 IP address is received on phys port #1, it is useful to redirect
traffic to port ID 0 directly to have these packets on correct CPU cores
from the very beginning to avoid SW mechanisms to pass from port #1 CPU
cores to port #0 CPU cores.

>>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> ---
>>   doc/guides/rel_notes/deprecation.rst | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst 
>> b/doc/guides/rel_notes/deprecation.rst
>> index d9c0e65921..6e6413c89f 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -158,3 +158,8 @@ Deprecation Notices
>>   * security: The functions ``rte_security_set_pkt_metadata`` and
>>     ``rte_security_get_userdata`` will be made inline functions and 
>> additional
>>     flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
>> +
>> +* ethdev: Definition of the flow API action PORT_ID is ambiguous and 
>> needs
>> +  clarification. Structure rte_flow_action_port_id will be extended to
>> +  specify traffic direction to represented entity or ethdev port 
>> itself in
>> +  DPDK 21.11.
>> -- 
>> 2.30.2
>>

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Ori Kam]

Hi Andrew,

I think before we can change the API we must agree on the meaning of representor.

PSB more comments

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Sunday, August 1, 2021 3:04 PM
> To: Eli Britstein <elibr@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Ori Kam
> <orika@nvidia.com>
> Cc: dev@dpdk.org; Ilya Maximets <i.maximets@ovn.org>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Matan Azrad <matan@nvidia.com>; Ivan
> Malov <ivan.malov@oktetlabs.ru>; Viacheslav Galaktionov
> <viacheslav.galaktionov@oktetlabs.ru>
> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID changes
> 
> On 8/1/21 1:57 PM, Eli Britstein wrote:
> >
> > On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> By its very name, action PORT_ID means that packets hit an ethdev
> >> with the given DPDK port ID. At least the current comments don't
> >> state the opposite.
> >> That said, since port representors had been adopted, applications
> >> like OvS have been misusing the action. They misread its purpose as
> >> sending packets to the opposite end of the "wire" plugged to the
> >> given ethdev, for example, redirecting packets to the VF itself
> >> rather than to its representor ethdev.
> >> Another example: OvS relies on this action with the admin PF's ethdev
> >> port ID specified in it in order to send offloaded packets to the
> >> physical port.
> >>
> >> Since there might be applications which use this action in its valid
> >> sense, one can't just change the documentation to greenlight the
> >> opposite meaning.
> >>
> >> The documentation must be clarified and rte_flow_action_port_id
> >> structure should be extended to support both meanings.
> >
> > I think the only clarification needed is that PORT_ID acts as if
> > rte_eth_tx_burst is called with the specified port-id.
> 
> Sorry, but I still think that it is opposite meaning to the current
> documentation which says "Directs matching traffic to a given DPDK port ID."
> Since it happens on switching level (transfer rule) "to a given DPDK port"
> means that it will be received on a given DPDK port.
> 
> Anyway, the goal of the deprecation notice is to highlight that it must be
> fixed and ensure that we can choose right decision even if it breaks API/ABI.
> 
Agree, it is good that you created the announcement.
I think we should continue our discussion on what is a representor.
I think for current implementation the doc should say "direct / matches
traffic to / from the switch port which the selected DPDK representor port
is connected to or to DPDK port if this port is not a representor."
If we go this way there is no need to change the API only the doc.

> > Regarding representors, it's not different. When using TX on a
> > representor port, the packets appear as RX on its represented port.
> >
> > Please elaborate if there is a use case for the PORT_ID~ in which the
> > app can get the packets using rte_eth_rx_burst on the specified port-id.
> 
> Multi-home host with a NIC with two physical ports and two PFs used by
> DPDK app with layer 3 (IP addresses). Different cores used to handle traffic
> from different ports plus routing in DPDK app. If traffic to port #0 IP address
> is received on phys port #1, it is useful to redirect traffic to port ID 0 directly
> to have these packets on correct CPU cores from the very beginning to avoid
> SW mechanisms to pass from port #1 CPU cores to port #0 CPU cores.
> 
To make sure I understand you are talking about a DPDK application that
is connected to number of ports and it is Eswitch manager, but it doesn't use
representors but the actual ports, right?
I think the definition I wrote above also works for this case.


Best,
Ori

> >>
> >> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> ---
> >>   doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>   1 file changed, 5 insertions(+)
> >>
> >> diff --git a/doc/guides/rel_notes/deprecation.rst
> >> b/doc/guides/rel_notes/deprecation.rst
> >> index d9c0e65921..6e6413c89f 100644
> >> --- a/doc/guides/rel_notes/deprecation.rst
> >> +++ b/doc/guides/rel_notes/deprecation.rst
> >> @@ -158,3 +158,8 @@ Deprecation Notices
> >>   * security: The functions ``rte_security_set_pkt_metadata`` and
> >>     ``rte_security_get_userdata`` will be made inline functions and
> >> additional
> >>     flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
> >> +
> >> +* ethdev: Definition of the flow API action PORT_ID is ambiguous and
> >> needs
> >> +  clarification. Structure rte_flow_action_port_id will be extended
> >> +to
> >> +  specify traffic direction to represented entity or ethdev port
> >> itself in
> >> +  DPDK 21.11.
> >> --
> >> 2.30.2
> >>

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Andrew Rybchenko]

Hi Ori,

On 8/1/21 3:23 PM, Ori Kam wrote:
> Hi Andrew,
> 
> I think before we can change the API we must agree on the meaning of representor.

The question is not directly related to a representor definition.
Just indirectly. PORT_ID action makes sense for non-representor
ports as well.

> PSB more comments
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Sunday, August 1, 2021 3:04 PM
>> To: Eli Britstein <elibr@nvidia.com>; NBU-Contact-Thomas Monjalon
>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Ori Kam
>> <orika@nvidia.com>
>> Cc: dev@dpdk.org; Ilya Maximets <i.maximets@ovn.org>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>; Matan Azrad <matan@nvidia.com>; Ivan
>> Malov <ivan.malov@oktetlabs.ru>; Viacheslav Galaktionov
>> <viacheslav.galaktionov@oktetlabs.ru>
>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID changes
>>
>> On 8/1/21 1:57 PM, Eli Britstein wrote:
>>>
>>> On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> By its very name, action PORT_ID means that packets hit an ethdev
>>>> with the given DPDK port ID. At least the current comments don't
>>>> state the opposite.
>>>> That said, since port representors had been adopted, applications
>>>> like OvS have been misusing the action. They misread its purpose as
>>>> sending packets to the opposite end of the "wire" plugged to the
>>>> given ethdev, for example, redirecting packets to the VF itself
>>>> rather than to its representor ethdev.
>>>> Another example: OvS relies on this action with the admin PF's ethdev
>>>> port ID specified in it in order to send offloaded packets to the
>>>> physical port.
>>>>
>>>> Since there might be applications which use this action in its valid
>>>> sense, one can't just change the documentation to greenlight the
>>>> opposite meaning.
>>>>
>>>> The documentation must be clarified and rte_flow_action_port_id
>>>> structure should be extended to support both meanings.
>>>
>>> I think the only clarification needed is that PORT_ID acts as if
>>> rte_eth_tx_burst is called with the specified port-id.
>>
>> Sorry, but I still think that it is opposite meaning to the current
>> documentation which says "Directs matching traffic to a given DPDK port ID."
>> Since it happens on switching level (transfer rule) "to a given DPDK port"
>> means that it will be received on a given DPDK port.
>>
>> Anyway, the goal of the deprecation notice is to highlight that it must be
>> fixed and ensure that we can choose right decision even if it breaks API/ABI.
>>
> Agree, it is good that you created the announcement.

Hopefully you agree that the area requires clarification and must
be improved. I think so hot discussions really prove it.

> I think we should continue our discussion on what is a representor.

Yes, but it is a hard topic. I'd like to unbind PORT_ID action from
the discussion, since the action makes sense for non-representors
as well.

> I think for current implementation the doc should say "direct / matches
> traffic to / from the switch port which the selected DPDK representor port
> is connected to or to DPDK port if this port is not a representor."

IMHO it is better to keep the definition of the action simple and
do not have any representor specifics in it. Representor is an ethdev
port. If we direct traffic to an ethdev port, it should be received
on the ethdev port regardless if it is a representor or not.
It is better to avoid exceptions and special cases.

> If we go this way there is no need to change the API only the doc.
> 
>>> Regarding representors, it's not different. When using TX on a
>>> representor port, the packets appear as RX on its represented port.
>>>
>>> Please elaborate if there is a use case for the PORT_ID~ in which the
>>> app can get the packets using rte_eth_rx_burst on the specified port-id.
>>
>> Multi-home host with a NIC with two physical ports and two PFs used by
>> DPDK app with layer 3 (IP addresses). Different cores used to handle traffic
>> from different ports plus routing in DPDK app. If traffic to port #0 IP address
>> is received on phys port #1, it is useful to redirect traffic to port ID 0 directly
>> to have these packets on correct CPU cores from the very beginning to avoid
>> SW mechanisms to pass from port #1 CPU cores to port #0 CPU cores.
>>
> To make sure I understand you are talking about a DPDK application that
> is connected to number of ports and it is Eswitch manager, but it doesn't use
> representors but the actual ports, right?
> I think the definition I wrote above also works for this case.

Other possible request is to direct traffic from phys port #0
to phys port #1 directly and say it in terms of PORT_ID action.

Thanks,
Andrew.

> Best,
> Ori
> 
>>>>
>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> ---
>>>>    doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>> b/doc/guides/rel_notes/deprecation.rst
>>>> index d9c0e65921..6e6413c89f 100644
>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>> @@ -158,3 +158,8 @@ Deprecation Notices
>>>>    * security: The functions ``rte_security_set_pkt_metadata`` and
>>>>      ``rte_security_get_userdata`` will be made inline functions and
>>>> additional
>>>>      flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
>>>> +
>>>> +* ethdev: Definition of the flow API action PORT_ID is ambiguous and
>>>> needs
>>>> +  clarification. Structure rte_flow_action_port_id will be extended
>>>> +to
>>>> +  specify traffic direction to represented entity or ethdev port
>>>> itself in
>>>> +  DPDK 21.11.
>>>> --
>>>> 2.30.2
>>>>
> 

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Ori Kam]

Hi Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Sunday, August 1, 2021 3:44 PM
> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID changes
> 
> Hi Ori,
> 
> On 8/1/21 3:23 PM, Ori Kam wrote:
> > Hi Andrew,
> >
> > I think before we can change the API we must agree on the meaning of
> representor.
> 
> The question is not directly related to a representor definition.
> Just indirectly. PORT_ID action makes sense for non-representor ports as
> well.
> 
> > PSB more comments
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Sunday, August 1, 2021 3:04 PM
> >> To: Eli Britstein <elibr@nvidia.com>; NBU-Contact-Thomas Monjalon
> >> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Ori Kam
> >> <orika@nvidia.com>
> >> Cc: dev@dpdk.org; Ilya Maximets <i.maximets@ovn.org>; Ajit Khaparde
> >> <ajit.khaparde@broadcom.com>; Matan Azrad <matan@nvidia.com>;
> Ivan
> >> Malov <ivan.malov@oktetlabs.ru>; Viacheslav Galaktionov
> >> <viacheslav.galaktionov@oktetlabs.ru>
> >> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
> >> changes
> >>
> >> On 8/1/21 1:57 PM, Eli Britstein wrote:
> >>>
> >>> On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
> >>>> External email: Use caution opening links or attachments
> >>>>
> >>>>
> >>>> By its very name, action PORT_ID means that packets hit an ethdev
> >>>> with the given DPDK port ID. At least the current comments don't
> >>>> state the opposite.
> >>>> That said, since port representors had been adopted, applications
> >>>> like OvS have been misusing the action. They misread its purpose as
> >>>> sending packets to the opposite end of the "wire" plugged to the
> >>>> given ethdev, for example, redirecting packets to the VF itself
> >>>> rather than to its representor ethdev.
> >>>> Another example: OvS relies on this action with the admin PF's
> >>>> ethdev port ID specified in it in order to send offloaded packets
> >>>> to the physical port.
> >>>>
> >>>> Since there might be applications which use this action in its
> >>>> valid sense, one can't just change the documentation to greenlight
> >>>> the opposite meaning.
> >>>>
> >>>> The documentation must be clarified and rte_flow_action_port_id
> >>>> structure should be extended to support both meanings.
> >>>
> >>> I think the only clarification needed is that PORT_ID acts as if
> >>> rte_eth_tx_burst is called with the specified port-id.
> >>
> >> Sorry, but I still think that it is opposite meaning to the current
> >> documentation which says "Directs matching traffic to a given DPDK port
> ID."
> >> Since it happens on switching level (transfer rule) "to a given DPDK port"
> >> means that it will be received on a given DPDK port.
> >>
> >> Anyway, the goal of the deprecation notice is to highlight that it
> >> must be fixed and ensure that we can choose right decision even if it
> breaks API/ABI.
> >>
> > Agree, it is good that you created the announcement.
> 
> Hopefully you agree that the area requires clarification and must be
> improved. I think so hot discussions really prove it.
> 
+1

> > I think we should continue our discussion on what is a representor.
> 
> Yes, but it is a hard topic. I'd like to unbind PORT_ID action from the
> discussion, since the action makes sense for non-representors as well.
> 
If this can be done great, I'm for it, but I'm not sure it can be, but let's try.

> > I think for current implementation the doc should say "direct /
> > matches traffic to / from the switch port which the selected DPDK
> > representor port is connected to or to DPDK port if this port is not a
> representor."
> 
> IMHO it is better to keep the definition of the action simple and do not have
> any representor specifics in it. Representor is an ethdev port. If we direct
> traffic to an ethdev port, it should be received on the ethdev port regardless
> if it is a representor or not.
> It is better to avoid exceptions and special cases.
> 

Lets see if I understand correctly, you suggest that port  action / item will be
for DPDK port, unless they are marked with some bit which means that
the traffic should be routed to the switch port which the DPDK port represent
am I correct?

> > If we go this way there is no need to change the API only the doc.
> >
> >>> Regarding representors, it's not different. When using TX on a
> >>> representor port, the packets appear as RX on its represented port.
> >>>
> >>> Please elaborate if there is a use case for the PORT_ID~ in which
> >>> the app can get the packets using rte_eth_rx_burst on the specified
> port-id.
> >>
> >> Multi-home host with a NIC with two physical ports and two PFs used
> >> by DPDK app with layer 3 (IP addresses). Different cores used to
> >> handle traffic from different ports plus routing in DPDK app. If
> >> traffic to port #0 IP address is received on phys port #1, it is
> >> useful to redirect traffic to port ID 0 directly to have these
> >> packets on correct CPU cores from the very beginning to avoid SW
> mechanisms to pass from port #1 CPU cores to port #0 CPU cores.
> >>
> > To make sure I understand you are talking about a DPDK application
> > that is connected to number of ports and it is Eswitch manager, but it
> > doesn't use representors but the actual ports, right?
> > I think the definition I wrote above also works for this case.
> 
> Other possible request is to direct traffic from phys port #0 to phys port #1
> directly and say it in terms of PORT_ID action.
> 
But we are talking using the switch layer(transfer mode) right?

Best,
Ori
> Thanks,
> Andrew.
> 
> > Best,
> > Ori
> >
> >>>>
> >>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> ---
> >>>>    doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>    1 file changed, 5 insertions(+)
> >>>>
> >>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>> b/doc/guides/rel_notes/deprecation.rst
> >>>> index d9c0e65921..6e6413c89f 100644
> >>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>> @@ -158,3 +158,8 @@ Deprecation Notices
> >>>>    * security: The functions ``rte_security_set_pkt_metadata`` and
> >>>>      ``rte_security_get_userdata`` will be made inline functions
> >>>> and additional
> >>>>      flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
> >>>> +
> >>>> +* ethdev: Definition of the flow API action PORT_ID is ambiguous
> >>>> +and
> >>>> needs
> >>>> +  clarification. Structure rte_flow_action_port_id will be
> >>>> +extended to
> >>>> +  specify traffic direction to represented entity or ethdev port
> >>>> itself in
> >>>> +  DPDK 21.11.
> >>>> --
> >>>> 2.30.2
> >>>>
> >

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Andrew Rybchenko]

On 8/1/21 3:56 PM, Ori Kam wrote:
> Hi Andrew,
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Sunday, August 1, 2021 3:44 PM
>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID changes
>>
>> Hi Ori,
>>
>> On 8/1/21 3:23 PM, Ori Kam wrote:
>>> Hi Andrew,
>>>
>>> I think before we can change the API we must agree on the meaning of
>> representor.
>>
>> The question is not directly related to a representor definition.
>> Just indirectly. PORT_ID action makes sense for non-representor ports as
>> well.
>>
>>> PSB more comments
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Sunday, August 1, 2021 3:04 PM
>>>> To: Eli Britstein <elibr@nvidia.com>; NBU-Contact-Thomas Monjalon
>>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Ori Kam
>>>> <orika@nvidia.com>
>>>> Cc: dev@dpdk.org; Ilya Maximets <i.maximets@ovn.org>; Ajit Khaparde
>>>> <ajit.khaparde@broadcom.com>; Matan Azrad <matan@nvidia.com>;
>> Ivan
>>>> Malov <ivan.malov@oktetlabs.ru>; Viacheslav Galaktionov
>>>> <viacheslav.galaktionov@oktetlabs.ru>
>>>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
>>>> changes
>>>>
>>>> On 8/1/21 1:57 PM, Eli Britstein wrote:
>>>>>
>>>>> On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> By its very name, action PORT_ID means that packets hit an ethdev
>>>>>> with the given DPDK port ID. At least the current comments don't
>>>>>> state the opposite.
>>>>>> That said, since port representors had been adopted, applications
>>>>>> like OvS have been misusing the action. They misread its purpose as
>>>>>> sending packets to the opposite end of the "wire" plugged to the
>>>>>> given ethdev, for example, redirecting packets to the VF itself
>>>>>> rather than to its representor ethdev.
>>>>>> Another example: OvS relies on this action with the admin PF's
>>>>>> ethdev port ID specified in it in order to send offloaded packets
>>>>>> to the physical port.
>>>>>>
>>>>>> Since there might be applications which use this action in its
>>>>>> valid sense, one can't just change the documentation to greenlight
>>>>>> the opposite meaning.
>>>>>>
>>>>>> The documentation must be clarified and rte_flow_action_port_id
>>>>>> structure should be extended to support both meanings.
>>>>>
>>>>> I think the only clarification needed is that PORT_ID acts as if
>>>>> rte_eth_tx_burst is called with the specified port-id.
>>>>
>>>> Sorry, but I still think that it is opposite meaning to the current
>>>> documentation which says "Directs matching traffic to a given DPDK port
>> ID."
>>>> Since it happens on switching level (transfer rule) "to a given DPDK port"
>>>> means that it will be received on a given DPDK port.
>>>>
>>>> Anyway, the goal of the deprecation notice is to highlight that it
>>>> must be fixed and ensure that we can choose right decision even if it
>> breaks API/ABI.
>>>>
>>> Agree, it is good that you created the announcement.
>>
>> Hopefully you agree that the area requires clarification and must be
>> improved. I think so hot discussions really prove it.
>>
> +1
> 
>>> I think we should continue our discussion on what is a representor.
>>
>> Yes, but it is a hard topic. I'd like to unbind PORT_ID action from the
>> discussion, since the action makes sense for non-representors as well.
>>
> If this can be done great, I'm for it, but I'm not sure it can be, but let's try.
> 
>>> I think for current implementation the doc should say "direct /
>>> matches traffic to / from the switch port which the selected DPDK
>>> representor port is connected to or to DPDK port if this port is not a
>> representor."
>>
>> IMHO it is better to keep the definition of the action simple and do not have
>> any representor specifics in it. Representor is an ethdev port. If we direct
>> traffic to an ethdev port, it should be received on the ethdev port regardless
>> if it is a representor or not.
>> It is better to avoid exceptions and special cases.
>>
> 
> Lets see if I understand correctly, you suggest that port  action / item will be
> for DPDK port, unless they are marked with some bit which means that
> the traffic should be routed to the switch port which the DPDK port represent
> am I correct?

Here I'm talking about PORT_ID action only. As for details, I've tried
to keep it out-of-scope of the deprecation notice.

However, since we are going to break something here, it is better to
break hard to be sure that every since usage is updated. So, I tend to
to solution suggested by Ilya [1] which is similar to Linux kernel.
I.e. add an enum with invalid zero value and two members to specify
direction.

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

as for PORT_ID pattern item, I think ingress/egress attributes define
direction. If it is an ingress flow rule, PORT_ID item should match
traffic coming from represented entity in the case of port representor
and associated network port in the case of ethdev port associated with
it. In egress case it otherwise matches traffic sent using Tx burst via
corresponding ethdev port.

>>> If we go this way there is no need to change the API only the doc.
>>>
>>>>> Regarding representors, it's not different. When using TX on a
>>>>> representor port, the packets appear as RX on its represented port.
>>>>>
>>>>> Please elaborate if there is a use case for the PORT_ID~ in which
>>>>> the app can get the packets using rte_eth_rx_burst on the specified
>> port-id.
>>>>
>>>> Multi-home host with a NIC with two physical ports and two PFs used
>>>> by DPDK app with layer 3 (IP addresses). Different cores used to
>>>> handle traffic from different ports plus routing in DPDK app. If
>>>> traffic to port #0 IP address is received on phys port #1, it is
>>>> useful to redirect traffic to port ID 0 directly to have these
>>>> packets on correct CPU cores from the very beginning to avoid SW
>> mechanisms to pass from port #1 CPU cores to port #0 CPU cores.
>>>>
>>> To make sure I understand you are talking about a DPDK application
>>> that is connected to number of ports and it is Eswitch manager, but it
>>> doesn't use representors but the actual ports, right?
>>> I think the definition I wrote above also works for this case.
>>
>> Other possible request is to direct traffic from phys port #0 to phys port #1
>> directly and say it in terms of PORT_ID action.
>>
> But we are talking using the switch layer(transfer mode) right?

Yes.

> Best,
> Ori
>> Thanks,
>> Andrew.
>>
>>> Best,
>>> Ori
>>>
>>>>>>
>>>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>>>> ---
>>>>>>     doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>>     1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>> index d9c0e65921..6e6413c89f 100644
>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>> @@ -158,3 +158,8 @@ Deprecation Notices
>>>>>>     * security: The functions ``rte_security_set_pkt_metadata`` and
>>>>>>       ``rte_security_get_userdata`` will be made inline functions
>>>>>> and additional
>>>>>>       flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
>>>>>> +
>>>>>> +* ethdev: Definition of the flow API action PORT_ID is ambiguous
>>>>>> +and
>>>>>> needs
>>>>>> +  clarification. Structure rte_flow_action_port_id will be
>>>>>> +extended to
>>>>>> +  specify traffic direction to represented entity or ethdev port
>>>>>> itself in
>>>>>> +  DPDK 21.11.
>>>>>> --
>>>>>> 2.30.2
>>>>>>
>>>
> 

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Ori Kam]

Hi  Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Sunday, August 1, 2021 4:24 PM
> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID changes
> 
> On 8/1/21 3:56 PM, Ori Kam wrote:
> > Hi Andrew,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Sunday, August 1, 2021 3:44 PM
> >> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
> >> changes
> >>
> >> Hi Ori,
> >>
> >> On 8/1/21 3:23 PM, Ori Kam wrote:
> >>> Hi Andrew,
> >>>
> >>> I think before we can change the API we must agree on the meaning of
> >> representor.
> >>
> >> The question is not directly related to a representor definition.
> >> Just indirectly. PORT_ID action makes sense for non-representor ports
> >> as well.
> >>
> >>> PSB more comments
> >>>
> >>>> -----Original Message-----
> >>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> Sent: Sunday, August 1, 2021 3:04 PM
> >>>> To: Eli Britstein <elibr@nvidia.com>; NBU-Contact-Thomas Monjalon
> >>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Ori
> >>>> Kam <orika@nvidia.com>
> >>>> Cc: dev@dpdk.org; Ilya Maximets <i.maximets@ovn.org>; Ajit
> Khaparde
> >>>> <ajit.khaparde@broadcom.com>; Matan Azrad <matan@nvidia.com>;
> >> Ivan
> >>>> Malov <ivan.malov@oktetlabs.ru>; Viacheslav Galaktionov
> >>>> <viacheslav.galaktionov@oktetlabs.ru>
> >>>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
> >>>> changes
> >>>>
> >>>> On 8/1/21 1:57 PM, Eli Britstein wrote:
> >>>>>
> >>>>> On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
> >>>>>> External email: Use caution opening links or attachments
> >>>>>>
> >>>>>>
> >>>>>> By its very name, action PORT_ID means that packets hit an ethdev
> >>>>>> with the given DPDK port ID. At least the current comments don't
> >>>>>> state the opposite.
> >>>>>> That said, since port representors had been adopted, applications
> >>>>>> like OvS have been misusing the action. They misread its purpose
> >>>>>> as sending packets to the opposite end of the "wire" plugged to
> >>>>>> the given ethdev, for example, redirecting packets to the VF
> >>>>>> itself rather than to its representor ethdev.
> >>>>>> Another example: OvS relies on this action with the admin PF's
> >>>>>> ethdev port ID specified in it in order to send offloaded packets
> >>>>>> to the physical port.
> >>>>>>
> >>>>>> Since there might be applications which use this action in its
> >>>>>> valid sense, one can't just change the documentation to
> >>>>>> greenlight the opposite meaning.
> >>>>>>
> >>>>>> The documentation must be clarified and rte_flow_action_port_id
> >>>>>> structure should be extended to support both meanings.
> >>>>>
> >>>>> I think the only clarification needed is that PORT_ID acts as if
> >>>>> rte_eth_tx_burst is called with the specified port-id.
> >>>>
> >>>> Sorry, but I still think that it is opposite meaning to the current
> >>>> documentation which says "Directs matching traffic to a given DPDK
> >>>> port
> >> ID."
> >>>> Since it happens on switching level (transfer rule) "to a given DPDK
> port"
> >>>> means that it will be received on a given DPDK port.
> >>>>
> >>>> Anyway, the goal of the deprecation notice is to highlight that it
> >>>> must be fixed and ensure that we can choose right decision even if
> >>>> it
> >> breaks API/ABI.
> >>>>
> >>> Agree, it is good that you created the announcement.
> >>
> >> Hopefully you agree that the area requires clarification and must be
> >> improved. I think so hot discussions really prove it.
> >>
> > +1
> >
> >>> I think we should continue our discussion on what is a representor.
> >>
> >> Yes, but it is a hard topic. I'd like to unbind PORT_ID action from
> >> the discussion, since the action makes sense for non-representors as well.
> >>
> > If this can be done great, I'm for it, but I'm not sure it can be, but let's try.
> >
> >>> I think for current implementation the doc should say "direct /
> >>> matches traffic to / from the switch port which the selected DPDK
> >>> representor port is connected to or to DPDK port if this port is not
> >>> a
> >> representor."
> >>
> >> IMHO it is better to keep the definition of the action simple and do
> >> not have any representor specifics in it. Representor is an ethdev
> >> port. If we direct traffic to an ethdev port, it should be received
> >> on the ethdev port regardless if it is a representor or not.
> >> It is better to avoid exceptions and special cases.
> >>
> >
> > Lets see if I understand correctly, you suggest that port  action /
> > item will be for DPDK port, unless they are marked with some bit which
> > means that the traffic should be routed to the switch port which the
> > DPDK port represent am I correct?
> 
> Here I'm talking about PORT_ID action only. As for details, I've tried to keep it
> out-of-scope of the deprecation notice.
> 
+1 but we need to check if we need it at all or just change doc.

> However, since we are going to break something here, it is better to break
> hard to be sure that every since usage is updated. So, I tend to to solution
> suggested by Ilya [1] which is similar to Linux kernel.
> I.e. add an enum with invalid zero value and two members to specify
> direction.
> 
> [1]
> https://patches.dpdk.org/project/dpdk/patch/20210601111420.5549-1-
> ivan.malov@oktetlabs.ru/#133431
> 
> as for PORT_ID pattern item, I think ingress/egress attributes define
> direction. If it is an ingress flow rule, PORT_ID item should match traffic
> coming from represented entity in the case of port representor and
> associated network port in the case of ethdev port associated with it. In
> egress case it otherwise matches traffic sent using Tx burst via corresponding
> ethdev port.
> 
I think that Ingress egress has only meaning when talking about NIC steering
and not E-Switch steering.
I think that we can just use original bit to mark if we want to send traffic
to DPDK port or to other port.

In any case I will be happy if we could have a meeting to discuss this
approach before sending your patch. 
I think this can save a lot of time.

Best,
Ori


> >>> If we go this way there is no need to change the API only the doc.
> >>>
> >>>>> Regarding representors, it's not different. When using TX on a
> >>>>> representor port, the packets appear as RX on its represented port.
> >>>>>
> >>>>> Please elaborate if there is a use case for the PORT_ID~ in which
> >>>>> the app can get the packets using rte_eth_rx_burst on the
> >>>>> specified
> >> port-id.
> >>>>
> >>>> Multi-home host with a NIC with two physical ports and two PFs used
> >>>> by DPDK app with layer 3 (IP addresses). Different cores used to
> >>>> handle traffic from different ports plus routing in DPDK app. If
> >>>> traffic to port #0 IP address is received on phys port #1, it is
> >>>> useful to redirect traffic to port ID 0 directly to have these
> >>>> packets on correct CPU cores from the very beginning to avoid SW
> >> mechanisms to pass from port #1 CPU cores to port #0 CPU cores.
> >>>>
> >>> To make sure I understand you are talking about a DPDK application
> >>> that is connected to number of ports and it is Eswitch manager, but
> >>> it doesn't use representors but the actual ports, right?
> >>> I think the definition I wrote above also works for this case.
> >>
> >> Other possible request is to direct traffic from phys port #0 to phys
> >> port #1 directly and say it in terms of PORT_ID action.
> >>
> > But we are talking using the switch layer(transfer mode) right?
> 
> Yes.
> 
> > Best,
> > Ori
> >> Thanks,
> >> Andrew.
> >>
> >>> Best,
> >>> Ori
> >>>
> >>>>>>
> >>>>>> Signed-off-by: Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>
> >>>>>> ---
> >>>>>>     doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>>>     1 file changed, 5 insertions(+)
> >>>>>>
> >>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>> index d9c0e65921..6e6413c89f 100644
> >>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>> @@ -158,3 +158,8 @@ Deprecation Notices
> >>>>>>     * security: The functions ``rte_security_set_pkt_metadata`` and
> >>>>>>       ``rte_security_get_userdata`` will be made inline functions
> >>>>>> and additional
> >>>>>>       flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
> >>>>>> +
> >>>>>> +* ethdev: Definition of the flow API action PORT_ID is ambiguous
> >>>>>> +and
> >>>>>> needs
> >>>>>> +  clarification. Structure rte_flow_action_port_id will be
> >>>>>> +extended to
> >>>>>> +  specify traffic direction to represented entity or ethdev port
> >>>>>> itself in
> >>>>>> +  DPDK 21.11.
> >>>>>> --
> >>>>>> 2.30.2
> >>>>>>
> >>>
> >

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Andrew Rybchenko]

On 8/1/21 7:13 PM, Ori Kam wrote:
> Hi  Andrew,
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Sunday, August 1, 2021 4:24 PM
>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID changes
>>
>> On 8/1/21 3:56 PM, Ori Kam wrote:
>>> Hi Andrew,
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Sunday, August 1, 2021 3:44 PM
>>>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
>>>> changes
>>>>
>>>> Hi Ori,
>>>>
>>>> On 8/1/21 3:23 PM, Ori Kam wrote:
>>>>> Hi Andrew,
>>>>>
>>>>> I think before we can change the API we must agree on the meaning of
>>>> representor.
>>>>
>>>> The question is not directly related to a representor definition.
>>>> Just indirectly. PORT_ID action makes sense for non-representor ports
>>>> as well.
>>>>
>>>>> PSB more comments
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>>>> Sent: Sunday, August 1, 2021 3:04 PM
>>>>>> To: Eli Britstein <elibr@nvidia.com>; NBU-Contact-Thomas Monjalon
>>>>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Ori
>>>>>> Kam <orika@nvidia.com>
>>>>>> Cc: dev@dpdk.org; Ilya Maximets <i.maximets@ovn.org>; Ajit
>> Khaparde
>>>>>> <ajit.khaparde@broadcom.com>; Matan Azrad <matan@nvidia.com>;
>>>> Ivan
>>>>>> Malov <ivan.malov@oktetlabs.ru>; Viacheslav Galaktionov
>>>>>> <viacheslav.galaktionov@oktetlabs.ru>
>>>>>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
>>>>>> changes
>>>>>>
>>>>>> On 8/1/21 1:57 PM, Eli Britstein wrote:
>>>>>>>
>>>>>>> On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>>
>>>>>>>>
>>>>>>>> By its very name, action PORT_ID means that packets hit an ethdev
>>>>>>>> with the given DPDK port ID. At least the current comments don't
>>>>>>>> state the opposite.
>>>>>>>> That said, since port representors had been adopted, applications
>>>>>>>> like OvS have been misusing the action. They misread its purpose
>>>>>>>> as sending packets to the opposite end of the "wire" plugged to
>>>>>>>> the given ethdev, for example, redirecting packets to the VF
>>>>>>>> itself rather than to its representor ethdev.
>>>>>>>> Another example: OvS relies on this action with the admin PF's
>>>>>>>> ethdev port ID specified in it in order to send offloaded packets
>>>>>>>> to the physical port.
>>>>>>>>
>>>>>>>> Since there might be applications which use this action in its
>>>>>>>> valid sense, one can't just change the documentation to
>>>>>>>> greenlight the opposite meaning.
>>>>>>>>
>>>>>>>> The documentation must be clarified and rte_flow_action_port_id
>>>>>>>> structure should be extended to support both meanings.
>>>>>>>
>>>>>>> I think the only clarification needed is that PORT_ID acts as if
>>>>>>> rte_eth_tx_burst is called with the specified port-id.
>>>>>>
>>>>>> Sorry, but I still think that it is opposite meaning to the current
>>>>>> documentation which says "Directs matching traffic to a given DPDK
>>>>>> port
>>>> ID."
>>>>>> Since it happens on switching level (transfer rule) "to a given DPDK
>> port"
>>>>>> means that it will be received on a given DPDK port.
>>>>>>
>>>>>> Anyway, the goal of the deprecation notice is to highlight that it
>>>>>> must be fixed and ensure that we can choose right decision even if
>>>>>> it
>>>> breaks API/ABI.
>>>>>>
>>>>> Agree, it is good that you created the announcement.
>>>>
>>>> Hopefully you agree that the area requires clarification and must be
>>>> improved. I think so hot discussions really prove it.
>>>>
>>> +1
>>>
>>>>> I think we should continue our discussion on what is a representor.
>>>>
>>>> Yes, but it is a hard topic. I'd like to unbind PORT_ID action from
>>>> the discussion, since the action makes sense for non-representors as well.
>>>>
>>> If this can be done great, I'm for it, but I'm not sure it can be, but let's try.
>>>
>>>>> I think for current implementation the doc should say "direct /
>>>>> matches traffic to / from the switch port which the selected DPDK
>>>>> representor port is connected to or to DPDK port if this port is not
>>>>> a
>>>> representor."
>>>>
>>>> IMHO it is better to keep the definition of the action simple and do
>>>> not have any representor specifics in it. Representor is an ethdev
>>>> port. If we direct traffic to an ethdev port, it should be received
>>>> on the ethdev port regardless if it is a representor or not.
>>>> It is better to avoid exceptions and special cases.
>>>>
>>>
>>> Lets see if I understand correctly, you suggest that port  action /
>>> item will be for DPDK port, unless they are marked with some bit which
>>> means that the traffic should be routed to the switch port which the
>>> DPDK port represent am I correct?
>>
>> Here I'm talking about PORT_ID action only. As for details, I've tried to keep it
>> out-of-scope of the deprecation notice.
>>
> +1 but we need to check if we need it at all or just change doc.
> 
>> However, since we are going to break something here, it is better to break
>> hard to be sure that every since usage is updated. So, I tend to to solution
>> suggested by Ilya [1] which is similar to Linux kernel.
>> I.e. add an enum with invalid zero value and two members to specify
>> direction.
>>
>> [1]
>> https://patches.dpdk.org/project/dpdk/patch/20210601111420.5549-1-
>> ivan.malov@oktetlabs.ru/#133431
>>
>> as for PORT_ID pattern item, I think ingress/egress attributes define
>> direction. If it is an ingress flow rule, PORT_ID item should match traffic
>> coming from represented entity in the case of port representor and
>> associated network port in the case of ethdev port associated with it. In
>> egress case it otherwise matches traffic sent using Tx burst via corresponding
>> ethdev port.
>>
> I think that Ingress egress has only meaning when talking about NIC steering
> and not E-Switch steering.

See [2]  12.2.2.4. Attribute: Transfer last paragraph.

[2] https://doc.dpdk.org/guides/prog_guide/rte_flow.html#attributes

In fact I was going to submit one more deprecation notice on the topic
to clarify it, but reread the documentation and now think that it is
good enough.

> I think that we can just use original bit to mark if we want to send traffic
> to DPDK port or to other port.

As I say the problem of the solution is that a silent breakage.
It is typically bad since  old code can simply misuse it.

> In any case I will be happy if we could have a meeting to discuss this
> approach before sending your patch.

Please, let the deprecation notice in. In whatever direction we fix it,
we'll break something in any case and DPDK users must be warned in
advance. We either change definition of the action or change support
of the action in drivers (in different ways in different drivers) or
do both.

> I think this can save a lot of time.

It is a good idea, let's schedule to the end of August. I guess many
of us have vacations now or in the nearest time. It will be simply
hard to find time in the nearest 3 weeks which is good for all or
at least majority of us.

Thanks,
Andrew.

> Best,
> Ori
> 
> 
>>>>> If we go this way there is no need to change the API only the doc.
>>>>>
>>>>>>> Regarding representors, it's not different. When using TX on a
>>>>>>> representor port, the packets appear as RX on its represented port.
>>>>>>>
>>>>>>> Please elaborate if there is a use case for the PORT_ID~ in which
>>>>>>> the app can get the packets using rte_eth_rx_burst on the
>>>>>>> specified
>>>> port-id.
>>>>>>
>>>>>> Multi-home host with a NIC with two physical ports and two PFs used
>>>>>> by DPDK app with layer 3 (IP addresses). Different cores used to
>>>>>> handle traffic from different ports plus routing in DPDK app. If
>>>>>> traffic to port #0 IP address is received on phys port #1, it is
>>>>>> useful to redirect traffic to port ID 0 directly to have these
>>>>>> packets on correct CPU cores from the very beginning to avoid SW
>>>> mechanisms to pass from port #1 CPU cores to port #0 CPU cores.
>>>>>>
>>>>> To make sure I understand you are talking about a DPDK application
>>>>> that is connected to number of ports and it is Eswitch manager, but
>>>>> it doesn't use representors but the actual ports, right?
>>>>> I think the definition I wrote above also works for this case.
>>>>
>>>> Other possible request is to direct traffic from phys port #0 to phys
>>>> port #1 directly and say it in terms of PORT_ID action.
>>>>
>>> But we are talking using the switch layer(transfer mode) right?
>>
>> Yes.
>>
>>> Best,
>>> Ori
>>>> Thanks,
>>>> Andrew.
>>>>
>>>>> Best,
>>>>> Ori
>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>
>>>>>>>> ---
>>>>>>>>      doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>>>>      1 file changed, 5 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>>> index d9c0e65921..6e6413c89f 100644
>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>>> @@ -158,3 +158,8 @@ Deprecation Notices
>>>>>>>>      * security: The functions ``rte_security_set_pkt_metadata`` and
>>>>>>>>        ``rte_security_get_userdata`` will be made inline functions
>>>>>>>> and additional
>>>>>>>>        flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
>>>>>>>> +
>>>>>>>> +* ethdev: Definition of the flow API action PORT_ID is ambiguous
>>>>>>>> +and
>>>>>>>> needs
>>>>>>>> +  clarification. Structure rte_flow_action_port_id will be
>>>>>>>> +extended to
>>>>>>>> +  specify traffic direction to represented entity or ethdev port
>>>>>>>> itself in
>>>>>>>> +  DPDK 21.11.
>>>>>>>> --
>>>>>>>> 2.30.2
>>>>>>>>
>>>>>
>>>
> 

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Ori Kam]

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> On 8/1/21 7:13 PM, Ori Kam wrote:
> > Hi  Andrew,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Sunday, August 1, 2021 4:24 PM
> >> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
> >> changes
> >>
> >> On 8/1/21 3:56 PM, Ori Kam wrote:
> >>> Hi Andrew,
> >>>
> >>>> -----Original Message-----
> >>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> Sent: Sunday, August 1, 2021 3:44 PM
> >>>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
> >>>> changes
> >>>>
> >>>> Hi Ori,
> >>>>
> >>>> On 8/1/21 3:23 PM, Ori Kam wrote:
> >>>>> Hi Andrew,
> >>>>>
> >>>>> I think before we can change the API we must agree on the meaning
> >>>>> of
> >>>> representor.
> >>>>
> >>>> The question is not directly related to a representor definition.
> >>>> Just indirectly. PORT_ID action makes sense for non-representor
> >>>> ports as well.
> >>>>
> >>>>> PSB more comments
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>>>> Sent: Sunday, August 1, 2021 3:04 PM
> >>>>>> To: Eli Britstein <elibr@nvidia.com>; NBU-Contact-Thomas Monjalon
> >>>>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Ori
> >>>>>> Kam <orika@nvidia.com>
> >>>>>> Cc: dev@dpdk.org; Ilya Maximets <i.maximets@ovn.org>; Ajit
> >> Khaparde
> >>>>>> <ajit.khaparde@broadcom.com>; Matan Azrad
> <matan@nvidia.com>;
> >>>> Ivan
> >>>>>> Malov <ivan.malov@oktetlabs.ru>; Viacheslav Galaktionov
> >>>>>> <viacheslav.galaktionov@oktetlabs.ru>
> >>>>>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
> >>>>>> changes
> >>>>>>
> >>>>>> On 8/1/21 1:57 PM, Eli Britstein wrote:
> >>>>>>>
> >>>>>>> On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
> >>>>>>>> External email: Use caution opening links or attachments
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> By its very name, action PORT_ID means that packets hit an
> >>>>>>>> ethdev with the given DPDK port ID. At least the current
> >>>>>>>> comments don't state the opposite.
> >>>>>>>> That said, since port representors had been adopted,
> >>>>>>>> applications like OvS have been misusing the action. They
> >>>>>>>> misread its purpose as sending packets to the opposite end of
> >>>>>>>> the "wire" plugged to the given ethdev, for example,
> >>>>>>>> redirecting packets to the VF itself rather than to its representor
> ethdev.
> >>>>>>>> Another example: OvS relies on this action with the admin PF's
> >>>>>>>> ethdev port ID specified in it in order to send offloaded
> >>>>>>>> packets to the physical port.
> >>>>>>>>
> >>>>>>>> Since there might be applications which use this action in its
> >>>>>>>> valid sense, one can't just change the documentation to
> >>>>>>>> greenlight the opposite meaning.
> >>>>>>>>
> >>>>>>>> The documentation must be clarified and rte_flow_action_port_id
> >>>>>>>> structure should be extended to support both meanings.
> >>>>>>>
> >>>>>>> I think the only clarification needed is that PORT_ID acts as if
> >>>>>>> rte_eth_tx_burst is called with the specified port-id.
> >>>>>>
> >>>>>> Sorry, but I still think that it is opposite meaning to the
> >>>>>> current documentation which says "Directs matching traffic to a
> >>>>>> given DPDK port
> >>>> ID."
> >>>>>> Since it happens on switching level (transfer rule) "to a given
> >>>>>> DPDK
> >> port"
> >>>>>> means that it will be received on a given DPDK port.
> >>>>>>
> >>>>>> Anyway, the goal of the deprecation notice is to highlight that
> >>>>>> it must be fixed and ensure that we can choose right decision
> >>>>>> even if it
> >>>> breaks API/ABI.
> >>>>>>
> >>>>> Agree, it is good that you created the announcement.
> >>>>
> >>>> Hopefully you agree that the area requires clarification and must
> >>>> be improved. I think so hot discussions really prove it.
> >>>>
> >>> +1
> >>>
> >>>>> I think we should continue our discussion on what is a representor.
> >>>>
> >>>> Yes, but it is a hard topic. I'd like to unbind PORT_ID action from
> >>>> the discussion, since the action makes sense for non-representors as
> well.
> >>>>
> >>> If this can be done great, I'm for it, but I'm not sure it can be, but let's
> try.
> >>>
> >>>>> I think for current implementation the doc should say "direct /
> >>>>> matches traffic to / from the switch port which the selected DPDK
> >>>>> representor port is connected to or to DPDK port if this port is
> >>>>> not a
> >>>> representor."
> >>>>
> >>>> IMHO it is better to keep the definition of the action simple and
> >>>> do not have any representor specifics in it. Representor is an
> >>>> ethdev port. If we direct traffic to an ethdev port, it should be
> >>>> received on the ethdev port regardless if it is a representor or not.
> >>>> It is better to avoid exceptions and special cases.
> >>>>
> >>>
> >>> Lets see if I understand correctly, you suggest that port  action /
> >>> item will be for DPDK port, unless they are marked with some bit
> >>> which means that the traffic should be routed to the switch port
> >>> which the DPDK port represent am I correct?
> >>
> >> Here I'm talking about PORT_ID action only. As for details, I've
> >> tried to keep it out-of-scope of the deprecation notice.
> >>
> > +1 but we need to check if we need it at all or just change doc.
> >
> >> However, since we are going to break something here, it is better to
> >> break hard to be sure that every since usage is updated. So, I tend
> >> to to solution suggested by Ilya [1] which is similar to Linux kernel.
> >> I.e. add an enum with invalid zero value and two members to specify
> >> direction.
> >>
> >> [1]
> >> https://patches.dpdk.org/project/dpdk/patch/20210601111420.5549-1-
> >> ivan.malov@oktetlabs.ru/#133431
> >>
> >> as for PORT_ID pattern item, I think ingress/egress attributes define
> >> direction. If it is an ingress flow rule, PORT_ID item should match
> >> traffic coming from represented entity in the case of port
> >> representor and associated network port in the case of ethdev port
> >> associated with it. In egress case it otherwise matches traffic sent
> >> using Tx burst via corresponding ethdev port.
> >>
> > I think that Ingress egress has only meaning when talking about NIC
> > steering and not E-Switch steering.
> 
> See [2]  12.2.2.4. Attribute: Transfer last paragraph.
> 
> [2] https://doc.dpdk.org/guides/prog_guide/rte_flow.html#attributes
> 
> In fact I was going to submit one more deprecation notice on the topic to
> clarify it, but reread the documentation and now think that it is good enough.
> 

I think this needs to change, 
" 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."
But if we route traffic between vports was is the app direction?
For example if sending traffic from VF A to VF B (app is on PF)
is it ingress or egress traffic? If the direction is reverse (B to A) does it change?
what if we are sending traffic from VF A to wire or from wire to A what is ingress / egress?
(Assuming that the VFs are connected to different application.)



> > I think that we can just use original bit to mark if we want to send
> > traffic to DPDK port or to other port.
> 
> As I say the problem of the solution is that a silent breakage.
> It is typically bad since  old code can simply misuse it.
> 
You have a point but then maybe we should also delete this bit.
Also I don't like the idea to break almost all apps that are using DPDK.
especially if it will not cause error on build.
just adding more fields will break the app logic not the compilation which
I think is the worst thing. (large number of application are based on
the current logic)

> > In any case I will be happy if we could have a meeting to discuss this
> > approach before sending your patch.
> 
> Please, let the deprecation notice in. In whatever direction we fix it, we'll
> break something in any case and DPDK users must be warned in advance.
> We either change definition of the action or change support of the action in
> drivers (in different ways in different drivers) or do both.

O.K.
> 
> > I think this can save a lot of time.
> 
> It is a good idea, let's schedule to the end of August. I guess many of us have
> vacations now or in the nearest time. It will be simply hard to find time in the
> nearest 3 weeks which is good for all or at least majority of us.
> 

Sure.
Best,
Ori
> Thanks,
> Andrew.
> 
> > Best,
> > Ori
> >
> >
> >>>>> If we go this way there is no need to change the API only the doc.
> >>>>>
> >>>>>>> Regarding representors, it's not different. When using TX on a
> >>>>>>> representor port, the packets appear as RX on its represented port.
> >>>>>>>
> >>>>>>> Please elaborate if there is a use case for the PORT_ID~ in
> >>>>>>> which the app can get the packets using rte_eth_rx_burst on the
> >>>>>>> specified
> >>>> port-id.
> >>>>>>
> >>>>>> Multi-home host with a NIC with two physical ports and two PFs
> >>>>>> used by DPDK app with layer 3 (IP addresses). Different cores
> >>>>>> used to handle traffic from different ports plus routing in DPDK
> >>>>>> app. If traffic to port #0 IP address is received on phys port
> >>>>>> #1, it is useful to redirect traffic to port ID 0 directly to
> >>>>>> have these packets on correct CPU cores from the very beginning
> >>>>>> to avoid SW
> >>>> mechanisms to pass from port #1 CPU cores to port #0 CPU cores.
> >>>>>>
> >>>>> To make sure I understand you are talking about a DPDK application
> >>>>> that is connected to number of ports and it is Eswitch manager,
> >>>>> but it doesn't use representors but the actual ports, right?
> >>>>> I think the definition I wrote above also works for this case.
> >>>>
> >>>> Other possible request is to direct traffic from phys port #0 to
> >>>> phys port #1 directly and say it in terms of PORT_ID action.
> >>>>
> >>> But we are talking using the switch layer(transfer mode) right?
> >>
> >> Yes.
> >>
> >>> Best,
> >>> Ori
> >>>> Thanks,
> >>>> Andrew.
> >>>>
> >>>>> Best,
> >>>>> Ori
> >>>>>
> >>>>>>>>
> >>>>>>>> Signed-off-by: Andrew Rybchenko
> >> <andrew.rybchenko@oktetlabs.ru>
> >>>>>>>> ---
> >>>>>>>>      doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>>>>>      1 file changed, 5 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> index d9c0e65921..6e6413c89f 100644
> >>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> @@ -158,3 +158,8 @@ Deprecation Notices
> >>>>>>>>      * security: The functions ``rte_security_set_pkt_metadata`` and
> >>>>>>>>        ``rte_security_get_userdata`` will be made inline
> >>>>>>>> functions and additional
> >>>>>>>>        flags will be added in structure ``rte_security_ctx`` in DPDK
> 21.11.
> >>>>>>>> +
> >>>>>>>> +* ethdev: Definition of the flow API action PORT_ID is
> >>>>>>>> +ambiguous and
> >>>>>>>> needs
> >>>>>>>> +  clarification. Structure rte_flow_action_port_id will be
> >>>>>>>> +extended to
> >>>>>>>> +  specify traffic direction to represented entity or ethdev
> >>>>>>>> +port
> >>>>>>>> itself in
> >>>>>>>> +  DPDK 21.11.
> >>>>>>>> --
> >>>>>>>> 2.30.2
> >>>>>>>>
> >>>>>
> >>>
> >

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Andrew Rybchenko]

Hi Ori,

On 8/2/21 10:28 AM, Ori Kam wrote:
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>
>> On 8/1/21 7:13 PM, Ori Kam wrote:
>>> Hi  Andrew,
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Sunday, August 1, 2021 4:24 PM
>>>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
>>>> changes
>>>>
>>>> On 8/1/21 3:56 PM, Ori Kam wrote:
>>>>> Hi Andrew,
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>>>> Sent: Sunday, August 1, 2021 3:44 PM
>>>>>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
>>>>>> changes
>>>>>>
>>>>>> Hi Ori,
>>>>>>
>>>>>> On 8/1/21 3:23 PM, Ori Kam wrote:
>>>>>>> Hi Andrew,
>>>>>>>
>>>>>>> I think before we can change the API we must agree on the meaning
>>>>>>> of
>>>>>> representor.
>>>>>>
>>>>>> The question is not directly related to a representor definition.
>>>>>> Just indirectly. PORT_ID action makes sense for non-representor
>>>>>> ports as well.
>>>>>>
>>>>>>> PSB more comments
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>>>>>> Sent: Sunday, August 1, 2021 3:04 PM
>>>>>>>> To: Eli Britstein <elibr@nvidia.com>; NBU-Contact-Thomas Monjalon
>>>>>>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Ori
>>>>>>>> Kam <orika@nvidia.com>
>>>>>>>> Cc: dev@dpdk.org; Ilya Maximets <i.maximets@ovn.org>; Ajit
>>>> Khaparde
>>>>>>>> <ajit.khaparde@broadcom.com>; Matan Azrad
>> <matan@nvidia.com>;
>>>>>> Ivan
>>>>>>>> Malov <ivan.malov@oktetlabs.ru>; Viacheslav Galaktionov
>>>>>>>> <viacheslav.galaktionov@oktetlabs.ru>
>>>>>>>> Subject: Re: [PATCH 1/2] ethdev: announce flow API action PORT_ID
>>>>>>>> changes
>>>>>>>>
>>>>>>>> On 8/1/21 1:57 PM, Eli Britstein wrote:
>>>>>>>>>
>>>>>>>>> On 8/1/2021 1:22 PM, Andrew Rybchenko wrote:
>>>>>>>>>> External email: Use caution opening links or attachments
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> By its very name, action PORT_ID means that packets hit an
>>>>>>>>>> ethdev with the given DPDK port ID. At least the current
>>>>>>>>>> comments don't state the opposite.
>>>>>>>>>> That said, since port representors had been adopted,
>>>>>>>>>> applications like OvS have been misusing the action. They
>>>>>>>>>> misread its purpose as sending packets to the opposite end of
>>>>>>>>>> the "wire" plugged to the given ethdev, for example,
>>>>>>>>>> redirecting packets to the VF itself rather than to its representor
>> ethdev.
>>>>>>>>>> Another example: OvS relies on this action with the admin PF's
>>>>>>>>>> ethdev port ID specified in it in order to send offloaded
>>>>>>>>>> packets to the physical port.
>>>>>>>>>>
>>>>>>>>>> Since there might be applications which use this action in its
>>>>>>>>>> valid sense, one can't just change the documentation to
>>>>>>>>>> greenlight the opposite meaning.
>>>>>>>>>>
>>>>>>>>>> The documentation must be clarified and rte_flow_action_port_id
>>>>>>>>>> structure should be extended to support both meanings.
>>>>>>>>>
>>>>>>>>> I think the only clarification needed is that PORT_ID acts as if
>>>>>>>>> rte_eth_tx_burst is called with the specified port-id.
>>>>>>>>
>>>>>>>> Sorry, but I still think that it is opposite meaning to the
>>>>>>>> current documentation which says "Directs matching traffic to a
>>>>>>>> given DPDK port
>>>>>> ID."
>>>>>>>> Since it happens on switching level (transfer rule) "to a given
>>>>>>>> DPDK
>>>> port"
>>>>>>>> means that it will be received on a given DPDK port.
>>>>>>>>
>>>>>>>> Anyway, the goal of the deprecation notice is to highlight that
>>>>>>>> it must be fixed and ensure that we can choose right decision
>>>>>>>> even if it
>>>>>> breaks API/ABI.
>>>>>>>>
>>>>>>> Agree, it is good that you created the announcement.
>>>>>>
>>>>>> Hopefully you agree that the area requires clarification and must
>>>>>> be improved. I think so hot discussions really prove it.
>>>>>>
>>>>> +1
>>>>>
>>>>>>> I think we should continue our discussion on what is a representor.
>>>>>>
>>>>>> Yes, but it is a hard topic. I'd like to unbind PORT_ID action from
>>>>>> the discussion, since the action makes sense for non-representors as
>> well.
>>>>>>
>>>>> If this can be done great, I'm for it, but I'm not sure it can be, but let's
>> try.
>>>>>
>>>>>>> I think for current implementation the doc should say "direct /
>>>>>>> matches traffic to / from the switch port which the selected DPDK
>>>>>>> representor port is connected to or to DPDK port if this port is
>>>>>>> not a
>>>>>> representor."
>>>>>>
>>>>>> IMHO it is better to keep the definition of the action simple and
>>>>>> do not have any representor specifics in it. Representor is an
>>>>>> ethdev port. If we direct traffic to an ethdev port, it should be
>>>>>> received on the ethdev port regardless if it is a representor or not.
>>>>>> It is better to avoid exceptions and special cases.
>>>>>>
>>>>>
>>>>> Lets see if I understand correctly, you suggest that port  action /
>>>>> item will be for DPDK port, unless they are marked with some bit
>>>>> which means that the traffic should be routed to the switch port
>>>>> which the DPDK port represent am I correct?
>>>>
>>>> Here I'm talking about PORT_ID action only. As for details, I've
>>>> tried to keep it out-of-scope of the deprecation notice.
>>>>
>>> +1 but we need to check if we need it at all or just change doc.
>>>
>>>> However, since we are going to break something here, it is better to
>>>> break hard to be sure that every since usage is updated. So, I tend
>>>> to to solution suggested by Ilya [1] which is similar to Linux kernel.
>>>> I.e. add an enum with invalid zero value and two members to specify
>>>> direction.
>>>>
>>>> [1]
>>>> https://patches.dpdk.org/project/dpdk/patch/20210601111420.5549-1-
>>>> ivan.malov@oktetlabs.ru/#133431
>>>>
>>>> as for PORT_ID pattern item, I think ingress/egress attributes define
>>>> direction. If it is an ingress flow rule, PORT_ID item should match
>>>> traffic coming from represented entity in the case of port
>>>> representor and associated network port in the case of ethdev port
>>>> associated with it. In egress case it otherwise matches traffic sent
>>>> using Tx burst via corresponding ethdev port.
>>>>
>>> I think that Ingress egress has only meaning when talking about NIC
>>> steering and not E-Switch steering.
>>
>> See [2]  12.2.2.4. Attribute: Transfer last paragraph.
>>
>> [2] https://doc.dpdk.org/guides/prog_guide/rte_flow.html#attributes
>>
>> In fact I was going to submit one more deprecation notice on the topic to
>> clarify it, but reread the documentation and now think that it is good enough.
>>
> 
> I think this needs to change,
> " 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."
> But if we route traffic between vports was is the app direction?
> For example if sending traffic from VF A to VF B (app is on PF)
> is it ingress or egress traffic? If the direction is reverse (B to A) does it change?

It is ingress since it would go DPDK app if we don't reroute it to VF B
directly. I think that egress is what is sent by DPDK app. Everything 
else is ingress. I.e. egress rules are applied to traffic which is
generated by the DPDK application itself.

> what if we are sending traffic from VF A to wire or from wire to A what is ingress / egress?
> (Assuming that the VFs are connected to different application.)

See above. It is all ingress rules, since it is applied on traffic which
is not generated by the DPDK application which inserts these rules.

>>> I think that we can just use original bit to mark if we want to send
>>> traffic to DPDK port or to other port.
>>
>> As I say the problem of the solution is that a silent breakage.
>> It is typically bad since  old code can simply misuse it.
>>
> You have a point but then maybe we should also delete this bit.
> Also I don't like the idea to break almost all apps that are using DPDK.
> especially if it will not cause error on build.

We can rename a field, to cause errors on build :)

> just adding more fields will break the app logic not the compilation which
> I think is the worst thing. (large number of application are based on
> the current logic)

The problem is that it could be different logic because of ambiguous
definition.

>>> In any case I will be happy if we could have a meeting to discuss this
>>> approach before sending your patch.
>>
>> Please, let the deprecation notice in. In whatever direction we fix it, we'll
>> break something in any case and DPDK users must be warned in advance.
>> We either change definition of the action or change support of the action in
>> drivers (in different ways in different drivers) or do both.
> 
> O.K.

Great, many thanks.

Andrew.

>>> I think this can save a lot of time.
>>
>> It is a good idea, let's schedule to the end of August. I guess many of us have
>> vacations now or in the nearest time. It will be simply hard to find time in the
>> nearest 3 weeks which is good for all or at least majority of us.
>>
> 
> Sure.
> Best,
> Ori
>> Thanks,
>> Andrew.
>>
>>> Best,
>>> Ori
>>>
>>>
>>>>>>> If we go this way there is no need to change the API only the doc.
>>>>>>>
>>>>>>>>> Regarding representors, it's not different. When using TX on a
>>>>>>>>> representor port, the packets appear as RX on its represented port.
>>>>>>>>>
>>>>>>>>> Please elaborate if there is a use case for the PORT_ID~ in
>>>>>>>>> which the app can get the packets using rte_eth_rx_burst on the
>>>>>>>>> specified
>>>>>> port-id.
>>>>>>>>
>>>>>>>> Multi-home host with a NIC with two physical ports and two PFs
>>>>>>>> used by DPDK app with layer 3 (IP addresses). Different cores
>>>>>>>> used to handle traffic from different ports plus routing in DPDK
>>>>>>>> app. If traffic to port #0 IP address is received on phys port
>>>>>>>> #1, it is useful to redirect traffic to port ID 0 directly to
>>>>>>>> have these packets on correct CPU cores from the very beginning
>>>>>>>> to avoid SW
>>>>>> mechanisms to pass from port #1 CPU cores to port #0 CPU cores.
>>>>>>>>
>>>>>>> To make sure I understand you are talking about a DPDK application
>>>>>>> that is connected to number of ports and it is Eswitch manager,
>>>>>>> but it doesn't use representors but the actual ports, right?
>>>>>>> I think the definition I wrote above also works for this case.
>>>>>>
>>>>>> Other possible request is to direct traffic from phys port #0 to
>>>>>> phys port #1 directly and say it in terms of PORT_ID action.
>>>>>>
>>>>> But we are talking using the switch layer(transfer mode) right?
>>>>
>>>> Yes.
>>>>
>>>>> Best,
>>>>> Ori
>>>>>> Thanks,
>>>>>> Andrew.
>>>>>>
>>>>>>> Best,
>>>>>>> Ori
>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Andrew Rybchenko
>>>> <andrew.rybchenko@oktetlabs.ru>
>>>>>>>>>> ---
>>>>>>>>>>       doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>>>>>>       1 file changed, 5 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> index d9c0e65921..6e6413c89f 100644
>>>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> @@ -158,3 +158,8 @@ Deprecation Notices
>>>>>>>>>>       * security: The functions ``rte_security_set_pkt_metadata`` and
>>>>>>>>>>         ``rte_security_get_userdata`` will be made inline
>>>>>>>>>> functions and additional
>>>>>>>>>>         flags will be added in structure ``rte_security_ctx`` in DPDK
>> 21.11.
>>>>>>>>>> +
>>>>>>>>>> +* ethdev: Definition of the flow API action PORT_ID is
>>>>>>>>>> +ambiguous and
>>>>>>>>>> needs
>>>>>>>>>> +  clarification. Structure rte_flow_action_port_id will be
>>>>>>>>>> +extended to
>>>>>>>>>> +  specify traffic direction to represented entity or ethdev
>>>>>>>>>> +port
>>>>>>>>>> itself in
>>>>>>>>>> +  DPDK 21.11.
>>>>>>>>>> --
>>>>>>>>>> 2.30.2
>>>>>>>>>>
>>>>>>>
>>>>>
>>>
> 

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Ori Kam]

Acked-by: Ori Kam <orika@nvidia.com>

Thanks,
Ori

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Ilya Maximets]

On 8/1/21 12:22 PM, Andrew Rybchenko wrote:
> By its very name, action PORT_ID means that packets hit an ethdev with the
> given DPDK port ID. At least the current comments don't state the opposite.
> That said, since port representors had been adopted, applications like OvS
> have been misusing the action. They misread its purpose as sending packets
> to the opposite end of the "wire" plugged to the given ethdev, for example,
> redirecting packets to the VF itself rather than to its representor ethdev.
> Another example: OvS relies on this action with the admin PF's ethdev port
> ID specified in it in order to send offloaded packets to the physical port.

Hi, Andrew.  The deprecation notice itself looks OK to me.  But I'd suggest
to avoid words like "misuse" and "misread" in the commit message, because I
don't think that it's correct.  Since documentation is ambiguous, different
people might interpret it differently.  And also, implementation in DPDK
matches with the way how OVS uses the API, otherwise offloading in OVS would
just not work.  So, OVS uses this API in a way as it is implemented in DPDK.
If the definition of a DPDK API allows interpretations that doesn't match with
the implementation inside the DPDK itself, that's not a fault of the external
application. And this can not be labeled as "misuse"/"misread".  Let's not
create a precedent.

Best regards, Ilya Maximets.

Re: [dpdk-dev] [PATCH 1/2] ethdev: announce flow API action PORT_ID changes

[Andrew Rybchenko]

On 8/2/21 6:49 PM, Ilya Maximets wrote:
> On 8/1/21 12:22 PM, Andrew Rybchenko wrote:
>> By its very name, action PORT_ID means that packets hit an ethdev with the
>> given DPDK port ID. At least the current comments don't state the opposite.
>> That said, since port representors had been adopted, applications like OvS
>> have been misusing the action. They misread its purpose as sending packets
>> to the opposite end of the "wire" plugged to the given ethdev, for example,
>> redirecting packets to the VF itself rather than to its representor ethdev.
>> Another example: OvS relies on this action with the admin PF's ethdev port
>> ID specified in it in order to send offloaded packets to the physical port.
> 
> Hi, Andrew.  The deprecation notice itself looks OK to me.  But I'd suggest
> to avoid words like "misuse" and "misread" in the commit message, because I
> don't think that it's correct.  Since documentation is ambiguous, different
> people might interpret it differently.  And also, implementation in DPDK
> matches with the way how OVS uses the API, otherwise offloading in OVS would
> just not work.  So, OVS uses this API in a way as it is implemented in DPDK.
> If the definition of a DPDK API allows interpretations that doesn't match with
> the implementation inside the DPDK itself, that's not a fault of the external
> application. And this can not be labeled as "misuse"/"misread".  Let's not
> create a precedent.

Yes, I agree. I'll send v2 tomorrow.

Thanks for feedback,
Andrew.

[dpdk-dev] [PATCH v2 1/2] ethdev: announce flow API action PORT_ID changes

[Andrew Rybchenko]

By its very name, action PORT_ID means that packets hit an ethdev with the
given DPDK port ID. At least the current comments don't state the opposite.

However some drivers implement it in a different way and direct traffic to
the opposite end of the "wire" plugged to the given ethdev. For example in
the case of a VF representor traffic is redirected to the corresponding VF
itself rather than to the representor ethdev and OvS uses PORT_ID action
this way.

The documentation must be clarified and, likely, rte_flow_action_port_id
structure should be extended to support both meanings.

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d9c0e65921..6e6413c89f 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -158,3 +158,8 @@ Deprecation Notices
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
+
+* ethdev: Definition of the flow API action PORT_ID is ambiguous and needs
+  clarification. Structure rte_flow_action_port_id will be extended to
+  specify traffic direction to represented entity or ethdev port itself in
+  DPDK 21.11.
-- 
2.30.2

[dpdk-dev] [PATCH v2 2/2] ethdev: announce clarification of implicit filter by port

[Andrew Rybchenko]

Transfer flow rules may be applied to traffic entering switch from
many sources. There are flow API pattern items which allow to specify
ingress port match criteria explicitly, but it is not documented
if ethdev port used to create flow rule adds any implicit match
criteria and how it coexists with explicit criterias.

These aspects should be documented and drivers and applications
which misuse it must be fixed.

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 6e6413c89f..4d174c8952 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -163,3 +163,8 @@ Deprecation Notices
   clarification. Structure rte_flow_action_port_id will be extended to
   specify traffic direction to 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.
-- 
2.30.2

[dpdk-dev] [PATCH v3 1/2] ethdev: announce flow API action PORT_ID changes

[Andrew Rybchenko]

By its very name, action PORT_ID means that packets hit an ethdev with the
given DPDK port ID. At least the current comments don't state the opposite.

However some drivers implement it in a different way and direct traffic to
the opposite end of the "wire" plugged to the given ethdev. For example in
the case of a VF representor traffic is redirected to the corresponding VF
itself rather than to the representor ethdev and OvS uses PORT_ID action
this way.

The documentation must be clarified and, likely, rte_flow_action_port_id
structure should be extended to support both meanings.

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d9c0e65921..6e6413c89f 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -158,3 +158,8 @@ Deprecation Notices
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
+
+* ethdev: Definition of the flow API action PORT_ID is ambiguous and needs
+  clarification. Structure rte_flow_action_port_id will be extended to
+  specify traffic direction to represented entity or ethdev port itself in
+  DPDK 21.11.
-- 
2.30.2

[dpdk-dev] [PATCH v3 2/2] ethdev: announce clarification of implicit filter by port

[Andrew Rybchenko]

Transfer flow rules may be applied to traffic entering switch from
many sources. There are flow API pattern items which allow to specify
ingress port match criteria explicitly, but it is not documented
if ethdev port used to create flow rule adds any implicit match
criteria and how it coexists with explicit ones.

These aspects should be documented and drivers and applications
which use it in a a different way must be fixed.

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 6e6413c89f..4d174c8952 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -163,3 +163,8 @@ Deprecation Notices
   clarification. Structure rte_flow_action_port_id will be extended to
   specify traffic direction to 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.
-- 
2.30.2

Re: [dpdk-dev] [PATCH v3 2/2] ethdev: announce clarification of implicit filter by port

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

On Mon, Aug 2, 2021 at 12:57 PM Andrew Rybchenko
<andrew.rybchenko@oktetlabs.ru> wrote:
>
> Transfer flow rules may be applied to traffic entering switch from
> many sources. There are flow API pattern items which allow to specify
> ingress port match criteria explicitly, but it is not documented
> if ethdev port used to create flow rule adds any implicit match
> criteria and how it coexists with explicit ones.
>
> These aspects should be documented and drivers and applications
> which use it in a a different way must be fixed.
nit! "in a different way..."
otherwise

Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Ori Kam <orika@nvidia.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 6e6413c89f..4d174c8952 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -163,3 +163,8 @@ Deprecation Notices
>    clarification. Structure rte_flow_action_port_id will be extended to
>    specify traffic direction to 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.
> --
> 2.30.2
>

Re: [dpdk-dev] [PATCH v3 2/2] ethdev: announce clarification of implicit filter by port

[Thomas Monjalon]

> > Transfer flow rules may be applied to traffic entering switch from
> > many sources. There are flow API pattern items which allow to specify
> > ingress port match criteria explicitly, but it is not documented
> > if ethdev port used to create flow rule adds any implicit match
> > criteria and how it coexists with explicit ones.
> >
> > These aspects should be documented and drivers and applications
> > which use it in a a different way must be fixed.
> nit! "in a different way..."
> otherwise
> 
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> 
> > Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > Acked-by: Ori Kam <orika@nvidia.com>

+1 for clarification
Acked-by: Thomas Monjalon <thomas@monjalon.net>

Applied, thanks.

Re: [dpdk-dev] [PATCH v3 1/2] ethdev: announce flow API action PORT_ID changes

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 1746 bytes --]

On Mon, Aug 2, 2021 at 12:57 PM Andrew Rybchenko
<andrew.rybchenko@oktetlabs.ru> wrote:
>
> By its very name, action PORT_ID means that packets hit an ethdev with the
> given DPDK port ID. At least the current comments don't state the opposite.
>
> However some drivers implement it in a different way and direct traffic to
> the opposite end of the "wire" plugged to the given ethdev. For example in
> the case of a VF representor traffic is redirected to the corresponding VF
> itself rather than to the representor ethdev and OvS uses PORT_ID action
> this way.
>
> The documentation must be clarified and, likely, rte_flow_action_port_id
> structure should be extended to support both meanings.
>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Ori Kam <orika@nvidia.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index d9c0e65921..6e6413c89f 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -158,3 +158,8 @@ Deprecation Notices
>  * security: The functions ``rte_security_set_pkt_metadata`` and
>    ``rte_security_get_userdata`` will be made inline functions and additional
>    flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
> +
> +* ethdev: Definition of the flow API action PORT_ID is ambiguous and needs
> +  clarification. Structure rte_flow_action_port_id will be extended to
> +  specify traffic direction to represented entity or ethdev port itself in
Nit! "to the represented entity"?
Otherwise
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

> +  DPDK 21.11.
> --
> 2.30.2
>

Re: [dpdk-dev] [PATCH v3 1/2] ethdev: announce flow API action PORT_ID changes

[Thomas Monjalon]

> > By its very name, action PORT_ID means that packets hit an ethdev with the
> > given DPDK port ID. At least the current comments don't state the opposite.
> >
> > However some drivers implement it in a different way and direct traffic to
> > the opposite end of the "wire" plugged to the given ethdev. For example in
> > the case of a VF representor traffic is redirected to the corresponding VF
> > itself rather than to the representor ethdev and OvS uses PORT_ID action
> > this way.
> >
> > The documentation must be clarified and, likely, rte_flow_action_port_id
> > structure should be extended to support both meanings.
> >
> > Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > Acked-by: Ori Kam <orika@nvidia.com>
> > ---
> > +* ethdev: Definition of the flow API action PORT_ID is ambiguous and needs
> > +  clarification. Structure rte_flow_action_port_id will be extended to
> > +  specify traffic direction to represented entity or ethdev port itself in
> Nit! "to the represented entity"?
> Otherwise
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

I agree clarification is welcome.
Acked-by: Thomas Monjalon <thomas@monjalon.net>

Applied with above change, thanks.