Dmitry Eremin-Solenikov(lumag) replied on github web page:

platform/linux-dpdk/odp_crypto.c
line 41
@@ -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;
                break;
        case ODP_AUTH_ALG_SHA1_HMAC:
                auth_xform->auth.algo = RTE_CRYPTO_AUTH_SHA1_HMAC;
-               auth_xform->auth.digest_length = 20;
                break;
        case ODP_AUTH_ALG_SHA512_HMAC:
                auth_xform->auth.algo = RTE_CRYPTO_AUTH_SHA512_HMAC;
-               auth_xform->auth.digest_length = 64;
                break;
        case ODP_AUTH_ALG_AES_GCM:
 #if ODP_DEPRECATED_API
        case ODP_AUTH_ALG_AES128_GCM:
 #endif
                auth_xform->auth.algo = RTE_CRYPTO_AUTH_AES_GCM;
-               auth_xform->auth.digest_length = 16;


Comment:
same issue here.

> Dmitry Eremin-Solenikov(lumag) wrote:
> 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_r161764621
updated_at 2018-01-16 14:21:10

Reply via email to