[PATCH 1/3] event/cnxk: fix return values for capability API

[PATCH 1/3] event/cnxk: fix return values for capability API

[Aakash Sasidharan]

crypto adapter capability now returns -ENOTSUP instead
of -EINVAL when event_cnxk or crypto_cnxk device is not
available. This would allow unit test to be skipped
when the devices are not available.

Fixes: 19f81cb59cb4 ("event/cnxk: add crypto adapter operations")

Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
---
 drivers/event/cnxk/cn10k_eventdev.c | 16 ++++++++--------
 drivers/event/cnxk/cn9k_eventdev.c  | 16 ++++++++--------
 drivers/event/cnxk/cnxk_eventdev.h  |  4 ++--
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/event/cnxk/cn10k_eventdev.c b/drivers/event/cnxk/cn10k_eventdev.c
index 499a3aace7..c5d4be0474 100644
--- a/drivers/event/cnxk/cn10k_eventdev.c
+++ b/drivers/event/cnxk/cn10k_eventdev.c
@@ -911,8 +911,8 @@ static int
 cn10k_crypto_adapter_caps_get(const struct rte_eventdev *event_dev,
 			      const struct rte_cryptodev *cdev, uint32_t *caps)
 {
-	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn10k");
-	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn10k");
+	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn10k", ENOTSUP);
+	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn10k", ENOTSUP);
 
 	*caps = RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD |
 		RTE_EVENT_CRYPTO_ADAPTER_CAP_SESSION_PRIVATE_DATA |
@@ -929,8 +929,8 @@ cn10k_crypto_adapter_qp_add(const struct rte_eventdev *event_dev,
 {
 	int ret;
 
-	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn10k");
-	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn10k");
+	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn10k", EINVAL);
+	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn10k", EINVAL);
 
 	cn10k_sso_fp_fns_set((struct rte_eventdev *)(uintptr_t)event_dev);
 
@@ -944,8 +944,8 @@ static int
 cn10k_crypto_adapter_qp_del(const struct rte_eventdev *event_dev, const struct rte_cryptodev *cdev,
 			    int32_t queue_pair_id)
 {
-	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn10k");
-	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn10k");
+	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn10k", EINVAL);
+	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn10k", EINVAL);
 
 	return cnxk_crypto_adapter_qp_del(cdev, queue_pair_id);
 }
@@ -963,8 +963,8 @@ cn10k_crypto_adapter_vec_limits(const struct rte_eventdev *event_dev,
 				const struct rte_cryptodev *cdev,
 				struct rte_event_crypto_adapter_vector_limits *limits)
 {
-	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn10k");
-	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn10k");
+	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn10k", EINVAL);
+	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn10k", EINVAL);
 
 	limits->log2_sz = false;
 	limits->min_sz = 0;
diff --git a/drivers/event/cnxk/cn9k_eventdev.c b/drivers/event/cnxk/cn9k_eventdev.c
index 6cce5477f0..f77a9d7085 100644
--- a/drivers/event/cnxk/cn9k_eventdev.c
+++ b/drivers/event/cnxk/cn9k_eventdev.c
@@ -942,11 +942,11 @@ cn9k_sso_tx_adapter_queue_del(uint8_t id, const struct rte_eventdev *event_dev,
 }
 
 static int
-cn9k_crypto_adapter_caps_get(const struct rte_eventdev *event_dev,
-			     const struct rte_cryptodev *cdev, uint32_t *caps)
+cn9k_crypto_adapter_caps_get(const struct rte_eventdev *event_dev, const struct rte_cryptodev *cdev,
+			     uint32_t *caps)
 {
-	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn9k");
-	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn9k");
+	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn9k", ENOTSUP);
+	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn9k", ENOTSUP);
 
 	*caps = RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_OP_FWD |
 		RTE_EVENT_CRYPTO_ADAPTER_CAP_SESSION_PRIVATE_DATA;
@@ -962,8 +962,8 @@ cn9k_crypto_adapter_qp_add(const struct rte_eventdev *event_dev,
 {
 	int ret;
 
-	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn9k");
-	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn9k");
+	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn9k", EINVAL);
+	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn9k", EINVAL);
 
 	cn9k_sso_fp_fns_set((struct rte_eventdev *)(uintptr_t)event_dev);
 
@@ -977,8 +977,8 @@ static int
 cn9k_crypto_adapter_qp_del(const struct rte_eventdev *event_dev, const struct rte_cryptodev *cdev,
 			   int32_t queue_pair_id)
 {
-	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn9k");
-	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn9k");
+	CNXK_VALID_DEV_OR_ERR_RET(event_dev->dev, "event_cn9k", EINVAL);
+	CNXK_VALID_DEV_OR_ERR_RET(cdev->device, "crypto_cn9k", EINVAL);
 
 	return cnxk_crypto_adapter_qp_del(cdev, queue_pair_id);
 }
diff --git a/drivers/event/cnxk/cnxk_eventdev.h b/drivers/event/cnxk/cnxk_eventdev.h
index 9d95092669..bd50de87c0 100644
--- a/drivers/event/cnxk/cnxk_eventdev.h
+++ b/drivers/event/cnxk/cnxk_eventdev.h
@@ -48,10 +48,10 @@
 	(min + val / ((max + cnt - 1) / cnt))
 #define CNXK_SSO_FLUSH_RETRY_MAX 0xfff
 
-#define CNXK_VALID_DEV_OR_ERR_RET(dev, drv_name)                               \
+#define CNXK_VALID_DEV_OR_ERR_RET(dev, drv_name, err_val)                      \
 	do {                                                                   \
 		if (strncmp(dev->driver->name, drv_name, strlen(drv_name)))    \
-			return -EINVAL;                                        \
+			return -err_val;                                       \
 	} while (0)
 
 typedef void *(*cnxk_sso_init_hws_mem_t)(void *dev, uint8_t port_id);
-- 
2.25.1

[PATCH 2/3] test/event_crypto_adapter: skip unsupported test

[Aakash Sasidharan]

Skip the test if rte_event_crypto_adapter_caps_get()
returns -ENOTSUP.

Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
---
 app/test/test_event_crypto_adapter.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/app/test/test_event_crypto_adapter.c b/app/test/test_event_crypto_adapter.c
index 24f66be69f..00b20fa443 100644
--- a/app/test/test_event_crypto_adapter.c
+++ b/app/test/test_event_crypto_adapter.c
@@ -284,6 +284,9 @@ test_crypto_adapter_params(void)
 	};
 
 	err = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID, &cap);
+	if (err == -ENOTSUP)
+		return TEST_SKIPPED;
+
 	TEST_ASSERT_SUCCESS(err, "Failed to get adapter capabilities\n");
 
 	if (cap & RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) {
@@ -1252,8 +1255,13 @@ test_crypto_adapter_create(void)
 		.enqueue_depth = 8,
 		.new_event_threshold = 1200,
 	};
+	uint32_t cap;
 	int ret;
 
+	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID, &cap);
+	if (ret == -ENOTSUP)
+		return ret;
+
 	/* Create adapter with default port creation callback */
 	ret = rte_event_crypto_adapter_create(TEST_ADAPTER_ID,
 					      evdev,
@@ -1274,6 +1282,9 @@ test_crypto_adapter_qp_add_del(void)
 	int ret;
 
 	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID, &cap);
+	if (ret == -ENOTSUP)
+		return TEST_SKIPPED;
+
 	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
 
 	if (cap & RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) {
@@ -1309,6 +1320,9 @@ configure_event_crypto_adapter(enum rte_event_crypto_adapter_mode mode)
 	int ret;
 
 	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID, &cap);
+	if (ret == -ENOTSUP)
+		return ret;
+
 	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
 
 	/* Skip mode and capability mismatch check for SW eventdev */
@@ -1475,6 +1489,9 @@ crypto_adapter_teardown(void)
 {
 	int ret;
 
+	if (!crypto_adapter_setup_done)
+		return;
+
 	ret = rte_event_crypto_adapter_stop(TEST_ADAPTER_ID);
 	if (ret < 0)
 		RTE_LOG(ERR, USER1, "Failed to stop adapter!");
-- 
2.25.1

[PATCH 3/3] test/event_crypto_adapter: fix crypto null dev creation

[Aakash Sasidharan]

Create crypto null device only if no other crypto devices
were found.

Fixes: 3c2c535ecfc0 ("test: add event crypto adapter auto-test")
Cc: abhinandan.gujjar@intel.com

Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
---
 app/test/test_event_crypto_adapter.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/test/test_event_crypto_adapter.c b/app/test/test_event_crypto_adapter.c
index 00b20fa443..0c56744ba0 100644
--- a/app/test/test_event_crypto_adapter.c
+++ b/app/test/test_event_crypto_adapter.c
@@ -1072,11 +1072,10 @@ configure_cryptodev(void)
 		return TEST_FAILED;
 	}
 
-	/* Create a NULL crypto device */
-	nb_devs = rte_cryptodev_device_count_by_driver(
-			rte_cryptodev_driver_id_get(
-			RTE_STR(CRYPTODEV_NAME_NULL_PMD)));
+
+	nb_devs = rte_cryptodev_count();
 	if (!nb_devs) {
+		/* Create a NULL crypto device */
 		ret = rte_vdev_init(
 			RTE_STR(CRYPTODEV_NAME_NULL_PMD), NULL);
 
-- 
2.25.1

RE: [PATCH 1/3] event/cnxk: fix return values for capability API

[Anoob Joseph]

> 
> crypto adapter capability now returns -ENOTSUP instead of -EINVAL when

[Anoob] Better to start with caps (crypto -> Crypto). May be reword to,

When event_cnxk or crypto_cnxk PMDs are not available, return -ENOTSUP instead of -EINVAL.

> event_cnxk or crypto_cnxk device is not available. This would allow unit test
> to be skipped when the devices are not available.
> 
> Fixes: 19f81cb59cb4 ("event/cnxk: add crypto adapter operations")
> 
> Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> ---
>  drivers/event/cnxk/cn10k_eventdev.c | 16 ++++++++--------
> drivers/event/cnxk/cn9k_eventdev.c  | 16 ++++++++--------
> drivers/event/cnxk/cnxk_eventdev.h  |  4 ++--
>  3 files changed, 18 insertions(+), 18 deletions(-)

With above change,

Acked-by: Anoob Joseph <anoobj@marvell.com>

RE: [PATCH 3/3] test/event_crypto_adapter: fix crypto null dev creation

[Anoob Joseph]

> Create crypto null device only if no other crypto devices were found.
> 
> Fixes: 3c2c535ecfc0 ("test: add event crypto adapter auto-test")
> Cc: abhinandan.gujjar@intel.com
> 
> Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> ---
>  app/test/test_event_crypto_adapter.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 

Acked-by: Anoob Joseph <anoobj@marvell.com>

RE: [PATCH 2/3] test/event_crypto_adapter: skip unsupported test

[Anoob Joseph]

> 
> Skip the test if rte_event_crypto_adapter_caps_get()
> returns -ENOTSUP.
> 
> Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> ---
>  app/test/test_event_crypto_adapter.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 

Acked-by: Anoob Joseph <anoobj@marvell.com>

RE: [PATCH 2/3] test/event_crypto_adapter: skip unsupported test

[Gujjar, Abhinandan S]

> -----Original Message-----
> From: Aakash Sasidharan <asasidharan@marvell.com>
> Sent: Tuesday, August 22, 2023 6:47 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> Cc: jerinj@marvell.com; sthotton@marvell.com; anoobj@marvell.com;
> dev@dpdk.org; asasidharan@marvell.com
> Subject: [PATCH 2/3] test/event_crypto_adapter: skip unsupported test
> 
> Skip the test if rte_event_crypto_adapter_caps_get()
> returns -ENOTSUP.
> 
> Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> ---
>  app/test/test_event_crypto_adapter.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/app/test/test_event_crypto_adapter.c
> b/app/test/test_event_crypto_adapter.c
> index 24f66be69f..00b20fa443 100644
> --- a/app/test/test_event_crypto_adapter.c
> +++ b/app/test/test_event_crypto_adapter.c
> @@ -284,6 +284,9 @@ test_crypto_adapter_params(void)
>  	};
> 
>  	err = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> &cap);
> +	if (err == -ENOTSUP)
> +		return TEST_SKIPPED;
> +
>  	TEST_ASSERT_SUCCESS(err, "Failed to get adapter capabilities\n");
> 
>  	if (cap &
> RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) { @@
> -1252,8 +1255,13 @@ test_crypto_adapter_create(void)
>  		.enqueue_depth = 8,
>  		.new_event_threshold = 1200,
>  	};
> +	uint32_t cap;
>  	int ret;
> 
> +	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> &cap);
> +	if (ret == -ENOTSUP)
> +		return ret;
> +
rte_event_crypto_adapter_caps_get() is basic thing to check what is supported or not.
Not sure, purpose of this patch. Did you come across a case whether existing test was not working?
If so, please mention that in the patch commit.

>  	/* Create adapter with default port creation callback */
>  	ret = rte_event_crypto_adapter_create(TEST_ADAPTER_ID,
>  					      evdev,
> @@ -1274,6 +1282,9 @@ test_crypto_adapter_qp_add_del(void)
>  	int ret;
> 
>  	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> &cap);
> +	if (ret == -ENOTSUP)
> +		return TEST_SKIPPED;
> +
>  	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
> 
>  	if (cap &
> RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) { @@
> -1309,6 +1320,9 @@ configure_event_crypto_adapter(enum
> rte_event_crypto_adapter_mode mode)
>  	int ret;
> 
>  	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> &cap);
> +	if (ret == -ENOTSUP)
> +		return ret;
> +
>  	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
> 
>  	/* Skip mode and capability mismatch check for SW eventdev */ @@
> -1475,6 +1489,9 @@ crypto_adapter_teardown(void)  {
>  	int ret;
> 
> +	if (!crypto_adapter_setup_done)
> +		return;
> +
>  	ret = rte_event_crypto_adapter_stop(TEST_ADAPTER_ID);
>  	if (ret < 0)
>  		RTE_LOG(ERR, USER1, "Failed to stop adapter!");
> --
> 2.25.1

RE: [PATCH 3/3] test/event_crypto_adapter: fix crypto null dev creation

[Gujjar, Abhinandan S]

> -----Original Message-----
> From: Aakash Sasidharan <asasidharan@marvell.com>
> Sent: Tuesday, August 22, 2023 6:47 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Akhil Goyal
> <gakhil@marvell.com>
> Cc: jerinj@marvell.com; sthotton@marvell.com; anoobj@marvell.com;
> dev@dpdk.org; asasidharan@marvell.com
> Subject: [PATCH 3/3] test/event_crypto_adapter: fix crypto null dev creation
> 
> Create crypto null device only if no other crypto devices were found.
> 
> Fixes: 3c2c535ecfc0 ("test: add event crypto adapter auto-test")
> Cc: abhinandan.gujjar@intel.com
> 
> Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> ---
>  app/test/test_event_crypto_adapter.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test/test_event_crypto_adapter.c
> b/app/test/test_event_crypto_adapter.c
> index 00b20fa443..0c56744ba0 100644
> --- a/app/test/test_event_crypto_adapter.c
> +++ b/app/test/test_event_crypto_adapter.c
> @@ -1072,11 +1072,10 @@ configure_cryptodev(void)
>  		return TEST_FAILED;
>  	}
> 
> -	/* Create a NULL crypto device */
> -	nb_devs = rte_cryptodev_device_count_by_driver(
> -			rte_cryptodev_driver_id_get(
> -			RTE_STR(CRYPTODEV_NAME_NULL_PMD)));
> +
> +	nb_devs = rte_cryptodev_count();
The test is intended to do on NULL cipher with NULL PMD. 
Right now, the test does cipher with " RTE_CRYPTO_CIPHER_NULL".
With this changes, does this test work, if you provide any other cryptodev like AESNI, QAT, OpenSSL other than NULL PMD?
>  	if (!nb_devs) {
> +		/* Create a NULL crypto device */
>  		ret = rte_vdev_init(
>  			RTE_STR(CRYPTODEV_NAME_NULL_PMD), NULL);
> 
> --
> 2.25.1

RE: [PATCH 3/3] test/event_crypto_adapter: fix crypto null dev creation

[Anoob Joseph]

Hi Abhinandan,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> Sent: Wednesday, August 23, 2023 10:19 AM
> To: Aakash Sasidharan <asasidharan@marvell.com>; Akhil Goyal
> <gakhil@marvell.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Shijith Thotton
> <sthotton@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> dev@dpdk.org
> Subject: [EXT] RE: [PATCH 3/3] test/event_crypto_adapter: fix crypto null
> dev creation
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> > -----Original Message-----
> > From: Aakash Sasidharan <asasidharan@marvell.com>
> > Sent: Tuesday, August 22, 2023 6:47 PM
> > To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Akhil Goyal
> > <gakhil@marvell.com>
> > Cc: jerinj@marvell.com; sthotton@marvell.com; anoobj@marvell.com;
> > dev@dpdk.org; asasidharan@marvell.com
> > Subject: [PATCH 3/3] test/event_crypto_adapter: fix crypto null dev
> > creation
> >
> > Create crypto null device only if no other crypto devices were found.
> >
> > Fixes: 3c2c535ecfc0 ("test: add event crypto adapter auto-test")
> > Cc: abhinandan.gujjar@intel.com
> >
> > Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> > ---
> >  app/test/test_event_crypto_adapter.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test/test_event_crypto_adapter.c
> > b/app/test/test_event_crypto_adapter.c
> > index 00b20fa443..0c56744ba0 100644
> > --- a/app/test/test_event_crypto_adapter.c
> > +++ b/app/test/test_event_crypto_adapter.c
> > @@ -1072,11 +1072,10 @@ configure_cryptodev(void)
> >  		return TEST_FAILED;
> >  	}
> >
> > -	/* Create a NULL crypto device */
> > -	nb_devs = rte_cryptodev_device_count_by_driver(
> > -			rte_cryptodev_driver_id_get(
> > -			RTE_STR(CRYPTODEV_NAME_NULL_PMD)));
> > +
> > +	nb_devs = rte_cryptodev_count();
> The test is intended to do on NULL cipher with NULL PMD.

[Anoob] This test is intended to verify any cryptodev with crypto adapter. Why do we want to restrict the usage with NULL devices only?

> Right now, the test does cipher with " RTE_CRYPTO_CIPHER_NULL".

[Anoob] It has support for asymmetric cases also.

> With this changes, does this test work, if you provide any other cryptodev
> like AESNI, QAT, OpenSSL other than NULL PMD?

[Anoob] If NULL cipher is supported by those PMDs, it should be. Because, this being a standard test application should stick to standard semantics and so shouldn't care about whether the PMD under test is NULL or QAT. If NULL cipher is not supported by the PMD, then the case should get skipped at capability layer. Or are you saying that crypto adapter is only supposed to be used with NULL PMD?

Also, what we have seen is, if there are other cryptodevs (like cnxk), that would be the first cryptodev. And the test application is only testing CDEV_ID 0. So creating a NULL crypto PMD when other PMDs are available is pointless.

> >  	if (!nb_devs) {
> > +		/* Create a NULL crypto device */
> >  		ret = rte_vdev_init(
> >  			RTE_STR(CRYPTODEV_NAME_NULL_PMD), NULL);
> >
> > --
> > 2.25.1

RE: [PATCH 3/3] test/event_crypto_adapter: fix crypto null dev creation

[Gujjar, Abhinandan S]

Hi Anoob,

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Wednesday, August 23, 2023 11:17 AM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Shijith Thotton
> <sthotton@marvell.com>; dev@dpdk.org; Aakash Sasidharan
> <asasidharan@marvell.com>; Akhil Goyal <gakhil@marvell.com>
> Subject: RE: [PATCH 3/3] test/event_crypto_adapter: fix crypto null dev
> creation
> 
> Hi Abhinandan,
> 
> Please see inline.
> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> > Sent: Wednesday, August 23, 2023 10:19 AM
> > To: Aakash Sasidharan <asasidharan@marvell.com>; Akhil Goyal
> > <gakhil@marvell.com>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Shijith Thotton
> > <sthotton@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> > dev@dpdk.org
> > Subject: [EXT] RE: [PATCH 3/3] test/event_crypto_adapter: fix crypto
> > null dev creation
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> >
> >
> > > -----Original Message-----
> > > From: Aakash Sasidharan <asasidharan@marvell.com>
> > > Sent: Tuesday, August 22, 2023 6:47 PM
> > > To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Akhil Goyal
> > > <gakhil@marvell.com>
> > > Cc: jerinj@marvell.com; sthotton@marvell.com; anoobj@marvell.com;
> > > dev@dpdk.org; asasidharan@marvell.com
> > > Subject: [PATCH 3/3] test/event_crypto_adapter: fix crypto null dev
> > > creation
> > >
> > > Create crypto null device only if no other crypto devices were found.
> > >
> > > Fixes: 3c2c535ecfc0 ("test: add event crypto adapter auto-test")
> > > Cc: abhinandan.gujjar@intel.com
> > >
> > > Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> > > ---
> > >  app/test/test_event_crypto_adapter.c | 7 +++----
> > >  1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/app/test/test_event_crypto_adapter.c
> > > b/app/test/test_event_crypto_adapter.c
> > > index 00b20fa443..0c56744ba0 100644
> > > --- a/app/test/test_event_crypto_adapter.c
> > > +++ b/app/test/test_event_crypto_adapter.c
> > > @@ -1072,11 +1072,10 @@ configure_cryptodev(void)
> > >  		return TEST_FAILED;
> > >  	}
> > >
> > > -	/* Create a NULL crypto device */
> > > -	nb_devs = rte_cryptodev_device_count_by_driver(
> > > -			rte_cryptodev_driver_id_get(
> > > -			RTE_STR(CRYPTODEV_NAME_NULL_PMD)));
> > > +
> > > +	nb_devs = rte_cryptodev_count();
> > The test is intended to do on NULL cipher with NULL PMD.
> 
> [Anoob] This test is intended to verify any cryptodev with crypto adapter.
> Why do we want to restrict the usage with NULL devices only?
No such restriction. Please refer below comment.
> 
> > Right now, the test does cipher with " RTE_CRYPTO_CIPHER_NULL".
> 
> [Anoob] It has support for asymmetric cases also.
Well, I was talking about cipher algo used in the test in the context of keeping it simple to verify the crypto adapter as it is agnostic to any underlying device.
Not sym vs asym support.
> 
> > With this changes, does this test work, if you provide any other
> > cryptodev like AESNI, QAT, OpenSSL other than NULL PMD?
> 
> [Anoob] If NULL cipher is supported by those PMDs, it should be. Because,
> this being a standard test application should stick to standard semantics and
> so shouldn't care about whether the PMD under test is NULL or QAT. If NULL
> cipher is not supported by the PMD, then the case should get skipped at
> capability layer. Or are you saying that crypto adapter is only supposed to be
> used with NULL PMD?
It is not about supporting other PMDs.
This is about keeping the test case simple with minimal conf overhead so that it is easy for testing and maintaining.
From the code, I see that few PMDs supports NULL too without much changes to the app. It should be ok.
> 
> Also, what we have seen is, if there are other cryptodevs (like cnxk), that
> would be the first cryptodev. And the test application is only testing CDEV_ID
> 0. So creating a NULL crypto PMD when other PMDs are available is pointless.
Sounds good.

Acked-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> 
> > >  	if (!nb_devs) {
> > > +		/* Create a NULL crypto device */
> > >  		ret = rte_vdev_init(
> > >  			RTE_STR(CRYPTODEV_NAME_NULL_PMD), NULL);
> > >
> > > --
> > > 2.25.1

RE: [PATCH 2/3] test/event_crypto_adapter: skip unsupported test

[Aakash Sasidharan]

Hi Abhinandan,

Please see inline.

> -----Original Message-----
> From: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> Sent: Wednesday, August 23, 2023 10:04 AM
> To: Aakash Sasidharan <asasidharan@marvell.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Shijith Thotton
> <sthotton@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> dev@dpdk.org
> Subject: [EXT] RE: [PATCH 2/3] test/event_crypto_adapter: skip unsupported
> test
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> > -----Original Message-----
> > From: Aakash Sasidharan <asasidharan@marvell.com>
> > Sent: Tuesday, August 22, 2023 6:47 PM
> > To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> > Cc: jerinj@marvell.com; sthotton@marvell.com; anoobj@marvell.com;
> > dev@dpdk.org; asasidharan@marvell.com
> > Subject: [PATCH 2/3] test/event_crypto_adapter: skip unsupported test
> >
> > Skip the test if rte_event_crypto_adapter_caps_get()
> > returns -ENOTSUP.
> >
> > Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> > ---
> >  app/test/test_event_crypto_adapter.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/app/test/test_event_crypto_adapter.c
> > b/app/test/test_event_crypto_adapter.c
> > index 24f66be69f..00b20fa443 100644
> > --- a/app/test/test_event_crypto_adapter.c
> > +++ b/app/test/test_event_crypto_adapter.c
> > @@ -284,6 +284,9 @@ test_crypto_adapter_params(void)
> >  	};
> >
> >  	err = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> &cap);
> > +	if (err == -ENOTSUP)
> > +		return TEST_SKIPPED;
> > +
> >  	TEST_ASSERT_SUCCESS(err, "Failed to get adapter capabilities\n");
> >
> >  	if (cap &
> > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) {
> @@
> > -1252,8 +1255,13 @@ test_crypto_adapter_create(void)
> >  		.enqueue_depth = 8,
> >  		.new_event_threshold = 1200,
> >  	};
> > +	uint32_t cap;
> >  	int ret;
> >
> > +	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> > &cap);
> > +	if (ret == -ENOTSUP)
> > +		return ret;
> > +
> rte_event_crypto_adapter_caps_get() is basic thing to check what is
> supported or not.
> Not sure, purpose of this patch. Did you come across a case whether existing
> test was not working?
> If so, please mention that in the patch commit.
> 
[Aakash] This is required for event_cnxk because it can support crypto adapter
only with crypto_cnxk and not with other cryptodevs.

