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.rybche...@oktetlabs.ru>
Sent: Sunday, August 1, 2021 3:04 PM
To: Eli Britstein <el...@nvidia.com>; NBU-Contact-Thomas Monjalon
<tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com>; Ori Kam
<or...@nvidia.com>
Cc: dev@dpdk.org; Ilya Maximets <i.maxim...@ovn.org>; Ajit Khaparde
<ajit.khapa...@broadcom.com>; Matan Azrad <ma...@nvidia.com>; Ivan
Malov <ivan.ma...@oktetlabs.ru>; Viacheslav Galaktionov
<viacheslav.galaktio...@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.rybche...@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



Reply via email to