[v4,2/5] test/crypto: add combined mode tests

Message ID 1631884523-836-3-git-send-email-anoobj@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series Add lookaside IPsec tests |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Anoob Joseph Sept. 17, 2021, 1:15 p.m. UTC
  Add framework to test IPsec features with all supported
combinations of ciphers.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>

---
 app/test/test_cryptodev.c                |  73 +++++++++++++++++++--
 app/test/test_cryptodev_security_ipsec.c | 107 +++++++++++++++++++++++++++++--
 app/test/test_cryptodev_security_ipsec.h |  52 ++++++++++++++-
 3 files changed, 223 insertions(+), 9 deletions(-)
  

Comments

Akhil Goyal Sept. 21, 2021, 4:22 p.m. UTC | #1
> Subject: [PATCH v4 2/5] test/crypto: add combined mode tests

Title: test/crypto: add combined mode IPSec  cases
> 

In description explain the meaning of combined mode.
Also mention that the encap and decap are done one after the other,
Hence known test vectors are not required.

Apart from that,
Acked-by: Akhil Goyal <gakhil@marvell.com>

> Add framework to test IPsec features with all supported
> combinations of ciphers.
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
>
  
Hemant Agrawal Sept. 24, 2021, 7:23 a.m. UTC | #2
HI Anoob,