> >  	/* Create adapter with default port creation callback */
> >  	ret = rte_event_crypto_adapter_create(TEST_ADAPTER_ID,
> >  					      evdev,
> > @@ -1274,6 +1282,9 @@ test_crypto_adapter_qp_add_del(void)
> >  	int ret;
> >
> >  	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> &cap);
> > +	if (ret == -ENOTSUP)
> > +		return TEST_SKIPPED;
> > +
> >  	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
> >
> >  	if (cap &
> > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) {
> @@
> > -1309,6 +1320,9 @@ configure_event_crypto_adapter(enum
> > rte_event_crypto_adapter_mode mode)
> >  	int ret;
> >
> >  	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> &cap);
> > +	if (ret == -ENOTSUP)
> > +		return ret;
> > +
> >  	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
> >
> >  	/* Skip mode and capability mismatch check for SW eventdev */ @@
> > -1475,6 +1489,9 @@ crypto_adapter_teardown(void)  {
> >  	int ret;
> >
> > +	if (!crypto_adapter_setup_done)
> > +		return;
> > +
> >  	ret = rte_event_crypto_adapter_stop(TEST_ADAPTER_ID);
> >  	if (ret < 0)
> >  		RTE_LOG(ERR, USER1, "Failed to stop adapter!");
> > --
> > 2.25.1

RE: [PATCH 2/3] test/event_crypto_adapter: skip unsupported test

[Aakash Sasidharan]

Hi Abhinandan,
 
Please see inline.

> -----Original Message-----
> From: Aakash Sasidharan
> Sent: Wednesday, August 23, 2023 1:18 PM
> To: 'Gujjar, Abhinandan S' <abhinandan.gujjar@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Shijith Thotton
> <sthotton@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> dev@dpdk.org
> Subject: RE: [PATCH 2/3] test/event_crypto_adapter: skip unsupported test
> 
> Hi Abhinandan,
> 
> Please see inline.
> 
> > -----Original Message-----
> > From: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> > Sent: Wednesday, August 23, 2023 10:04 AM
> > To: Aakash Sasidharan <asasidharan@marvell.com>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Shijith Thotton
> > <sthotton@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> > dev@dpdk.org
> > Subject: [EXT] RE: [PATCH 2/3] test/event_crypto_adapter: skip
> > unsupported test
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> >
> >
> > > -----Original Message-----
> > > From: Aakash Sasidharan <asasidharan@marvell.com>
> > > Sent: Tuesday, August 22, 2023 6:47 PM
> > > To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> > > Cc: jerinj@marvell.com; sthotton@marvell.com; anoobj@marvell.com;
> > > dev@dpdk.org; asasidharan@marvell.com
> > > Subject: [PATCH 2/3] test/event_crypto_adapter: skip unsupported
> > > test
> > >
> > > Skip the test if rte_event_crypto_adapter_caps_get()
> > > returns -ENOTSUP.
> > >
> > > Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> > > ---
> > >  app/test/test_event_crypto_adapter.c | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > >
> > > diff --git a/app/test/test_event_crypto_adapter.c
> > > b/app/test/test_event_crypto_adapter.c
> > > index 24f66be69f..00b20fa443 100644
> > > --- a/app/test/test_event_crypto_adapter.c
> > > +++ b/app/test/test_event_crypto_adapter.c
> > > @@ -284,6 +284,9 @@ test_crypto_adapter_params(void)
> > >  	};
> > >
> > >  	err = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> > &cap);
> > > +	if (err == -ENOTSUP)
> > > +		return TEST_SKIPPED;
> > > +
> > >  	TEST_ASSERT_SUCCESS(err, "Failed to get adapter capabilities\n");
> > >
> > >  	if (cap &
> > > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) {
> > @@
> > > -1252,8 +1255,13 @@ test_crypto_adapter_create(void)
> > >  		.enqueue_depth = 8,
> > >  		.new_event_threshold = 1200,
> > >  	};
> > > +	uint32_t cap;
> > >  	int ret;
> > >
> > > +	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> > > &cap);
> > > +	if (ret == -ENOTSUP)
> > > +		return ret;
> > > +
> > rte_event_crypto_adapter_caps_get() is basic thing to check what is
> > supported or not.
> > Not sure, purpose of this patch. Did you come across a case whether
> > existing test was not working?
> > If so, please mention that in the patch commit.
> >
> [Aakash] This is required for event_cnxk because it can support crypto
> adapter only with crypto_cnxk and not with other cryptodevs.
> 
[Aakash] Just clarifying, PMD may return -ENOTSUP if the event device that is compatible with
the specified crypto adapter is not available. Above check is to handle the same.

