[dpdk-dev,02/11] examples/ipsec-secgw: Fixed init of aead crypto devices

Message ID 1507987683-12315-2-git-send-email-aviadye@dev.mellanox.co.il (mailing list archive)
State Changes Requested, archived
Delegated to: Pablo de Lara Guarch
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Aviad Yehezkel Oct. 14, 2017, 1:27 p.m. UTC
  From: Aviad Yehezkel <aviadye@mellanox.com>

This was broken since new aead xfrom was introduced

Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
---
 examples/ipsec-secgw/ipsec-secgw.c | 29 +++++++++++++++++++----------
 examples/ipsec-secgw/ipsec.h       |  1 +
 2 files changed, 20 insertions(+), 10 deletions(-)
  

Comments

Aviad Yehezkel Oct. 15, 2017, 12:54 p.m. UTC | #1
On 10/14/2017 4:27 PM, aviadye@dev.mellanox.co.il wrote:
> From: Aviad Yehezkel <aviadye@mellanox.com>
>
> This was broken since new aead xfrom was introduced
>
> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
> ---
>   examples/ipsec-secgw/ipsec-secgw.c | 29 +++++++++++++++++++----------
>   examples/ipsec-secgw/ipsec.h       |  1 +
>   2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index 99dc270..7bf692c 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -1113,7 +1113,8 @@ add_mapping(struct rte_hash *map, const char *str, uint16_t cdev_id,
>   		uint16_t qp, struct lcore_params *params,
>   		struct ipsec_ctx *ipsec_ctx,
>   		const struct rte_cryptodev_capabilities *cipher,
> -		const struct rte_cryptodev_capabilities *auth)
> +		const struct rte_cryptodev_capabilities *auth,
> +		const struct rte_cryptodev_capabilities *aead)
>   {
>   	int32_t ret = 0;
>   	unsigned long i;
> @@ -1124,6 +1125,8 @@ add_mapping(struct rte_hash *map, const char *str, uint16_t cdev_id,
>   		key.cipher_algo = cipher->sym.cipher.algo;
>   	if (auth)
>   		key.auth_algo = auth->sym.auth.algo;
> +	if (aead)
> +		key.aead_algo = aead->sym.aead.algo;
>   
>   	ret = rte_hash_lookup(map, &key);
>   	if (ret != -ENOENT)
> @@ -1192,19 +1195,25 @@ add_cdev_mapping(struct rte_cryptodev_info *dev_info, uint16_t cdev_id,
>   		if (i->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC)
>   			continue;
>   
> -		if (i->sym.xform_type != RTE_CRYPTO_SYM_XFORM_CIPHER)
> +		if (i->sym.xform_type == RTE_CRYPTO_SYM_XFORM_AEAD) {
> +			ret |= add_mapping(map, str, cdev_id, qp, params,
> +					ipsec_ctx, NULL, NULL, i);
>   			continue;
> +		}
>   
> -		for (j = dev_info->capabilities;
> -				j->op != RTE_CRYPTO_OP_TYPE_UNDEFINED; j++) {
> -			if (j->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC)
> -				continue;
> +		if (i->sym.xform_type == RTE_CRYPTO_SYM_XFORM_CIPHER) {
> +			for (j = dev_info->capabilities;
> +					j->op != RTE_CRYPTO_OP_TYPE_UNDEFINED; j++) {
> +				if (j->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC)
> +					continue;
>   
> -			if (j->sym.xform_type != RTE_CRYPTO_SYM_XFORM_AUTH)
> -				continue;
> +				if (j->sym.xform_type != RTE_CRYPTO_SYM_XFORM_AUTH)
> +					continue;
>   
> -			ret |= add_mapping(map, str, cdev_id, qp, params,
> -					ipsec_ctx, i, j);
> +				ret |= add_mapping(map, str, cdev_id, qp, params,
> +						ipsec_ctx, i, j, NULL);
> +			}
> +			continue;
>   		}
>   	}
>   
> diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
> index da1fb1b..7d057ae 100644
> --- a/examples/ipsec-secgw/ipsec.h
> +++ b/examples/ipsec-secgw/ipsec.h
> @@ -150,6 +150,7 @@ struct cdev_key {
>   	uint16_t lcore_id;
>   	uint8_t cipher_algo;
>   	uint8_t auth_algo;
> +	uint8_t aead_algo;
>   };
>   
>   struct socket_ctx {

Tested-by: Aviad Yehezkel <aviadye@mellanox.com>
  
De Lara Guarch, Pablo Oct. 16, 2017, 3:23 p.m. UTC | #2
Hi Aviad,

> -----Original Message-----

> From: Aviad Yehezkel [mailto:aviadye@dev.mellanox.co.il]

> Sent: Sunday, October 15, 2017 1:54 PM

> To: dev@dpdk.org; Gonzalez Monroy, Sergio

> <sergio.gonzalez.monroy@intel.com>; De Lara Guarch, Pablo

> <pablo.de.lara.guarch@intel.com>; aviadye@mellanox.com

> Cc: borisp@mellanox.com; akhil.goyal@nxp.com;

> hemant.agrawal@nxp.com; Nicolau, Radu <radu.nicolau@intel.com>;

> Doherty, Declan <declan.doherty@intel.com>; liranl@mellanox.com;

> nelio.laranjeiro@6wind.com; thomas@monjalon.net

> Subject: Re: [dpdk-dev][PATCH 02/11] examples/ipsec-secgw: Fixed init of

> aead crypto devices

> 


Commit titles should start with infinitive and not with lowercase.
e.g. examples/ipsec-secgw: fix init of aead crypto devices

Also, since this is a fix, you should include a Fixes line with the commit id
where the issues was introduced, and CC stable, if the issue was not introduced 
in the current release.

Take a look at the following document, that explains in detail the contribution guidelines:
http://dpdk.org/doc/guides/contributing/patches.html

Also, I have a comment below.

Thanks,
Pablo


> 

> 

> On 10/14/2017 4:27 PM, aviadye@dev.mellanox.co.il wrote:

> > From: Aviad Yehezkel <aviadye@mellanox.com>

> >

> > This was broken since new aead xfrom was introduced

> >

> > Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>


...

> >   	if (ret != -ENOENT)

> > @@ -1192,19 +1195,25 @@ add_cdev_mapping(struct

> rte_cryptodev_info *dev_info, uint16_t cdev_id,

> >   		if (i->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC)

> >   			continue;

> >



I think it is simpler to leave the code as it is, and add:

+		if (i->sym.xform_type == RTE_CRYPTO_SYM_XFORM_AEAD) {
+			ret |= add_mapping(map, str, cdev_id, qp, params,
+					ipsec_ctx, NULL, NULL, i);
+			continue;
+		}

And just add NULL in the existing add_mapping() function, without modifying the for loop.
The other changes were OK to me.

> > -		if (i->sym.xform_type !=

> RTE_CRYPTO_SYM_XFORM_CIPHER)

> > +		if (i->sym.xform_type == RTE_CRYPTO_SYM_XFORM_AEAD)

> {

> > +			ret |= add_mapping(map, str, cdev_id, qp, params,

> > +					ipsec_ctx, NULL, NULL, i);

> >   			continue;

> > +		}
  

Patch

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 99dc270..7bf692c 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -1113,7 +1113,8 @@  add_mapping(struct rte_hash *map, const char *str, uint16_t cdev_id,
 		uint16_t qp, struct lcore_params *params,
 		struct ipsec_ctx *ipsec_ctx,
 		const struct rte_cryptodev_capabilities *cipher,
-		const struct rte_cryptodev_capabilities *auth)
+		const struct rte_cryptodev_capabilities *auth,
+		const struct rte_cryptodev_capabilities *aead)
 {
 	int32_t ret = 0;
 	unsigned long i;
@@ -1124,6 +1125,8 @@  add_mapping(struct rte_hash *map, const char *str, uint16_t cdev_id,
 		key.cipher_algo = cipher->sym.cipher.algo;
 	if (auth)
 		key.auth_algo = auth->sym.auth.algo;
+	if (aead)
+		key.aead_algo = aead->sym.aead.algo;
 
 	ret = rte_hash_lookup(map, &key);
 	if (ret != -ENOENT)
@@ -1192,19 +1195,25 @@  add_cdev_mapping(struct rte_cryptodev_info *dev_info, uint16_t cdev_id,
 		if (i->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC)
 			continue;
 
-		if (i->sym.xform_type != RTE_CRYPTO_SYM_XFORM_CIPHER)
+		if (i->sym.xform_type == RTE_CRYPTO_SYM_XFORM_AEAD) {
+			ret |= add_mapping(map, str, cdev_id, qp, params,
+					ipsec_ctx, NULL, NULL, i);
 			continue;
+		}
 
-		for (j = dev_info->capabilities;
-				j->op != RTE_CRYPTO_OP_TYPE_UNDEFINED; j++) {
-			if (j->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC)
-				continue;
+		if (i->sym.xform_type == RTE_CRYPTO_SYM_XFORM_CIPHER) {
+			for (j = dev_info->capabilities;
+					j->op != RTE_CRYPTO_OP_TYPE_UNDEFINED; j++) {
+				if (j->op != RTE_CRYPTO_OP_TYPE_SYMMETRIC)
+					continue;
 
-			if (j->sym.xform_type != RTE_CRYPTO_SYM_XFORM_AUTH)
-				continue;
+				if (j->sym.xform_type != RTE_CRYPTO_SYM_XFORM_AUTH)
+					continue;
 
-			ret |= add_mapping(map, str, cdev_id, qp, params,
-					ipsec_ctx, i, j);
+				ret |= add_mapping(map, str, cdev_id, qp, params,
+						ipsec_ctx, i, j, NULL);
+			}
+			continue;
 		}
 	}
 
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index da1fb1b..7d057ae 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -150,6 +150,7 @@  struct cdev_key {
 	uint16_t lcore_id;
 	uint8_t cipher_algo;
 	uint8_t auth_algo;
+	uint8_t aead_algo;
 };
 
 struct socket_ctx {