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
