[dpdk-dev] [PATCH v2] ethdev: support flow aging

Andrew Rybchenko arybchenko at solarflare.com
Tue Apr 21 17:59:49 CEST 2020


On 4/21/20 1:04 PM, Ferruh Yigit wrote:
> On 4/20/2020 5:10 PM, Thomas Monjalon wrote:
>> 20/04/2020 16:06, Ferruh Yigit:
>>> On 4/18/2020 10:44 AM, Thomas Monjalon wrote:
>>>> 18/04/2020 07:04, Bill Zhou:
>>>>> From: Ferruh Yigit <ferruh.yigit at intel.com>
>>>>>> On 4/14/2020 9:32 AM, Dong Zhou wrote:
>>>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>>>> @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
>>>>>>>  	RTE_ETH_EVENT_NEW,      /**< port is probed */
>>>>>>>  	RTE_ETH_EVENT_DESTROY,  /**< port is released */
>>>>>>>  	RTE_ETH_EVENT_IPSEC,    /**< IPsec offload related event */
>>>>>>> +	RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected
>>>>>> */
>>>>>>>  	RTE_ETH_EVENT_MAX       /**< max value of this enum */
>>>>>>>  };
>>>>>>
>>>>>>
>>>>>> Just recognized that this is failing in ABI check [1], as far as last time for a
>>>>>> similar enum warning a QAT patch has been dropped, should this need to
>>>>>> wait for
>>>>>> 20.11 too?
>>>>>
>>>>> This patch is commonly used for flow aging, there are 2 other patches have 
>>>>> implement flow aging in mlx5 driver reply to this patch.
>> [...]
>>>> These MAX values in enums are a pain.
>>>> We can try to think what can be done, waiting 20.11.
>>>> Not sure there is a solution, except hijacking an existing value
>>>> not used in the PMD, waiting the definitive value in 20.11...
>>>
>>> Dropping from the tree as of now, to not cause more merge conflicts, we can add
>>> it later when issue is resolved.
>>
>> Thanks for dropping, that's the right thing to do
>> when a patch is breaking ABI check.
>>
>> After some thoughts, I think it is acceptable to make a v3
>> which ignore this specific enum change. I explain my thought below:
>>
>> An enum can accept a new value at 2 conditions:
>> 	- added as last value (not changing old values)
>> 	- new value not used by existing API
>>
>> The value RTE_ETH_EVENT_FLOW_AGED meet the above 2 conditions:
>> 	- only RTE_ETH_EVENT_MAX is changed, which is consistent
>> 	- new value sent to the app only if the app registered for it
>>
> 
> Same here, as far as I can see it is safe to get this change.
> 
> If any DPDK API returns this enum, either as return of the API or as output
> parameter, this still can be problem, because application may use that returned
> value, this was the concern in the QAT sample.
> 
> But here application registers an event and DPDK library process callback for
> it, so application callbacks won't be called for anything that application
> doesn't already know about, in that respect this should be safe for old
> applications.
> 
> Not sure if we can generalize above two conditions for all enum changes, but we
> can investigate them case by case as we get the warnings.
> 
>> So, except if I miss something, I suggest we add this exception:
>> Allow new value in rte_eth_event_type if added just before RTE_ETH_EVENT_MAX.
>> In other words, allow changing the value of RTE_ETH_EVENT_MAX.
>> The file to add such exception is devtools/libabigail.abignore.
>>
> 
> OK to exception.

Me too


More information about the dev mailing list