diff mbox

[API-NEXT,PATCHv4] api: crypto: change iv_len field to feature min-max len

Message ID 1495203810-9148-1-git-send-email-balakrishna.garapati@linaro.org
State New
Headers show

Commit Message

Balakrishna Garapati May 19, 2017, 2:23 p.m. UTC 
dpdk pmd's require to minatain min, max, inc fields for all
crypto capabilitites. By making iv_len to feature like dpdk pmd's
it is easier to create cipher entires in odp based on key_len.

Signed-off-by: Balakrishna Garapati <balakrishna.garapati@linaro.org>

---
 since v1: fixed comments from v1,
           - updated the description about the patch
           - Fixed the bug in crypto validation
 since v2: if statement correction in validation "check_cipher_options"
 since v3: updated doing key_len check prior to iv_len and corrected iv_len
 data type in validation "check_cipher_options"

 include/odp/api/spec/crypto.h                      | 12 +++-
 platform/linux-generic/odp_crypto.c                |  8 +--
 .../validation/api/crypto/odp_crypto_test_inp.c    | 81 ++++++----------------
 3 files changed, 37 insertions(+), 64 deletions(-)

--
1.9.1

Comments

Balasubramanian Manoharan May 19, 2017, 9:59 p.m. UTC | #1 
Regards,
Bala


On 19 May 2017 at 07:23, Balakrishna Garapati
<balakrishna.garapati@linaro.org> wrote:
> dpdk pmd's require to minatain min, max, inc fields for all

> crypto capabilitites. By making iv_len to feature like dpdk pmd's

> it is easier to create cipher entires in odp based on key_len.

>

> Signed-off-by: Balakrishna Garapati <balakrishna.garapati@linaro.org>

> ---

>  since v1: fixed comments from v1,

>            - updated the description about the patch

>            - Fixed the bug in crypto validation

>  since v2: if statement correction in validation "check_cipher_options"

>  since v3: updated doing key_len check prior to iv_len and corrected iv_len

>  data type in validation "check_cipher_options"

>

>  include/odp/api/spec/crypto.h                      | 12 +++-

>  platform/linux-generic/odp_crypto.c                |  8 +--

>  .../validation/api/crypto/odp_crypto_test_inp.c    | 81 ++++++----------------

>  3 files changed, 37 insertions(+), 64 deletions(-)

>

> diff --git a/include/odp/api/spec/crypto.h b/include/odp/api/spec/crypto.h

> index c47d314..a1b540a 100644

> --- a/include/odp/api/spec/crypto.h

> +++ b/include/odp/api/spec/crypto.h

> @@ -497,7 +497,17 @@ typedef struct odp_crypto_cipher_capability_t {

>         uint32_t key_len;

>

>         /** IV length in bytes */

> -       uint32_t iv_len;

> +       struct {

> +               /** Minimum iv length in bytes */

> +               uint32_t min;

> +

> +               /** Maximum iv length in bytes */

> +               uint32_t max;

> +

> +               /** Increment of supported lengths between min and max

> +                *  (in bytes) */

> +               uint32_t inc;

> +       } iv_len;

>

>  } odp_crypto_cipher_capability_t;


This is part of capability API which will be in control path and not
in the data path, IMO I feel this change adds more complexity.

Regards,
Bala
>

> diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c

> index a0f3f7e..3e9b607 100644

> --- a/platform/linux-generic/odp_crypto.c

> +++ b/platform/linux-generic/odp_crypto.c

> @@ -35,16 +35,16 @@

>   * Keep sorted: first by key length, then by IV length

>   */

>  static const odp_crypto_cipher_capability_t cipher_capa_des[] = {

> -{.key_len = 24, .iv_len = 8} };

> +{.key_len = 24, .iv_len = {.min = 8, .max = 8, .inc = 0} } };

>

>  static const odp_crypto_cipher_capability_t cipher_capa_trides_cbc[] = {

> -{.key_len = 24, .iv_len = 8} };

> +{.key_len = 24, .iv_len = {.min = 8, .max = 8, .inc = 0} } };

>

>  static const odp_crypto_cipher_capability_t cipher_capa_aes_cbc[] = {

> -{.key_len = 16, .iv_len = 16} };

> +{.key_len = 16, .iv_len = {.min = 16, .max = 16, .inc = 0} } };

