[dpdk-dev] [EXT] Re: [PATCH v2 4/4] add ABI checks

[Trahe, Fiona]

Hi Anoob,

> -----Original Message-----
> From: Anoob Joseph <anoobj at marvell.com>
> Sent: Wednesday, February 5, 2020 6:16 AM
> To: Neil Horman <nhorman at tuxdriver.com>; Akhil Goyal <akhil.goyal at nxp.com>; Trahe, Fiona
> <fiona.trahe at intel.com>
> Cc: Thomas Monjalon <thomas at monjalon.net>; Yigit, Ferruh <ferruh.yigit at intel.com>; Ananyev,
> Konstantin <konstantin.ananyev at intel.com>; Trahe, Fiona <fiona.trahe at intel.com>; dev at dpdk.org;
> David Marchand <david.marchand at redhat.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal at intel.com>;
> Richardson, Bruce <bruce.richardson at intel.com>; Mcnamara, John <john.mcnamara at intel.com>;
> dodji at seketeli.net; Andrew Rybchenko <arybchenko at solarflare.com>; aconole at redhat.com;
> bluca at debian.org; ktraynor at redhat.com
> Subject: RE: [EXT] Re: [dpdk-dev] [PATCH v2 4/4] add ABI checks
> 
> Hi Akhil, Neil, Fiona
> 
> Sorry for the late response. I want to propose a new change in line with what you folks had proposed.
> 
> May be we can treat the new features EXPERIMENTAL until a new stable release.
> 
> enum rte_crypto_aead_algorithm {
>          RTE_CRYPTO_AEAD_AES_CCM = 1,
>          /**< AES algorithm in CCM mode. */
>          RTE_CRYPTO_AEAD_AES_GCM,
>          /**< AES algorithm in GCM mode. */
>          RTE_CRYPTO_AEAD_LIST_END,
>          /**< List end for stable */
>          /** EXPERIMENTAL */
>          RTE_CRYPTO_AEAD_CHACHA20_POLY1305,
>          /**< Chacha20 cipher with poly1305 authenticator */
>          RTE_CRYPTO_AEAD_LIST_END_EXPERIMENTAL
>          /**< List end */
>  };
> 
> And then introduce an experimental API,
> 
> const struct rte_cryptodev_capabilities *
> rte_cryptodev_get_exp_capabilites(uint8_t dev_id);
> 
> The PMD owner is expected to add new capabilities (only new ones) to this one until the new feature is
> deemed stable (ie, in one of the next stable releases). We don't expect users to change their API/ABI.
> For applications using EXPERIMENTAL is allowed to use the above capabilities to get the EXPERIMENTAL
> features.
> 
> This does involve moving around code in PMD when one feature is added, but that's the risk PMD
> owner is taking by upstreaming as EXPERIMENTAL and not in stable release.
> 
> Thanks,
> Anoob

[Fiona] Thanks for the suggestion Anoob.
I like the enum part of the idea - but not the new temporary API as the applications need to be aware of it and would have to change again when it's removed.
I explored an alternative way of using the current experimental infrastructure, i.e.:
enum rte_crypto_aead_algorithm {
         RTE_CRYPTO_AEAD_AES_CCM = 1,
         /**< AES algorithm in CCM mode. */
         RTE_CRYPTO_AEAD_AES_GCM,
         /**< AES algorithm in GCM mode. */
#ifdef ALLOW_EXPERIMENTAL_API
         RTE_CRYPTO_AEAD_CHACHA20_POLY1305,
         /**< Chacha20 cipher with poly1305 authenticator */
#endif
         RTE_CRYPTO_AEAD_LIST_END,
         /**< List end */
};

No new rte_cryptodev_get_exp_capabilities() needed.

Any PMD that implements the experimental API must do the same:
#ifdef ALLOW_EXPERIMENTAL_API
	<PMD code processing new enum>
#endif
Same with test code.
Any 19.11 production code that  wants to run against shared objects from
20.02 can be expected to build DPDK with ALLOW_EXPERIMENTAL_API disabled,
so will not pick up the new feature.
However, it appears the flag is not globally consistent, i.e. most PMDs have it set, even if the application doesn't set it. So this probably wouldn't work.
We're testing the approach outlined yesterday and believe it satisfactorily resolves the issue, so will stick with that.