diff mbox series

[v3,03/15] crypto/mlx5: support session operations

Message ID 20210504210857.3398397-4-matan@nvidia.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers show
Series drivers: introduce mlx5 crypto PMD | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Matan Azrad May 4, 2021, 9:08 p.m. UTC
From: Shiri Kuzin <shirik@nvidia.com>

Sessions are used in symmetric transformations in order to prepare
objects and data for packet processing stage.

A mlx5 session includes iv_offset, pointer to mlx5_crypto_dek struct,
bsf_size, bsf_p_type, encryption_order and encryption standard.

Implement the next session operations:
        mlx5_crypto_sym_session_get_size- returns the size of the mlx5
	session struct.
	mlx5_crypto_sym_session_configure- prepares the DEK hash-list
	and saves all the session data.
	mlx5_crypto_sym_session_clear - destroys the DEK hash-list.

Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/crypto/mlx5/mlx5_crypto.c | 96 ++++++++++++++++++++++++++++++-
 1 file changed, 93 insertions(+), 3 deletions(-)

Comments

Akhil Goyal May 8, 2021, 12:44 p.m. UTC | #1
> +
> +static void
> +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> +			      struct rte_cryptodev_sym_session *sess)
> +{
> +	struct mlx5_crypto_priv *priv = dev->data->dev_private;
> +	struct mlx5_crypto_session *sess_private_data =
> +			get_sym_session_private_data(sess, dev->driver_id);
> +
> +	if (unlikely(sess_private_data == NULL)) {
> +		DRV_LOG(ERR, "Failed to get session %p private data.",
> +				sess_private_data);
> +		return;
> +	}
> +	mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> +	DRV_LOG(DEBUG, "Session %p was cleared.", sess_private_data);
> +}

Memory leakage, mempool is not freed.
IMO, this driver is not properly tested with the unit test app.
Please add a note in the documentation that it is tested with autotest.
Matan Azrad May 9, 2021, 8:03 a.m. UTC | #2
From: Akhil Goyal
> > +static void
> > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> > +                           struct rte_cryptodev_sym_session *sess) {
> > +     struct mlx5_crypto_priv *priv = dev->data->dev_private;
> > +     struct mlx5_crypto_session *sess_private_data =
> > +                     get_sym_session_private_data(sess,
> > +dev->driver_id);
> > +
> > +     if (unlikely(sess_private_data == NULL)) {
> > +             DRV_LOG(ERR, "Failed to get session %p private data.",
> > +                             sess_private_data);
> > +             return;
> > +     }
> > +     mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> > +     DRV_LOG(DEBUG, "Session %p was cleared.", sess_private_data); }
> 
> Memory leakage, mempool is not freed.

Yes, good catch, this part was missed.

> IMO, this driver is not properly tested with the unit test app.

The only app we tested until now is l2fwd_crypto and it works fine!
We can add it to doc.

> Please add a note in the documentation that it is tested with autotest.