> > >  	/* Create adapter with default port creation callback */
> > >  	ret = rte_event_crypto_adapter_create(TEST_ADAPTER_ID,
> > >  					      evdev,
> > > @@ -1274,6 +1282,9 @@ test_crypto_adapter_qp_add_del(void)
> > >  	int ret;
> > >
> > >  	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> > &cap);
> > > +	if (ret == -ENOTSUP)
> > > +		return TEST_SKIPPED;
> > > +
> > >  	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
> > >
> > >  	if (cap &
> > > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) {
> > @@
> > > -1309,6 +1320,9 @@ configure_event_crypto_adapter(enum
> > > rte_event_crypto_adapter_mode mode)
> > >  	int ret;
> > >
> > >  	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> > &cap);
> > > +	if (ret == -ENOTSUP)
> > > +		return ret;
> > > +
> > >  	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
> > >
> > >  	/* Skip mode and capability mismatch check for SW eventdev */ @@
> > > -1475,6 +1489,9 @@ crypto_adapter_teardown(void)  {
> > >  	int ret;
> > >
> > > +	if (!crypto_adapter_setup_done)
> > > +		return;
> > > +
> > >  	ret = rte_event_crypto_adapter_stop(TEST_ADAPTER_ID);
> > >  	if (ret < 0)
> > >  		RTE_LOG(ERR, USER1, "Failed to stop adapter!");
> > > --
> > > 2.25.1

RE: [PATCH 2/3] test/event_crypto_adapter: skip unsupported test

[Gujjar, Abhinandan S]

> -----Original Message-----
> From: Aakash Sasidharan <asasidharan@marvell.com>
> Sent: Wednesday, August 23, 2023 1:19 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Shijith Thotton
> <sthotton@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> dev@dpdk.org
> Subject: RE: [PATCH 2/3] test/event_crypto_adapter: skip unsupported test
> 
> Hi Abhinandan,
> 
> Please see inline.
> 
> > -----Original Message-----
> > From: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> > Sent: Wednesday, August 23, 2023 10:04 AM
> > To: Aakash Sasidharan <asasidharan@marvell.com>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Shijith Thotton
> > <sthotton@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> > dev@dpdk.org
> > Subject: [EXT] RE: [PATCH 2/3] test/event_crypto_adapter: skip
> > unsupported test
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> >
> >
> > > -----Original Message-----
> > > From: Aakash Sasidharan <asasidharan@marvell.com>
> > > Sent: Tuesday, August 22, 2023 6:47 PM
> > > To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> > > Cc: jerinj@marvell.com; sthotton@marvell.com; anoobj@marvell.com;
> > > dev@dpdk.org; asasidharan@marvell.com
> > > Subject: [PATCH 2/3] test/event_crypto_adapter: skip unsupported
> > > test
> > >
> > > Skip the test if rte_event_crypto_adapter_caps_get()
> > > returns -ENOTSUP.
> > >
> > > Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> > > ---
> > >  app/test/test_event_crypto_adapter.c | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > >
> > > diff --git a/app/test/test_event_crypto_adapter.c
> > > b/app/test/test_event_crypto_adapter.c
> > > index 24f66be69f..00b20fa443 100644
> > > --- a/app/test/test_event_crypto_adapter.c
> > > +++ b/app/test/test_event_crypto_adapter.c
> > > @@ -284,6 +284,9 @@ test_crypto_adapter_params(void)
> > >  	};
> > >
> > >  	err = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> > &cap);
> > > +	if (err == -ENOTSUP)
> > > +		return TEST_SKIPPED;
> > > +
> > >  	TEST_ASSERT_SUCCESS(err, "Failed to get adapter capabilities\n");
> > >
> > >  	if (cap &
> > > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) {
> > @@
> > > -1252,8 +1255,13 @@ test_crypto_adapter_create(void)
> > >  		.enqueue_depth = 8,
> > >  		.new_event_threshold = 1200,
> > >  	};
> > > +	uint32_t cap;
> > >  	int ret;
> > >
> > > +	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> > > &cap);
> > > +	if (ret == -ENOTSUP)
> > > +		return ret;
> > > +
> > rte_event_crypto_adapter_caps_get() is basic thing to check what is
> > supported or not.
> > Not sure, purpose of this patch. Did you come across a case whether
> > existing test was not working?
> > If so, please mention that in the patch commit.
> >
> [Aakash] This is required for event_cnxk because it can support crypto
> adapter only with crypto_cnxk and not with other cryptodevs.
Ok.