>

>  static const odp_crypto_cipher_capability_t cipher_capa_aes_gcm[] = {

> -{.key_len = 16, .iv_len = 12} };

> +{.key_len = 16, .iv_len = {.min = 12, .max = 12, .inc = 0} } };

>

>  /*

>   * Authentication algorithm capabilities

> diff --git a/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c b/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c

> index 24ea493..0545384 100644

> --- a/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c

> +++ b/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c

> @@ -93,10 +93,6 @@ static void alg_test(odp_crypto_op_t op,

>         odp_crypto_op_param_t op_params;

>         uint8_t *data_addr;

>         int data_off;

> -       odp_crypto_cipher_capability_t cipher_capa[MAX_ALG_CAPA];

> -       odp_crypto_auth_capability_t   auth_capa[MAX_ALG_CAPA];

> -       int num, i;

> -       int found;

>

>         rc = odp_crypto_capability(&capa);

>         CU_ASSERT(!rc);

> @@ -136,59 +132,6 @@ static void alg_test(odp_crypto_op_t op,

>         CU_ASSERT(!rc);

>         CU_ASSERT((~capa.auths.all_bits & capa.hw_auths.all_bits) == 0);

>

> -       num = odp_crypto_cipher_capability(cipher_alg, cipher_capa,

> -                                          MAX_ALG_CAPA);

> -

> -       if (cipher_alg != ODP_CIPHER_ALG_NULL) {

> -               CU_ASSERT(num > 0);

> -               found = 0;

> -       } else {

> -               CU_ASSERT(num == 0);

> -               found = 1;

> -       }

> -

> -       CU_ASSERT(num <= MAX_ALG_CAPA);

> -

> -       if (num > MAX_ALG_CAPA)

> -               num = MAX_ALG_CAPA;

> -

> -       /* Search for the test case */

> -       for (i = 0; i < num; i++) {

> -               if (cipher_capa[i].key_len == cipher_key.length &&

> -                   cipher_capa[i].iv_len  == ses_iv.length) {

> -                       found = 1;

> -                       break;

> -               }

> -       }

> -

> -       CU_ASSERT(found);

> -

> -       num = odp_crypto_auth_capability(auth_alg, auth_capa, MAX_ALG_CAPA);

> -

> -       if (auth_alg != ODP_AUTH_ALG_NULL) {

> -               CU_ASSERT(num > 0);

> -               found = 0;

> -       } else {

> -               CU_ASSERT(num == 0);

> -               found = 1;

> -       }

> -

> -       CU_ASSERT(num <= MAX_ALG_CAPA);

> -

> -       if (num > MAX_ALG_CAPA)

> -               num = MAX_ALG_CAPA;

> -

> -       /* Search for the test case */

> -       for (i = 0; i < num; i++) {

> -               if (auth_capa[i].digest_len == digest_len &&

> -                   auth_capa[i].key_len    == auth_key.length) {

> -                       found = 1;

> -                       break;

> -               }

> -       }

> -

> -       CU_ASSERT(found);

> -

>         /* Create a crypto session */

>         odp_crypto_session_param_init(&ses_params);

>         ses_params.op = op;