The next app we want to test with, is test-crypto-perf.
Akhil Goyal May 9, 2021, 9:21 a.m. UTC | #3
> > > +static void
> > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> > > +                           struct rte_cryptodev_sym_session *sess) {
> > > +     struct mlx5_crypto_priv *priv = dev->data->dev_private;
> > > +     struct mlx5_crypto_session *sess_private_data =
> > > +                     get_sym_session_private_data(sess,
> > > +dev->driver_id);
> > > +
> > > +     if (unlikely(sess_private_data == NULL)) {
> > > +             DRV_LOG(ERR, "Failed to get session %p private data.",
> > > +                             sess_private_data);
> > > +             return;
> > > +     }
> > > +     mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> > > +     DRV_LOG(DEBUG, "Session %p was cleared.", sess_private_data); }
> >
> > Memory leakage, mempool is not freed.
> 
> Yes, good catch, this part was missed.
> 
> > IMO, this driver is not properly tested with the unit test app.
> 
> The only app we tested until now is l2fwd_crypto and it works fine!
> We can add it to doc.
> 
> > Please add a note in the documentation that it is tested with autotest.
> 
> 
> The next app we want to test with, is test-crypto-perf.
> 
I would recommend to run the test app first.
It will catch most of your basic bugs like the one above.
Matan Azrad May 9, 2021, 2:25 p.m. UTC | #4
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Sunday, May 9, 2021 12:21 PM
> To: Matan Azrad <matan@nvidia.com>; dev@dpdk.org
> Cc: Suanming Mou <suanmingm@nvidia.com>; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>; Shiri Kuzin <shirik@nvidia.com>
> Subject: RE: [EXT] [PATCH v3 03/15] crypto/mlx5: support session operations
> 
> External email: Use caution opening links or attachments
> 
> 
> > > > +static void
> > > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> > > > +                           struct rte_cryptodev_sym_session *sess) {
> > > > +     struct mlx5_crypto_priv *priv = dev->data->dev_private;
> > > > +     struct mlx5_crypto_session *sess_private_data =
> > > > +                     get_sym_session_private_data(sess,
> > > > +dev->driver_id);
> > > > +
> > > > +     if (unlikely(sess_private_data == NULL)) {
> > > > +             DRV_LOG(ERR, "Failed to get session %p private data.",
> > > > +                             sess_private_data);
> > > > +             return;
> > > > +     }
> > > > +     mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> > > > +     DRV_LOG(DEBUG, "Session %p was cleared.", sess_private_data); }
> > >
> > > Memory leakage, mempool is not freed.
> >
> > Yes, good catch, this part was missed.
> >
> > > IMO, this driver is not properly tested with the unit test app.
> >
> > The only app we tested until now is l2fwd_crypto and it works fine!
> > We can add it to doc.
> >
> > > Please add a note in the documentation that it is tested with autotest.
> >
> >
> > The next app we want to test with, is test-crypto-perf.
> >
> I would recommend to run the test app first.
> It will catch most of your basic bugs like the one above.

It is too late for this, will add the test adjustment later.
Akhil Goyal May 11, 2021, 5:30 p.m. UTC | #5
> >
> > > > > +static void
> > > > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> > > > > +                           struct rte_cryptodev_sym_session *sess) {
> > > > > +     struct mlx5_crypto_priv *priv = dev->data->dev_private;
> > > > > +     struct mlx5_crypto_session *sess_private_data =
> > > > > +                     get_sym_session_private_data(sess,
> > > > > +dev->driver_id);
> > > > > +
> > > > > +     if (unlikely(sess_private_data == NULL)) {
> > > > > +             DRV_LOG(ERR, "Failed to get session %p private data.",
> > > > > +                             sess_private_data);
> > > > > +             return;
> > > > > +     }
> > > > > +     mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> > > > > +     DRV_LOG(DEBUG, "Session %p was cleared.", sess_private_data);
> }
> > > >
> > > > Memory leakage, mempool is not freed.
> > >
> > > Yes, good catch, this part was missed.
> > >
> > > > IMO, this driver is not properly tested with the unit test app.
> > >
> > > The only app we tested until now is l2fwd_crypto and it works fine!
> > > We can add it to doc.
> > >
> > > > Please add a note in the documentation that it is tested with autotest.
> > >
> > >
> > > The next app we want to test with, is test-crypto-perf.
> > >
> > I would recommend to run the test app first.
> > It will catch most of your basic bugs like the one above.
> 
> It is too late for this, will add the test adjustment later.

Can we postpone the PMD to next release. I believe test application makes
The PMD look robust as per the DPDK crypto PMD API usage.
I haven't seen a PMD getting merged without test app.
And I apologize I did not mentioned it earlier, but it is kind of obvious thing
to run test app before sending it to upstream.
L2fwd-crypto is not doing data validation hence you cannot be sure that it is
working fine as per other standard stacks like Linux.

