Dmitry Eremin-Solenikov(lumag) replied on github web page: platform/linux-dpdk/odp_crypto.c line 171 @@ -663,76 +666,105 @@ int odp_crypto_auth_capability(odp_auth_alg_t auth, return idx; } -static int get_crypto_dev(struct rte_crypto_sym_xform *cipher_xform, - struct rte_crypto_sym_xform *auth_xform, +static int get_crypto_dev(struct rte_crypto_sym_xform *first_xform, + enum crypto_chain_order res, uint16_t iv_length, uint8_t *dev_id) { uint8_t cdev_id, id; const struct rte_cryptodev_capabilities *cap; + struct rte_crypto_sym_xform *auth_xform = NULL; + struct rte_crypto_sym_xform *cipher_xform = NULL; enum rte_crypto_cipher_algorithm cap_cipher_algo; enum rte_crypto_auth_algorithm cap_auth_algo; enum rte_crypto_cipher_algorithm app_cipher_algo; enum rte_crypto_auth_algorithm app_auth_algo; + switch (res) { + case CRYPTO_CHAIN_ONLY_CIPHER: + cipher_xform = first_xform; + break; + case CRYPTO_CHAIN_ONLY_AUTH: + auth_xform = first_xform; + break; + case CRYPTO_CHAIN_CIPHER_AUTH: + cipher_xform = first_xform; + auth_xform = first_xform->next; + break; + case CRYPTO_CHAIN_AUTH_CIPHER: + auth_xform = first_xform; + cipher_xform = first_xform->next; + break; + default: + return -1; + } + for (id = 0; id < global->enabled_crypto_devs; id++) { struct rte_cryptodev_info dev_info; int i = 0; cdev_id = global->enabled_crypto_dev_ids[id]; rte_cryptodev_info_get(cdev_id, &dev_info); - app_cipher_algo = cipher_xform->cipher.algo; cap = &dev_info.capabilities[i]; - while (cap->op != RTE_CRYPTO_OP_TYPE_UNDEFINED) { - cap_cipher_algo = cap->sym.cipher.algo; + while (cipher_xform && cap->op != + RTE_CRYPTO_OP_TYPE_UNDEFINED) { if (cap->sym.xform_type == RTE_CRYPTO_SYM_XFORM_CIPHER) { + app_cipher_algo = cipher_xform->cipher.algo; + cap_cipher_algo = cap->sym.cipher.algo; if (cap_cipher_algo == app_cipher_algo) break; } - cap = &dev_info.capabilities[++i]; + cap = &dev_info.capabilities[++i]; } if (cap->op == RTE_CRYPTO_OP_TYPE_UNDEFINED) continue; - /* Check if key size is supported by the algorithm. */ - if (cipher_xform->cipher.key.length) { - if (is_valid_size(cipher_xform->cipher.key.length, - cap->sym.cipher.key_size.min, - cap->sym.cipher.key_size.max, - cap->sym.cipher.key_size. - increment) != 0) { - ODP_ERR("Unsupported cipher key length\n"); - return -1; - } - /* No size provided, use minimum size. */ - } else - cipher_xform->cipher.key.length = - cap->sym.cipher.key_size.min; - - /* Check if iv length is supported by the algorithm. */ - if (iv_length) { - if (is_valid_size(iv_length, - cap->sym.cipher.iv_size.min, - cap->sym.cipher.iv_size.max, - cap->sym.cipher.iv_size. - increment) != 0) { - ODP_ERR("Unsupported iv length\n"); - return -1; + if (cipher_xform) { + /* Check if key size is supported by the algorithm. */ + if (cipher_xform->cipher.key.length) { + if (is_valid_size( + cipher_xform->cipher.key.length, + cap->sym.cipher.key_size.min, + cap->sym.cipher.key_size.max, + cap->sym.cipher.key_size. + increment) != 0) { + ODP_ERR("Invalid cipher key length\n"); + return -1; + } + /* No size provided, use minimum size. */ + } else + cipher_xform->cipher.key.length = + cap->sym.cipher.key_size.min; + + /* Check if iv length is supported by the algorithm. */ + if (iv_length) {
Comment: 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_r161766200 updated_at 2018-01-16 14:21:10