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 >
