| Message ID | 1494852174-9403-1-git-send-email-balakrishna.garapati@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 15.05.2017 15:42, Balakrishna Garapati wrote: > This change is needed to support dpdk crypto pmd's Could you please be more specific, what are you trying to express. If you need, you can provide several entries, one for each IV len. > > Signed-off-by: Balakrishna Garapati <balakrishna.garapati@linaro.org> > --- > include/odp/api/spec/crypto.h | 12 +++- > platform/linux-generic/odp_crypto.c | 8 +-- > .../validation/api/crypto/odp_crypto_test_inp.c | 76 +++++----------------- > 3 files changed, 31 insertions(+), 65 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; > > 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..628f150 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); > - NAK for this part. First, you have removed auth-related checks. Second, you have removed the assertion that there really is a capability supporting provided params. > /* Create a crypto session */ > odp_crypto_session_param_init(&ses_params); > ses_params.op = op; > @@ -385,9 +328,21 @@ static int check_cipher_options(odp_cipher_alg_t cipher, uint32_t key_len, > 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) > - break; > + uint8_t inc = cipher_capa[i].iv_len.inc; > + uint8_t iv_min = cipher_capa[i].iv_len.min; > + uint8_t iv_max = cipher_capa[i].iv_len.max; > + > + if (inc == 0) { > + if (iv_len == iv_min && > + key_len == cipher_capa[i].key_len) > + break; > + } else { > + for (uint8_t iv = iv_min; iv <= iv_max; iv = iv + inc) { > + if (iv_len == iv && > + key_len == cipher_capa[i].key_len) > + break; This will break only inner loop. Outer loop will continue afterwards. Have you tested your changes with inc != 0? > + } > + } > } > > if (i == num) { > @@ -396,6 +351,7 @@ static int check_cipher_options(odp_cipher_alg_t cipher, uint32_t key_len, > iv_len); > return 0; > } > + > return 1; > } Unnecessary change. -- With best wishes Dmitry
On 15 May 2017 at 19:38, Dmitry Eremin-Solenikov < dmitry.ereminsolenikov@linaro.org> wrote: > On 15.05.2017 15:42, Balakrishna Garapati wrote: > > This change is needed to support dpdk crypto pmd's > > Could you please be more specific, what are you trying to express. If > you need, you can provide several entries, one for each IV len. > Yes you can create multiple entries of iv. But the issue comes when you have the varied key_len as well (min, max, inc). So the permutation of creating number of entries will be complex. This patch now can create multiple entries based on key_len. I will send out a new version with clear description. > > > > > Signed-off-by: Balakrishna Garapati <balakrishna.garapati@linaro.org> > > --- > > include/odp/api/spec/crypto.h | 12 +++- > > platform/linux-generic/odp_crypto.c | 8 +-- > > .../validation/api/crypto/odp_crypto_test_inp.c | 76 > +++++----------------- > > 3 files changed, 31 insertions(+), 65 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; > > > > 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..628f150 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); > > - > > NAK for this part. First, you have removed auth-related checks. Second, > you have removed the assertion that there really is a capability > supporting provided params. We are already doing this check with in the each test case "check_cipher_options", "check_auth_options". That's why removed the check from here. > > > /* Create a crypto session */ > > odp_crypto_session_param_init(&ses_params); > > ses_params.op = op; > > @@ -385,9 +328,21 @@ static int check_cipher_options(odp_cipher_alg_t > cipher, uint32_t key_len, > > 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) > > - break; > > + uint8_t inc = cipher_capa[i].iv_len.inc; > > + uint8_t iv_min = cipher_capa[i].iv_len.min; > > + uint8_t iv_max = cipher_capa[i].iv_len.max; > > + > > + if (inc == 0) { > > + if (iv_len == iv_min && > > + key_len == cipher_capa[i].key_len) > > + break; > > + } else { > > + for (uint8_t iv = iv_min; iv <= iv_max; iv = iv + > inc) { > > + if (iv_len == iv && > > + key_len == cipher_capa[i].key_len) > > + break; > > This will break only inner loop. Outer loop will continue afterwards. > Have you tested your changes with inc != 0? > No didn't check. But agree it's a bug here. Will fix it in the next version. /Krishna > > > + } > > + } > > } > > > > if (i == num) { > > @@ -396,6 +351,7 @@ static int check_cipher_options(odp_cipher_alg_t > cipher, uint32_t key_len, > > iv_len); > > return 0; > > } > > + > > return 1; > > } > > Unnecessary change. > > > -- > With best wishes > Dmitry >
On 16.05.2017 10:43, Krishna Garapati wrote: > > > On 15 May 2017 at 19:38, Dmitry Eremin-Solenikov > <dmitry.ereminsolenikov@linaro.org > <mailto:dmitry.ereminsolenikov@linaro.org>> wrote: > > On 15.05.2017 15:42, Balakrishna Garapati wrote: > > This change is needed to support dpdk crypto pmd's > > Could you please be more specific, what are you trying to express. If > you need, you can provide several entries, one for each IV len. > > Yes you can create multiple entries of iv. But the issue comes when you > have the varied key_len as well (min, max, inc). So the permutation of > creating number of entries will be complex. This patch now can create > multiple entries based on key_len. I will send out a new version with > clear description. Just go with multiple entries, that is clearer way compared with iv_inc, in my opinion. Could you please be more specific and specify, what would be the entries in case of DPDK? -- With best wishes Dmitry
On 16 May 2017 at 17:46, Dmitry Eremin-Solenikov < dmitry.ereminsolenikov@linaro.org> wrote: > On 16.05.2017 10:43, Krishna Garapati wrote: > > > > > > On 15 May 2017 at 19:38, Dmitry Eremin-Solenikov > > <dmitry.ereminsolenikov@linaro.org > > <mailto:dmitry.ereminsolenikov@linaro.org>> wrote: > > > > On 15.05.2017 15:42, Balakrishna Garapati wrote: > > > This change is needed to support dpdk crypto pmd's > > > > Could you please be more specific, what are you trying to express. If > > you need, you can provide several entries, one for each IV len. > > > > Yes you can create multiple entries of iv. But the issue comes when you > > have the varied key_len as well (min, max, inc). So the permutation of > > creating number of entries will be complex. This patch now can create > > multiple entries based on key_len. I will send out a new version with > > clear description. > > Just go with multiple entries, that is clearer way compared with iv_inc, > in my opinion. Could you please be more specific and specify, what would > be the entries in case of DPDK? The entries in DPDK is as mentioned in this patch (min, max, inc) for both key_len & iv_len. It would be easier and cleaner creating and cross verifying the entries based on one field ex: key_len. /Krishna > > -- > With best wishes > Dmitry >
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..628f150 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; @@ -385,9 +328,21 @@ static int check_cipher_options(odp_cipher_alg_t cipher, uint32_t key_len, 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) - break; + uint8_t inc = cipher_capa[i].iv_len.inc; + uint8_t iv_min = cipher_capa[i].iv_len.min; + uint8_t iv_max = cipher_capa[i].iv_len.max; + + if (inc == 0) { + if (iv_len == iv_min && + key_len == cipher_capa[i].key_len) + break; + } else { + for (uint8_t iv = iv_min; iv <= iv_max; iv = iv + inc) { + if (iv_len == iv && + key_len == cipher_capa[i].key_len) + break; + } + } } if (i == num) { @@ -396,6 +351,7 @@ static int check_cipher_options(odp_cipher_alg_t cipher, uint32_t key_len, iv_len); return 0; } + return 1; }
This change is needed to support dpdk crypto pmd's Signed-off-by: Balakrishna Garapati <balakrishna.garapati@linaro.org> --- include/odp/api/spec/crypto.h | 12 +++- platform/linux-generic/odp_crypto.c | 8 +-- .../validation/api/crypto/odp_crypto_test_inp.c | 76 +++++----------------- 3 files changed, 31 insertions(+), 65 deletions(-) -- 1.9.1