Re: [dpdk-dev] DPDK Release Status Meeting 16/01/2020

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

On 1/16/2020 4:42 PM, Akhil Goyal wrote:
> Hi Konstantin,
> 
>> Hi Akhil,
>>
>>>>>
>>>>> * next-net-crypto
>>>>>   * Pull request sent
>>>>>   * There is a performance concern on some ipsec-gw patches,
>>>>>     they can go in -rc2 if the issue is solved
>>>>>   * CPU crypto from last release may be breaking ABI, need to confirm
>>>>
>>>> AFAIK, there is no ABI breakage.
>>>
>>> This is the output of validate-abi.sh.
>>>
>>> 	Change 							Effect
>>> 1 Field sym_cpu_process has been added to this type. 	              1) This field will
>> not be initialized by old clients.
>>>                                                                                                                    2) Size of the
>> inclusive type has been changed.
>>>
>>> 								NOTE: this
>> field should be accessed only from the new library
>>> functions, otherwise it may result in crash or incorrect behavior of applications.
>>> 2 Size of this type has been changed from 128 bytes to 136 bytes. 	The
>> fields or parameters of such data type may be incorrectly
>>> initialized or accessed by old client applications.
>>
>> This is struct rte_cryptodev_ops, which is AFAIK, not part of public API.
>> So not sure, why do you consider it as ABI breakage?
>>
> 
> If this is not an issue, than I am fine with it.

The ABI change between cryptodev and PMDs are allowed, that is contained within
DPDK and not a user interface [1].

[1] Unless some inline functions are directly accessing the dev_ops, as
(unfortunately) done in the ethdev.

> 
>>>
>>> Apart from that, IPSEC also has breakage, but that is experimental, so not an
>> issue.
>>>
>>>>
>>>>>     and discussed dummy variable is missing, may be postponed to next
>> release
>>>>
>>>> Not sure I understand last sentence, could the author explain
>>>> what dummy variables we are talking about.
>>>
>>> In one of the techboard meeting around 19.11 timeframe, during the
>> discussion for approving methodology for CPU-crypto, it was
>>> proposed that in order to avoid delay, a dummy variable can be introduced in
>> cryptodev API/ABI to avoid any ABI breakage in
>>> upcoming releases. But this was not done.
>>
>> Dummy variable for what?
>> If you are talking about sym_cpu_process - we just added it into
>> rte_cryptodev_ops, instead of
>> ' struct rte_cryptodev' instead.
>> That way we avoid any ABI breakage without introducing any churn in
>> rte_cryptodev itself , see above.
> 
> Then why was there so much resistance on this approach when there is no ABI breakage.
> I thought there was ABI breakage because of this change.
> 
> BTW this patchset is a bit late and it came after merge deadline 15 Jan.
> Ideally all library related patches should go in RC1.
> I would check if I could make it to the RC2.
> I have 3 IPSec series to work on before RC2.
> 
> -Akhil
>