[dpdk-dev] [PATCH 0/4] add eventdev vdev uninit support

[dpdk-dev] [PATCH 0/4] add eventdev vdev uninit support

[Jerin Jacob]

This patchset adds eventdev vdev uinit support and a unit
test case to verify the same

Jerin Jacob (4):
  eventdev: fix event driver name to eventdev lookup
  evendev: add vdev uninit support
  event/skeleton: add vdev uninit support
  app/test: unit test case to exercise eventdev vdev uninit

 app/test/test_eventdev.c                     | 60 +++++++++++++++++++++++++++-
 drivers/event/skeleton/skeleton_eventdev.c   | 42 +++----------------
 lib/librte_eventdev/rte_eventdev.c           | 43 ++++++++++++++++++--
 lib/librte_eventdev/rte_eventdev_pmd.h       | 17 ++++++--
 lib/librte_eventdev/rte_eventdev_version.map |  1 +
 5 files changed, 117 insertions(+), 46 deletions(-)

-- 
2.5.5

[dpdk-dev] [PATCH 1/4] eventdev: fix event driver name to eventdev lookup

[Jerin Jacob]

- Removed uninitialized max_devs value
- Corrected dev assignment

Fixes: 8ca610e0487c ("eventdev: implement the northbound APIs")

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 lib/librte_eventdev/rte_eventdev_pmd.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index 15843c6..4eea618 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -149,7 +149,6 @@ struct rte_eventdev_driver {
 /** Global structure used for maintaining state of allocated event devices */
 struct rte_eventdev_global {
 	uint8_t nb_devs;	/**< Number of devices found */
-	uint8_t max_devs;	/**< Max number of devices */
 };
 
 extern struct rte_eventdev_global *rte_eventdev_globals;
@@ -175,8 +174,8 @@ rte_event_pmd_get_named_dev(const char *name)
 	if (name == NULL)
 		return NULL;
 
-	for (i = 0, dev = &rte_eventdevs[i];
-			i < rte_eventdev_globals->max_devs; i++) {
+	for (i = 0; i < RTE_EVENT_MAX_DEVS; i++) {
+		dev = &rte_eventdevs[i];
 		if ((dev->attached == RTE_EVENTDEV_ATTACHED) &&
 				(strcmp(dev->data->name, name) == 0))
 			return dev;
-- 
2.5.5

[dpdk-dev] [PATCH 2/4] evendev: add vdev uninit support

[Jerin Jacob]

Added eventdev vdev uninit support to release the resources
allocated in eventdev vdev init.

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 lib/librte_eventdev/rte_eventdev.c           | 43 +++++++++++++++++++++++++---
 lib/librte_eventdev/rte_eventdev_pmd.h       | 12 +++++++-
 lib/librte_eventdev/rte_eventdev_version.map |  1 +
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index 2b39fb2..09fc274 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -1143,6 +1143,8 @@ int
 rte_event_pmd_release(struct rte_eventdev *eventdev)
 {
 	int ret;
+	char mz_name[RTE_EVENTDEV_NAME_MAX_LEN];
+	const struct rte_memzone *mz;
 
 	if (eventdev == NULL)
 		return -EINVAL;
@@ -1153,8 +1155,26 @@ rte_event_pmd_release(struct rte_eventdev *eventdev)
 
 	eventdev->attached = RTE_EVENTDEV_DETACHED;
 	eventdev_globals.nb_devs--;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		rte_free(eventdev->data->dev_private);
+
+		/* Generate memzone name */
+		ret = snprintf(mz_name, sizeof(mz_name), "rte_eventdev_data_%u",
+				eventdev->data->dev_id);
+		if (ret >= (int)sizeof(mz_name))
+			return -EINVAL;
+
+		mz = rte_memzone_lookup(mz_name);
+		if (mz == NULL)
+			return -ENOMEM;
+
+		ret = rte_memzone_free(mz);
+		if (ret)
+			return ret;
+	}
+
 	eventdev->data = NULL;
-
 	return 0;
 }
 
@@ -1186,6 +1206,24 @@ rte_event_pmd_vdev_init(const char *name, size_t dev_private_size,
 }
 
 int
+rte_event_pmd_vdev_uninit(const char *name)
+{
+	struct rte_eventdev *eventdev;
+
+	if (name == NULL)
+		return -EINVAL;
+
+	eventdev = rte_event_pmd_get_named_dev(name);
+	if (eventdev == NULL)
+		return -ENODEV;
+
+	/* Free the event device */
+	rte_event_pmd_release(eventdev);
+
+	return 0;
+}
+
+int
 rte_event_pmd_pci_probe(struct rte_pci_driver *pci_drv,
 			struct rte_pci_device *pci_dev)
 {
@@ -1275,9 +1313,6 @@ rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev)
 	/* Free event device */
 	rte_event_pmd_release(eventdev);
 
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
-		rte_free(eventdev->data->dev_private);
-
 	eventdev->pci_dev = NULL;
 	eventdev->driver = NULL;
 
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index 4eea618..be5c784 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -198,7 +198,7 @@ rte_event_pmd_is_valid_dev(uint8_t dev_id)
 {
 	struct rte_eventdev *dev;
 
-	if (dev_id >= rte_eventdev_globals->nb_devs)
+	if (dev_id >= RTE_EVENT_MAX_DEVS)
 		return 0;
 
 	dev = &rte_eventdevs[dev_id];
@@ -552,6 +552,16 @@ struct rte_eventdev *
 rte_event_pmd_vdev_init(const char *name, size_t dev_private_size,
 		int socket_id);
 
+/**
+ * Destroy the given virtual event device
+ *
+ * @param name
+ *   PMD type name
+ * @return
+ *   - 0 on success, negative on error
+ */
+int
+rte_event_pmd_vdev_uninit(const char *name);
 
 /**
  * Wrapper for use by pci drivers as a .probe function to attach to a event
diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
index b138eb3..c572c4d 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -35,6 +35,7 @@ DPDK_17.02 {
 	rte_event_pmd_allocate;
 	rte_event_pmd_release;
 	rte_event_pmd_vdev_init;
+	rte_event_pmd_vdev_uninit;
 	rte_event_pmd_pci_probe;
 	rte_event_pmd_pci_remove;
 
-- 
2.5.5

[dpdk-dev] [PATCH 3/4] event/skeleton: add vdev uninit support

[Jerin Jacob]

Removed global index based device name
generation as vdev uninit needs the exact driver
name used vdev init.

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 drivers/event/skeleton/skeleton_eventdev.c | 42 ++++--------------------------
 1 file changed, 5 insertions(+), 37 deletions(-)

diff --git a/drivers/event/skeleton/skeleton_eventdev.c b/drivers/event/skeleton/skeleton_eventdev.c
index 085cb86..46e75d8 100644
--- a/drivers/event/skeleton/skeleton_eventdev.c
+++ b/drivers/event/skeleton/skeleton_eventdev.c
@@ -439,44 +439,15 @@ RTE_PMD_REGISTER_PCI_TABLE(event_skeleton_pci, pci_id_skeleton_map);
 
 /* VDEV based event device */
 
-/**
- * Global static parameter used to create a unique name for each skeleton
- * event device.
- */
-static unsigned int skeleton_unique_id;
-
-static inline int
-skeleton_create_unique_device_name(char *name, size_t size)
-{
-	int ret;
-
-	if (name == NULL)
-		return -EINVAL;
-
-	ret = snprintf(name, size, "%s_%u", RTE_STR(EVENTDEV_NAME_SKELETON_PMD),
-			skeleton_unique_id++);
-	if (ret < 0)
-		return ret;
-	return 0;
-}
-
 static int
-skeleton_eventdev_create(int socket_id)
+skeleton_eventdev_create(const char *name, int socket_id)
 {
 	struct rte_eventdev *eventdev;
-	char eventdev_name[RTE_EVENTDEV_NAME_MAX_LEN];
 
-	/* Create a unique device name */
-	if (skeleton_create_unique_device_name(eventdev_name,
-			RTE_EVENTDEV_NAME_MAX_LEN) != 0) {
-		PMD_DRV_ERR("Failed to create unique eventdev name");
-		return -EINVAL;
-	}
-
-	eventdev = rte_event_pmd_vdev_init(eventdev_name,
+	eventdev = rte_event_pmd_vdev_init(name,
 			sizeof(struct skeleton_eventdev), socket_id);
 	if (eventdev == NULL) {
-		PMD_DRV_ERR("Failed to create eventdev vdev");
+		PMD_DRV_ERR("Failed to create eventdev vdev %s", name);
 		goto fail;
 	}
 
@@ -497,18 +468,15 @@ skeleton_eventdev_probe(const char *name, __rte_unused const char *input_args)
 {
 	RTE_LOG(INFO, PMD, "Initializing %s on NUMA node %d\n", name,
 			rte_socket_id());
-	return skeleton_eventdev_create(rte_socket_id());
+	return skeleton_eventdev_create(name, rte_socket_id());
 }
 
 static int
 skeleton_eventdev_remove(const char *name)
 {
-	if (name == NULL)
-		return -EINVAL;
-
 	PMD_DRV_LOG(INFO, "Closing %s on NUMA node %d", name, rte_socket_id());
 
-	return 0;
+	return rte_event_pmd_vdev_uninit(name);
 }
 
 static struct rte_vdev_driver vdev_eventdev_skeleton_pmd = {
-- 
2.5.5

[dpdk-dev] [PATCH 4/4] app/test: unit test case to exercise eventdev vdev uninit

[Jerin Jacob]

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 app/test/test_eventdev.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/app/test/test_eventdev.c b/app/test/test_eventdev.c
index 042a446..e817838 100644
--- a/app/test/test_eventdev.c
+++ b/app/test/test_eventdev.c
@@ -51,7 +51,7 @@ testsuite_setup(void)
 	if (!count) {
 		printf("Failed to find a valid event device,"
 			" testing with event_skeleton device\n");
-		return rte_eal_vdev_init("event_skeleton", NULL);
+		return rte_eal_vdev_init("event_skeleton0", NULL);
 	}
 	return TEST_SUCCESS;
 }
@@ -720,6 +720,62 @@ test_eventdev_close(void)
 	return rte_event_dev_close(TEST_DEV_ID);
 }
 
+#define TEST_EVENTDEV_COUNT 8
+
+static int
+test_eventdev_create_destroy(void)
+{
+#ifdef RTE_LIBRTE_PMD_SKELETON_EVENTDEV
+	int i, ret;
+	uint8_t curr_count;
+	char name[RTE_EVENTDEV_NAME_MAX_LEN];
+
+	curr_count = rte_event_dev_count();
+
+	/* Start from one to avoid overlap with active event_skeleton0 dev */
+	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
+		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
+				 "%s%u", "event_skeleton", i);
+		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
+		ret = rte_eal_vdev_init(name, NULL);
+		TEST_ASSERT_SUCCESS(ret, "Failed to init event_skeleton dev %s",
+					name);
+	}
+	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
+		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
+				 "%s%u", "event_skeleton", i);
+		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
+		ret = rte_eal_vdev_uninit(name);
+		TEST_ASSERT_SUCCESS(ret, "Failed to uninit skeleton dev %s",
+					name);
+	}
+	TEST_ASSERT(curr_count == rte_event_dev_count(),
+			 "init/uninit pairs count mismatch");
+
+	/* Test in reverse order */
+	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
+		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
+				 "%s%u", "event_skeleton", i);
+		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
+		ret = rte_eal_vdev_init(name, NULL);
+		TEST_ASSERT_SUCCESS(ret, "Failed to init event_skeleton dev %s",
+					name);
+	}
+	for (i = TEST_EVENTDEV_COUNT; i > 0; i--) {
+		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
+				 "%s%u", "event_skeleton", i);
+		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
+		ret = rte_eal_vdev_uninit(name);
+		TEST_ASSERT_SUCCESS(ret, "Failed to uninit skeleton dev %s",
+					name);
+	}
+	TEST_ASSERT(curr_count == rte_event_dev_count(),
+			 "init/uninit pairs count mismatch");
+#else
+	printf("Skipping eventdev_create_destroy test due to unavailability of event skeleton device\n");
+#endif
+	return TEST_SUCCESS;
+}
 static struct unit_test_suite eventdev_common_testsuite  = {
 	.suite_name = "eventdev common code unit test suite",
 	.setup = testsuite_setup,
@@ -765,6 +821,8 @@ static struct unit_test_suite eventdev_common_testsuite  = {
 			test_eventdev_link_get),
 		TEST_CASE_ST(eventdev_setup_device, NULL,
 			test_eventdev_close),
+		TEST_CASE_ST(NULL, NULL,
+			test_eventdev_create_destroy),
 		TEST_CASES_END() /**< NULL terminate unit test array */
 	}
 };