On 9/17/2021 6:45 PM, Anoob Joseph wrote:
> Add framework to test IPsec features with all supported
> combinations of ciphers.
>
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
>
> ---
>   app/test/test_cryptodev.c                |  73 +++++++++++++++++++--
>   app/test/test_cryptodev_security_ipsec.c | 107 +++++++++++++++++++++++++++++--
>   app/test/test_cryptodev_security_ipsec.h |  52 ++++++++++++++-
>   3 files changed, 223 insertions(+), 9 deletions(-)
>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index 9c7875c..7fd246e 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -8904,7 +8904,8 @@ static int
>   test_ipsec_proto_process(const struct ipsec_test_data td[],
>   			 struct ipsec_test_data res_d[],
>   			 int nb_td,
> -			 bool silent)
> +			 bool silent,
> +			 const struct ipsec_test_flags *flags)
>   {
>   	struct crypto_testsuite_params *ts_params = &testsuite_params;
>   	struct crypto_unittest_params *ut_params = &unittest_params;
> @@ -9021,7 +9022,7 @@ test_ipsec_proto_process(const struct ipsec_test_data td[],
>   		/* Process crypto operation */
>   		process_crypto_request(dev_id, ut_params->op);
>   
> -		ret = test_ipsec_status_check(ut_params->op, dir);
> +		ret = test_ipsec_status_check(ut_params->op, flags, dir);
>   		if (ret != TEST_SUCCESS)
>   			goto crypto_op_free;
>   
> @@ -9029,7 +9030,7 @@ test_ipsec_proto_process(const struct ipsec_test_data td[],
>   			res_d_tmp = &res_d[i];
>   
>   		ret = test_ipsec_post_process(ut_params->ibuf, &td[i],
> -					      res_d_tmp, silent);
> +					      res_d_tmp, silent, flags);
>   		if (ret != TEST_SUCCESS)
>   			goto crypto_op_free;
>   
> @@ -9057,11 +9058,71 @@ test_ipsec_proto_process(const struct ipsec_test_data td[],
>   static int
>   test_ipsec_proto_known_vec_inb(const void *td_outb)
>   {
> +	struct ipsec_test_flags flags;
>   	struct ipsec_test_data td_inb;
>   
> +	memset(&flags, 0, sizeof(flags));
> +
>   	test_ipsec_td_in_from_out(td_outb, &td_inb);
>   
> -	return test_ipsec_proto_process(&td_inb, NULL, 1, false);
> +	return test_ipsec_proto_process(&td_inb, NULL, 1, false, &flags);

In most places, you have hardcoded this silent flag - this is for debug 
prints.

Will you please driver it from some macro or variable or log-level, 
where it can be changed easily.
  
Anoob Joseph Sept. 24, 2021, 8:12 a.m. UTC | #3
Hi Hemant,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Hemant Agrawal <hemant.agrawal@oss.nxp.com>
> Sent: Friday, September 24, 2021 12:54 PM
> To: Anoob Joseph <anoobj@marvell.com>; Akhil Goyal <gakhil@marvell.com>;
> Declan Doherty <declan.doherty@intel.com>; Fan Zhang
> <roy.fan.zhang@intel.com>; Konstantin Ananyev
> <konstantin.ananyev@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Archana Muniganti
> <marchana@marvell.com>; Tejasree Kondoj <ktejasree@marvell.com>; Hemant
> Agrawal <hemant.agrawal@nxp.com>; Radu Nicolau
> <radu.nicolau@intel.com>; Ciara Power <ciara.power@intel.com>; Gagandeep
> Singh <g.singh@nxp.com>; dev@dpdk.org
> Subject: [EXT] Re: [dpdk-dev] [PATCH v4 2/5] test/crypto: add combined mode
> tests
> 
> External Email
> 
> ----------------------------------------------------------------------
> HI Anoob,
> 
> On 9/17/2021 6:45 PM, Anoob Joseph wrote:
> > Add framework to test IPsec features with all supported combinations
> > of ciphers.
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > Signed-off-by: Tejasree Kondoj <ktejasree@marvell.com>
> >
> > ---
> >   app/test/test_cryptodev.c                |  73 +++++++++++++++++++--
> >   app/test/test_cryptodev_security_ipsec.c | 107
> +++++++++++++++++++++++++++++--
> >   app/test/test_cryptodev_security_ipsec.h |  52 ++++++++++++++-
> >   3 files changed, 223 insertions(+), 9 deletions(-)
> >
> > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> > index 9c7875c..7fd246e 100644
> > --- a/app/test/test_cryptodev.c
> > +++ b/app/test/test_cryptodev.c
> > @@ -8904,7 +8904,8 @@ static int
> >   test_ipsec_proto_process(const struct ipsec_test_data td[],
> >   			 struct ipsec_test_data res_d[],
> >   			 int nb_td,
> > -			 bool silent)
> > +			 bool silent,
> > +			 const struct ipsec_test_flags *flags)
> >   {
> >   	struct crypto_testsuite_params *ts_params = &testsuite_params;
> >   	struct crypto_unittest_params *ut_params = &unittest_params; @@
> > -9021,7 +9022,7 @@ test_ipsec_proto_process(const struct ipsec_test_data
> td[],
> >   		/* Process crypto operation */
> >   		process_crypto_request(dev_id, ut_params->op);
> >
> > -		ret = test_ipsec_status_check(ut_params->op, dir);
> > +		ret = test_ipsec_status_check(ut_params->op, flags, dir);
> >   		if (ret != TEST_SUCCESS)
> >   			goto crypto_op_free;
> >
> > @@ -9029,7 +9030,7 @@ test_ipsec_proto_process(const struct
> ipsec_test_data td[],
> >   			res_d_tmp = &res_d[i];
> >
> >   		ret = test_ipsec_post_process(ut_params->ibuf, &td[i],
> > -					      res_d_tmp, silent);
> > +					      res_d_tmp, silent, flags);
> >   		if (ret != TEST_SUCCESS)
> >   			goto crypto_op_free;
> >
> > @@ -9057,11 +9058,71 @@ test_ipsec_proto_process(const struct
> ipsec_test_data td[],
> >   static int
> >   test_ipsec_proto_known_vec_inb(const void *td_outb)
> >   {
> > +	struct ipsec_test_flags flags;
> >   	struct ipsec_test_data td_inb;
> >
> > +	memset(&flags, 0, sizeof(flags));
> > +
> >   	test_ipsec_td_in_from_out(td_outb, &td_inb);
> >
> > -	return test_ipsec_proto_process(&td_inb, NULL, 1, false);
> > +	return test_ipsec_proto_process(&td_inb, NULL, 1, false, &flags);
> 
> In most places, you have hardcoded this silent flag - this is for debug prints.
> 
> Will you please driver it from some macro or variable or log-level, where it can
> be changed easily.
> 

[Anoob] Currently, for known vector tests, we have "silent" set as false to have more verbose output by default. For combined mode, since we attempt multiple ciphers, there would be lot of prints. Hence we kept "silent" flag as true to disable prints for such cases. The idea is, if a developer wants to quickly analyze one combined mode test case, he can do so by changing silent flag for just that. Enabling it universally could dump more info than desired. 

Making it a global flag would mean, we would have same kind of prints with known vector & combined mode (which attempts combinations). Do you think a global setting is better? If yes, what do you recommend? A custom flag or RTE_LOG?
  

Patch

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9c7875c..7fd246e 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -8904,7 +8904,8 @@  static int
 test_ipsec_proto_process(const struct ipsec_test_data td[],
 			 struct ipsec_test_data res_d[],
 			 int nb_td,
-			 bool silent)
+			 bool silent,
+			 const struct ipsec_test_flags *flags)
 {
 	struct crypto_testsuite_params *ts_params = &testsuite_params;
 	struct crypto_unittest_params *ut_params = &unittest_params;
@@ -9021,7 +9022,7 @@  test_ipsec_proto_process(const struct ipsec_test_data td[],
 		/* Process crypto operation */
 		process_crypto_request(dev_id, ut_params->op);
 
-		ret = test_ipsec_status_check(ut_params->op, dir);
+		ret = test_ipsec_status_check(ut_params->op, flags, dir);
 		if (ret != TEST_SUCCESS)
 			goto crypto_op_free;
 
@@ -9029,7 +9030,7 @@  test_ipsec_proto_process(const struct ipsec_test_data td[],
 			res_d_tmp = &res_d[i];
 
 		ret = test_ipsec_post_process(ut_params->ibuf, &td[i],
-					      res_d_tmp, silent);
+					      res_d_tmp, silent, flags);
 		if (ret != TEST_SUCCESS)
 			goto crypto_op_free;
 
@@ -9057,11 +9058,71 @@  test_ipsec_proto_process(const struct ipsec_test_data td[],
 static int
 test_ipsec_proto_known_vec_inb(const void *td_outb)
 {
+	struct ipsec_test_flags flags;
 	struct ipsec_test_data td_inb;
 
+	memset(&flags, 0, sizeof(flags));
+
 	test_ipsec_td_in_from_out(td_outb, &td_inb);
 
-	return test_ipsec_proto_process(&td_inb, NULL, 1, false);
+	return test_ipsec_proto_process(&td_inb, NULL, 1, false, &flags);
+}
+
+static int
+test_ipsec_proto_all(const struct ipsec_test_flags *flags)
+{
+	struct ipsec_test_data td_outb[IPSEC_TEST_PACKETS_MAX];
+	struct ipsec_test_data td_inb[IPSEC_TEST_PACKETS_MAX];
+	unsigned int i, nb_pkts = 1, pass_cnt = 0;
+	int ret;
+
+	for (i = 0; i < RTE_DIM(aead_list); i++) {
+		test_ipsec_td_prepare(&aead_list[i],
+				      NULL,
+				      flags,
+				      td_outb,
+				      nb_pkts);
+
+		ret = test_ipsec_proto_process(td_outb, td_inb, nb_pkts, true,
+					       flags);
+		if (ret == TEST_SKIPPED)
+			continue;
+
+		if (ret == TEST_FAILED)
+			return TEST_FAILED;
+
+		test_ipsec_td_update(td_inb, td_outb, nb_pkts, flags);
+
+		ret = test_ipsec_proto_process(td_inb, NULL, nb_pkts, true,
+					       flags);
+		if (ret == TEST_SKIPPED)
+			continue;
+
+		if (ret == TEST_FAILED)
+			return TEST_FAILED;
+
+		if (flags->display_alg)
+			test_ipsec_display_alg(&aead_list[i], NULL);
+
+		pass_cnt++;
+	}
+
+	if (pass_cnt > 0)
+		return TEST_SUCCESS;
+	else
+		return TEST_SKIPPED;
+}
+
+static int
+test_ipsec_proto_display_list(const void *data __rte_unused)
+{
+	struct ipsec_test_flags flags;
+
+	memset(&flags, 0, sizeof(flags));
+
+	flags.display_alg = true;
+
+	return test_ipsec_proto_all(&flags);
 }
 
 static int
@@ -13971,6 +14032,10 @@  static struct unit_test_suite ipsec_proto_testsuite  = {
 			"Inbound known vector (ESP tunnel mode IPv4 AES-GCM 256)",
 			ut_setup_security, ut_teardown,
 			test_ipsec_proto_known_vec_inb, &pkt_aes_256_gcm),
+		TEST_CASE_NAMED_ST(
+			"Combined test alg list",
+			ut_setup_security, ut_teardown,
+			test_ipsec_proto_display_list),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
 };
diff --git a/app/test/test_cryptodev_security_ipsec.c b/app/test/test_cryptodev_security_ipsec.c
index 2431fcb..d08e093 100644
--- a/app/test/test_cryptodev_security_ipsec.c
+++ b/app/test/test_cryptodev_security_ipsec.c
@@ -10,6 +10,8 @@ 
 #include "test.h"
 #include "test_cryptodev_security_ipsec.h"
 
+extern struct ipsec_test_data pkt_aes_256_gcm;
+
 int
 test_ipsec_sec_caps_verify(struct rte_security_ipsec_xform *ipsec_xform,
 			   const struct rte_security_capability *sec_cap,
@@ -128,6 +130,68 @@  test_ipsec_td_in_from_out(const struct ipsec_test_data *td_out,
 	}
 }
 
+void
+test_ipsec_td_prepare(const struct crypto_param *param1,
+		      const struct crypto_param *param2,
+		      const struct ipsec_test_flags *flags,
+		      struct ipsec_test_data *td_array,
+		      int nb_td)
+
+{
+	struct ipsec_test_data *td;
+	int i;
+
+	memset(td_array, 0, nb_td * sizeof(*td));
+
+	for (i = 0; i < nb_td; i++) {
+		td = &td_array[i];
+		/* Copy template for packet & key fields */
+		memcpy(td, &pkt_aes_256_gcm, sizeof(*td));
+
+		/* Override fields based on param */
+
+		if (param1->type == RTE_CRYPTO_SYM_XFORM_AEAD)
+			td->aead = true;
+		else
+			td->aead = false;
+
+		td->xform.aead.aead.algo = param1->alg.aead;
+		td->xform.aead.aead.key.length = param1->key_length;
+	}
+
+	RTE_SET_USED(flags);
+	RTE_SET_USED(param2);
+}
+
+void
+test_ipsec_td_update(struct ipsec_test_data td_inb[],
+		     const struct ipsec_test_data td_outb[],
+		     int nb_td,
+		     const struct ipsec_test_flags *flags)
+{
+	int i;
+
+	for (i = 0; i < nb_td; i++) {
+		memcpy(td_inb[i].output_text.data, td_outb[i].input_text.data,
+		       td_outb[i].input_text.len);
+		td_inb[i].output_text.len = td_outb->input_text.len;
+	}
+
+	RTE_SET_USED(flags);
+}
+
+void
+test_ipsec_display_alg(const struct crypto_param *param1,
+		       const struct crypto_param *param2)
+{
+	if (param1->type == RTE_CRYPTO_SYM_XFORM_AEAD)
+		printf("\t%s [%d]\n",
+		       rte_crypto_aead_algorithm_strings[param1->alg.aead],
+		       param1->key_length);
+
+	RTE_SET_USED(param2);
+}
+
 static int
 test_ipsec_tunnel_hdr_len_get(const struct ipsec_test_data *td)
 {
@@ -148,7 +212,7 @@  test_ipsec_tunnel_hdr_len_get(const struct ipsec_test_data *td)
 
 static int
 test_ipsec_td_verify(struct rte_mbuf *m, const struct ipsec_test_data *td,
-		     bool silent)
+		     bool silent, const struct ipsec_test_flags *flags)
 {
 	uint8_t *output_text = rte_pktmbuf_mtod(m, uint8_t *);
 	uint32_t skip, len = rte_pktmbuf_pkt_len(m);
@@ -177,12 +241,37 @@  test_ipsec_td_verify(struct rte_mbuf *m, const struct ipsec_test_data *td,
 		return TEST_FAILED;
 	}
 
+	RTE_SET_USED(flags);
+
+	return TEST_SUCCESS;
+}
+
+static int
+test_ipsec_res_d_prepare(struct rte_mbuf *m, const struct ipsec_test_data *td,
+		   struct ipsec_test_data *res_d)
+{
+	uint8_t *output_text = rte_pktmbuf_mtod(m, uint8_t *);
+	uint32_t len = rte_pktmbuf_pkt_len(m);
+
+	memcpy(res_d, td, sizeof(*res_d));
+	memcpy(res_d->input_text.data, output_text, len);
+	res_d->input_text.len = len;
+
+	res_d->ipsec_xform.direction = RTE_SECURITY_IPSEC_SA_DIR_INGRESS;
+	if (res_d->aead) {
+		res_d->xform.aead.aead.op = RTE_CRYPTO_AEAD_OP_DECRYPT;
+	} else {
+		printf("Only AEAD supported\n");
+		return TEST_SKIPPED;
+	}
+
 	return TEST_SUCCESS;
 }
 
 int
 test_ipsec_post_process(struct rte_mbuf *m, const struct ipsec_test_data *td,
-			struct ipsec_test_data *res_d, bool silent)
+			struct ipsec_test_data *res_d, bool silent,
+			const struct ipsec_test_flags *flags)
 {
 	/*
 	 * In case of known vector tests & all inbound tests, res_d provided
@@ -190,13 +279,22 @@  test_ipsec_post_process(struct rte_mbuf *m, const struct ipsec_test_data *td,
 	 * For inbound, output_text would be plain packet and for outbound
 	 * output_text would IPsec packet. Validate by comparing against
 	 * known vectors.
+	 *
+	 * In case of combined mode tests, the output_text from outbound
+	 * operation (ie, IPsec packet) would need to be inbound processed to
+	 * obtain the plain text. Copy output_text to result data, 'res_d', so
+	 * that inbound processing can be done.
 	 */
-	RTE_SET_USED(res_d);
-	return test_ipsec_td_verify(m, td, silent);
+
+	if (res_d == NULL)
+		return test_ipsec_td_verify(m, td, silent, flags);
+	else
+		return test_ipsec_res_d_prepare(m, td, res_d);
 }
 
 int
 test_ipsec_status_check(struct rte_crypto_op *op,
+			const struct ipsec_test_flags *flags,
 			enum rte_security_ipsec_sa_direction dir)
 {
 	int ret = TEST_SUCCESS;
@@ -206,6 +304,7 @@  test_ipsec_status_check(struct rte_crypto_op *op,
 		ret = TEST_FAILED;
 	}
 
+	RTE_SET_USED(flags);
 	RTE_SET_USED(dir);
 
 	return ret;
diff --git a/app/test/test_cryptodev_security_ipsec.h b/app/test/test_cryptodev_security_ipsec.h
index 5f1b46d..cbb3ee4 100644
--- a/app/test/test_cryptodev_security_ipsec.h
+++ b/app/test/test_cryptodev_security_ipsec.h
@@ -8,6 +8,8 @@ 
 #include <rte_cryptodev.h>
 #include <rte_security.h>
 
+#define IPSEC_TEST_PACKETS_MAX 32
+
 struct ipsec_test_data {
 	struct {
 		uint8_t data[32];
@@ -45,6 +47,38 @@  struct ipsec_test_data {
 	} xform;
 };
 
+struct ipsec_test_flags {
+	bool display_alg;
+};
+
+struct crypto_param {
+	enum rte_crypto_sym_xform_type type;
+	union {
+		enum rte_crypto_cipher_algorithm cipher;
+		enum rte_crypto_auth_algorithm auth;
+		enum rte_crypto_aead_algorithm aead;
+	} alg;
+	uint16_t key_length;
+};
+
+static const struct crypto_param aead_list[] = {
+	{
+		.type = RTE_CRYPTO_SYM_XFORM_AEAD,
+		.alg.aead =  RTE_CRYPTO_AEAD_AES_GCM,
+		.key_length = 16,
+	},
+	{
+		.type = RTE_CRYPTO_SYM_XFORM_AEAD,
+		.alg.aead = RTE_CRYPTO_AEAD_AES_GCM,
+		.key_length = 24,
+	},
+	{
+		.type = RTE_CRYPTO_SYM_XFORM_AEAD,
+		.alg.aead = RTE_CRYPTO_AEAD_AES_GCM,
+		.key_length = 32
+	},
+};
+
 int test_ipsec_sec_caps_verify(struct rte_security_ipsec_xform *ipsec_xform,
 			       const struct rte_security_capability *sec_cap,
 			       bool silent);
@@ -56,11 +90,27 @@  int test_ipsec_crypto_caps_aead_verify(
 void test_ipsec_td_in_from_out(const struct ipsec_test_data *td_out,
 			       struct ipsec_test_data *td_in);
 
+void test_ipsec_td_prepare(const struct crypto_param *param1,
+			   const struct crypto_param *param2,
+			   const struct ipsec_test_flags *flags,
+			   struct ipsec_test_data *td_array,
+			   int nb_td);
+
+void test_ipsec_td_update(struct ipsec_test_data td_inb[],
+			  const struct ipsec_test_data td_outb[],
+			  int nb_td,
+			  const struct ipsec_test_flags *flags);
+
+void test_ipsec_display_alg(const struct crypto_param *param1,
+			    const struct crypto_param *param2);
+
 int test_ipsec_post_process(struct rte_mbuf *m,
 			    const struct ipsec_test_data *td,
-			    struct ipsec_test_data *res_d, bool silent);
+			    struct ipsec_test_data *res_d, bool silent,
+			    const struct ipsec_test_flags *flags);
 
 int test_ipsec_status_check(struct rte_crypto_op *op,
+			    const struct ipsec_test_flags *flags,
 			    enum rte_security_ipsec_sa_direction dir);
 
 #endif