[PATCH 0/2] add ensure consistent device data in multiprocess mode

[PATCH 0/2] add ensure consistent device data in multiprocess mode

[Marcin Kerlin]

This patch ensure not overwrite device data in the multiprocess application.

1)Changes in the library introduces continuity in device data rte_eth_dev_data[]
common for to all processes. Functionality detach cleans data of detachable 
device and leaves space for other devices or for the next run app.

2)Changes in application testpmd allow secondary process to attach the mempool
created by primary process rather than create new and in the case of quit or
force quit to free devices of this process from shared array rte_eth_dev_data[].

Marcin Kerlin (2):
  lib/librte_ether: ensure not overwrite device data in multiprocess app
  app/testpmd: fix handling of multiprocess

 app/test-pmd/testpmd.c                 | 30 +++++++++++-
 app/test-pmd/testpmd.h                 |  1 +
 lib/librte_ether/rte_ethdev.c          | 87 ++++++++++++++++++++++++++++++----
 lib/librte_ether/rte_ethdev.h          | 23 +++++++++
 lib/librte_ether/rte_ether_version.map |  8 ++++
 5 files changed, 139 insertions(+), 10 deletions(-)

-- 
1.9.1

[PATCH 1/2] lib/librte_ether: ensure not overwrite device data

[Marcin Kerlin]

Added ensure consistent device data in the multiprocess application. Primary
process creates array rte_eth_dev_data[] and secondary process appends in the
first free place rather than at the beginning. This behavior prevents 
overwriting devices of primary process by secondary process. Two arrays 
rte_eth_dev_data[] and rte_eth_devices[] are shifted relative to each other 
and it required the addition new functions to search separately data.

Signed-off-by: Marcin Kerlin <marcinx.kerlin@intel.com>
---
 lib/librte_ether/rte_ethdev.c          | 87 ++++++++++++++++++++++++++++++----
 lib/librte_ether/rte_ethdev.h          | 23 +++++++++
 lib/librte_ether/rte_ether_version.map |  8 ++++
 3 files changed, 110 insertions(+), 8 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index f62a9ec..a8f89c9 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -177,6 +177,19 @@ rte_eth_dev_allocated(const char *name)
 	return NULL;
 }
 
+struct rte_eth_dev_data *
+rte_eth_dev_data_shared_allocated(const char *name)
+{
+	unsigned i;
+
+	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+		if (strcmp(rte_eth_dev_data[i].name, name) == 0)
+			return &rte_eth_dev_data[i];
+	}
+
+	return NULL;
+}
+
 static uint8_t
 rte_eth_dev_find_free_port(void)
 {
@@ -189,10 +202,43 @@ rte_eth_dev_find_free_port(void)
 	return RTE_MAX_ETHPORTS;
 }
 
+static uint8_t
+rte_eth_dev_find_free_dev_data_id(void)
+{
+	unsigned i;
+
+	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+		if (!strlen(rte_eth_dev_data[i].name))
+			return i;
+	}
+	return RTE_MAX_ETHPORTS;
+}
+
+int
+rte_eth_dev_release_dev_data(uint8_t port_id)
+{
+	char device[RTE_ETH_NAME_MAX_LEN];
+	struct rte_eth_dev_data *eth_dev_data = NULL;
+
+	/* get device name by port id */
+	if (rte_eth_dev_get_name_by_port(port_id, device))
+		return -EINVAL;
+
+	/* look for an entry in the shared device data */
+	eth_dev_data = rte_eth_dev_data_shared_allocated(device);
+	if (eth_dev_data == NULL)
+		return -EINVAL;
+
+	/* clear an entry in the shared device data */
+	memset(eth_dev_data, 0, sizeof(struct rte_eth_dev_data));
+
+	return 0;
+}
+
 struct rte_eth_dev *
 rte_eth_dev_allocate(const char *name, enum rte_eth_dev_type type)
 {
-	uint8_t port_id;
+	uint8_t port_id, dev_data_id;
 	struct rte_eth_dev *eth_dev;
 
 	port_id = rte_eth_dev_find_free_port();
@@ -204,14 +250,28 @@ rte_eth_dev_allocate(const char *name, enum rte_eth_dev_type type)
 	if (rte_eth_dev_data == NULL)
 		rte_eth_dev_data_alloc();
 
+	dev_data_id = rte_eth_dev_find_free_dev_data_id();
+
+	if (dev_data_id == RTE_MAX_ETHPORTS) {
+		RTE_PMD_DEBUG_TRACE("Reached maximum number of Ethernet ports by all "
+				"the processes\n");
+		return NULL;
+	}
+
 	if (rte_eth_dev_allocated(name) != NULL) {
 		RTE_PMD_DEBUG_TRACE("Ethernet Device with name %s already allocated!\n",
 				name);
 		return NULL;
 	}
 
+	if (rte_eth_dev_data_shared_allocated(name) != NULL) {
+		RTE_PMD_DEBUG_TRACE("Ethernet Device with name %s already allocated by "
+				"another process!\n", name);
+		return NULL;
+	}
+
 	eth_dev = &rte_eth_devices[port_id];
-	eth_dev->data = &rte_eth_dev_data[port_id];
+	eth_dev->data = &rte_eth_dev_data[dev_data_id];
 	snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
 	eth_dev->data->port_id = port_id;
 	eth_dev->attached = DEV_ATTACHED;
@@ -237,6 +297,7 @@ rte_eth_dev_create_unique_device_name(char *name, size_t size,
 int
 rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
 {
+
 	if (eth_dev == NULL)
 		return -EINVAL;
 
@@ -418,9 +479,7 @@ rte_eth_dev_get_name_by_port(uint8_t port_id, char *name)
 		return -EINVAL;
 	}
 
-	/* shouldn't check 'rte_eth_devices[i].data',
-	 * because it might be overwritten by VDEV PMD */
-	tmp = rte_eth_dev_data[port_id].name;
+	tmp = rte_eth_devices[port_id].data->name;
 	strcpy(name, tmp);
 	return 0;
 }
@@ -439,9 +498,7 @@ rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id)
 
 	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
 