Regards,
Akhil
Matan Azrad May 12, 2021, 5:51 a.m. UTC | #6
From: Akhil Goyal
> > > > > > +static void
> > > > > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> > > > > > +                           struct rte_cryptodev_sym_session *sess) {
> > > > > > +     struct mlx5_crypto_priv *priv = dev->data->dev_private;
> > > > > > +     struct mlx5_crypto_session *sess_private_data =
> > > > > > +                     get_sym_session_private_data(sess,
> > > > > > +dev->driver_id);
> > > > > > +
> > > > > > +     if (unlikely(sess_private_data == NULL)) {
> > > > > > +             DRV_LOG(ERR, "Failed to get session %p private data.",
> > > > > > +                             sess_private_data);
> > > > > > +             return;
> > > > > > +     }
> > > > > > +     mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> > > > > > +     DRV_LOG(DEBUG, "Session %p was cleared.",
> > > > > > + sess_private_data);
> > }
> > > > >
> > > > > Memory leakage, mempool is not freed.
> > > >
> > > > Yes, good catch, this part was missed.
> > > >
> > > > > IMO, this driver is not properly tested with the unit test app.
> > > >
> > > > The only app we tested until now is l2fwd_crypto and it works fine!
> > > > We can add it to doc.
> > > >
> > > > > Please add a note in the documentation that it is tested with autotest.
> > > >
> > > >
> > > > The next app we want to test with, is test-crypto-perf.
> > > >
> > > I would recommend to run the test app first.
> > > It will catch most of your basic bugs like the one above.
> >
> > It is too late for this, will add the test adjustment later.
> 
> Can we postpone the PMD to next release.

We can, but this is not our plan.
We met all the DPDK rules to push it on time.

> I believe test application makes
> The PMD look robust as per the DPDK crypto PMD API usage.

Every test will add robustness to the PMD.

> I haven't seen a PMD getting merged without test app.

compress/mlx5, vdpa/mlx5, regex/mlx5, net/mlx5, vdev_netvsc....

> And I apologize I did not mentioned it earlier, but it is kind of obvious thing to
> run test app before sending it to upstream.

In fact, no, I added more than one PMD, no one require specific test.

> L2fwd-crypto is not doing data validation hence you cannot be sure that it is
> working fine as per other standard stacks like Linux.

It is not do data validation, but we check that the packet payload return back has the expected encrypted\decrypted data using open-ssl.
Also for us it was mandatory requirement to check it.

For us, the current validation is enough, we don't support a lot of things in the crypto PMD currently, only one algorithm in the most basic modes.

For sure we will continue to add tests and to increase robustness.
Also adding more features is in plan for future. 

If you postpone it, it yours, we don't agree with it.

> Regards,
> Akhil
Akhil Goyal May 12, 2021, 6:47 a.m. UTC | #7
> From: Akhil Goyal
> > > > > > > +static void
> > > > > > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> > > > > > > +                           struct rte_cryptodev_sym_session *sess) {
> > > > > > > +     struct mlx5_crypto_priv *priv = dev->data->dev_private;
> > > > > > > +     struct mlx5_crypto_session *sess_private_data =
> > > > > > > +                     get_sym_session_private_data(sess,
> > > > > > > +dev->driver_id);
> > > > > > > +
> > > > > > > +     if (unlikely(sess_private_data == NULL)) {
> > > > > > > +             DRV_LOG(ERR, "Failed to get session %p private data.",
> > > > > > > +                             sess_private_data);
> > > > > > > +             return;
> > > > > > > +     }
> > > > > > > +     mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> > > > > > > +     DRV_LOG(DEBUG, "Session %p was cleared.",
> > > > > > > + sess_private_data);
> > > }
> > > > > >
> > > > > > Memory leakage, mempool is not freed.
> > > > >
> > > > > Yes, good catch, this part was missed.
> > > > >
> > > > > > IMO, this driver is not properly tested with the unit test app.
> > > > >
> > > > > The only app we tested until now is l2fwd_crypto and it works fine!
> > > > > We can add it to doc.
> > > > >
> > > > > > Please add a note in the documentation that it is tested with
> autotest.
> > > > >
> > > > >
> > > > > The next app we want to test with, is test-crypto-perf.
> > > > >
> > > > I would recommend to run the test app first.
> > > > It will catch most of your basic bugs like the one above.
> > >
> > > It is too late for this, will add the test adjustment later.
> >
> > Can we postpone the PMD to next release.
> 
> We can, but this is not our plan.
> We met all the DPDK rules to push it on time.

