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

["Ananyev, Konstantin" <konstantin.ananyev@intel.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?

Probably the only way to avoid such situation in future -
don't accept API without actual implementation.

> 
> 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.

Ok, no objections from my side to that.
Konstantin