-- 
2.5.5

Re: [dpdk-dev] [PATCH 1/4] eventdev: fix event driver name to eventdev lookup

[Van Haaren, Harry]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, February 6, 2017 5:24 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Eads, Gage
> <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH 1/4] eventdev: fix event driver name to eventdev lookup
> 
> - Removed uninitialized max_devs value
> - Corrected dev assignment
> 
> Fixes: 8ca610e0487c ("eventdev: implement the northbound APIs")
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Re: [dpdk-dev] [PATCH 2/4] evendev: add vdev uninit support

[Van Haaren, Harry]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, February 6, 2017 5:24 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Eads, Gage
> <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH 2/4] evendev: add vdev uninit support
> 
> Added eventdev vdev uninit support to release the resources
> allocated in eventdev vdev init.
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---

One comment below to update version.map 17.05, but once that's fixed

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>


>  lib/librte_eventdev/rte_eventdev.c           | 43 +++++++++++++++++++++++++---
>  lib/librte_eventdev/rte_eventdev_pmd.h       | 12 +++++++-
>  lib/librte_eventdev/rte_eventdev_version.map |  1 +
>  3 files changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> index 2b39fb2..09fc274 100644
> --- a/lib/librte_eventdev/rte_eventdev.c
> +++ b/lib/librte_eventdev/rte_eventdev.c
> @@ -1143,6 +1143,8 @@ int
>  rte_event_pmd_release(struct rte_eventdev *eventdev)
>  {
>  	int ret;
> +	char mz_name[RTE_EVENTDEV_NAME_MAX_LEN];
> +	const struct rte_memzone *mz;
> 
>  	if (eventdev == NULL)
>  		return -EINVAL;
> @@ -1153,8 +1155,26 @@ rte_event_pmd_release(struct rte_eventdev *eventdev)
> 
>  	eventdev->attached = RTE_EVENTDEV_DETACHED;
>  	eventdev_globals.nb_devs--;
> +
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +		rte_free(eventdev->data->dev_private);
> +
> +		/* Generate memzone name */
> +		ret = snprintf(mz_name, sizeof(mz_name), "rte_eventdev_data_%u",
> +				eventdev->data->dev_id);
> +		if (ret >= (int)sizeof(mz_name))
> +			return -EINVAL;
> +
> +		mz = rte_memzone_lookup(mz_name);
> +		if (mz == NULL)
> +			return -ENOMEM;
> +
> +		ret = rte_memzone_free(mz);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	eventdev->data = NULL;
> -
>  	return 0;
>  }
> 
> @@ -1186,6 +1206,24 @@ rte_event_pmd_vdev_init(const char *name, size_t dev_private_size,
>  }
> 
>  int
> +rte_event_pmd_vdev_uninit(const char *name)
> +{
> +	struct rte_eventdev *eventdev;
> +
> +	if (name == NULL)
> +		return -EINVAL;
> +
> +	eventdev = rte_event_pmd_get_named_dev(name);
> +	if (eventdev == NULL)
> +		return -ENODEV;
> +
> +	/* Free the event device */
> +	rte_event_pmd_release(eventdev);
> +
> +	return 0;
> +}
> +
> +int
>  rte_event_pmd_pci_probe(struct rte_pci_driver *pci_drv,
>  			struct rte_pci_device *pci_dev)
>  {
> @@ -1275,9 +1313,6 @@ rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev)
>  	/* Free event device */
>  	rte_event_pmd_release(eventdev);
> 
> -	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> -		rte_free(eventdev->data->dev_private);