On time!! Really? Incomplete v1 was submitted before RC1,
The complete PMD was submitted only during RC2.

> 
> > I believe test application makes
> > The PMD look robust as per the DPDK crypto PMD API usage.
> 
> Every test will add robustness to the PMD.
> 
> > I haven't seen a PMD getting merged without test app.
> 
> compress/mlx5, vdpa/mlx5, regex/mlx5, net/mlx5, vdev_netvsc....
> 
All these are 'not' crypto PMDs.
For crypto, UT is a must. I would like to add a statement in the guidelines if it is not there.

> > And I apologize I did not mentioned it earlier, but it is kind of obvious thing
> to
> > run test app before sending it to upstream.
> 
> In fact, no, I added more than one PMD, no one require specific test.
> 
> > L2fwd-crypto is not doing data validation hence you cannot be sure that it
> is
> > working fine as per other standard stacks like Linux.
> 
> It is not do data validation, but we check that the packet payload return back
> has the expected encrypted\decrypted data using open-ssl.
> Also for us it was mandatory requirement to check it.
> 
> For us, the current validation is enough, we don't support a lot of things in
> the crypto PMD currently, only one algorithm in the most basic modes.
> 
> For sure we will continue to add tests and to increase robustness.
> Also adding more features is in plan for future.
> 
> If you postpone it, it yours, we don't agree with it.
> 
Please fix the review comments by today, if you want it  to be merged in RC3.
IMO, the driver is not ready to be merged. 
I want to postpone it as it is not feasible to fix all the comments by RC3 timeline.
I believe all my comments are valid and need to be addressed.
Can you get somebody else(outside Nvidia) to ack your patch and counter my comments?
I will merge it without review based on that Ack.

Regards,
Akhil
Thomas Monjalon May 12, 2021, 7:01 a.m. UTC | #8
12/05/2021 07:51, Matan Azrad:
> From: Akhil Goyal
> > > > > > > +static void
> > > > > > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> > > > > > > +                           struct rte_cryptodev_sym_session *sess) {
> > > > > > > +     struct mlx5_crypto_priv *priv = dev->data->dev_private;
> > > > > > > +     struct mlx5_crypto_session *sess_private_data =
> > > > > > > +                     get_sym_session_private_data(sess,
> > > > > > > +dev->driver_id);
> > > > > > > +
> > > > > > > +     if (unlikely(sess_private_data == NULL)) {
> > > > > > > +             DRV_LOG(ERR, "Failed to get session %p private data.",
> > > > > > > +                             sess_private_data);
> > > > > > > +             return;
> > > > > > > +     }
> > > > > > > +     mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> > > > > > > +     DRV_LOG(DEBUG, "Session %p was cleared.",
> > > > > > > + sess_private_data);
> > > }
> > > > > >
> > > > > > Memory leakage, mempool is not freed.
> > > > >
> > > > > Yes, good catch, this part was missed.
> > > > >
> > > > > > IMO, this driver is not properly tested with the unit test app.
> > > > >
> > > > > The only app we tested until now is l2fwd_crypto and it works fine!
> > > > > We can add it to doc.
> > > > >
> > > > > > Please add a note in the documentation that it is tested with autotest.
> > > > >
> > > > >
> > > > > The next app we want to test with, is test-crypto-perf.
> > > > >
> > > > I would recommend to run the test app first.
> > > > It will catch most of your basic bugs like the one above.
> > >
> > > It is too late for this, will add the test adjustment later.
> > 
> > Can we postpone the PMD to next release.
> 
> We can, but this is not our plan.
> We met all the DPDK rules to push it on time.