Acked-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>

> 
> > >  	/* Create adapter with default port creation callback */
> > >  	ret = rte_event_crypto_adapter_create(TEST_ADAPTER_ID,
> > >  					      evdev,
> > > @@ -1274,6 +1282,9 @@ test_crypto_adapter_qp_add_del(void)
> > >  	int ret;
> > >
> > >  	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> > &cap);
> > > +	if (ret == -ENOTSUP)
> > > +		return TEST_SKIPPED;
> > > +
> > >  	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
> > >
> > >  	if (cap &
> > > RTE_EVENT_CRYPTO_ADAPTER_CAP_INTERNAL_PORT_QP_EV_BIND) {
> > @@
> > > -1309,6 +1320,9 @@ configure_event_crypto_adapter(enum
> > > rte_event_crypto_adapter_mode mode)
> > >  	int ret;
> > >
> > >  	ret = rte_event_crypto_adapter_caps_get(evdev, TEST_CDEV_ID,
> > &cap);
> > > +	if (ret == -ENOTSUP)
> > > +		return ret;
> > > +
> > >  	TEST_ASSERT_SUCCESS(ret, "Failed to get adapter capabilities\n");
> > >
> > >  	/* Skip mode and capability mismatch check for SW eventdev */ @@
> > > -1475,6 +1489,9 @@ crypto_adapter_teardown(void)  {
> > >  	int ret;
> > >
> > > +	if (!crypto_adapter_setup_done)
> > > +		return;
> > > +
> > >  	ret = rte_event_crypto_adapter_stop(TEST_ADAPTER_ID);
> > >  	if (ret < 0)
> > >  		RTE_LOG(ERR, USER1, "Failed to stop adapter!");
> > > --
> > > 2.25.1

Re: [PATCH 3/3] test/event_crypto_adapter: fix crypto null dev creation

[Jerin Jacob]

On Wed, Aug 23, 2023 at 11:47 AM Gujjar, Abhinandan S
<abhinandan.gujjar@intel.com> wrote:
>

> It is not about supporting other PMDs.
> This is about keeping the test case simple with minimal conf overhead so that it is easy for testing and maintaining.
> From the code, I see that few PMDs supports NULL too without much changes to the app. It should be ok.
> >
> > Also, what we have seen is, if there are other cryptodevs (like cnxk), that
> > would be the first cryptodev. And the test application is only testing CDEV_ID
> > 0. So creating a NULL crypto PMD when other PMDs are available is pointless.
> Sounds good.
>
> Acked-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>

Series applied to dpdk-next-net-eventdev/for-main. Thanks