>  	eventdev->pci_dev = NULL;
>  	eventdev->driver = NULL;
> 
> diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
> index 4eea618..be5c784 100644
> --- a/lib/librte_eventdev/rte_eventdev_pmd.h
> +++ b/lib/librte_eventdev/rte_eventdev_pmd.h
> @@ -198,7 +198,7 @@ rte_event_pmd_is_valid_dev(uint8_t dev_id)
>  {
>  	struct rte_eventdev *dev;
> 
> -	if (dev_id >= rte_eventdev_globals->nb_devs)
> +	if (dev_id >= RTE_EVENT_MAX_DEVS)
>  		return 0;
> 
>  	dev = &rte_eventdevs[dev_id];
> @@ -552,6 +552,16 @@ struct rte_eventdev *
>  rte_event_pmd_vdev_init(const char *name, size_t dev_private_size,
>  		int socket_id);
> 
> +/**
> + * Destroy the given virtual event device
> + *
> + * @param name
> + *   PMD type name
> + * @return
> + *   - 0 on success, negative on error
> + */
> +int
> +rte_event_pmd_vdev_uninit(const char *name);
> 
>  /**
>   * Wrapper for use by pci drivers as a .probe function to attach to a event
> diff --git a/lib/librte_eventdev/rte_eventdev_version.map
> b/lib/librte_eventdev/rte_eventdev_version.map
> index b138eb3..c572c4d 100644
> --- a/lib/librte_eventdev/rte_eventdev_version.map
> +++ b/lib/librte_eventdev/rte_eventdev_version.map
> @@ -35,6 +35,7 @@ DPDK_17.02 {


Version to 17.05 I think?


>  	rte_event_pmd_allocate;
>  	rte_event_pmd_release;
>  	rte_event_pmd_vdev_init;
> +	rte_event_pmd_vdev_uninit;
>  	rte_event_pmd_pci_probe;
>  	rte_event_pmd_pci_remove;
> 
> --
> 2.5.5

Re: [dpdk-dev] [PATCH 3/4] event/skeleton: add vdev uninit support

[Van Haaren, Harry]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, February 6, 2017 5:24 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Eads, Gage
> <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH 3/4] event/skeleton: add vdev uninit support
> 
> Removed global index based device name
> generation as vdev uninit needs the exact driver
> name used vdev init.
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Re: [dpdk-dev] [PATCH 4/4] app/test: unit test case to exercise eventdev vdev uninit

[Van Haaren, Harry]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, February 6, 2017 5:24 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Eads, Gage
> <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH 4/4] app/test: unit test case to exercise eventdev vdev uninit
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---

Comments inline,

>  app/test/test_eventdev.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test/test_eventdev.c b/app/test/test_eventdev.c
> index 042a446..e817838 100644
> --- a/app/test/test_eventdev.c
> +++ b/app/test/test_eventdev.c
> @@ -51,7 +51,7 @@ testsuite_setup(void)
>  	if (!count) {
>  		printf("Failed to find a valid event device,"
>  			" testing with event_skeleton device\n");
> -		return rte_eal_vdev_init("event_skeleton", NULL);
> +		return rte_eal_vdev_init("event_skeleton0", NULL);

I think these hard-coded eventdev names need to be removed from the testing framework, and we should let the app/test >> propt accept the name of the vdev to use instead. That would allow running tests on each device by:

RTE>> eventdev_common_autotest event_skeleton0

RTE>> eventdev_common_autotest event_sw0


>  	}
>  	return TEST_SUCCESS;
>  }
> @@ -720,6 +720,62 @@ test_eventdev_close(void)
>  	return rte_event_dev_close(TEST_DEV_ID);
>  }
> 
> +#define TEST_EVENTDEV_COUNT 8
> +
> +static int
> +test_eventdev_create_destroy(void)
> +{
> +#ifdef RTE_LIBRTE_PMD_SKELETON_EVENTDEV

Same issue as above, compile-time selection of the tests to be run isn't flexible enough to allow testing multiple eventdevs.
I think using a parameter to eventdev_common_autotest to pass the "name" to use would work again? I am open to other solutions if there's a better way :)


> +	int i, ret;
> +	uint8_t curr_count;
> +	char name[RTE_EVENTDEV_NAME_MAX_LEN];
> +
> +	curr_count = rte_event_dev_count();
> +
> +	/* Start from one to avoid overlap with active event_skeleton0 dev */
> +	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
> +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> +				 "%s%u", "event_skeleton", i);
> +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> +		ret = rte_eal_vdev_init(name, NULL);
> +		TEST_ASSERT_SUCCESS(ret, "Failed to init event_skeleton dev %s",
> +					name);
> +	}
> +	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
> +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> +				 "%s%u", "event_skeleton", i);
> +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> +		ret = rte_eal_vdev_uninit(name);
> +		TEST_ASSERT_SUCCESS(ret, "Failed to uninit skeleton dev %s",
> +					name);
> +	}
> +	TEST_ASSERT(curr_count == rte_event_dev_count(),
> +			 "init/uninit pairs count mismatch");
> +
> +	/* Test in reverse order */
> +	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
> +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> +				 "%s%u", "event_skeleton", i);
> +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> +		ret = rte_eal_vdev_init(name, NULL);
> +		TEST_ASSERT_SUCCESS(ret, "Failed to init event_skeleton dev %s",
> +					name);
> +	}
> +	for (i = TEST_EVENTDEV_COUNT; i > 0; i--) {
> +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> +				 "%s%u", "event_skeleton", i);
> +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> +		ret = rte_eal_vdev_uninit(name);
> +		TEST_ASSERT_SUCCESS(ret, "Failed to uninit skeleton dev %s",
> +					name);
> +	}
> +	TEST_ASSERT(curr_count == rte_event_dev_count(),
> +			 "init/uninit pairs count mismatch");
> +#else
> +	printf("Skipping eventdev_create_destroy test due to unavailability of event skeleton
> device\n");
> +#endif
> +	return TEST_SUCCESS;
> +}
>  static struct unit_test_suite eventdev_common_testsuite  = {
>  	.suite_name = "eventdev common code unit test suite",
>  	.setup = testsuite_setup,
> @@ -765,6 +821,8 @@ static struct unit_test_suite eventdev_common_testsuite  = {
>  			test_eventdev_link_get),
>  		TEST_CASE_ST(eventdev_setup_device, NULL,
>  			test_eventdev_close),
> +		TEST_CASE_ST(NULL, NULL,
> +			test_eventdev_create_destroy),
>  		TEST_CASES_END() /**< NULL terminate unit test array */
>  	}
>  };
> --
> 2.5.5

