[dpdk-dev] [PATCH v1 0/1] crypto/openssl: replace macros by inline routines

[dpdk-dev] [PATCH v1 0/1] crypto/openssl: replace macros by inline routines

[Ashish Gupta]

From: Shally Verma <shally.verma@caviumnetworks.com>

Replace macros by static inline functions in openssl
compatibility layer. These changes are based on
the feedback given on openssl PMD patch V5 here:

Link: https://patches.dpdk.org/patch/43273/#84042

Ashish Gupta (1):
  crypto/openssl: replace macros by static inline functions

 drivers/crypto/openssl/compat.h              | 265 ++++++++++++++++++---------
 drivers/crypto/openssl/rte_openssl_pmd.c     |  10 +-
 drivers/crypto/openssl/rte_openssl_pmd_ops.c |  12 +-
 3 files changed, 188 insertions(+), 99 deletions(-)

-- 
2.14.3

[dpdk-dev] [PATCH v1 1/1] crypto/openssl: replace macros by static inline functions

[Ashish Gupta]

Replace macros by static inline functions in openssl version
compatibility layer

Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
---
 drivers/crypto/openssl/compat.h              | 265 ++++++++++++++++++---------
 drivers/crypto/openssl/rte_openssl_pmd.c     |  10 +-
 drivers/crypto/openssl/rte_openssl_pmd_ops.c |  12 +-
 3 files changed, 188 insertions(+), 99 deletions(-)

diff --git a/drivers/crypto/openssl/compat.h b/drivers/crypto/openssl/compat.h
index 45f9a33dc..eecb7d369 100644
--- a/drivers/crypto/openssl/compat.h
+++ b/drivers/crypto/openssl/compat.h
@@ -7,101 +7,190 @@
 
 #if (OPENSSL_VERSION_NUMBER < 0x10100000L)
 
-#define set_rsa_params(rsa, p, q, ret) \
-	do {rsa->p = p; rsa->q = q; ret = 0; } while (0)
-
-#define set_rsa_crt_params(rsa, dmp1, dmq1, iqmp, ret) \
-	do { \
-		rsa->dmp1 = dmp1; \
-		rsa->dmq1 = dmq1; \
-		rsa->iqmp = iqmp; \
-		ret = 0; \
-	} while (0)
-
-#define set_rsa_keys(rsa, n, e, d, ret) \
-	do { \
-		rsa->n = n; rsa->e = e; rsa->d = d; ret = 0; \
-	} while (0)
-
-#define set_dh_params(dh, p, g, ret) \
-	do { \
-		dh->p = p; \
-		dh->q = NULL; \
-		dh->g = g; \
-		ret = 0; \
-	} while (0)
-
-#define set_dh_priv_key(dh, priv_key, ret) \
-	do { dh->priv_key = priv_key; ret = 0; } while (0)
-
-#define set_dsa_params(dsa, p, q, g, ret) \
-	do { dsa->p = p; dsa->q = q; dsa->g = g; ret = 0; } while (0)
-
-#define get_dh_pub_key(dh, pub_key) \
-	(pub_key = dh->pub_key)
-
-#define get_dh_priv_key(dh, priv_key) \
-	(priv_key = dh->priv_key)
-
-#define set_dsa_sign(sign, r, s) \
-	do { sign->r = r; sign->s = s; } while (0)
-
-#define get_dsa_sign(sign, r, s) \
-	do { r = sign->r; s = sign->s; } while (0)
-
-#define set_dsa_keys(dsa, pub, priv, ret) \
-	do { dsa->pub_key = pub; dsa->priv_key = priv; ret = 0; } while (0)
-
-#define set_dsa_pub_key(dsa, pub_key) \
-	(dsa->pub_key = pub_key)
-
-#define get_dsa_priv_key(dsa, priv_key) \
-	(priv_key = dsa->priv_key)
+static __rte_always_inline int
+set_rsa_params(RSA *rsa, BIGNUM *p, BIGNUM *q)
+{
+	rsa->p = p;
+	rsa->q = q;
+	return 0;
+}
+
+static __rte_always_inline int
+set_rsa_crt_params(RSA *rsa, BIGNUM *dmp1, BIGNUM *dmq1, BIGNUM *iqmp)
+{
+	rsa->dmp1 = dmp1;
+	rsa->dmq1 = dmq1;
+	rsa->iqmp = iqmp;
+	return 0;
+}
+
+static __rte_always_inline int
+set_rsa_keys(RSA *rsa, BIGNUM *n, BIGNUM *e, BIGNUM *d)
+{
+	rsa->n = n;
+	rsa->e = e;
+	rsa->d = d;
+	return 0;
+}
+
+static __rte_always_inline int
+set_dh_params(DH *dh, BIGNUM *p, BIGNUM *g)
+{
+	dh->p = p;
+	dh->q = NULL;
+	dh->g = g;
+	return 0;
+}
+
+static __rte_always_inline int
+set_dh_priv_key(DH *dh, BIGNUM *priv_key)
+{
+	dh->priv_key = priv_key;
+	return 0;
+}
+
+static __rte_always_inline int
+set_dsa_params(DSA *dsa, BIGNUM *p, BIGNUM *q, BIGNUM *g)
+{
+	dsa->p = p;
+	dsa->q = q;
+	dsa->g = g;
+	return 0;
+}
+
+static __rte_always_inline void
+get_dh_pub_key(DH *dh, const BIGNUM **pub_key)
+{
+	*pub_key = dh->pub_key;
+}
+
+static __rte_always_inline void
+get_dh_priv_key(DH *dh, const BIGNUM **priv_key)
+{
+	*priv_key = dh->priv_key;
+}
+
+static __rte_always_inline void
+set_dsa_sign(DSA_SIG *sign, BIGNUM *r, BIGNUM *s)
+{
+	sign->r = r;
+	sign->s = s;
+}
+
+static __rte_always_inline void
+get_dsa_sign(DSA_SIG *sign, const BIGNUM **r, const BIGNUM **s)
+{
+	*r = sign->r;
+	*s = sign->s;
+}
+
+static __rte_always_inline int
+set_dsa_keys(DSA *dsa, BIGNUM *pub, BIGNUM *priv)
+{
+	dsa->pub_key = pub;
+	dsa->priv_key = priv;
+	return 0;
+}
+
+static __rte_always_inline void
+set_dsa_pub_key(DSA *dsa, BIGNUM *pub)
+{
+	dsa->pub_key = pub;
+}
+
+static __rte_always_inline void
+get_dsa_priv_key(DSA *dsa, BIGNUM **priv_key)
+{
+	*priv_key = dsa->priv_key;
+}
 
 #else
 
