[dpdk-stable] patch 'test/crypto: fix missing operation status check' has been queued to stable release 19.11.1
Kevin Traynor
ktraynor at redhat.com
Tue Feb 11 13:10:07 CET 2020
On 11/02/2020 11:52, Luca Boccassi wrote:
> Hi,
>
> That's a question for Kevin, the 18.11 maintainer.
>
> On Tue, 2020-02-11 at 11:37 +0000, Dybkowski, AdamX wrote:
>> Hi Luca.
>>
>> I originally targeted this patch for DPDK 18.11. That's good it'll go
>> into 19.11, too. But will it be applied to 18.11 as well?
>>
I tried to apply it on 18.11 branch last week, but it doesn't apply and
I have a note to say there was too much churn. So, you'll need to send a
rebased version on 18.11 branch. It will be on the list of commits that
didn't apply and asking authors for rebase that I'll send out a bit later.
>> Adam
>>
>>> -----Original Message-----
>>> From:
>>> luca.boccassi at gmail.com
>>> [mailto:
>>> luca.boccassi at gmail.com
>>> ]
>>> Sent: Tuesday, 11 February, 2020 12:19
>>> To: Dybkowski, AdamX <
>>> adamx.dybkowski at intel.com
>>>>
>>> Cc: Trahe, Fiona <
>>> fiona.trahe at intel.com
>>>> ; Ankur Dwivedi
>>> <
>>> adwivedi at marvell.com
>>>> ; Anoob Joseph <
>>> anoobj at marvell.com
>>>> ; dpdk
>>> stable <
>>> stable at dpdk.org
>>>>
>>> Subject: patch 'test/crypto: fix missing operation status check'
>>> has been
>>> queued to stable release 19.11.1
>>>
>>> Hi,
>>>
>>> FYI, your patch has been queued to stable release 19.11.1
>>>
>>> Note it hasn't been pushed to
>>> http://dpdk.org/browse/dpdk-stable
>>> yet.
>>> It will be pushed if I get no objections before 02/13/20. So please
>>> shout if
>>> anyone has objections.
>>>
>>> Also note that after the patch there's a diff of the upstream
>>> commit vs the
>>> patch applied to the branch. This will indicate if there was any
>>> rebasing
>>> needed to apply to the stable branch. If there were code changes
>>> for
>>> rebasing
>>> (ie: not only metadata diffs), please double check that the rebase
>>> was
>>> correctly done.
>>>
>>> Thanks.
>>>
>>> Luca Boccassi
>>>
>>> ---
>>> From ce8302172f9f8e06833a49abf8b283a71b07dc3b Mon Sep 17 00:00:00
>>> 2001
>>> From: Adam Dybkowski <
>>> adamx.dybkowski at intel.com
>>>>
>>> Date: Fri, 20 Dec 2019 13:58:52 +0100
>>> Subject: [PATCH] test/crypto: fix missing operation status check
>>>
>>> [ upstream commit b26ef1a11f21ecde63582ed6db281c93ce9fbf23 ]
>>>
>>> This patch adds checking of the symmetric crypto operation status
>>> that was
>>> silently skipped before. It fixes the wireless algorithms session
>>> creation
>>> (SNOW3G, KASUMI, ZUC) and passing of the digest data for the
>>> verification
>>> by PMD. Also fixed the missing aad padding issue revealed after op
>>> status
>>> checking was introduced.
>>>
>>> Fixes: c0f87eb5252b ("cryptodev: change burst API to be crypto op
>>> oriented")
>>> Fixes: 77a217a19bb7 ("test/crypto: add AES-CCM tests")
>>>
>>> Signed-off-by: Adam Dybkowski <
>>> adamx.dybkowski at intel.com
>>>>
>>> Acked-by: Fiona Trahe <
>>> fiona.trahe at intel.com
>>>>
>>> Tested-by: Ankur Dwivedi <
>>> adwivedi at marvell.com
>>>>
>>> Reviewed-by: Anoob Joseph <
>>> anoobj at marvell.com
>>>>
>>> ---
>>> app/test/test_cryptodev.c | 96 +++++++++++++++++++++------------
>>> ------
>>> 1 file changed, 52 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
>>> index
>>> 1b561456d7..79ced809de 100644
>>> --- a/app/test/test_cryptodev.c
>>> +++ b/app/test/test_cryptodev.c
>>> @@ -143,7 +143,7 @@ static struct rte_crypto_op *
>>> process_crypto_request(uint8_t dev_id, struct rte_crypto_op *op) {
>>> if (rte_cryptodev_enqueue_burst(dev_id, 0, &op, 1) != 1) {
>>> - printf("Error sending packet for encryption");
>>> + RTE_LOG(ERR, USER1, "Error sending packet for
>>> encryption\n");
>>> return NULL;
>>> }
>>>
>>> @@ -152,6 +152,11 @@ process_crypto_request(uint8_t dev_id, struct
>>> rte_crypto_op *op)
>>> while (rte_cryptodev_dequeue_burst(dev_id, 0, &op, 1) == 0)
>>> rte_pause();
>>>
>>> + if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
>>> + RTE_LOG(DEBUG, USER1, "Operation status %d\n", op-
>>>> status);
>>>
>>> + return NULL;
>>> + }
>>> +
>>> return op;
>>> }
>>>
>>> @@ -2823,9 +2828,18 @@
>>> create_wireless_algo_auth_cipher_session(uint8_t dev_id,
>>> ut_params->sess = rte_cryptodev_sym_session_create(
>>> ts_params->session_mpool);
>>>
>>> - status = rte_cryptodev_sym_session_init(dev_id, ut_params-
>>>> sess,
>>> - &ut_params->auth_xform,
>>> - ts_params->session_priv_mpool);
>>> + if (cipher_op == RTE_CRYPTO_CIPHER_OP_DECRYPT) {
>>> + ut_params->auth_xform.next = NULL;
>>> + ut_params->cipher_xform.next = &ut_params->auth_xform;
>>> + status = rte_cryptodev_sym_session_init(dev_id,
>>> ut_params-
>>>> sess,
>>>
>>> + &ut_params->cipher_xform,
>>> + ts_params->session_priv_mpool);
>>> +
>>> + } else
>>> + status = rte_cryptodev_sym_session_init(dev_id,
>>> ut_params-
>>>> sess,
>>>
>>> + &ut_params->auth_xform,
>>> + ts_params->session_priv_mpool);
>>> +
>>> TEST_ASSERT_EQUAL(status, 0, "session init failed");
>>> TEST_ASSERT_NOT_NULL(ut_params->sess, "Session creation
>>> failed");
>>>
>>> @@ -3018,13 +3032,14 @@
>>> create_wireless_algo_cipher_hash_operation(const uint8_t
>>> *auth_tag, }
>>>
>>> static int
>>> -create_wireless_algo_auth_cipher_operation(unsigned int
>>> auth_tag_len,
>>> +create_wireless_algo_auth_cipher_operation(
>>> + const uint8_t *auth_tag, unsigned int auth_tag_len,
>>> const uint8_t *cipher_iv, uint8_t cipher_iv_len,
>>> const uint8_t *auth_iv, uint8_t auth_iv_len,
>>> unsigned int data_pad_len,
>>> unsigned int cipher_len, unsigned int cipher_offset,
>>> unsigned int auth_len, unsigned int auth_offset,
>>> - uint8_t op_mode, uint8_t do_sgl)
>>> + uint8_t op_mode, uint8_t do_sgl, uint8_t verify)
>>> {
>>> struct crypto_testsuite_params *ts_params = &testsuite_params;
>>> struct crypto_unittest_params *ut_params = &unittest_params; @@
>>> -3081,6 +3096,10 @@
>>> create_wireless_algo_auth_cipher_operation(unsigned int
>>> auth_tag_len,
>>> }
>>> }
>>>
>>> + /* Copy digest for the verification */
>>> + if (verify)
>>> + memcpy(sym_op->auth.digest.data, auth_tag,
>>> auth_tag_len);
>>> +
>>> /* Copy cipher and auth IVs at the end of the crypto operation
>>> */
>>> uint8_t *iv_ptr = rte_crypto_op_ctod_offset(
>>> ut_params->op, uint8_t *, IV_OFFSET); @@
>>> -4643,7
>>> +4662,7 @@ test_snow3g_auth_cipher(const struct snow3g_test_data
>>> *tdata,
>>>
>>> /* Create SNOW 3G operation */
>>> retval = create_wireless_algo_auth_cipher_operation(
>>> - tdata->digest.len,
>>> + tdata->digest.data, tdata->digest.len,
>>> tdata->cipher_iv.data, tdata->cipher_iv.len,
>>> tdata->auth_iv.data, tdata->auth_iv.len,
>>> (tdata->digest.offset_bytes == 0 ?
>>> @@ -4653,7 +4672,7 @@ test_snow3g_auth_cipher(const struct
>>> snow3g_test_data *tdata,
>>> tdata->cipher.offset_bits,
>>> tdata->validAuthLenInBits.len,
>>> tdata->auth.offset_bits,
>>> - op_mode, 0);
>>> + op_mode, 0, verify);
>>>
>>> if (retval < 0)
>>> return retval;
>>> @@ -4819,7 +4838,7 @@ test_snow3g_auth_cipher_sgl(const struct
>>> snow3g_test_data *tdata,
>>>
>>> /* Create SNOW 3G operation */
>>> retval = create_wireless_algo_auth_cipher_operation(
>>> - tdata->digest.len,
>>> + tdata->digest.data, tdata->digest.len,
>>> tdata->cipher_iv.data, tdata->cipher_iv.len,
>>> tdata->auth_iv.data, tdata->auth_iv.len,
>>> (tdata->digest.offset_bytes == 0 ?
>>> @@ -4829,7 +4848,7 @@ test_snow3g_auth_cipher_sgl(const struct
>>> snow3g_test_data *tdata,
>>> tdata->cipher.offset_bits,
>>> tdata->validAuthLenInBits.len,
>>> tdata->auth.offset_bits,
>>> - op_mode, 1);
>>> + op_mode, 1, verify);
>>>
>>> if (retval < 0)
>>> return retval;
>>> @@ -4988,7 +5007,7 @@ test_kasumi_auth_cipher(const struct
>>> kasumi_test_data *tdata,
>>>
>>> /* Create KASUMI operation */
>>> retval = create_wireless_algo_auth_cipher_operation(
>>> - tdata->digest.len,
>>> + tdata->digest.data, tdata->digest.len,
>>> tdata->cipher_iv.data, tdata->cipher_iv.len,
>>> NULL, 0,
>>> (tdata->digest.offset_bytes == 0 ?
>>> @@ -4998,7 +5017,7 @@ test_kasumi_auth_cipher(const struct
>>> kasumi_test_data *tdata,
>>> tdata->validCipherOffsetInBits.len,
>>> tdata->validAuthLenInBits.len,
>>> 0,
>>> - op_mode, 0);
>>> + op_mode, 0, verify);
>>>
>>> if (retval < 0)
>>> return retval;
>>> @@ -5165,7 +5184,7 @@ test_kasumi_auth_cipher_sgl(const struct
>>> kasumi_test_data *tdata,
>>>
>>> /* Create KASUMI operation */
>>> retval = create_wireless_algo_auth_cipher_operation(
>>> - tdata->digest.len,
>>> + tdata->digest.data, tdata->digest.len,
>>> tdata->cipher_iv.data, tdata->cipher_iv.len,
>>> NULL, 0,
>>> (tdata->digest.offset_bytes == 0 ?
>>> @@ -5175,7 +5194,7 @@ test_kasumi_auth_cipher_sgl(const struct
>>> kasumi_test_data *tdata,
>>> tdata->validCipherOffsetInBits.len,
>>> tdata->validAuthLenInBits.len,
>>> 0,
>>> - op_mode, 1);
>>> + op_mode, 1, verify);
>>>
>>> if (retval < 0)
>>> return retval;
>>> @@ -5666,7 +5685,7 @@ test_zuc_auth_cipher(const struct
>>> wireless_test_data *tdata,
>>>
>>> /* Create ZUC operation */
>>> retval = create_wireless_algo_auth_cipher_operation(
>>> - tdata->digest.len,
>>> + tdata->digest.data, tdata->digest.len,
>>> tdata->cipher_iv.data, tdata->cipher_iv.len,
>>> tdata->auth_iv.data, tdata->auth_iv.len,
>>> (tdata->digest.offset_bytes == 0 ?
>>> @@ -5676,7 +5695,7 @@ test_zuc_auth_cipher(const struct
>>> wireless_test_data *tdata,
>>> tdata->validCipherOffsetInBits.len,
>>> tdata->validAuthLenInBits.len,
>>> 0,
>>> - op_mode, 0);
>>> + op_mode, 0, verify);
>>>
>>> if (retval < 0)
>>> return retval;
>>> @@ -5852,7 +5871,7 @@ test_zuc_auth_cipher_sgl(const struct
>>> wireless_test_data *tdata,
>>>
>>> /* Create ZUC operation */
>>> retval = create_wireless_algo_auth_cipher_operation(
>>> - tdata->digest.len,
>>> + tdata->digest.data, tdata->digest.len,
>>> tdata->cipher_iv.data, tdata->cipher_iv.len,
>>> NULL, 0,
>>> (tdata->digest.offset_bytes == 0 ?
>>> @@ -5862,7 +5881,7 @@ test_zuc_auth_cipher_sgl(const struct
>>> wireless_test_data *tdata,
>>> tdata->validCipherOffsetInBits.len,
>>> tdata->validAuthLenInBits.len,
>>> 0,
>>> - op_mode, 1);
>>> + op_mode, 1, verify);
>>>
>>> if (retval < 0)
>>> return retval;
>>> @@ -6643,7 +6662,7 @@ test_mixed_auth_cipher(const struct
>>> mixed_cipher_auth_test_data *tdata,
>>>
>>> /* Create the operation */
>>> retval = create_wireless_algo_auth_cipher_operation(
>>> - tdata->digest_enc.len,
>>> + tdata->digest_enc.data, tdata->digest_enc.len,
>>> tdata->cipher_iv.data, tdata->cipher_iv.len,
>>> tdata->auth_iv.data, tdata->auth_iv.len,
>>> (tdata->digest_enc.offset == 0 ?
>>> @@ -6653,7 +6672,7 @@ test_mixed_auth_cipher(const struct
>>> mixed_cipher_auth_test_data *tdata,
>>> tdata->cipher.offset_bits,
>>> tdata->validAuthLen.len_bits,
>>> tdata->auth.offset_bits,
>>> - op_mode, 0);
>>> + op_mode, 0, verify);
>>>
>>> if (retval < 0)
>>> return retval;
>>> @@ -6827,7 +6846,7 @@ test_mixed_auth_cipher_sgl(const struct
>>> mixed_cipher_auth_test_data *tdata,
>>>
>>> /* Create the operation */
>>> retval = create_wireless_algo_auth_cipher_operation(
>>> - tdata->digest_enc.len,
>>> + tdata->digest_enc.data, tdata->digest_enc.len,
>>> tdata->cipher_iv.data, tdata->cipher_iv.len,
>>> tdata->auth_iv.data, tdata->auth_iv.len,
>>> (tdata->digest_enc.offset == 0 ?
>>> @@ -6837,7 +6856,7 @@ test_mixed_auth_cipher_sgl(const struct
>>> mixed_cipher_auth_test_data *tdata,
>>> tdata->cipher.offset_bits,
>>> tdata->validAuthLen.len_bits,
>>> tdata->auth.offset_bits,
>>> - op_mode, 1);
>>> + op_mode, 1, verify);
>>>
>>> if (retval < 0)
>>> return retval;
>>> @@ -10818,13 +10837,8 @@
>>> test_authentication_verify_fail_when_data_corruption(
>>>
>>> ut_params->op = process_crypto_request(ts_params-
>>>> valid_devs[0],
>>> ut_params->op);
>>> - TEST_ASSERT_NOT_NULL(ut_params->op, "failed crypto process");
>>> - TEST_ASSERT_NOT_EQUAL(ut_params->op->status,
>>> - RTE_CRYPTO_OP_STATUS_SUCCESS,
>>> - "authentication not failed");
>>>
>>> - ut_params->obuf = ut_params->op->sym->m_src;
>>> - TEST_ASSERT_NOT_NULL(ut_params->obuf, "failed to retrieve
>>> obuf");
>>> + TEST_ASSERT_NULL(ut_params->op, "authentication not failed");
>>>
>>> return 0;
>>> }
>>> @@ -10879,13 +10893,8 @@
>>> test_authentication_verify_GMAC_fail_when_corruption(
>>>
>>> ut_params->op = process_crypto_request(ts_params-
>>>> valid_devs[0],
>>> ut_params->op);
>>> - TEST_ASSERT_NOT_NULL(ut_params->op, "failed crypto process");
>>> - TEST_ASSERT_NOT_EQUAL(ut_params->op->status,
>>> - RTE_CRYPTO_OP_STATUS_SUCCESS,
>>> - "authentication not failed");
>>>
>>> - ut_params->obuf = ut_params->op->sym->m_src;
>>> - TEST_ASSERT_NOT_NULL(ut_params->obuf, "failed to retrieve
>>> obuf");
>>> + TEST_ASSERT_NULL(ut_params->op, "authentication not failed");
>>>
>>> return 0;
>>> }
>>> @@ -10940,13 +10949,7 @@
>>> test_authenticated_decryption_fail_when_corruption(
>>> ut_params->op = process_crypto_request(ts_params-
>>>> valid_devs[0],
>>> ut_params->op);
>>>
>>> - TEST_ASSERT_NOT_NULL(ut_params->op, "failed crypto process");
>>> - TEST_ASSERT_NOT_EQUAL(ut_params->op->status,
>>> - RTE_CRYPTO_OP_STATUS_SUCCESS,
>>> - "authentication not failed");
>>> -
>>> - ut_params->obuf = ut_params->op->sym->m_src;
>>> - TEST_ASSERT_NOT_NULL(ut_params->obuf, "failed to retrieve
>>> obuf");
>>> + TEST_ASSERT_NULL(ut_params->op, "authentication not failed");
>>>
>>> return 0;
>>> }
>>> @@ -11149,6 +11152,7 @@ create_aead_operation_SGL(enum
>>> rte_crypto_aead_operation op,
>>> const unsigned int auth_tag_len = tdata->auth_tag.len;
>>> const unsigned int iv_len = tdata->iv.len;
>>> unsigned int aad_len = tdata->aad.len;
>>> + unsigned int aad_len_pad = 0;
>>>
>>> /* Generate Crypto op data structure */
>>> ut_params->op = rte_crypto_op_alloc(ts_params->op_mpool,
>>> @@ -11203,8 +11207,10 @@ create_aead_operation_SGL(enum
>>> rte_crypto_aead_operation op,
>>>
>>> rte_memcpy(iv_ptr, tdata->iv.data, iv_len);
>>>
>>> + aad_len_pad = RTE_ALIGN_CEIL(aad_len, 16);
>>> +
>>> sym_op->aead.aad.data = (uint8_t *)rte_pktmbuf_prepend(
>>> - ut_params->ibuf, aad_len);
>>> + ut_params->ibuf, aad_len_pad);
>>> TEST_ASSERT_NOT_NULL(sym_op->aead.aad.data,
>>> "no room to prepend aad");
>>> sym_op->aead.aad.phys_addr = rte_pktmbuf_iova( @@ -
>>> 11219,7 +11225,7 @@ create_aead_operation_SGL(enum
>>> rte_crypto_aead_operation op,
>>> }
>>>
>>> sym_op->aead.data.length = tdata->plaintext.len;
>>> - sym_op->aead.data.offset = aad_len;
>>> + sym_op->aead.data.offset = aad_len_pad;
>>>
>>> return 0;
>>> }
>>> @@ -11252,7 +11258,7 @@ test_authenticated_encryption_SGL(const
>>> struct
>>> aead_test_data *tdata,
>>> int ecx = 0;
>>> void *digest_mem = NULL;
>>>
>>> - uint32_t prepend_len = tdata->aad.len;
>>> + uint32_t prepend_len = RTE_ALIGN_CEIL(tdata->aad.len, 16);
>>>
>>> if (tdata->plaintext.len % fragsz != 0) {
>>> if (tdata->plaintext.len / fragsz + 1 > SGL_MAX_NO) @@
>>> -
>>> 11915,6 +11921,8 @@ static struct unit_test_suite
>>> cryptodev_qat_testsuite =
>>> {
>>>
>>> test_AES_GCM_auth_encrypt_SGL_out_of_place_400B_400B),
>>> TEST_CASE_ST(ut_setup, ut_teardown,
>>>
>>> test_AES_GCM_auth_encrypt_SGL_out_of_place_1500B_2000B),
>>> + TEST_CASE_ST(ut_setup, ut_teardown,
>>> +
>>> test_AES_GCM_auth_encrypt_SGL_out_of_place_400B_1seg),
>>> TEST_CASE_ST(ut_setup, ut_teardown,
>>>
>>> test_AES_GCM_authenticated_encryption_test_case_1),
>>> TEST_CASE_ST(ut_setup, ut_teardown,
>>> --
>>> 2.20.1
>>>
>>> ---
>>> Diff of the applied patch vs upstream commit (please double-check
>>> if non-
>>> empty:
>>> ---
>>> --- - 2020-02-11 11:17:39.986002596 +0000
>>> +++ 0023-test-crypto-fix-missing-operation-status-check.patch
>>> 2020-02-11
>>> 11:17:38.332000075 +0000
>>> @@ -1,8 +1,10 @@
>>> -From b26ef1a11f21ecde63582ed6db281c93ce9fbf23 Mon Sep 17 00:00:00
>>> 2001
>>> +From ce8302172f9f8e06833a49abf8b283a71b07dc3b Mon Sep 17 00:00:00
>>> 2001
>>> From: Adam Dybkowski <
>>> adamx.dybkowski at intel.com
>>>>
>>> Date: Fri, 20 Dec 2019 13:58:52 +0100
>>> Subject: [PATCH] test/crypto: fix missing operation status check
>>>
>>> +[ upstream commit b26ef1a11f21ecde63582ed6db281c93ce9fbf23 ]
>>> +
>>> This patch adds checking of the symmetric crypto operation
>>> status that was
>>> silently skipped before. It fixes the wireless algorithms session
>>> creation
>>> (SNOW3G, KASUMI, ZUC) and passing of the digest @@ -11,7 +13,6 @@
>>>
>>> Fixes: c0f87eb5252b ("cryptodev: change burst API to be crypto op
>>> oriented")
>>> Fixes: 77a217a19bb7 ("test/crypto: add AES-CCM tests")
>>> -Cc:
>>> stable at dpdk.org
>>>
>>>
>>> Signed-off-by: Adam Dybkowski <
>>> adamx.dybkowski at intel.com
>>>>
>>> Acked-by: Fiona Trahe <
>>> fiona.trahe at intel.com
>>>>
More information about the stable
mailing list