Dmitry Eremin-Solenikov(lumag) replied on github web page:
platform/linux-generic/odp_ipsec_sad.c
line 12
@@ -303,13 +303,15 @@ odp_ipsec_sa_t odp_ipsec_sa_create(const
odp_ipsec_sa_param_t *param)
case ODP_CIPHER_ALG_3DES_CBC:
ipsec_sa->esp_iv_len = 8;
ipsec_sa->esp_block_len = 8;
+ crypto_param.iv.length = 8;
break;
#if ODP_DEPRECATED_API
case ODP_CIPHER_ALG_AES128_CBC:
#endif
case ODP_CIPHER_ALG_AES_CBC:
ipsec_sa->esp_iv_len = 16;
ipsec_sa->esp_block_len = 16;
+ crypto_param.iv.length = 16;
Comment:
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_r161768090
updated_at 2018-01-16 14:21:10