-#define set_rsa_params(rsa, p, q, ret) \
-	(ret = !RSA_set0_factors(rsa, p, q))
+static __rte_always_inline int
+set_rsa_params(RSA *rsa, BIGNUM *p, BIGNUM *q)
+{
+	return !(RSA_set0_factors(rsa, p, q));
+}
 
-#define set_rsa_crt_params(rsa, dmp1, dmq1, iqmp, ret) \
-	(ret = !RSA_set0_crt_params(rsa, dmp1, dmq1, iqmp))
+static __rte_always_inline int
+set_rsa_crt_params(RSA *rsa, BIGNUM *dmp1, BIGNUM *dmq1, BIGNUM *iqmp)
+{
+	return !(RSA_set0_crt_params(rsa, dmp1, dmq1, iqmp));
+}
 
 /* n, e must be non-null, d can be NULL */
-#define set_rsa_keys(rsa, n, e, d, ret) \
-	(ret = !RSA_set0_key(rsa, n, e, d))
-
-#define set_dh_params(dh, p, g, ret) \
-	(ret = !DH_set0_pqg(dh, p, NULL, g))
-
-#define set_dh_priv_key(dh, priv_key, ret) \
-	(ret = !DH_set0_key(dh, NULL, priv_key))
-
-#define get_dh_pub_key(dh, pub_key) \
-	(DH_get0_key(dh_key, &pub_key, NULL))
-
-#define get_dh_priv_key(dh, priv_key) \
-	(DH_get0_key(dh_key, NULL, &priv_key))
-
-#define set_dsa_params(dsa, p, q, g, ret) \
-	(ret = !DSA_set0_pqg(dsa, p, q, g))
-
-#define set_dsa_priv_key(dsa, priv_key) \
-	(DSA_set0_key(dsa, NULL, priv_key))
-
-#define set_dsa_sign(sign, r, s) \
-	(DSA_SIG_set0(sign, r, s))
-
-#define get_dsa_sign(sign, r, s) \
-	(DSA_SIG_get0(sign, &r, &s))
-
-#define set_dsa_keys(dsa, pub, priv, ret) \
-	(ret = !DSA_set0_key(dsa, pub, priv))
-
-#define set_dsa_pub_key(dsa, pub_key) \
-	(DSA_set0_key(dsa, pub_key, NULL))
 
