DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Tootoonchian, Amin" <amin.tootoonchian@intel.com>
To: "thomas.monjalon@6wind.com" <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: [dpdk-dev] [PATCH] ethdev: ensure consistent port id assignment
Date: Tue, 12 Jul 2016 02:01:17 +0000	[thread overview]
Message-ID: <5905C8E33883CA46A8878E2D7724E21516DF5D@ORSMSX109.amr.corp.intel.com> (raw)

The rte_eth_dev_allocate() code has an implicit assumption that the port
id assignment in the secondary process is consistent with that of the
primary. The current code breaks if the enumeration of ethdevs in
primary and secondary processes are not identical (e.g., when the
black/whitelist and vdev args of the primary and secondary do not match,
or when the primary dynamically adds/removes ethdevs).

To fix this problem, rte_eth_dev_allocate() now looks up port id by name
in a secondary process (making it explicit that ethdevs can only be
created and initialized by the primary process). Upon releasing a port,
the primary process zeros out eth_dev->data to avoid false positives in
port id lookup by rte_eth_dev_get_port_by_name().

Signed-off-by: Amin Tootoonchian <amin.tootoonchian@intel.com>
---
 lib/librte_ether/rte_ethdev.c | 44 +++++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 0a6e3f1..1801f57 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -195,25 +195,37 @@ rte_eth_dev_allocate(const char *name, enum rte_eth_dev_type type)
 	uint8_t port_id;
 	struct rte_eth_dev *eth_dev;
 
-	port_id = rte_eth_dev_find_free_port();
-	if (port_id == RTE_MAX_ETHPORTS) {
-		RTE_PMD_DEBUG_TRACE("Reached maximum number of Ethernet ports\n");
-		return NULL;
-	}
-
 	if (rte_eth_dev_data == NULL)
 		rte_eth_dev_data_alloc();
 
-	if (rte_eth_dev_allocated(name) != NULL) {
-		RTE_PMD_DEBUG_TRACE("Ethernet Device with name %s already allocated!\n",
-				name);
-		return NULL;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		port_id = rte_eth_dev_find_free_port();
+		if (port_id == RTE_MAX_ETHPORTS) {
+			RTE_PMD_DEBUG_TRACE("Reached maximum number of Ethernet ports\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;
+		}
+	} else {
+		if (rte_eth_dev_get_port_by_name(name, &port_id) != 0) {
+			RTE_PMD_DEBUG_TRACE("Ethernet Device with name %s could not be found!\n",
+					name);
+			return NULL;
+		}
 	}
 
 	eth_dev = &rte_eth_devices[port_id];
 	eth_dev->data = &rte_eth_dev_data[port_id];
-	snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
-	eth_dev->data->port_id = port_id;
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
+		eth_dev->data->port_id = port_id;
+	}
+
 	eth_dev->attached = DEV_ATTACHED;
 	eth_dev->dev_type = type;
 	nb_ports++;
@@ -293,8 +305,10 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv,
 			pci_drv->name,
 			(unsigned) pci_dev->id.vendor_id,
 			(unsigned) pci_dev->id.device_id);
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		rte_free(eth_dev->data->dev_private);
+		memset(eth_dev->data, 0, sizeof(*rte_eth_dev_data));
+	}
 	rte_eth_dev_release_port(eth_dev);
 	return diag;
 }
@@ -330,8 +344,10 @@ rte_eth_dev_uninit(struct rte_pci_device *pci_dev)
 	/* free ether device */
 	rte_eth_dev_release_port(eth_dev);
 
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		rte_free(eth_dev->data->dev_private);
+		memset(eth_dev->data, 0, sizeof(*rte_eth_dev_data));
+	}
 
 	eth_dev->pci_dev = NULL;
 	eth_dev->driver = NULL;
-- 
2.8.1

             reply	other threads:[~2016-07-12  2:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-12  2:01 Tootoonchian, Amin [this message]
2016-07-20  8:51 ` Kerlin, MarcinX
2016-07-20 15:07   ` Tootoonchian, Amin
2016-07-20 15:11     ` Thomas Monjalon
2016-07-20 17:25       ` Tootoonchian, Amin
2016-08-24 22:17       ` Tootoonchian, Amin
2017-09-04 14:53         ` Sergio Gonzalez Monroy
2018-12-21 15:30           ` Ferruh Yigit
2016-07-21 13:54     ` Kerlin, MarcinX
2016-07-22 19:56       ` Tootoonchian, Amin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5905C8E33883CA46A8878E2D7724E21516DF5D@ORSMSX109.amr.corp.intel.com \
    --to=amin.tootoonchian@intel.com \
    --cc=dev@dpdk.org \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).