Thread (91 messages) 91 messages, 9 authors, 2021-01-20

Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri

From: Zhang, Qi Z <hidden>
Date: 2021-01-09 01:01:31

-----Original Message-----
From: Thomas Monjalon <redacted>
Sent: Friday, January 8, 2021 6:36 PM
To: Yigit, Ferruh <redacted>; Zhang, Qi Z <redacted>;
Guo, Jia [off-list ref]; Andrew Rybchenko
[off-list ref]
Cc: Wu, Jingjing <redacted>; Yang, Qiming
[off-list ref]; Wang, Haiyue [off-list ref];
dev@dpdk.org; orika@nvidia.com; getelson@nvidia.com
Subject: Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri

08/01/2021 10:29, Andrew Rybchenko:
quoted
On 1/8/21 11:57 AM, Ferruh Yigit wrote:
quoted
On 1/8/2021 1:41 AM, Zhang, Qi Z wrote:
quoted
From: Thomas Monjalon <redacted>
quoted
07/01/2021 16:24, Zhang, Qi Z:
quoted
From: Thomas Monjalon <redacted>
quoted
07/01/2021 13:47, Zhang, Qi Z:
quoted
From: Thomas Monjalon <redacted>
quoted
07/01/2021 10:32, Guo, Jia:
quoted
From: Thomas Monjalon <redacted>
quoted
24/12/2020 07:59, Jeff Guo:
quoted
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1219,6 +1219,7 @@ enum rte_eth_tunnel_type {
      RTE_TUNNEL_TYPE_IP_IN_GRE,
      RTE_L2_TUNNEL_TYPE_E_TAG,
      RTE_TUNNEL_TYPE_VXLAN_GPE,
+    RTE_TUNNEL_TYPE_ECPRI,
      RTE_TUNNEL_TYPE_MAX,
  };
We tried to remove all these legacy API in DPDK 20.11.
Andrew decided to not remove this one because it is not yet
completely replaced by rte_flow in all drivers.
However, I am against continuing to update this API.
The opposite work should be done: migrate to rte_flow.
Agree but seems that the legacy api and driver legacy
implementation still keep in this release, and there is no a
general way to replace the legacy by rte_flow right now.
I think rte_flow is a complete replacement with more features.
Thomas, I may not agree with this.

Actually the "enum rte_eth_tunnel_type" is used by
rte_eth_dev_udp_tunnel_port_add A packet with specific dst udp
port will be recognized as a specific tunnel packet type (e.g.
vxlan, vxlan-gpe,
ecpri...) In Intel NIC, the API actually changes the
configuration of the packet parser in HW but not add a filter
rule and I guess all other devices may enable it in a similar way.
quoted
so naturally it should be a device (port) level configuration
but not a rte_flow
rule for match, encap, decap...

I don't understand how it helps to identify an UDP port if there
is no rule for this tunnel.
What is the usage?
Yes, in general It is a rule, it matches a udp packet's dst port
and the action is
"now the packet is identified as vxlan packet" then all other
rte_flow rules that match for a vlxan as pattern will take effect.
but somehow, I think they are not rules in the same domain, just
like we have dedicate API for mac/vlan filter, we'd better have a
dedicate API for this also. ( RFC for Vxlan explains why we need
this. https://tools.ietf.org/html/rfc7348).
quoted
"Destination Port: IANA has assigned the value 4789 for the VXLAN
UDP port, and this value SHOULD be used by default as the
destination UDP port.  Some early implementations of VXLAN have
used other values for the destination port.  To enable
interoperability with these implementations, the destination port
SHOULD be configurable."
quoted
quoted
quoted
quoted
Yes the port number is free.
But isn't it more natural to specify this port number as part of
the rte_flow rule?
I think if we have a rte_flow action type that can be used to set a
packet's tunnel type xxx, like below #flow create eth/ipv4/udp port
is 4789/... action set_tunnel_type VxLAN / end then we may replace
it with rte_flow, but I'm not sure if it's necessary, please share
if you have a better idea.
Of course we can specify the UDP port in rte_flow rule.
Please check rte_flow_item_udp.
That's a basic of rte_flow.
Its not about the pattern match, it's about the action, what we need is a rte_flow action to "define a packet's tunnel type", but we don't have.

#flow create eth/ipv4/udp port is 4789/... action set_tunnel_type VxLAN

I think rte_eth_dev_udp_tunnel_port_add does this job well already, if we plan to move it to rte_flow, at least we need a replacement solution.
quoted
quoted
Isn't this more a device configuration than filtering, not sure
about using rte_flow for this.
+1
A device configuration? No, setting an UDP port is a stack configuration.

quoted
quoted
quoted
BTW, are we going to move all other filter like mac , VLAN
filter/strip/insert into rte_flow finally?
Yes I think it should be the direction.
All of this can be achieved with rte_flow.

quoted
quoted
quoted
if that's the plan, though I don't have much inputs for this right
now, but I think we may not need to prevent new features be added
based on current API if it does not introduce more complexity and
not break anything.
If we continue updating old API, we are just forking ourself:
having 2 APIs for the same feature is a non-sense.
We must remove APIs which are superseded by rte_flow.
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help