There is no guarantee a patch will be pushed. This is not due.

> > I believe test application makes
> > The PMD look robust as per the DPDK crypto PMD API usage.
> 
> Every test will add robustness to the PMD.
> 
> > I haven't seen a PMD getting merged without test app.
> 
> compress/mlx5, vdpa/mlx5, regex/mlx5, net/mlx5, vdev_netvsc....

There are functional test app for regex, and testpmd for net driver.
Running these tests are basic requirements.

> > And I apologize I did not mentioned it earlier, but it is kind of obvious thing to
> > run test app before sending it to upstream.
> 
> In fact, no, I added more than one PMD, no one require specific test.

Each driver class may have its own expectations.

> > L2fwd-crypto is not doing data validation hence you cannot be sure that it is
> > working fine as per other standard stacks like Linux.
> 
> It is not do data validation, but we check that the packet payload return back has the expected encrypted\decrypted data using open-ssl.
> Also for us it was mandatory requirement to check it.
> 
> For us, the current validation is enough, we don't support a lot of things in the crypto PMD currently, only one algorithm in the most basic modes.
> 
> For sure we will continue to add tests and to increase robustness.
> Also adding more features is in plan for future. 
> 
> If you postpone it, it yours, we don't agree with it.

Not clear who is "we".
I am OK with postponing this PMD.

Thank you Akhil for your time.
Matan Azrad May 12, 2021, 7:25 a.m. UTC | #9
From: Akhil Goyal
> > From: Akhil Goyal
> > > > > > > > +static void
> > > > > > > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> > > > > > > > +                           struct rte_cryptodev_sym_session *sess) {
> > > > > > > > +     struct mlx5_crypto_priv *priv = dev->data->dev_private;
> > > > > > > > +     struct mlx5_crypto_session *sess_private_data =
> > > > > > > > +                     get_sym_session_private_data(sess,
> > > > > > > > +dev->driver_id);
> > > > > > > > +
> > > > > > > > +     if (unlikely(sess_private_data == NULL)) {
> > > > > > > > +             DRV_LOG(ERR, "Failed to get session %p private data.",
> > > > > > > > +                             sess_private_data);
> > > > > > > > +             return;
> > > > > > > > +     }
> > > > > > > > +     mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> > > > > > > > +     DRV_LOG(DEBUG, "Session %p was cleared.",
> > > > > > > > + sess_private_data);
> > > > }
> > > > > > >
> > > > > > > Memory leakage, mempool is not freed.
> > > > > >
> > > > > > Yes, good catch, this part was missed.
> > > > > >
> > > > > > > IMO, this driver is not properly tested with the unit test app.
> > > > > >
> > > > > > The only app we tested until now is l2fwd_crypto and it works fine!
> > > > > > We can add it to doc.
> > > > > >
> > > > > > > Please add a note in the documentation that it is tested
> > > > > > > with
> > autotest.
> > > > > >
> > > > > >
> > > > > > The next app we want to test with, is test-crypto-perf.
> > > > > >
> > > > > I would recommend to run the test app first.
> > > > > It will catch most of your basic bugs like the one above.
> > > >
> > > > It is too late for this, will add the test adjustment later.
> > >
> > > Can we postpone the PMD to next release.
> >
> > We can, but this is not our plan.
> > We met all the DPDK rules to push it on time.
> 
> On time!! Really? Incomplete v1 was submitted before RC1, The complete
> PMD was submitted only during RC2.

Yes, on time, you got draft in RC1 -- most of your critical comments are relevant for that version. 
 