Re: [dpdk-dev] [PATCH 4/4] app/test: unit test case to exercise eventdev vdev uninit

[Jerin Jacob]

On Tue, Feb 07, 2017 at 03:17:30PM +0000, Van Haaren, Harry wrote:
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, February 6, 2017 5:24 AM
> > To: dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Eads, Gage
> > <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jerin Jacob
> > <jerin.jacob@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH 4/4] app/test: unit test case to exercise eventdev vdev uninit
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> 
> Comments inline,
> 
> >  app/test/test_eventdev.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> > 
> > diff --git a/app/test/test_eventdev.c b/app/test/test_eventdev.c
> > index 042a446..e817838 100644
> > --- a/app/test/test_eventdev.c
> > +++ b/app/test/test_eventdev.c
> > @@ -51,7 +51,7 @@ testsuite_setup(void)
> >  	if (!count) {
> >  		printf("Failed to find a valid event device,"
> >  			" testing with event_skeleton device\n");
> > -		return rte_eal_vdev_init("event_skeleton", NULL);
> > +		return rte_eal_vdev_init("event_skeleton0", NULL);
> 
> I think these hard-coded eventdev names need to be removed from the testing framework, and we should let the app/test >> propt accept the name of the vdev to use instead. That would allow running tests on each device by:
> 
> RTE>> eventdev_common_autotest event_skeleton0
> 
> RTE>> eventdev_common_autotest event_sw0

It checks for "count = rte_event_dev_count()" before the calling rte_eal_vdev_init.
That's would translate to use any eventdev
driver if it is available in the EAL command line argument.So I think, we
are good here and in case none of the drivers provided in EAL argument
it choose the skeleton driver as default.

sudo ./build/app/test -c 2 --vdev='event_sw0'
sudo ./build/app/test -c 2 --vdev='event_skeleton0'

> 
> 
> >  	}
> >  	return TEST_SUCCESS;
> >  }
> > @@ -720,6 +720,62 @@ test_eventdev_close(void)
> >  	return rte_event_dev_close(TEST_DEV_ID);
> >  }
> > 
> > +#define TEST_EVENTDEV_COUNT 8
> > +
> > +static int
> > +test_eventdev_create_destroy(void)
> > +{
> > +#ifdef RTE_LIBRTE_PMD_SKELETON_EVENTDEV
> 
> Same issue as above, compile-time selection of the tests to be run isn't flexible enough to allow testing multiple eventdevs.
> I think using a parameter to eventdev_common_autotest to pass the "name" to use would work again? I am open to other solutions if there's a better way :)

In general I agree with your comment. But in this specific case,

1) Device can be PCI device too. But,This test case will be applicable only to vdev
device. So does it make sense to generalize.
2) Currently app/test won't accepts the command line arguments for a specific
test case.Is there any other alternatives? If none, I will check for possibility
of adding that change to app/test first.