> @@ -379,14 +322,34 @@ static int check_cipher_options(odp_cipher_alg_t cipher, uint32_t key_len,

>  {

>         int i;

>         int num;

> +       int found = 0;

> +

>         odp_crypto_cipher_capability_t cipher_capa[MAX_ALG_CAPA];

>

>         num = odp_crypto_cipher_capability(cipher, cipher_capa, MAX_ALG_CAPA);

>         CU_ASSERT_FATAL(num >= 1);

>

>         for (i = 0; i < num; i++) {

> -               if (key_len == cipher_capa[i].key_len &&

> -                   iv_len == cipher_capa[i].iv_len)

> +               if (key_len != cipher_capa[i].key_len)

> +                       continue;

> +

> +               uint32_t inc = cipher_capa[i].iv_len.inc;

> +               uint32_t iv_min = cipher_capa[i].iv_len.min;

> +               uint32_t iv_max = cipher_capa[i].iv_len.max;

> +

> +               if (inc == 0 && iv_len == iv_min) {

> +                               found = 1;

> +               } else {

> +                       for (uint32_t iv = iv_min; iv <= iv_max;

> +                                                  iv = iv + inc) {

> +                               if (iv_len == iv) {

> +                                       found = 1;

> +                                       break;

> +                               }

> +                       }

> +               }

> +

> +               if (found == 1)

>                         break;

>         }

>

> --

> 1.9.1

>
Savolainen, Petri (Nokia - FI/Espoo) May 23, 2017, 1:14 p.m. UTC | #2 
> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> Balakrishna Garapati

> Sent: Friday, May 19, 2017 5:24 PM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [API-NEXT PATCHv4] api: crypto: change iv_len field to

> feature min-max len

> 

> dpdk pmd's require to minatain min, max, inc fields for all

> crypto capabilitites. By making iv_len to feature like dpdk pmd's

> it is easier to create cipher entires in odp based on key_len.

> 

> Signed-off-by: Balakrishna Garapati <balakrishna.garapati@linaro.org>

> ---

>  since v1: fixed comments from v1,

>            - updated the description about the patch

>            - Fixed the bug in crypto validation

>  since v2: if statement correction in validation "check_cipher_options"

>  since v3: updated doing key_len check prior to iv_len and corrected

> iv_len

>  data type in validation "check_cipher_options"

> 

>  include/odp/api/spec/crypto.h                      | 12 +++-

>  platform/linux-generic/odp_crypto.c                |  8 +--

>  .../validation/api/crypto/odp_crypto_test_inp.c    | 81 ++++++-----------

> -----

>  3 files changed, 37 insertions(+), 64 deletions(-)

> 

> diff --git a/include/odp/api/spec/crypto.h b/include/odp/api/spec/crypto.h

> index c47d314..a1b540a 100644

> --- a/include/odp/api/spec/crypto.h

> +++ b/include/odp/api/spec/crypto.h

> @@ -497,7 +497,17 @@ typedef struct odp_crypto_cipher_capability_t {

>  	uint32_t key_len;

> 

>  	/** IV length in bytes */

> -	uint32_t iv_len;

> +	struct {

> +		/** Minimum iv length in bytes */

> +		uint32_t min;

> +

> +		/** Maximum iv length in bytes */

> +		uint32_t max;

> +

> +		/** Increment of supported lengths between min and

> max

> +		 *  (in bytes) */

> +		uint32_t inc;

> +	} iv_len;



I choose to not use this mechanism since:
* number of key_len / iv_len combination should not be too large
* this is inflexible if there are holes in the range: e.g. iv_len == 4, 8, 16 could not be expressed with min/max/inc
* application should have an easy job to scroll through a sorted list of key/iv lengths
* implementation can have predefined arrays sorted already at build time

-Petri
diff mbox

Patch

diff --git a/include/odp/api/spec/crypto.h b/include/odp/api/spec/crypto.h
index c47d314..a1b540a 100644
--- a/include/odp/api/spec/crypto.h
+++ b/include/odp/api/spec/crypto.h
@@ -497,7 +497,17 @@  typedef struct odp_crypto_cipher_capability_t {
 	uint32_t key_len;

 	/** IV length in bytes */
-	uint32_t iv_len;
+	struct {
+		/** Minimum iv length in bytes */
+		uint32_t min;
+
+		/** Maximum iv length in bytes */
+		uint32_t max;
+
+		/** Increment of supported lengths between min and max
+		 *  (in bytes) */
+		uint32_t inc;
+	} iv_len;

 } odp_crypto_cipher_capability_t;

diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c
index a0f3f7e..3e9b607 100644
--- a/platform/linux-generic/odp_crypto.c
+++ b/platform/linux-generic/odp_crypto.c
@@ -35,16 +35,16 @@ 
  * Keep sorted: first by key length, then by IV length
  */
 static const odp_crypto_cipher_capability_t cipher_capa_des[] = {
-{.key_len = 24, .iv_len = 8} };
+{.key_len = 24, .iv_len = {.min = 8, .max = 8, .inc = 0} } };

 static const odp_crypto_cipher_capability_t cipher_capa_trides_cbc[] = {
-{.key_len = 24, .iv_len = 8} };
+{.key_len = 24, .iv_len = {.min = 8, .max = 8, .inc = 0} } };

 static const odp_crypto_cipher_capability_t cipher_capa_aes_cbc[] = {
-{.key_len = 16, .iv_len = 16} };
+{.key_len = 16, .iv_len = {.min = 16, .max = 16, .inc = 0} } };

 static const odp_crypto_cipher_capability_t cipher_capa_aes_gcm[] = {
-{.key_len = 16, .iv_len = 12} };
+{.key_len = 16, .iv_len = {.min = 12, .max = 12, .inc = 0} } };

 /*
  * Authentication algorithm capabilities
diff --git a/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c b/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c
index 24ea493..0545384 100644
--- a/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c
+++ b/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c
@@ -93,10 +93,6 @@  static void alg_test(odp_crypto_op_t op,
 	odp_crypto_op_param_t op_params;
 	uint8_t *data_addr;
 	int data_off;
-	odp_crypto_cipher_capability_t cipher_capa[MAX_ALG_CAPA];
-	odp_crypto_auth_capability_t   auth_capa[MAX_ALG_CAPA];
-	int num, i;
-	int found;

 	rc = odp_crypto_capability(&capa);
 	CU_ASSERT(!rc);
@@ -136,59 +132,6 @@  static void alg_test(odp_crypto_op_t op,
 	CU_ASSERT(!rc);
 	CU_ASSERT((~capa.auths.all_bits & capa.hw_auths.all_bits) == 0);

-	num = odp_crypto_cipher_capability(cipher_alg, cipher_capa,
-					   MAX_ALG_CAPA);
-
-	if (cipher_alg != ODP_CIPHER_ALG_NULL) {
-		CU_ASSERT(num > 0);
-		found = 0;
-	} else {
-		CU_ASSERT(num == 0);
-		found = 1;
-	}
-
-	CU_ASSERT(num <= MAX_ALG_CAPA);
-
-	if (num > MAX_ALG_CAPA)
-		num = MAX_ALG_CAPA;
-
-	/* Search for the test case */
-	for (i = 0; i < num; i++) {
-		if (cipher_capa[i].key_len == cipher_key.length &&
-		    cipher_capa[i].iv_len  == ses_iv.length) {
-			found = 1;
-			break;
-		}
-	}
-
-	CU_ASSERT(found);
-
-	num = odp_crypto_auth_capability(auth_alg, auth_capa, MAX_ALG_CAPA);
-
-	if (auth_alg != ODP_AUTH_ALG_NULL) {
-		CU_ASSERT(num > 0);
-		found = 0;
-	} else {
-		CU_ASSERT(num == 0);
-		found = 1;
-	}
-
-	CU_ASSERT(num <= MAX_ALG_CAPA);
-
-	if (num > MAX_ALG_CAPA)
-		num = MAX_ALG_CAPA;
-
-	/* Search for the test case */
-	for (i = 0; i < num; i++) {
-		if (auth_capa[i].digest_len == digest_len &&
-		    auth_capa[i].key_len    == auth_key.length) {
-			found = 1;
-			break;
-		}
-	}
-
-	CU_ASSERT(found);
-
 	/* Create a crypto session */
 	odp_crypto_session_param_init(&ses_params);
 	ses_params.op = op;
@@ -379,14 +322,34 @@  static int check_cipher_options(odp_cipher_alg_t cipher, uint32_t key_len,
 {
 	int i;
 	int num;
+	int found = 0;
+
 	odp_crypto_cipher_capability_t cipher_capa[MAX_ALG_CAPA];

 	num = odp_crypto_cipher_capability(cipher, cipher_capa, MAX_ALG_CAPA);
 	CU_ASSERT_FATAL(num >= 1);

 	for (i = 0; i < num; i++) {
-		if (key_len == cipher_capa[i].key_len &&
-		    iv_len == cipher_capa[i].iv_len)
+		if (key_len != cipher_capa[i].key_len)
+			continue;
+
+		uint32_t inc = cipher_capa[i].iv_len.inc;
+		uint32_t iv_min = cipher_capa[i].iv_len.min;
+		uint32_t iv_max = cipher_capa[i].iv_len.max;
+
+		if (inc == 0 && iv_len == iv_min) {
+				found = 1;
+		} else {
+			for (uint32_t iv = iv_min; iv <= iv_max;
+						   iv = iv + inc) {
+				if (iv_len == iv) {
+					found = 1;
+					break;
+				}
+			}
+		}
+
+		if (found == 1)
 			break;
 	}