Dmitry Eremin-Solenikov(lumag) replied on github web page:
platform/linux-dpdk/odp_crypto.c
line 26
@@ -122,29 +130,24 @@ static int auth_alg_odp_to_rte(odp_auth_alg_t auth_alg,
case ODP_AUTH_ALG_MD5_96:
#endif
auth_xform->auth.algo = RTE_CRYPTO_AUTH_MD5_HMAC;
- auth_xform->auth.digest_length = 12;
break;
case ODP_AUTH_ALG_SHA256_HMAC:
#if ODP_DEPRECATED_API
case ODP_AUTH_ALG_SHA256_128:
#endif
auth_xform->auth.algo = RTE_CRYPTO_AUTH_SHA256_HMAC;
- auth_xform->auth.digest_length = 16;
Comment:
Same issue here.
> Dmitry Eremin-Solenikov(lumag) wrote:
> `auth.digest_length` should be set for MD5_96 here, but not for MD5_HMAC.
>> Dmitry Eremin-Solenikov(lumag) wrote:
>> It's really not a problem at this moment, this can be fixed later.
>>> GBalakrishna wrote
>>> oke I see. I will make the update in the next version.
>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>> both = Encrypt-then-MAC and MAC-cleartext
>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>> Could you please add tests cases rather than modify existing ones?
>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>> Implementation should not set digest len for cases other than several
>>>>>> deprecated auth algos.
>>>>>>> GBalakrishna wrote
>>>>>>> setting digest_length is moved to get_crypto_dev(), where it reads
>>>>>>> from PMD and set the min digest length if application doesn't set it.
>>>>>>> Reagrding the QAT, I need to look at it and understand. may be as a
>>>>>>> separate PR.
>>>>>>>> GBalakrishna wrote
>>>>>>>> I have updated the tests so that it works with both the platforms
>>>>>>>> linux-generci & linux-dpdk.
>>>>>>>>> GBalakrishna wrote
>>>>>>>>> what do you mean by testing both combinations ?. We are testing both
>>>>>>>>> the combinations ENCODE & DECODE. It's the default value we are
>>>>>>>>> changing it.
>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>> Checkpatch flags this line:
>>>>>>>>>> ```
>>>>>>>>>> WARNING: line over 80 characters
>>>>>>>>>> #363: FILE: platform/linux-dpdk/test/wrapper-script.sh:3:
>>>>>>>>>> +export ODP_PLATFORM_PARAMS=${ODP_PLATFORM_PARAMS:--n 4 --vdev
>>>>>>>>>> "crypto_openssl" --vdev "crypto_null"}
>>>>>>>>>> total: 0 errors, 1 warnings, 0 checks, 530 lines checked
>>>>>>>>>> NOTE: Ignored message types: AVOID_EXTERNS BIT_MACRO
>>>>>>>>>> COMPARISON_TO_NULL DEPRECATED_VARIABLE NEW_TYPEDEFS PREFER_PRINTF
>>>>>>>>>> PREFER_SCANF SPLIT_STRING SSCANF_TO_KSTRTO VOLATILE
>>>>>>>>>> 0001-linux-dpdk-crypto-support-for-cipher-auth-only-featu.patch has
>>>>>>>>>> style problems, please review.
>>>>>>>>>> ```
>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>> I suggest to factor out cipher and auth checks as separate
>>>>>>>>>>> functions and apply them only if corresponding algorithm is not
>>>>>>>>>>> NULL. Code would be simpler.
>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>> For deprecated MD5_96 (and SHA256_128/AES128_GCM) application
>>>>>>>>>>>> won't set `auth.digest_len`, so you have to enforce 12 and 16
>>>>>>>>>>>> bytes digest len. For MD5_HMAC/SHA256_HMAC (and others)
>>>>>>>>>>>> application will set `auth.digest_len`.
>>>>>>>>>>>>
>>>>>>>>>>>> IIRC QAT driver should support truncated digests out-of-box. Also
>>>>>>>>>>>> you can try expanding pmd_openssl to support truncated lengths, it
>>>>>>>>>>>> should not be very complicated. Also note, that full-length HMAC
>>>>>>>>>>>> won't be useful for IPsec.
>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>> "Do not change tests". See my previous comment.
>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>> Theoretically we'd better be testing both combinations. The
>>>>>>>>>>>>>> problem with test changes in your commit is that you change
>>>>>>>>>>>>>> tests rather than expand them. Can you import tests from #379?
>>>>>>>>>>>>>>> GBalakrishna wrote
>>>>>>>>>>>>>>> not sure what do u mean here.
>>>>>>>>>>>>>>>> GBalakrishna wrote
>>>>>>>>>>>>>>>> Not sure if I have fallowed. I introduced Chain order because
>>>>>>>>>>>>>>>> dpdk PMD's doesn't have support for combination of NULL +
>>>>>>>>>>>>>>>> Valid algo. So linux-dpdk implementation converts the NULL
>>>>>>>>>>>>>>>> also to chain order as if it cipher only & auth only.
>>>>>>>>>>>>>>>>> GBalakrishna wrote
>>>>>>>>>>>>>>>>> my understanding of auth_cipher_text = true means do encode
>>>>>>>>>>>>>>>>> then authenticate. and when it is applied together with if
>>>>>>>>>>>>>>>>> (ODP_CRYPTO_OP_ENCODE == param->op) sets the
>>>>>>>>>>>>>>>>> "entry->do_cipher_first" to true by default and if the
>>>>>>>>>>>>>>>>> param->op == DECODE it sets the !auth_cipher_text. I felt
>>>>>>>>>>>>>>>>> this is more easier for understanding.
>>>>>>>>>>>>>>>>>> GBalakrishna wrote
>>>>>>>>>>>>>>>>>> openssl PMD in 17.02 supports the digest length to be min 16
>>>>>>>>>>>>>>>>>> bytes. But I have now removed the setting digest_lengths
>>>>>>>>>>>>>>>>>> from here as it sets it from the application.
>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>> why?
>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>> `non tangere testos meos`
>>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>>> I'd suggest to use ALG_NULL checks directly, rather than
>>>>>>>>>>>>>>>>>>>>> introduce chain order.
>>>>>>>>>>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>>>>>>>>>>> This is wrong. MD5_96 should use 12-byte digest length.
>>>>>>>>>>>>>>>>>>>>>> MD5_HMAC should use digest length specified by
>>>>>>>>>>>>>>>>>>>>>> application.
https://github.com/Linaro/odp/pull/385#discussion_r161764573
updated_at 2018-01-16 14:21:10