Another options could be,
1) environment variable
2) new eventdev APIs to get driver name from dev_id.But does not makes much
sense in case PCIe device.

Thoughts?

> 
> 
> > +	int i, ret;
> > +	uint8_t curr_count;
> > +	char name[RTE_EVENTDEV_NAME_MAX_LEN];
> > +
> > +	curr_count = rte_event_dev_count();
> > +
> > +	/* Start from one to avoid overlap with active event_skeleton0 dev */
> > +	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
> > +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> > +				 "%s%u", "event_skeleton", i);
> > +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> > +		ret = rte_eal_vdev_init(name, NULL);
> > +		TEST_ASSERT_SUCCESS(ret, "Failed to init event_skeleton dev %s",
> > +					name);
> > +	}
> > +	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
> > +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> > +				 "%s%u", "event_skeleton", i);
> > +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> > +		ret = rte_eal_vdev_uninit(name);
> > +		TEST_ASSERT_SUCCESS(ret, "Failed to uninit skeleton dev %s",
> > +					name);
> > +	}
> > +	TEST_ASSERT(curr_count == rte_event_dev_count(),
> > +			 "init/uninit pairs count mismatch");
> > +
> > +	/* Test in reverse order */
> > +	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
> > +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> > +				 "%s%u", "event_skeleton", i);
> > +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> > +		ret = rte_eal_vdev_init(name, NULL);
> > +		TEST_ASSERT_SUCCESS(ret, "Failed to init event_skeleton dev %s",
> > +					name);
> > +	}
> > +	for (i = TEST_EVENTDEV_COUNT; i > 0; i--) {
> > +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> > +				 "%s%u", "event_skeleton", i);
> > +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> > +		ret = rte_eal_vdev_uninit(name);
> > +		TEST_ASSERT_SUCCESS(ret, "Failed to uninit skeleton dev %s",
> > +					name);
> > +	}
> > +	TEST_ASSERT(curr_count == rte_event_dev_count(),
> > +			 "init/uninit pairs count mismatch");
> > +#else
> > +	printf("Skipping eventdev_create_destroy test due to unavailability of event skeleton
> > device\n");
> > +#endif
> > +	return TEST_SUCCESS;
> > +}
> >  static struct unit_test_suite eventdev_common_testsuite  = {
> >  	.suite_name = "eventdev common code unit test suite",
> >  	.setup = testsuite_setup,
> > @@ -765,6 +821,8 @@ static struct unit_test_suite eventdev_common_testsuite  = {
> >  			test_eventdev_link_get),
> >  		TEST_CASE_ST(eventdev_setup_device, NULL,
> >  			test_eventdev_close),
> > +		TEST_CASE_ST(NULL, NULL,
> > +			test_eventdev_create_destroy),
> >  		TEST_CASES_END() /**< NULL terminate unit test array */
> >  	}
> >  };
> > --
> > 2.5.5
> 

