Dmitry Eremin-Solenikov(lumag) replied on github web page:
platform/linux-dpdk/odp_crypto.c
line 215
@@ -776,6 +808,22 @@ static int get_crypto_dev(struct rte_crypto_sym_xform
*cipher_xform,
return -1;
}
+static void set_chain_order(struct rte_crypto_sym_xform **first_xform,
+ struct rte_crypto_sym_xform *auth_xform,
+ struct rte_crypto_sym_xform *cipher_xform,
+ enum crypto_chain_order *res)
Comment:
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_r161766946
updated_at 2018-01-16 14:21:10
