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