-		if (!strncmp(name,
-			rte_eth_dev_data[i].name, strlen(name))) {
-
+		if (!strncmp(name, rte_eth_devices[i].data->name, strlen(name))) {
 			*port_id = i;
 
 			return 0;
@@ -632,6 +689,8 @@ int
 rte_eth_dev_detach(uint8_t port_id, char *name)
 {
 	struct rte_pci_addr addr;
+	struct rte_eth_dev_data *eth_dev_data = NULL;
+	char device[RTE_ETH_NAME_MAX_LEN];
 	int ret = -1;
 
 	if (name == NULL) {
@@ -643,6 +702,15 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
 	if (rte_eth_dev_is_detachable(port_id))
 		goto err;
 
+	/* get device name by port id */
+	if (rte_eth_dev_get_name_by_port(port_id, device))
+		goto err;
+
+	/* look for an entry in the shared device data */
+	eth_dev_data = rte_eth_dev_data_shared_allocated(device);
+	if (eth_dev_data == NULL)
+		goto err;
+
 	if (rte_eth_dev_get_device_type(port_id) == RTE_ETH_DEV_PCI) {
 		ret = rte_eth_dev_get_addr_by_port(port_id, &addr);
 		if (ret < 0)
@@ -662,6 +730,9 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
 			goto err;
 	}
 
+	/* clear an entry in the shared device data */
+	memset(eth_dev_data, 0, sizeof(struct rte_eth_dev_data));
+
 	return 0;
 
 err:
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index b0fe033..9de231b 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1748,6 +1748,29 @@ struct rte_eth_dev *rte_eth_dev_allocated(const char *name);
 
 /**
  * @internal
+ * Returns a shared ethdev slot specified by the unique identifier name.
+ *
+ * @param	name
+ *  The pointer to the Unique identifier name for each shared Ethernet device
+ *  between multiple processes.
+ * @return
+ *   - The pointer to the shared ethdev slot, on success. NULL on error
+ */
+struct rte_eth_dev_data *rte_eth_dev_data_shared_allocated(const char *name);
+
+/**
+ * @internal
+ * Release device data kept in a common memory for all processes.
+ *
+ * @param	port_id
+ *   The port identifier of the device to release device data.
+ * @return
+ *   - 0 on success, negative on error
+ */
+int rte_eth_dev_release_dev_data(uint8_t port_id);
+
+/**
+ * @internal
  * Allocates a new ethdev slot for an ethernet device and returns the pointer
  * to that slot for the driver to use.
  *
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 45ddf44..25ae80d 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -139,3 +139,10 @@ DPDK_16.07 {
 	rte_eth_dev_get_port_by_name;
 	rte_eth_xstats_get_names;
 } DPDK_16.04;
+
+DPDK_16.11 {
+	global:
+
+	rte_eth_dev_data_shared_allocated;
+	rte_eth_dev_release_dev_data;
+} DPDK_16.07
-- 
1.9.1

[PATCH 2/2] app/testpmd: fix handling of multiprocess

[Marcin Kerlin]

Added lookup for pool name because secondary process should attach to
mempool created by primary process rather than create new.

Added function free_mp_shared_dev_data() which causes that if secondary
process quit or force quit then detach own devices from common array
rte_eth_dev_data[] for all processes. This allows to have a up-to-date
list of devices and run again secondary application with the same name.

Signed-off-by: Marcin Kerlin <marcinx.kerlin@intel.com>
---
 app/test-pmd/testpmd.c | 30 ++++++++++++++++++++++++++++--
 app/test-pmd/testpmd.h |  1 +
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1428974..2fa33d0 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -453,8 +453,10 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			rte_mempool_obj_iter(rte_mp, rte_pktmbuf_init, NULL);
 		} else {
 			/* wrapper to rte_mempool_create() */
-			rte_mp = rte_pktmbuf_pool_create(pool_name, nb_mbuf,
-				mb_mempool_cache, 0, mbuf_seg_size, socket_id);
+			rte_mp = rte_mempool_lookup(pool_name);
+			if (rte_mp == NULL)
+				rte_mp = rte_pktmbuf_pool_create(pool_name, nb_mbuf,
+						mb_mempool_cache, 0, mbuf_seg_size, socket_id);
 		}
 	}
 
@@ -1610,6 +1612,29 @@ detach_port(uint8_t port_id)
 	return;
 }
 
+void free_mp_shared_dev_data(portid_t pid)
+{
+	portid_t pi;
+
+	if (port_id_is_invalid(pid, ENABLED_WARN))
+		return;
+
+	/* free data only if the secondary process exits */
+	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
+		return;
+
+	FOREACH_PORT(pi, ports) {
+		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+			continue;
+
+		if (!port_is_closed(pi))
+			return;
+
+		if (rte_eth_dev_release_dev_data(pi) < 0)
+			return;
+	}
+}
+
 void
 pmd_test_exit(void)
 {
@@ -1625,6 +1650,7 @@ pmd_test_exit(void)
 			fflush(stdout);
 			stop_port(pt_id);
 			close_port(pt_id);
+			free_mp_shared_dev_data(pt_id);
 		}
 	}
 	printf("\nBye...\n");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2b281cc..63f82f7 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -553,6 +553,7 @@ void attach_port(char *identifier);
 void detach_port(uint8_t port_id);
 int all_ports_stopped(void);
 int port_is_started(portid_t port_id);