Re: [dpdk-dev] [PATCH 1/4] eventdev: fix event driver name to eventdev lookup

[Jerin Jacob]

On Tue, Feb 07, 2017 at 03:04:19PM +0000, Van Haaren, Harry wrote:
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, February 6, 2017 5:24 AM
> > To: dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Eads, Gage
> > <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jerin Jacob
> > <jerin.jacob@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH 1/4] eventdev: fix event driver name to eventdev lookup
> > 
> > - Removed uninitialized max_devs value
> > - Corrected dev assignment
> > 
> > Fixes: 8ca610e0487c ("eventdev: implement the northbound APIs")
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Applied to dpdk-next-eventdev/master. Thanks.

Re: [dpdk-dev] [PATCH 2/4] evendev: add vdev uninit support

[Jerin Jacob]

On Tue, Feb 07, 2017 at 03:11:01PM +0000, Van Haaren, Harry wrote:
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, February 6, 2017 5:24 AM
> > To: dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Eads, Gage
> > <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jerin Jacob
> > <jerin.jacob@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH 2/4] evendev: add vdev uninit support
> > 
> > Added eventdev vdev uninit support to release the resources
> > allocated in eventdev vdev init.
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> 
> One comment below to update version.map 17.05, but once that's fixed

Fixed it.
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Applied to dpdk-next-eventdev/master. Thanks.

