Re: [dpdk-dev] [PATCH] security: remove experimental tag

[Hemant Agrawal <hemant.agrawal@nxp.com>]

On 11/13/2018 9:06 PM, Ananyev, Konstantin wrote:
>
>>>>> -----Original Message-----
>>>>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>>>>> Sent: Tuesday, November 13, 2018 11:28 AM
>>>>> To: dev@dpdk.org
>>>>> Cc: thomas@monjalon.net; Ananyev, Konstantin
>>>>> <konstantin.ananyev@intel.com>; jerin.jacob@caviumnetworks.com;
>>>>> anoob.joseph@caviumnetworks.com; Nicolau, Radu
>>>>> <radu.nicolau@intel.com>; Doherty, Declan
>>>>> <declan.doherty@intel.com>; Hemant Agrawal
>>>>> <hemant.agrawal@nxp.com>; Akhil Goyal <akhil.goyal@nxp.com>
>>>>> Subject: [PATCH] security: remove experimental tag
>>>>>
>>>>> rte_security has been experimental since DPDK 17.11 release.
>>>>> Now the library has matured and expermental tag is removed in this
>>>>> patch.
>>>> I agree that it is present for a while in dpdk.org, but as I can see
>>>> we still have unimplemented API here.
>>>> Which makes me doubt that it is ok to remove experimental tag from it.
>>>> Konstantin
>>> 3 vendors(Intel/Cavium/NXP) have tested their PMDs on security and
>>> made the changes that they need.
>>> Which APIs are missing?
>> What I am aware about:
>> a) rte_security_ops. get_userdata
>> [Akhil] I believe Cavium added some patches in ipsec-secgw app for its usage and I believe they do have implementation for that.
> ipsec-secgw has some code that refers it, but at present moment there is no PMD in dpdk.org that supports it
> (at least I can't find any).
>
>> Also I
>> cannot see any changes in rte_security for its support in PMDs.
> Might be, but wouldn't you expect function to be at least implemented
> to call it 'mature'?
>
>> b) RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL
>>
>> [Akhil] Cavium supports it.
> Might be, but again it is not in dpdk.org right now
> (AFAIK it is planned for 19.02).
>
>> c) rte_security_capability.ol_flags:
>>      RTE_SECURITY_PDCP_ORDERING_CAP
>>      RTE_SECURITY_PDCP_DUP_DETECT_CAP
>>
>> [Akhil] PDCP is not currently supported by any of the vendors except NXP and NXP do not support these capabilities.
>> For this also, I don’t see any change in the library. It would be only PMD which needs to support it.
>>
>>      RTE_SECURITY_TX_HW_TRAILER_OFFLOAD
>>      RTE_SECURITY_RX_HW_TRAILER_OFFLOAD
>>
>> [Akhil] Same here, these are all PMD capabilities which do not require any change in rte_security.
> Without real implementation, how can we be certain about it?
> Might be while implementing feature X we would realize that something else is needed.
> Another question - what users who build their products on top of rte_security
> have to do?
> Should they include support for all these unimplemented capabilities into their
> code or not?
> Considering the fact, that right now there is no way for them to test/try it.
>
>>> I believe addition of protocols is not an issue  even if we remove
>>> experimental tag.
>> After another thought - it is probably unfair to keep whole lib as experimental because few things are missing.
>> But I think things that are unimplemented (or related to them) need to stay in 'experimental' state.
>>
>> [Akhil] I do not foresee any changes in library, so I believe experimental is not required. Please correct me if this is incorrect understanding.
> The only change I am personally plan to do in 19.02 -
> add opaque userdata field into rte_security_session:
> struct rte_security_session {
>   	void *sess_private_data;
>   	/**< Private session material */
> +	uint64_t userdata;
> +	/**< Opaque user defined data */
>   };
>
> Might be in future extra changes would be needed to pass ipsec
> sqn/replay_window data between HW/SW.
> Not aware about any other changes.
> Though these future changes is not my main concern.
> After all we have a defined process for making changes into non-experimental API.
> I just don't see how we can consider API that has un-implemented parts as a 'mature'.
> Probably we have different views on what experimental/mature API means.
>  From my perspective to name it a 'mature' it needs to at least to be implemented and
> tested (proved working),  plus stable enough(not major changes coming).

NXP want to start pushing the IPSEC offload etc to VPP and other 
projects, but it needs to come out of experimental first.

Other projects will not adapt to it till it come out of experimental tag.

It can be a different kind of debate and problem.  When we proposed the 
original APIs, we only provided APIs supported by NXP. However other 
vendors jumped in and suggested them to make them more generic. But 
these vendors are yet to implement them. e.g. We have not yet seen 
Mellanox providing their rte_security driver yet, but they contributed 
few of the APIs.  Now should we wait indefinately to get implementation 
of those APIs.

Note that in some cases segregating them in experimental and 
non-experimental is not feasible.

The similar case is with PDCP,  the original APIs we proposed was 
supported by NXP driver. However when Cavium reviewed it and provided 
suggestions, we ended up adding few extra things for their completeness. 
Now, NXP don't support/implement them and others vendors are not yet 
implementing them.

so, will it never come out of experimental?

I think except few APIs, we shall make it non-experimental and if any 
one need any changes, they need to come with standard route.


> Konstantin