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

[Thomas Monjalon <thomas@monjalon.net>]

07/01/2021 13:47, Zhang, Qi Z:
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Thursday, January 7, 2021 6:12 PM
> > To: Guo, Jia <jia.guo@intel.com>
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> > Yang, Qiming <qiming.yang@intel.com>; Wang, Haiyue
> > <haiyue.wang@intel.com>; dev@dpdk.org; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; andrew.rybchenko@oktetlabs.ru; orika@nvidia.com;
> > getelson@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 <thomas@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.guo@intel.com>
> > > > > Reviewed-by: Qi Zhang <qi.z.zhang@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?


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