DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Akhil Goyal <akhil.goyal@nxp.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "thomas@monjalon.net" <thomas@monjalon.net>,
	"jerin.jacob@caviumnetworks.com" <jerin.jacob@caviumnetworks.com>,
	"anoob.joseph@caviumnetworks.com"
	<anoob.joseph@caviumnetworks.com>,
	"Nicolau, Radu" <radu.nicolau@intel.com>,
	"Doherty, Declan" <declan.doherty@intel.com>,
	Hemant Agrawal <hemant.agrawal@nxp.com>
Subject: Re: [dpdk-dev] [PATCH] security: remove experimental tag
Date: Tue, 13 Nov 2018 12:23:11 +0000	[thread overview]
Message-ID: <2601191342CEEE43887BDE71AB977258010CE49D56@IRSMSX106.ger.corp.intel.com> (raw)
In-Reply-To: <e672e434-7ff7-dc25-d9d2-bff664b019a2@nxp.com>



> 
> Hi Konstantin,
> 
> On 11/13/2018 5:19 PM, Ananyev, Konstantin wrote:
> > Hi Akhil,
> >
> >> -----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
b) RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL
c) rte_security_capability.ol_flags:
    RTE_SECURITY_PDCP_ORDERING_CAP
    RTE_SECURITY_PDCP_DUP_DETECT_CAP
    RTE_SECURITY_TX_HW_TRAILER_OFFLOAD
    RTE_SECURITY_RX_HW_TRAILER_OFFLOAD

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

