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.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. 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.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 > >>