> > > I believe test application makes
> > > The PMD look robust as per the DPDK crypto PMD API usage.
> >
> > Every test will add robustness to the PMD.
> >
> > > I haven't seen a PMD getting merged without test app.
> >
> > compress/mlx5, vdpa/mlx5, regex/mlx5, net/mlx5, vdev_netvsc....
> >
> All these are 'not' crypto PMDs.
> For crypto, UT is a must. I would like to add a statement in the guidelines if it
> is not there.
> 
> > > And I apologize I did not mentioned it earlier, but it is kind of
> > > obvious thing
> > to
> > > run test app before sending it to upstream.
> >
> > In fact, no, I added more than one PMD, no one require specific test.
> >
> > > L2fwd-crypto is not doing data validation hence you cannot be sure
> > > that it
> > is
> > > working fine as per other standard stacks like Linux.
> >
> > It is not do data validation, but we check that the packet payload
> > return back has the expected encrypted\decrypted data using open-ssl.
> > Also for us it was mandatory requirement to check it.
> >
> > For us, the current validation is enough, we don't support a lot of
> > things in the crypto PMD currently, only one algorithm in the most basic
> modes.
> >
> > For sure we will continue to add tests and to increase robustness.
> > Also adding more features is in plan for future.
> >
> > If you postpone it, it yours, we don't agree with it.
> >
> Please fix the review comments by today, if you want it  to be merged in RC3.
> IMO, the driver is not ready to be merged.
> I want to postpone it as it is not feasible to fix all the comments by RC3
> timeline.

Yes, it is too late.
Let's continue in next release.

Thanks for your time.

> I believe all my comments are valid and need to be addressed.
> Can you get somebody else(outside Nvidia) to ack your patch and counter
> my comments?
> I will merge it without review based on that Ack.
> Regards,
> Akhil
diff mbox series

Patch

diff --git a/drivers/crypto/mlx5/mlx5_crypto.c b/drivers/crypto/mlx5/mlx5_crypto.c
index 2bdfb1a10f..32f5077066 100644
--- a/drivers/crypto/mlx5/mlx5_crypto.c
+++ b/drivers/crypto/mlx5/mlx5_crypto.c
@@ -3,6 +3,7 @@ 
  */
 
 #include <rte_malloc.h>
+#include <rte_mempool.h>
 #include <rte_errno.h>
 #include <rte_log.h>
 #include <rte_pci.h>
@@ -36,6 +37,24 @@  static const struct rte_driver mlx5_drv = {
 
 static struct cryptodev_driver mlx5_cryptodev_driver;
 
+struct mlx5_crypto_session {
+	uint32_t bs_bpt_eo_es;
+	/*
+	 * bsf_size, bsf_p_type, encryption_order and encryption standard,
+	 * saved in big endian format.
+	 */
+	uint32_t iv_offset:16;
+	/* Starting point for Initialisation Vector. */
+	struct mlx5_crypto_dek *dek; /* Pointer to dek struct. */
+	uint32_t dek_id; /* DEK ID */
+} __rte_packed;
+
+static unsigned int
+mlx5_crypto_sym_session_get_size(struct rte_cryptodev *dev __rte_unused)
+{
+	return sizeof(struct mlx5_crypto_session);
+}
+
 static int
 mlx5_crypto_dev_configure(struct rte_cryptodev *dev,
 		struct rte_cryptodev_config *config __rte_unused)
@@ -58,6 +77,77 @@  mlx5_crypto_dev_close(struct rte_cryptodev *dev)
 	return 0;
 }
 