> 
> 
> >  lib/librte_eventdev/rte_eventdev.c           | 43 +++++++++++++++++++++++++---
> >  lib/librte_eventdev/rte_eventdev_pmd.h       | 12 +++++++-
> >  lib/librte_eventdev/rte_eventdev_version.map |  1 +
> >  3 files changed, 51 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> > index 2b39fb2..09fc274 100644
> > --- a/lib/librte_eventdev/rte_eventdev.c
> > +++ b/lib/librte_eventdev/rte_eventdev.c
> > @@ -1143,6 +1143,8 @@ int
> >  rte_event_pmd_release(struct rte_eventdev *eventdev)
> >  {
> >  	int ret;
> > +	char mz_name[RTE_EVENTDEV_NAME_MAX_LEN];
> > +	const struct rte_memzone *mz;
> > 
> >  	if (eventdev == NULL)
> >  		return -EINVAL;
> > @@ -1153,8 +1155,26 @@ rte_event_pmd_release(struct rte_eventdev *eventdev)
> > 
> >  	eventdev->attached = RTE_EVENTDEV_DETACHED;
> >  	eventdev_globals.nb_devs--;
> > +
> > +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> > +		rte_free(eventdev->data->dev_private);
> > +
> > +		/* Generate memzone name */
> > +		ret = snprintf(mz_name, sizeof(mz_name), "rte_eventdev_data_%u",
> > +				eventdev->data->dev_id);
> > +		if (ret >= (int)sizeof(mz_name))
> > +			return -EINVAL;
> > +
> > +		mz = rte_memzone_lookup(mz_name);
> > +		if (mz == NULL)
> > +			return -ENOMEM;
> > +
> > +		ret = rte_memzone_free(mz);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> >  	eventdev->data = NULL;
> > -
> >  	return 0;
> >  }
> > 
> > @@ -1186,6 +1206,24 @@ rte_event_pmd_vdev_init(const char *name, size_t dev_private_size,
> >  }
> > 
> >  int
> > +rte_event_pmd_vdev_uninit(const char *name)
> > +{
> > +	struct rte_eventdev *eventdev;
> > +
> > +	if (name == NULL)
> > +		return -EINVAL;
> > +
> > +	eventdev = rte_event_pmd_get_named_dev(name);
> > +	if (eventdev == NULL)
> > +		return -ENODEV;
> > +
> > +	/* Free the event device */
> > +	rte_event_pmd_release(eventdev);
> > +
> > +	return 0;
> > +}
> > +
> > +int
> >  rte_event_pmd_pci_probe(struct rte_pci_driver *pci_drv,
> >  			struct rte_pci_device *pci_dev)
> >  {
> > @@ -1275,9 +1313,6 @@ rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev)
> >  	/* Free event device */
> >  	rte_event_pmd_release(eventdev);
> > 
> > -	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> > -		rte_free(eventdev->data->dev_private);
> 
> 
> 
> >  	eventdev->pci_dev = NULL;
> >  	eventdev->driver = NULL;
> > 
> > diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
> > index 4eea618..be5c784 100644
> > --- a/lib/librte_eventdev/rte_eventdev_pmd.h
> > +++ b/lib/librte_eventdev/rte_eventdev_pmd.h
> > @@ -198,7 +198,7 @@ rte_event_pmd_is_valid_dev(uint8_t dev_id)
> >  {
> >  	struct rte_eventdev *dev;
> > 
> > -	if (dev_id >= rte_eventdev_globals->nb_devs)
> > +	if (dev_id >= RTE_EVENT_MAX_DEVS)
> >  		return 0;
> > 
> >  	dev = &rte_eventdevs[dev_id];
> > @@ -552,6 +552,16 @@ struct rte_eventdev *
> >  rte_event_pmd_vdev_init(const char *name, size_t dev_private_size,
> >  		int socket_id);
> > 
> > +/**
> > + * Destroy the given virtual event device
> > + *
> > + * @param name
> > + *   PMD type name
> > + * @return
> > + *   - 0 on success, negative on error
> > + */
> > +int
> > +rte_event_pmd_vdev_uninit(const char *name);
> > 
> >  /**
> >   * Wrapper for use by pci drivers as a .probe function to attach to a event
> > diff --git a/lib/librte_eventdev/rte_eventdev_version.map
> > b/lib/librte_eventdev/rte_eventdev_version.map
> > index b138eb3..c572c4d 100644
> > --- a/lib/librte_eventdev/rte_eventdev_version.map
> > +++ b/lib/librte_eventdev/rte_eventdev_version.map
> > @@ -35,6 +35,7 @@ DPDK_17.02 {
> 
> 
> Version to 17.05 I think?
> 
> 
> >  	rte_event_pmd_allocate;
> >  	rte_event_pmd_release;
> >  	rte_event_pmd_vdev_init;
> > +	rte_event_pmd_vdev_uninit;
> >  	rte_event_pmd_pci_probe;
> >  	rte_event_pmd_pci_remove;
> > 
> > --
> > 2.5.5
> 

Re: [dpdk-dev] [PATCH 3/4] event/skeleton: add vdev uninit support

[Jerin Jacob]

On Tue, Feb 07, 2017 at 03:11:50PM +0000, Van Haaren, Harry wrote:
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, February 6, 2017 5:24 AM
> > To: dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Eads, Gage
> > <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jerin Jacob
> > <jerin.jacob@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH 3/4] event/skeleton: add vdev uninit support
> > 
> > Removed global index based device name
> > generation as vdev uninit needs the exact driver
> > name used vdev init.
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Applied to dpdk-next-eventdev/master. Thanks.

>