From: "Power, Ciara" <ciara.power@intel.com>
To: "Zhang, Fan" <fanzhang.oss@gmail.com>, Akhil Goyal <gakhil@marvell.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Ji, Kai" <kai.ji@intel.com>
Subject: RE: [PATCH] test/crypto: add further ZUC testcases
Date: Thu, 5 Jan 2023 13:45:16 +0000 [thread overview]
Message-ID: <MN2PR11MB3821018EE8A7D3CC9DD084EDE6FA9@MN2PR11MB3821.namprd11.prod.outlook.com> (raw)
In-Reply-To: <11bb2aaa-220c-86e1-96f1-c81a68d37c6e@gmail.com>
Hi Fan,
> -----Original Message-----
> From: Zhang, Fan <fanzhang.oss@gmail.com>
> Sent: Wednesday 21 December 2022 15:21
> To: Power, Ciara <ciara.power@intel.com>; Akhil Goyal
> <gakhil@marvell.com>
> Cc: dev@dpdk.org; Ji, Kai <kai.ji@intel.com>; fanzhang.oss@gmail.com
> Subject: Re: [PATCH] test/crypto: add further ZUC testcases
>
> Hi Ciara,
>
> On 12/21/2022 2:04 PM, Ciara Power wrote:
> > Previously no ZUC decryption only or hash verify testcases existed,
> > only encryption and authentication.
> > This commit adds testcases for ZUC 128 and 256 decryption, and hash
> > verify.
> <snip>
> > + if (direction == RTE_CRYPTO_CIPHER_OP_ENCRYPT) {
> <snip>
> > + } else {
> > + if (ut_params->obuf)
> > + plaintext = rte_pktmbuf_mtod(ut_params->obuf,
> uint8_t *);
> > + else
> > + plaintext = ciphertext;
> > +
> > + debug_hexdump(stdout, "plaintext:", plaintext,
> ciphertext_len);
> > +
> The below line looks a bit off: bits len = bytes len * 8 right?
[CP]
Yes think this should be : (tdata->validCipherOffsetInBits.len >> 3)
> > + const uint8_t *reference_plaintext = tdata->plaintext.data +
> > + (tdata->validCipherOffsetInBits.len);
> > +
> > + /* Validate obuf */
> > + TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
> > + plaintext,
> > + reference_plaintext,
> > + tdata->validCipherLenInBits.len,
> > + "ZUC Plaintext data not as expected");
> <snip>
> > static int
> > -test_zuc_encryption_sgl(const struct wireless_test_data *tdata)
> > +test_zuc_cipher_sgl(const struct wireless_test_data *tdata,
> > + enum rte_crypto_cipher_operation direction)
> > {
> > struct crypto_testsuite_params *ts_params = &testsuite_params;
> > struct crypto_unittest_params *ut_params = &unittest_params;
> >
> > int retval;
> >
> > - unsigned int plaintext_pad_len;
> > - unsigned int plaintext_len;
> > - const uint8_t *ciphertext;
> > - uint8_t ciphertext_buffer[2048];
> > + unsigned int plaintext_pad_len, ciphertext_pad_len;
> > + unsigned int plaintext_len, ciphertext_len;
> > + const uint8_t *ciphertext, *plaintext;
>
> Just a piece of advice: Instead of allocating 2 buffers and we may use only
> one in either direction,
>
> we may use
>
> uint8_t buffers[2048];
>
> uint8_t *ciphertext_buffer = NULL, *plaintext_buffer = NULL;
>
> And pointing either ciphertext_buffer or plaintext_buffer to buffers based
> on the direction value?
[CP]
Good idea.
Probably no need to even have individual ct/pt buffer pointers either here
>
> > + uint8_t ciphertext_buffer[2048], plaintext_buffer[2048];
> > struct rte_cryptodev_info dev_info;
> >
> > /* Check if device supports ZUC EEA3 */ @@ -6074,21 +6110,36 @@
> > test_zuc_encryption_sgl(const struct wireless_test_data *tdata)
> > return TEST_SKIPPED;
> > }
> >
> > - plaintext_len = ceil_byte_length(tdata->plaintext.len);
> > + if (direction == RTE_CRYPTO_CIPHER_OP_ENCRYPT) {
> > + plaintext_len = ceil_byte_length(tdata->plaintext.len);
> >
> > - /* Append data which is padded to a multiple */
> > - /* of the algorithms block size */
> > - plaintext_pad_len = RTE_ALIGN_CEIL(plaintext_len, 8);
> > + /* Append data which is padded to a multiple */
> > + /* of the algorithms block size */
> > + plaintext_pad_len = RTE_ALIGN_CEIL(plaintext_len, 8);
> >
> > - ut_params->ibuf = create_segmented_mbuf(ts_params-
> >mbuf_pool,
> > - plaintext_pad_len, 10, 0);
> > + ut_params->ibuf = create_segmented_mbuf(ts_params-
> >mbuf_pool,
> > + plaintext_pad_len, 10, 0);
> >
> > - pktmbuf_write(ut_params->ibuf, 0, plaintext_len,
> > - tdata->plaintext.data);
> > + pktmbuf_write(ut_params->ibuf, 0, plaintext_len,
> > + tdata->plaintext.data);
> > + } else {
> > + ciphertext_len = ceil_byte_length(tdata->ciphertext.len);
> > +
> > + /* Append data which is padded to a multiple */
> > + /* of the algorithms block size */
> > + ciphertext_pad_len = RTE_ALIGN_CEIL(ciphertext_len, 8);
> > +
> > + ut_params->ibuf = create_segmented_mbuf(ts_params-
> >mbuf_pool,
> > + ciphertext_pad_len, 10, 0);
> > +
> > + pktmbuf_write(ut_params->ibuf, 0, ciphertext_len,
> > + tdata->ciphertext.data);
> > +
> > + }
> >
> > /* Create ZUC session */
> > retval = create_wireless_algo_cipher_session(ts_params-
> >valid_devs[0],
> > - RTE_CRYPTO_CIPHER_OP_ENCRYPT,
> > + direction,
> > RTE_CRYPTO_CIPHER_ZUC_EEA3,
> > tdata->key.data, tdata->key.len,
> > tdata->cipher_iv.len);
> > @@ -6096,8 +6147,10 @@ test_zuc_encryption_sgl(const struct
> wireless_test_data *tdata)
> > return retval;
> >
> > /* Clear mbuf payload */
> > -
> > - pktmbuf_write(ut_params->ibuf, 0, plaintext_len, tdata-
> >plaintext.data);
> > + if (direction == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
> > + pktmbuf_write(ut_params->ibuf, 0, plaintext_len, tdata-
> >plaintext.data);
> > + else
> > + pktmbuf_write(ut_params->ibuf, 0, ciphertext_len,
> > +tdata->ciphertext.data);
> >
> > /* Create ZUC operation */
> > retval =
> > create_wireless_algo_cipher_operation(tdata->cipher_iv.data,
> > @@ -6115,28 +6168,48 @@ test_zuc_encryption_sgl(const struct
> wireless_test_data *tdata)
> > TEST_ASSERT_NOT_NULL(ut_params->op, "failed to retrieve obuf");
> >
> > ut_params->obuf = ut_params->op->sym->m_dst;
> > - if (ut_params->obuf)
> > - ciphertext = rte_pktmbuf_read(ut_params->obuf,
> > - 0, plaintext_len, ciphertext_buffer);
> > - else
> > - ciphertext = rte_pktmbuf_read(ut_params->ibuf,
> > - 0, plaintext_len, ciphertext_buffer);
> >
> > - /* Validate obuf */
> > - debug_hexdump(stdout, "ciphertext:", ciphertext, plaintext_len);
> > + if (direction == RTE_CRYPTO_CIPHER_OP_ENCRYPT) {
> > + if (ut_params->obuf)
> > + ciphertext = rte_pktmbuf_read(ut_params->obuf,
> > + 0, plaintext_len, ciphertext_buffer);
> > + else
> > + ciphertext = rte_pktmbuf_read(ut_params->ibuf,
> > + 0, plaintext_len, ciphertext_buffer);
> >
> > - /* Validate obuf */
> > - TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
> > - ciphertext,
> > - tdata->ciphertext.data,
> > - tdata->validCipherLenInBits.len,
> > - "ZUC Ciphertext data not as expected");
> > + /* Validate obuf */
> > + debug_hexdump(stdout, "ciphertext:", ciphertext,
> plaintext_len);
> > +
> > + /* Validate obuf */
> > + TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
> > + ciphertext,
> > + tdata->ciphertext.data,
> > + tdata->validCipherLenInBits.len,
> > + "ZUC Ciphertext data not as expected");
> > + } else {
> > + if (ut_params->obuf)
> > + plaintext = rte_pktmbuf_read(ut_params->obuf,
> > + 0, ciphertext_len, plaintext_buffer);
> > + else
> > + plaintext = rte_pktmbuf_read(ut_params->ibuf,
> > + 0, ciphertext_len, plaintext_buffer);
> > +
> > + /* Validate obuf */
> > + debug_hexdump(stdout, "plaintext:", plaintext,
> ciphertext_len);
> > +
> > + /* Validate obuf */
> > + TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
> > + plaintext,
> > + tdata->plaintext.data,
> > + tdata->validCipherLenInBits.len,
> > + "ZUC Plaintext data not as expected");
> > + }
> >
> > return 0;
> > }
> >
> > static int
> > -test_zuc_authentication(const struct wireless_test_data *tdata)
>
> Just a nit,
>
> we may pass "enum rte_crypto_auth_operation" as parameter below,
> instead of a "verify" for passing.
>
> This also makes the changes below more readable.
>
[CP]
Yes, will make this change, thanks.
> > +test_zuc_authentication(const struct wireless_test_data *tdata,
> > +uint8_t verify)
[CP]
I missed these suggestions for my v2, which has since been merged.
I will send a small improvement patch to make these changes - thanks!
Ciara
prev parent reply other threads:[~2023-01-05 13:45 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-21 14:04 Ciara Power
2022-12-21 14:25 ` [PATCH v2] " Ciara Power
2023-01-04 15:09 ` [EXT] " Tejasree Kondoj
2023-01-05 11:01 ` Akhil Goyal
2022-12-21 15:20 ` [PATCH] " Zhang, Fan
2023-01-05 13:45 ` Power, Ciara [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN2PR11MB3821018EE8A7D3CC9DD084EDE6FA9@MN2PR11MB3821.namprd11.prod.outlook.com \
--to=ciara.power@intel.com \
--cc=dev@dpdk.org \
--cc=fanzhang.oss@gmail.com \
--cc=gakhil@marvell.com \
--cc=kai.ji@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).