[1/2] security: introduce per session event metadata
Checks
Commit Message
Implement API to set/get event data per security session.
Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>
Acked-by: Anoob Joseph <anoobj@marvell.com>
---
.../prog_guide/event_crypto_adapter.rst | 4 +-
lib/security/rte_security.h | 43 +++++++++++++++++++
2 files changed, 45 insertions(+), 2 deletions(-)
Comments
Acked-by: Abhinandan Gujjar <Abhinandan.gujjar@intel.com>
> -----Original Message-----
> From: Volodymyr Fialko <vfialko@marvell.com>
> Sent: Friday, March 25, 2022 4:46 PM
> To: dev@dpdk.org; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Akhil
> Goyal <gakhil@marvell.com>
> Cc: jerinj@marvell.com; Volodymyr Fialko <vfialko@marvell.com>; Anoob
> Joseph <anoobj@marvell.com>
> Subject: [PATCH 1/2] security: introduce per session event metadata
>
> Implement API to set/get event data per security session.
>
> Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
> Acked-by: Akhil Goyal <gakhil@marvell.com>
> Acked-by: Anoob Joseph <anoobj@marvell.com>
> ---
> .../prog_guide/event_crypto_adapter.rst | 4 +-
> lib/security/rte_security.h | 43 +++++++++++++++++++
> 2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/prog_guide/event_crypto_adapter.rst
> b/doc/guides/prog_guide/event_crypto_adapter.rst
> index 4fb5c688e0..227b36b4b7 100644
> --- a/doc/guides/prog_guide/event_crypto_adapter.rst
> +++ b/doc/guides/prog_guide/event_crypto_adapter.rst
> @@ -246,9 +246,9 @@ by ``rte_cryptodev_sym_session_get_user_data()`` API.
> The RTE_EVENT_CRYPTO_ADAPTER_CAP_SESSION_PRIVATE_DATA capability
> indicates whether HW or SW supports this feature.
>
> -For security session, ``rte_security_session_set_private_data()`` API
> +For security session, ``rte_security_session_set_event_mdata()`` API
> will be used to set request/response data. The same data will be obtained -by
> ``rte_security_session_get_private_data()`` API.
> +by ``rte_security_session_get_event_mdata()`` API.
>
> For session-less it is mandatory to place the request/response data with the
> ``rte_crypto_op``.
> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h index
> b080d10c2c..29ec514504 100644
> --- a/lib/security/rte_security.h
> +++ b/lib/security/rte_security.h
> @@ -526,6 +526,8 @@ struct rte_security_session {
> /**< Private session material */
> uint64_t opaque_data;
> /**< Opaque user defined data */
> + void *event_mdata;
> + /**< Event request/response information */
> };
>
> /**
> @@ -729,6 +731,47 @@ set_sec_session_private_data(struct
> rte_security_session *sess,
> sess->sess_private_data = private_data; }
>
> +/**
> + * Get event meta data attached to a security session.
> + *
> + * @param sess Session pointer allocated by
> + * *rte_security_session_create*.
> + *
> + * @return
> + * - On success return pointer to the event crypto meta data which is set
> + * using *rte_security_session_set_event_mdata*
> + * - On failure returns NULL.
> + */
> +__rte_experimental
> +static inline void *
> +rte_security_session_get_event_mdata(const struct rte_security_session
> +*sess) {
> + return sess->event_mdata;
> +}
> +
> +/**
> + * Attach event crypto meta data to a security session.
> + *
> + * Application can allocate memory for *rte_event_crypto_metadata* and
> +set the
> + * reference pointer using this API which the PMD can retrieve using
> + * *rte_security_session_get_event_mdata*
> + *
> + * The API should be used only in case session is used for event crypto
> + * adapter.
> + *
> + * @param sess Session pointer allocated by
> + * *rte_security_session_create*.
> + * @param ev_mdata Pointer to the event crypto meta data
> + * (aka *union rte_event_crypto_metadata*)
> + */
> +__rte_experimental
> +static inline void
> +rte_security_session_set_event_mdata(struct rte_security_session *sess,
> + void *ev_mdata)
> +{
> + sess->event_mdata = ev_mdata;
> +}
> +
> /**
> * Attach a session to a crypto operation.
> * This API is needed only in case of
> RTE_SECURITY_SESS_CRYPTO_PROTO_OFFLOAD
> --
> 2.25.1
Hi Abhinandan,
> ----------------------------------------------------------------------
> Acked-by: Abhinandan Gujjar <Abhinandan.gujjar@intel.com>
>
This change would be an ABI breakage. So to avoid that, we are planning to
Propose a better solution compared to this patch.
We plan to add a new cryptodev op to set the event metadata. A single API
which can be used in all cases - sym/asym/security sessions.
As currently in case of sym crypto, userdata is being used for storing the event
Metadata and it is then dereferenced in the PMD which is wrong.
User data is meant only for user to use and PMD should not dereference it.
Hence a new cryptodev op can be used to set session event metadata
explicitly if event mode is enabled.
I will be sending the proposal soon. Would need your help in testing the
Intel usecases.
Regards,
Akhil
> > -----Original Message-----
> > From: Volodymyr Fialko <vfialko@marvell.com>
> > Sent: Friday, March 25, 2022 4:46 PM
> > To: dev@dpdk.org; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>;
> Akhil
> > Goyal <gakhil@marvell.com>
> > Cc: jerinj@marvell.com; Volodymyr Fialko <vfialko@marvell.com>; Anoob
> > Joseph <anoobj@marvell.com>
> > Subject: [PATCH 1/2] security: introduce per session event metadata
> >
> > Implement API to set/get event data per security session.
> >
> > Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
> > Acked-by: Akhil Goyal <gakhil@marvell.com>
> > Acked-by: Anoob Joseph <anoobj@marvell.com>
> > ---
> > .../prog_guide/event_crypto_adapter.rst | 4 +-
> > lib/security/rte_security.h | 43 +++++++++++++++++++
> > 2 files changed, 45 insertions(+), 2 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/event_crypto_adapter.rst
> > b/doc/guides/prog_guide/event_crypto_adapter.rst
> > index 4fb5c688e0..227b36b4b7 100644
> > --- a/doc/guides/prog_guide/event_crypto_adapter.rst
> > +++ b/doc/guides/prog_guide/event_crypto_adapter.rst
> > @@ -246,9 +246,9 @@ by ``rte_cryptodev_sym_session_get_user_data()``
> API.
> > The RTE_EVENT_CRYPTO_ADAPTER_CAP_SESSION_PRIVATE_DATA capability
> > indicates whether HW or SW supports this feature.
> >
> > -For security session, ``rte_security_session_set_private_data()`` API
> > +For security session, ``rte_security_session_set_event_mdata()`` API
> > will be used to set request/response data. The same data will be obtained -by
> > ``rte_security_session_get_private_data()`` API.
> > +by ``rte_security_session_get_event_mdata()`` API.
> >
> > For session-less it is mandatory to place the request/response data with the
> > ``rte_crypto_op``.
> > diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h index
> > b080d10c2c..29ec514504 100644
> > --- a/lib/security/rte_security.h
> > +++ b/lib/security/rte_security.h
> > @@ -526,6 +526,8 @@ struct rte_security_session {
> > /**< Private session material */
> > uint64_t opaque_data;
> > /**< Opaque user defined data */
> > + void *event_mdata;
> > + /**< Event request/response information */
> > };
> >
> > /**
> > @@ -729,6 +731,47 @@ set_sec_session_private_data(struct
> > rte_security_session *sess,
> > sess->sess_private_data = private_data; }
> >
> > +/**
> > + * Get event meta data attached to a security session.
> > + *
> > + * @param sess Session pointer allocated by
> > + * *rte_security_session_create*.
> > + *
> > + * @return
> > + * - On success return pointer to the event crypto meta data which is set
> > + * using *rte_security_session_set_event_mdata*
> > + * - On failure returns NULL.
> > + */
> > +__rte_experimental
> > +static inline void *
> > +rte_security_session_get_event_mdata(const struct rte_security_session
> > +*sess) {
> > + return sess->event_mdata;
> > +}
> > +
> > +/**
> > + * Attach event crypto meta data to a security session.
> > + *
> > + * Application can allocate memory for *rte_event_crypto_metadata* and
> > +set the
> > + * reference pointer using this API which the PMD can retrieve using
> > + * *rte_security_session_get_event_mdata*
> > + *
> > + * The API should be used only in case session is used for event crypto
> > + * adapter.
> > + *
> > + * @param sess Session pointer allocated by
> > + * *rte_security_session_create*.
> > + * @param ev_mdata Pointer to the event crypto meta data
> > + * (aka *union rte_event_crypto_metadata*)
> > + */
> > +__rte_experimental
> > +static inline void
> > +rte_security_session_set_event_mdata(struct rte_security_session *sess,
> > + void *ev_mdata)
> > +{
> > + sess->event_mdata = ev_mdata;
> > +}
> > +
> > /**
> > * Attach a session to a crypto operation.
> > * This API is needed only in case of
> > RTE_SECURITY_SESS_CRYPTO_PROTO_OFFLOAD
> > --
> > 2.25.1
+ @Jayatheerthan, Jay & @Vangati, Narender
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Monday, April 4, 2022 3:19 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Volodymyr Fialko
> <vfialko@marvell.com>; dev@dpdk.org
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Anoob Joseph
> <anoobj@marvell.com>
> Subject: RE: [PATCH 1/2] security: introduce per session event metadata
>
> Hi Abhinandan,
> > ----------------------------------------------------------------------
> > Acked-by: Abhinandan Gujjar <Abhinandan.gujjar@intel.com>
> >
> This change would be an ABI breakage. So to avoid that, we are planning to
> Propose a better solution compared to this patch.
> We plan to add a new cryptodev op to set the event metadata. A single API
> which can be used in all cases - sym/asym/security sessions.
>
> As currently in case of sym crypto, userdata is being used for storing the event
> Metadata and it is then dereferenced in the PMD which is wrong.
> User data is meant only for user to use and PMD should not dereference it.
>
> Hence a new cryptodev op can be used to set session event metadata explicitly
> if event mode is enabled.
>
> I will be sending the proposal soon. Would need your help in testing the Intel
> usecases.
>
> Regards,
> Akhil
> > > -----Original Message-----
> > > From: Volodymyr Fialko <vfialko@marvell.com>
> > > Sent: Friday, March 25, 2022 4:46 PM
> > > To: dev@dpdk.org; Gujjar, Abhinandan S
> > > <abhinandan.gujjar@intel.com>;
> > Akhil
> > > Goyal <gakhil@marvell.com>
> > > Cc: jerinj@marvell.com; Volodymyr Fialko <vfialko@marvell.com>;
> > > Anoob Joseph <anoobj@marvell.com>
> > > Subject: [PATCH 1/2] security: introduce per session event metadata
> > >
> > > Implement API to set/get event data per security session.
> > >
> > > Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
> > > Acked-by: Akhil Goyal <gakhil@marvell.com>
> > > Acked-by: Anoob Joseph <anoobj@marvell.com>
> > > ---
> > > .../prog_guide/event_crypto_adapter.rst | 4 +-
> > > lib/security/rte_security.h | 43 +++++++++++++++++++
> > > 2 files changed, 45 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/doc/guides/prog_guide/event_crypto_adapter.rst
> > > b/doc/guides/prog_guide/event_crypto_adapter.rst
> > > index 4fb5c688e0..227b36b4b7 100644
> > > --- a/doc/guides/prog_guide/event_crypto_adapter.rst
> > > +++ b/doc/guides/prog_guide/event_crypto_adapter.rst
> > > @@ -246,9 +246,9 @@ by ``rte_cryptodev_sym_session_get_user_data()``
> > API.
> > > The RTE_EVENT_CRYPTO_ADAPTER_CAP_SESSION_PRIVATE_DATA
> capability
> > > indicates whether HW or SW supports this feature.
> > >
> > > -For security session, ``rte_security_session_set_private_data()``
> > > API
> > > +For security session, ``rte_security_session_set_event_mdata()``
> > > +API
> > > will be used to set request/response data. The same data will be
> > > obtained -by ``rte_security_session_get_private_data()`` API.
> > > +by ``rte_security_session_get_event_mdata()`` API.
> > >
> > > For session-less it is mandatory to place the request/response data
> > > with the ``rte_crypto_op``.
> > > diff --git a/lib/security/rte_security.h
> > > b/lib/security/rte_security.h index
> > > b080d10c2c..29ec514504 100644
> > > --- a/lib/security/rte_security.h
> > > +++ b/lib/security/rte_security.h
> > > @@ -526,6 +526,8 @@ struct rte_security_session {
> > > /**< Private session material */
> > > uint64_t opaque_data;
> > > /**< Opaque user defined data */
> > > + void *event_mdata;
> > > + /**< Event request/response information */
> > > };
> > >
> > > /**
> > > @@ -729,6 +731,47 @@ set_sec_session_private_data(struct
> > > rte_security_session *sess,
> > > sess->sess_private_data = private_data; }
> > >
> > > +/**
> > > + * Get event meta data attached to a security session.
> > > + *
> > > + * @param sess Session pointer allocated by
> > > + * *rte_security_session_create*.
> > > + *
> > > + * @return
> > > + * - On success return pointer to the event crypto meta data which is set
> > > + * using *rte_security_session_set_event_mdata*
> > > + * - On failure returns NULL.
> > > + */
> > > +__rte_experimental
> > > +static inline void *
> > > +rte_security_session_get_event_mdata(const struct
> > > +rte_security_session
> > > +*sess) {
> > > + return sess->event_mdata;
> > > +}
> > > +
> > > +/**
> > > + * Attach event crypto meta data to a security session.
> > > + *
> > > + * Application can allocate memory for *rte_event_crypto_metadata*
> > > +and set the
> > > + * reference pointer using this API which the PMD can retrieve
> > > +using
> > > + * *rte_security_session_get_event_mdata*
> > > + *
> > > + * The API should be used only in case session is used for event
> > > +crypto
> > > + * adapter.
> > > + *
> > > + * @param sess Session pointer allocated by
> > > + * *rte_security_session_create*.
> > > + * @param ev_mdata Pointer to the event crypto meta data
> > > + * (aka *union rte_event_crypto_metadata*)
> > > + */
> > > +__rte_experimental
> > > +static inline void
> > > +rte_security_session_set_event_mdata(struct rte_security_session *sess,
> > > + void *ev_mdata)
> > > +{
> > > + sess->event_mdata = ev_mdata;
> > > +}
> > > +
> > > /**
> > > * Attach a session to a crypto operation.
> > > * This API is needed only in case of
> > > RTE_SECURITY_SESS_CRYPTO_PROTO_OFFLOAD
> > > --
> > > 2.25.1
Hi Abhinandan and others,
> + @Jayatheerthan, Jay & @Vangati, Narender
>
> > This change would be an ABI breakage. So to avoid that, we are planning to
> > Propose a better solution compared to this patch.
> > We plan to add a new cryptodev op to set the event metadata. A single API
> > which can be used in all cases - sym/asym/security sessions.
> >
> > As currently in case of sym crypto, userdata is being used for storing the event
> > Metadata and it is then dereferenced in the PMD which is wrong.
> > User data is meant only for user to use and PMD should not dereference it.
> >
> > Hence a new cryptodev op can be used to set session event metadata explicitly
> > if event mode is enabled.
> >
> > I will be sending the proposal soon. Would need your help in testing the Intel
> > usecases.
> >
We may need to stick to the approach introduced in this patch only.
As if we propose, a new single API for all type of sessions, the driver would need to
Get the event metadata from the session private data. This is not possible with
Your use case which gets it inside the eventdev library for sw adapter case as it cannot
Get the session private data without knowing the cdev_id.
Hence, we will take this patch as is in next release for security sessions(as it is an ABI break)
And would also introduce a similar change for crypto sessions in next release.
This way we can get rid of using userdata which is wrong implementation.
Regards,
Akhil
Hi Abhinandan and others,
> We may need to stick to the approach introduced in this patch only.
> As if we propose, a new single API for all type of sessions, the driver would need
> to
> Get the event metadata from the session private data. This is not possible with
> Your use case which gets it inside the eventdev library for sw adapter case as it
> cannot
> Get the session private data without knowing the cdev_id.
>
> Hence, we will take this patch as is in next release for security sessions(as it is an
> ABI break)
> And would also introduce a similar change for crypto sessions in next release.
> This way we can get rid of using userdata which is wrong implementation.
On another thought, we have posted a new patchset to use event crypto adapter.
Please review and provide feedback.
Thanks,
Akhil
@@ -246,9 +246,9 @@ by ``rte_cryptodev_sym_session_get_user_data()`` API. The
RTE_EVENT_CRYPTO_ADAPTER_CAP_SESSION_PRIVATE_DATA capability indicates
whether HW or SW supports this feature.
-For security session, ``rte_security_session_set_private_data()`` API
+For security session, ``rte_security_session_set_event_mdata()`` API
will be used to set request/response data. The same data will be obtained
-by ``rte_security_session_get_private_data()`` API.
+by ``rte_security_session_get_event_mdata()`` API.
For session-less it is mandatory to place the request/response data with
the ``rte_crypto_op``.
@@ -526,6 +526,8 @@ struct rte_security_session {
/**< Private session material */
uint64_t opaque_data;
/**< Opaque user defined data */
+ void *event_mdata;
+ /**< Event request/response information */
};
/**
@@ -729,6 +731,47 @@ set_sec_session_private_data(struct rte_security_session *sess,
sess->sess_private_data = private_data;
}
+/**
+ * Get event meta data attached to a security session.
+ *
+ * @param sess Session pointer allocated by
+ * *rte_security_session_create*.
+ *
+ * @return
+ * - On success return pointer to the event crypto meta data which is set
+ * using *rte_security_session_set_event_mdata*
+ * - On failure returns NULL.
+ */
+__rte_experimental
+static inline void *
+rte_security_session_get_event_mdata(const struct rte_security_session *sess)
+{
+ return sess->event_mdata;
+}
+
+/**
+ * Attach event crypto meta data to a security session.
+ *
+ * Application can allocate memory for *rte_event_crypto_metadata* and set the
+ * reference pointer using this API which the PMD can retrieve using
+ * *rte_security_session_get_event_mdata*
+ *
+ * The API should be used only in case session is used for event crypto
+ * adapter.
+ *
+ * @param sess Session pointer allocated by
+ * *rte_security_session_create*.
+ * @param ev_mdata Pointer to the event crypto meta data
+ * (aka *union rte_event_crypto_metadata*)
+ */
+__rte_experimental
+static inline void
+rte_security_session_set_event_mdata(struct rte_security_session *sess,
+ void *ev_mdata)
+{
+ sess->event_mdata = ev_mdata;
+}
+
/**
* Attach a session to a crypto operation.
* This API is needed only in case of RTE_SECURITY_SESS_CRYPTO_PROTO_OFFLOAD