GBalakrishna replied on github web page:
platform/linux-dpdk/test/wrapper-script.sh
line 5
@@ -1,6 +1,7 @@
#!/bin/bash
-export ODP_PLATFORM_PARAMS=${ODP_PLATFORM_PARAMS:--n 4 --vdev "crypto_openssl"}
+export ODP_PLATFORM_PARAMS=${ODP_PLATFORM_PARAMS:--n 4 "--vdev \
+"crypto_openssl" --vdev "crypto_null""}
Comment:
yes it works.
> GBalakrishna wrote
> it is set in get_crypto_dev()
>> GBalakrishna wrote
>> it is set in get_crypto_dev()
>>> GBalakrishna wrote
>>> it is set in get_crypto_dev(): auth_xform->auth.digest_length =
>>> cap->sym.auth.digest_size.min
>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>> All test updates should go in separate commit.
>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>> Name of the test suggests, that test is using truncated hmac, but then
>>>>> you specify full auth_digest_len.
>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>> and this
>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>> Will it work w/o crypto_null driver?
>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>> `res` does not quite follow `crypto_chain_order`, does it? Please
>>>>>>>> rename to 'order' or smth. like that.
>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>> Push this logic to `set_chain_order`
>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>> Make it return `crypto_chain_order`.
>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>> No need to keep this under `if (iv_length)`
>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>> 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_r161794540
updated_at 2018-01-16 15:37:37