+static int
+mlx5_crypto_sym_session_configure(struct rte_cryptodev *dev,
+				  struct rte_crypto_sym_xform *xform,
+				  struct rte_cryptodev_sym_session *session,
+				  struct rte_mempool *mp)
+{
+	struct mlx5_crypto_priv *priv = dev->data->dev_private;
+	struct mlx5_crypto_session *sess_private_data;
+	struct rte_crypto_cipher_xform *cipher;
+	uint8_t encryption_order;
+	int ret;
+
+	if (unlikely(xform->next != NULL)) {
+		DRV_LOG(ERR, "Xform next is not supported.");
+		return -ENOTSUP;
+	}
+	if (unlikely((xform->type != RTE_CRYPTO_SYM_XFORM_CIPHER) ||
+		     (xform->cipher.algo != RTE_CRYPTO_CIPHER_AES_XTS))) {
+		DRV_LOG(ERR, "Only AES-XTS algorithm is supported.");
+		return -ENOTSUP;
+	}
+	ret = rte_mempool_get(mp, (void *)&sess_private_data);
+	if (ret != 0) {
+		DRV_LOG(ERR,
+			"Failed to get session %p private data from mempool.",
+			sess_private_data);
+		return -ENOMEM;
+	}
+	cipher = &xform->cipher;
+	sess_private_data->dek = mlx5_crypto_dek_prepare(priv, cipher);
+	if (sess_private_data->dek == NULL) {
+		rte_mempool_put(mp, sess_private_data);
+		DRV_LOG(ERR, "Failed to prepare dek.");
+		return -ENOMEM;
+	}
+	if (cipher->op == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
+		encryption_order = MLX5_ENCRYPTION_ORDER_ENCRYPTED_RAW_MEMORY;
+	else
+		encryption_order = MLX5_ENCRYPTION_ORDER_ENCRYPTED_RAW_WIRE;
+	sess_private_data->bs_bpt_eo_es = rte_cpu_to_be_32
+			(MLX5_BSF_SIZE_64B << MLX5_BSF_SIZE_OFFSET |
+			 MLX5_BSF_P_TYPE_CRYPTO << MLX5_BSF_P_TYPE_OFFSET |
+			 encryption_order << MLX5_ENCRYPTION_ORDER_OFFSET |
+			 MLX5_ENCRYPTION_STANDARD_AES_XTS);
+	sess_private_data->iv_offset = cipher->iv.offset;
+	sess_private_data->dek_id =
+			rte_cpu_to_be_32(sess_private_data->dek->obj->id &
+					 0xffffff);
+	set_sym_session_private_data(session, dev->driver_id,
+				     sess_private_data);
+	DRV_LOG(DEBUG, "Session %p was configured.", sess_private_data);
+	return 0;
+}
+
+static void
+mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
+			      struct rte_cryptodev_sym_session *sess)
+{
+	struct mlx5_crypto_priv *priv = dev->data->dev_private;
+	struct mlx5_crypto_session *sess_private_data =
+			get_sym_session_private_data(sess, dev->driver_id);
+
+	if (unlikely(sess_private_data == NULL)) {
+		DRV_LOG(ERR, "Failed to get session %p private data.",
+				sess_private_data);
+		return;
+	}
+	mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
+	DRV_LOG(DEBUG, "Session %p was cleared.", sess_private_data);
+}
+
 static struct rte_cryptodev_ops mlx5_crypto_ops = {
 	.dev_configure			= mlx5_crypto_dev_configure,
 	.dev_start			= NULL,
@@ -68,9 +158,9 @@  static struct rte_cryptodev_ops mlx5_crypto_ops = {
 	.stats_reset			= NULL,
 	.queue_pair_setup		= NULL,
 	.queue_pair_release		= NULL,
-	.sym_session_get_size		= NULL,
-	.sym_session_configure		= NULL,
-	.sym_session_clear		= NULL,
+	.sym_session_get_size		= mlx5_crypto_sym_session_get_size,
+	.sym_session_configure		= mlx5_crypto_sym_session_configure,
+	.sym_session_clear		= mlx5_crypto_sym_session_clear,
 	.sym_get_raw_dp_ctx_size	= NULL,
 	.sym_configure_raw_dp_ctx	= NULL,
 };