> 
> -Akhil
> >> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> >> ---
> >>   MAINTAINERS                                  |  2 +-
> >>   lib/librte_security/Makefile                 |  1 -
> >>   lib/librte_security/meson.build              |  1 -
> >>   lib/librte_security/rte_security.c           | 18 +++++++++---------
> >>   lib/librte_security/rte_security.h           | 27 +++++++++++++--------------
> >>   lib/librte_security/rte_security_driver.h    |  1 -
> >>   lib/librte_security/rte_security_version.map |  2 +-
> >>   7 files changed, 24 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 19353ac..bdd6747 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -347,7 +347,7 @@ F: lib/librte_cryptodev/
> >>   F: test/test/test_cryptodev*
> >>   F: examples/l2fwd-crypto/
> >>
> >> -Security API - EXPERIMENTAL
> >> +Security API
> >>   M: Akhil Goyal <akhil.goyal@nxp.com>
> >>   M: Declan Doherty <declan.doherty@intel.com>
> >>   T: git://dpdk.org/next/dpdk-next-crypto
> >> diff --git a/lib/librte_security/Makefile b/lib/librte_security/Makefile
> >> index 8daebea..bd92343 100644
> >> --- a/lib/librte_security/Makefile
> >> +++ b/lib/librte_security/Makefile
> >> @@ -10,7 +10,6 @@ LIB = librte_security.a
> >>   LIBABIVER := 1
> >>
> >>   # build flags
> >> -CFLAGS += -DALLOW_EXPERIMENTAL_API
> >>   CFLAGS += -O3
> >>   CFLAGS += $(WERROR_FLAGS)
> >>   LDLIBS += -lrte_eal -lrte_mempool
> >> diff --git a/lib/librte_security/meson.build b/lib/librte_security/meson.build
> >> index 4c85894..532953f 100644
> >> --- a/lib/librte_security/meson.build
> >> +++ b/lib/librte_security/meson.build
> >> @@ -1,7 +1,6 @@
> >>   # SPDX-License-Identifier: BSD-3-Clause
> >>   # Copyright(c) 2017 Intel Corporation
> >>
> >> -allow_experimental_apis = true
> >>   sources = files('rte_security.c')
> >>   headers = files('rte_security.h', 'rte_security_driver.h')
> >>   deps += ['mempool', 'cryptodev']
> >> diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c
> >> index c6355de..bc81ce1 100644
> >> --- a/lib/librte_security/rte_security.c
> >> +++ b/lib/librte_security/rte_security.c
> >> @@ -10,7 +10,7 @@
> >>   #include "rte_security_driver.h"
> >>
> >>   struct rte_security_session *
> >> -__rte_experimental rte_security_session_create(struct rte_security_ctx *instance,
> >> +rte_security_session_create(struct rte_security_ctx *instance,
> >>   			    struct rte_security_session_conf *conf,
> >>   			    struct rte_mempool *mp)
> >>   {
> >> @@ -33,7 +33,7 @@ __rte_experimental rte_security_session_create(struct rte_security_ctx *instance
> >>   	return sess;
> >>   }
> >>
> >> -int __rte_experimental
> >> +int
> >>   rte_security_session_update(struct rte_security_ctx *instance,
> >>   			    struct rte_security_session *sess,
> >>   			    struct rte_security_session_conf *conf)
> >> @@ -42,14 +42,14 @@ rte_security_session_update(struct rte_security_ctx *instance,
> >>   	return instance->ops->session_update(instance->device, sess, conf);
> >>   }
> >>
> >> -unsigned int __rte_experimental
> >> +unsigned int
> >>   rte_security_session_get_size(struct rte_security_ctx *instance)
> >>   {
> >>   	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->session_get_size, 0);
> >>   	return instance->ops->session_get_size(instance->device);
> >>   }
> >>
> >> -int __rte_experimental
> >> +int
> >>   rte_security_session_stats_get(struct rte_security_ctx *instance,
> >>   			       struct rte_security_session *sess,
> >>   			       struct rte_security_stats *stats)
> >> @@ -58,7 +58,7 @@ rte_security_session_stats_get(struct rte_security_ctx *instance,
> >>   	return instance->ops->session_stats_get(instance->device, sess, stats);
> >>   }
> >>
> >> -int __rte_experimental
> >> +int
> >>   rte_security_session_destroy(struct rte_security_ctx *instance,
> >>   			     struct rte_security_session *sess)
> >>   {
> >> @@ -76,7 +76,7 @@ rte_security_session_destroy(struct rte_security_ctx *instance,
> >>   	return ret;
> >>   }
> >>
> >> -int __rte_experimental
> >> +int
> >>   rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
> >>   			      struct rte_security_session *sess,
> >>   			      struct rte_mbuf *m, void *params)
> >> @@ -86,7 +86,7 @@ rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
> >>   					       sess, m, params);
> >>   }
> >>
> >> -void * __rte_experimental
> >> +void *
> >>   rte_security_get_userdata(struct rte_security_ctx *instance, uint64_t md)
> >>   {
> >>   	void *userdata = NULL;
> >> @@ -98,14 +98,14 @@ rte_security_get_userdata(struct rte_security_ctx *instance, uint64_t md)
> >>   	return userdata;
> >>   }
> >>
> >> -const struct rte_security_capability * __rte_experimental
> >> +const struct rte_security_capability *
> >>   rte_security_capabilities_get(struct rte_security_ctx *instance)
> >>   {
> >>   	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->capabilities_get, NULL);
> >>   	return instance->ops->capabilities_get(instance->device);
> >>   }
> >>
> >> -const struct rte_security_capability * __rte_experimental
> >> +const struct rte_security_capability *
> >>   rte_security_capability_get(struct rte_security_ctx *instance,
> >>   			    struct rte_security_capability_idx *idx)
> >>   {
> >> diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h
> >> index 1431b4d..e07b132 100644
> >> --- a/lib/librte_security/rte_security.h
> >> +++ b/lib/librte_security/rte_security.h
> >> @@ -8,7 +8,6 @@
> >>
> >>   /**
> >>    * @file rte_security.h
> >> - * @b EXPERIMENTAL: this API may change without prior notice
> >>    *
> >>    * RTE Security Common Definitions
> >>    *
> >> @@ -330,7 +329,7 @@ struct rte_security_session {
> >>    *  - On success, pointer to session
> >>    *  - On failure, NULL
> >>    */
> >> -struct rte_security_session * __rte_experimental
> >> +struct rte_security_session *
> >>   rte_security_session_create(struct rte_security_ctx *instance,
> >>   			    struct rte_security_session_conf *conf,
> >>   			    struct rte_mempool *mp);
> >> @@ -345,7 +344,7 @@ rte_security_session_create(struct rte_security_ctx *instance,
> >>    *  - On success returns 0
> >>    *  - On failure return errno
> >>    */
> >> -int __rte_experimental
> >> +int
> >>   rte_security_session_update(struct rte_security_ctx *instance,
> >>   			    struct rte_security_session *sess,
> >>   			    struct rte_security_session_conf *conf);
> >> @@ -359,7 +358,7 @@ rte_security_session_update(struct rte_security_ctx *instance,
> >>    *   - Size of the private data, if successful
> >>    *   - 0 if device is invalid or does not support the operation.
> >>    */
> >> -unsigned int __rte_experimental
> >> +unsigned int
> >>   rte_security_session_get_size(struct rte_security_ctx *instance);
> >>
> >>   /**
> >> @@ -374,7 +373,7 @@ rte_security_session_get_size(struct rte_security_ctx *instance);
> >>    *  - -EINVAL if session is NULL.
> >>    *  - -EBUSY if not all device private data has been freed.
> >>    */
> >> -int __rte_experimental
> >> +int
> >>   rte_security_session_destroy(struct rte_security_ctx *instance,
> >>   			     struct rte_security_session *sess);
> >>
> >> @@ -391,7 +390,7 @@ rte_security_session_destroy(struct rte_security_ctx *instance,
> >>    *  - On success, zero.
> >>    *  - On failure, a negative value.
> >>    */
> >> -int __rte_experimental
> >> +int
> >>   rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
> >>   			      struct rte_security_session *sess,
> >>   			      struct rte_mbuf *mb, void *params);
> >> @@ -413,7 +412,7 @@ rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
> >>    *  - On success, userdata
> >>    *  - On failure, NULL
> >>    */
> >> -void * __rte_experimental
> >> +void *
> >>   rte_security_get_userdata(struct rte_security_ctx *instance, uint64_t md);
> >>
> >>   /**
> >> @@ -422,7 +421,7 @@ rte_security_get_userdata(struct rte_security_ctx *instance, uint64_t md);
> >>    * @param	sym_op	crypto operation
> >>    * @param	sess	security session
> >>    */
> >> -static inline int __rte_experimental
> >> +static inline int
> >>   __rte_security_attach_session(struct rte_crypto_sym_op *sym_op,
> >>   			      struct rte_security_session *sess)
> >>   {
> >> @@ -431,13 +430,13 @@ __rte_security_attach_session(struct rte_crypto_sym_op *sym_op,
> >>   	return 0;
> >>   }
> >>
> >> -static inline void * __rte_experimental
> >> +static inline void *
> >>   get_sec_session_private_data(const struct rte_security_session *sess)
> >>   {
> >>   	return sess->sess_private_data;
> >>   }
> >>
> >> -static inline void __rte_experimental
> >> +static inline void
> >>   set_sec_session_private_data(struct rte_security_session *sess,
> >>   			     void *private_data)
> >>   {
> >> @@ -453,7 +452,7 @@ set_sec_session_private_data(struct rte_security_session *sess,
> >>    * @param	op	crypto operation
> >>    * @param	sess	security session
> >>    */
> >> -static inline int __rte_experimental
> >> +static inline int
> >>   rte_security_attach_session(struct rte_crypto_op *op,
> >>   			    struct rte_security_session *sess)
> >>   {
> >> @@ -500,7 +499,7 @@ struct rte_security_stats {
> >>    *  - On success return 0
> >>    *  - On failure errno
> >>    */
> >> -int __rte_experimental
> >> +int
> >>   rte_security_session_stats_get(struct rte_security_ctx *instance,
> >>   			       struct rte_security_session *sess,
> >>   			       struct rte_security_stats *stats);
> >> @@ -608,7 +607,7 @@ struct rte_security_capability_idx {
> >>    *   - Returns array of security capabilities.
> >>    *   - Return NULL if no capabilities available.
> >>    */
> >> -const struct rte_security_capability * __rte_experimental
> >> +const struct rte_security_capability *
> >>   rte_security_capabilities_get(struct rte_security_ctx *instance);
> >>
> >>   /**
> >> @@ -622,7 +621,7 @@ rte_security_capabilities_get(struct rte_security_ctx *instance);
> >>    *     index criteria.
> >>    *   - Return NULL if the capability not matched on security instance.
> >>    */
> >> -const struct rte_security_capability * __rte_experimental
> >> +const struct rte_security_capability *
> >>   rte_security_capability_get(struct rte_security_ctx *instance,
> >>   			    struct rte_security_capability_idx *idx);
> >>
> >> diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h
> >> index 42f42ff..1b561f8 100644
> >> --- a/lib/librte_security/rte_security_driver.h
> >> +++ b/lib/librte_security/rte_security_driver.h
> >> @@ -8,7 +8,6 @@
> >>
> >>   /**
> >>    * @file rte_security_driver.h
> >> - * @b EXPERIMENTAL: this API may change without prior notice
> >>    *
> >>    * RTE Security Common Definitions
> >>    *
> >> diff --git a/lib/librte_security/rte_security_version.map b/lib/librte_security/rte_security_version.map
> >> index 5a1c8ae..a77ca4b 100644
> >> --- a/lib/librte_security/rte_security_version.map
> >> +++ b/lib/librte_security/rte_security_version.map
> >> @@ -1,4 +1,4 @@
> >> -EXPERIMENTAL {
> >> +DPDK_18.11 {
> >>   	global:
> >>
> >>   	rte_security_attach_session;
> >> --
> >> 2.7.4


  reply	other threads:[~2018-11-13 12:23 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-13 11:28 Akhil Goyal
2018-11-13 11:49 ` Ananyev, Konstantin
2018-11-13 11:59   ` Akhil Goyal
2018-11-13 12:23     ` Ananyev, Konstantin [this message]
2018-11-13 12:41       ` Akhil Goyal
2018-11-13 15:36         ` Ananyev, Konstantin
2018-11-14  8:30           ` Hemant Agrawal
2018-11-14  8:53             ` Ananyev, Konstantin
2018-11-14  9:39 ` Joseph, Anoob
2018-11-14 12:40 ` Hemant Agrawal
2018-11-14 17:07 ` Boris Pismenny
2018-11-18 16:51 ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2601191342CEEE43887BDE71AB977258010CE49D56@IRSMSX106.ger.corp.intel.com \
    --to=konstantin.ananyev@intel.com \
    --cc=akhil.goyal@nxp.com \
    --cc=anoob.joseph@caviumnetworks.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=radu.nicolau@intel.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).