+void free_mp_shared_dev_data(portid_t pid);
 void pmd_test_exit(void);
 void fdir_get_infos(portid_t port_id);
 void fdir_set_flex_mask(portid_t port_id,
-- 
1.9.1

Re: [PATCH 0/2] add ensure consistent device data in multiprocess mode

[Kerlin, MarcinX]

Self-Nack this patch because the commit log needs change.

> -----Original Message-----
> From: Kerlin, MarcinX
> Sent: Friday, July 29, 2016 5:57 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> thomas.monjalon@6wind.com; Kerlin, MarcinX <marcinx.kerlin@intel.com>
> Subject: [PATCH 0/2] add ensure consistent device data in multiprocess mode
> 
> This patch ensure not overwrite device data in the multiprocess application.
> 
> 1)Changes in the library introduces continuity in device data
> rte_eth_dev_data[] common for to all processes. Functionality detach cleans
> data of detachable device and leaves space for other devices or for the next
> run app.
> 
> 2)Changes in application testpmd allow secondary process to attach the
> mempool created by primary process rather than create new and in the case of
> quit or force quit to free devices of this process from shared array
> rte_eth_dev_data[].
> 
> Marcin Kerlin (2):
>   lib/librte_ether: ensure not overwrite device data in multiprocess app
>   app/testpmd: fix handling of multiprocess
> 
>  app/test-pmd/testpmd.c                 | 30 +++++++++++-
>  app/test-pmd/testpmd.h                 |  1 +
>  lib/librte_ether/rte_ethdev.c          | 87 ++++++++++++++++++++++++++++++---
> -
>  lib/librte_ether/rte_ethdev.h          | 23 +++++++++
>  lib/librte_ether/rte_ether_version.map |  8 ++++
>  5 files changed, 139 insertions(+), 10 deletions(-)
> 
> --
> 1.9.1