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

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/8/2021 12:38 PM, Kinsella, Ray wrote:
> 
> 
>> -----Original Message-----
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Friday 8 January 2021 10:24
>> To: Guo, Jia <jia.guo@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>> Yigit, Ferruh <ferruh.yigit@intel.com>
>> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Yang, Qiming
>> <qiming.yang@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>;
>> dev@dpdk.org; andrew.rybchenko@oktetlabs.ru; orika@nvidia.com;
>> getelson@nvidia.com; Dodji Seketeli <dodji@redhat.com>; Kinsella, Ray
>> <ray.kinsella@intel.com>
>> Subject: Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type
>> for ecpri
>>
>> 08/01/2021 10:22, Ferruh Yigit:
>>> On 1/7/2021 1:33 PM, Thomas Monjalon wrote:
>>>> 07/01/2021 13:47, Zhang, Qi Z:
>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>> 07/01/2021 10:32, Guo, Jia:
>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>>> 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.
>>
>> But RTE_TUNNEL_TYPE_MAX is part of lib/librte_ethdev/rte_ethdev.h so it
>> is exposed to the application.
>> I think it is a case of ABI breakage.
>>
> 
> Really a lot depends on context, Thomas is right it is hard to predict how these _MAX values are used.
> 
> We have seen cases in the past where _MAX enumeration values have been used to size arrays the like - I don't immediately see that issue here. My understanding is that the only consumer of this enumeration is rte_eth_dev_udp_tunnel_port_add and rte_eth_dev_udp_tunnel_port_delete, right? On face value, impact looks negligible.
> 
> I will take a look at why libabigail doesn't complain.
> 

Application can use the enum, including MAX as they desire, we can't really 
assume anything there.

In previous case, library was providing an enum value back to application. And 
the problem was application can use those values blindly and new unexpected 
values may cause trouble.

For this case, even the application create a table with RTE_TUNNEL_TYPE_MAX 
size, library is not sending any type of this enum to application to cause any 
problem, at least abigail seems not able to finding any instance of it.