On 1/7/2021 1:33 PM, Thomas Monjalon wrote:
07/01/2021 13:47, Zhang, Qi Z:

-----Original Message-----
From: Thomas Monjalon <tho...@monjalon.net>
Sent: Thursday, January 7, 2021 6:12 PM
To: Guo, Jia <jia....@intel.com>
Cc: Zhang, Qi Z <qi.z.zh...@intel.com>; Wu, Jingjing <jingjing...@intel.com>;
Yang, Qiming <qiming.y...@intel.com>; Wang, Haiyue
<haiyue.w...@intel.com>; dev@dpdk.org; Yigit, Ferruh
<ferruh.yi...@intel.com>; andrew.rybche...@oktetlabs.ru; or...@nvidia.com;
getel...@nvidia.com
Subject: Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri

07/01/2021 10:32, Guo, Jia:
From: Thomas Monjalon <tho...@monjalon.net>
24/12/2020 07:59, Jeff Guo:
Add type of RTE_TUNNEL_TYPE_ECPRI into the enum of ethdev tunnel
type.

Signed-off-by: Jeff Guo <jia....@intel.com>
Reviewed-by: Qi Zhang <qi.z.zh...@intel.com>
[...]
--- 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.
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?

So I think it's not a good idea to replace
the rte_eth_dev_udp_tunnel_port_add with rte_flow config
and also there is no existing rte_flow_action
can cover this requirement unless we introduce some new one.

You can match, encap, decap.
There is even a new API to get tunnel infos after decap.
What is missing?

I still don't see which use case is missing.


Sorry, it is a nack.
BTW, it is probably breaking the ABI because of RTE_TUNNEL_TYPE_MAX.

Yes that may break the ABI but fortunately the checking-abi-compatibility tool 
shows negative :) , thanks Ferruh' s guide.
https://github.com/ferruhy/dpdk/actions/runs/468859673

That's very strange. An enum value is changed.
Why it is not flagged by libabigail?


As long as the enum values not sent to the application and kept within the library, changing their values shouldn't be problem.


Oh, the ABI break should be a problem.

PS: please Cc ethdev maintainers for such patch, thanks.
tip: use --cc-cmd devtools/get-maintainer.sh

Thanks for your helpful tip.




Reply via email to