Dmitry Eremin-Solenikov(lumag) replied on github web page:
platform/linux-dpdk/odp_crypto.c
line 62
@@ -615,7 +618,7 @@ int odp_crypto_auth_capability(odp_auth_alg_t auth,
while (cap->op != RTE_CRYPTO_OP_TYPE_UNDEFINED) {
cap_auth_algo = cap->sym.auth.algo;
if (cap->sym.xform_type ==
- RTE_CRYPTO_SYM_XFORM_CIPHER) {
+ RTE_CRYPTO_SYM_XFORM_AUTH) {
if (cap_auth_algo == auth_xform.auth.algo)
Comment:
Liked this typo. It shows that the code never worked as expected.
> Dmitry Eremin-Solenikov(lumag) wrote:
> 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_r161764746
updated_at 2018-01-16 14:21:10