-#define get_dsa_priv_key(dsa, priv_key) \
-	(DSA_get0_key(dsa, NULL, &priv_key))
+static __rte_always_inline  int
+set_rsa_keys(RSA *rsa, BIGNUM *n, BIGNUM *e, BIGNUM *d)
+{
+	return !(RSA_set0_key(rsa, n, e, d));
+}
+
+static __rte_always_inline int
+set_dh_params(DH *dh, BIGNUM *p, BIGNUM *g)
+{
+	return !(DH_set0_pqg(dh, p, NULL, g));
+}
+
+static __rte_always_inline int
+set_dh_priv_key(DH *dh, BIGNUM *priv_key)
+{
+	return !(DH_set0_key(dh, NULL, priv_key));
+}
+
+static __rte_always_inline void
+get_dh_pub_key(DH *dh_key, const BIGNUM **pub_key)
+{
+	DH_get0_key(dh_key, pub_key, NULL);
+}
+
+static __rte_always_inline void
+get_dh_priv_key(DH *dh_key, const BIGNUM **priv_key)
+{
+	DH_get0_key(dh_key, NULL, priv_key);
+}
+
+static __rte_always_inline int
+set_dsa_params(DSA *dsa, BIGNUM *p, BIGNUM *q, BIGNUM *g)
+{
+	return !(DSA_set0_pqg(dsa, p, q, g));
+}
+
+static __rte_always_inline void
+set_dsa_priv_key(DSA *dsa, BIGNUM *priv_key)
+{
+	DSA_set0_key(dsa, NULL, priv_key);
+}
+
+static __rte_always_inline void
+set_dsa_sign(DSA_SIG *sign, BIGNUM *r, BIGNUM *s)
+{
+	DSA_SIG_set0(sign, r, s);
+}
+
+static __rte_always_inline void
+get_dsa_sign(DSA_SIG *sign, const BIGNUM **r, const BIGNUM **s)
+{
+	DSA_SIG_get0(sign, r, s);
+}
+
+static __rte_always_inline int
+set_dsa_keys(DSA *dsa, BIGNUM *pub, BIGNUM *priv)
+{
+	return !(DSA_set0_key(dsa, pub, priv));
+}
+
+static __rte_always_inline void
+set_dsa_pub_key(DSA *dsa, BIGNUM *pub_key)
+{
+	DSA_set0_key(dsa, pub_key, NULL);
+}
+
+static __rte_always_inline void
+get_dsa_priv_key(DSA *dsa, const BIGNUM **priv_key)
+{
+	DSA_get0_key(dsa, NULL, priv_key);
+}
 
 #endif /* version < 10100000 */
 
diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 7d263aba3..7cf42199c 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -1564,7 +1564,7 @@ process_openssl_dsa_sign_op(struct rte_crypto_op *cop,
 		cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
 	} else {
 		const BIGNUM *r = NULL, *s = NULL;
-		get_dsa_sign(sign, r, s);
+		get_dsa_sign(sign, &r, &s);
 
 		op->r.length = BN_bn2bin(r, op->r.data);
 		op->s.length = BN_bn2bin(s, op->s.data);
@@ -1666,7 +1666,7 @@ process_openssl_dh_op(struct rte_crypto_op *cop,
 			cop->status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
 			return -1;
 		}
-		set_dh_priv_key(dh_key, priv_key, ret);
+		ret = set_dh_priv_key(dh_key, priv_key);
 		if (ret) {
 			OPENSSL_LOG(ERR, "Failed to set private key\n");
 			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
@@ -1715,7 +1715,7 @@ process_openssl_dh_op(struct rte_crypto_op *cop,
 			cop->status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
 			return -1;
 		}
-		set_dh_priv_key(dh_key, priv_key, ret);
+		ret = set_dh_priv_key(dh_key, priv_key);
 		if (ret) {
 			OPENSSL_LOG(ERR, "Failed to set private key\n");
 			cop->status = RTE_CRYPTO_OP_STATUS_ERROR;
@@ -1743,7 +1743,7 @@ process_openssl_dh_op(struct rte_crypto_op *cop,
 				__func__, __LINE__);
 
 		/* get the generated keys */
-		get_dh_pub_key(dh_key, pub_key);
+		get_dh_pub_key(dh_key, &pub_key);
 
 		/* output public key */
 		op->pub_key.length = BN_bn2bin(pub_key,
@@ -1758,7 +1758,7 @@ process_openssl_dh_op(struct rte_crypto_op *cop,
 				__func__, __LINE__);
 
 		/* get the generated keys */
-		get_dh_priv_key(dh_key, priv_key);
+		get_dh_priv_key(dh_key, &priv_key);
 
 		/* provide generated private key back to user */
 		op->priv_key.length = BN_bn2bin(priv_key,
diff --git a/drivers/crypto/openssl/rte_openssl_pmd_ops.c b/drivers/crypto/openssl/rte_openssl_pmd_ops.c
index de2284390..4ad8fa1f8 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd_ops.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd_ops.c
@@ -875,14 +875,14 @@ static int openssl_set_asym_session_parameters(
 				RSA_free(rsa);
 				goto err_rsa;
 			}
-			set_rsa_params(rsa, p, q, ret);
+			ret = set_rsa_params(rsa, p, q);
 			if (ret) {
 				OPENSSL_LOG(ERR,
 					"failed to set rsa params\n");
 				RSA_free(rsa);
 				goto err_rsa;
 			}
-			set_rsa_crt_params(rsa, dmp1, dmq1, iqmp, ret);
+			ret = set_rsa_crt_params(rsa, dmp1, dmq1, iqmp);
 			if (ret) {
 				OPENSSL_LOG(ERR,
 					"failed to set crt params\n");
@@ -896,7 +896,7 @@ static int openssl_set_asym_session_parameters(
 			}
 		}
 
-		set_rsa_keys(rsa, n, e, d, ret);
+		ret = set_rsa_keys(rsa, n, e, d);
 		if (ret) {
 			OPENSSL_LOG(ERR, "Failed to load rsa keys\n");
 			RSA_free(rsa);
@@ -1005,7 +1005,7 @@ static int openssl_set_asym_session_parameters(
 				"failed to allocate resources\n");
 			goto err_dh;
 		}
-		set_dh_params(dh, p, g, ret);
+		ret = set_dh_params(dh, p, g);
 		if (ret) {
 			DH_free(dh);
 			goto err_dh;
@@ -1087,7 +1087,7 @@ static int openssl_set_asym_session_parameters(
 			goto err_dsa;
 		}
 
-		set_dsa_params(dsa, p, q, g, ret);
+		ret = set_dsa_params(dsa, p, q, g);
 		if (ret) {
 			DSA_free(dsa);
 			OPENSSL_LOG(ERR, "Failed to dsa params\n");
@@ -1101,7 +1101,7 @@ static int openssl_set_asym_session_parameters(
 		 * both versions
 		 */
 		/* just set dummy public for very 1st call */
-		set_dsa_keys(dsa, pub_key, priv_key, ret);
+		ret = set_dsa_keys(dsa, pub_key, priv_key);
 		if (ret) {
 			DSA_free(dsa);
 			OPENSSL_LOG(ERR, "Failed to set keys\n");
-- 
2.14.3

Re: [dpdk-dev] [PATCH v1 1/1] crypto/openssl: replace macros by static inline functions

[De Lara Guarch, Pablo]

Hi Ashish,

> -----Original Message-----
> From: Ashish Gupta [mailto:Ashish.Gupta@caviumnetworks.com]
> Sent: Tuesday, July 31, 2018 3:16 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; narayanaprasad.athreya@cavium.com;
> nidadavolu.murthy@cavium.com; Sunila Sahu
> <sunila.sahu@caviumnetworks.com>; Shally Verma
> <shally.verma@caviumnetworks.com>; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>
> Subject: [PATCH v1 1/1] crypto/openssl: replace macros by static inline functions
> 
> Replace macros by static inline functions in openssl version compatibility layer
> 
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>

Thanks for the patch. Since this is not a fix, I will leave it for an early merge at start of the 18.11 cycle.

Pablo

Re: [dpdk-dev] [PATCH v1 1/1] crypto/openssl: replace macros by static inline functions

[Akhil Goyal]

On 7/31/2018 7:46 PM, Ashish Gupta wrote:

> Replace macros by static inline functions in openssl version
> compatibility layer
>
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> ---
>   drivers/crypto/openssl/compat.h              | 265 ++++++++++++++++++---------
>   drivers/crypto/openssl/rte_openssl_pmd.c     |  10 +-
>   drivers/crypto/openssl/rte_openssl_pmd_ops.c |  12 +-
>   3 files changed, 188 insertions(+), 99 deletions(-)
>
>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

Re: [dpdk-dev] [PATCH v1 1/1] crypto/openssl: replace macros by static inline functions

[Akhil Goyal]

On 7/31/2018 7:46 PM, Ashish Gupta wrote:
> Replace macros by static inline functions in openssl version
> compatibility layer
>
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> ---
>
Applied to dpdk-next-crypto

Thanks