Hi Ori,
On 8/2/21 10:28 AM, Ori Kam wrote:
-----Original Message-----
From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
On 8/1/21 7:13 PM, Ori Kam wrote:
Hi Andrew,
-----Original Message-----
From: Andrew Rybchenko <andrew.rybche...@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.rybche...@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.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.
+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.ma...@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.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
