Re: [PATCH v2 1/6] ethdev: fix port data mismatched in multiple process model
From: Thomas Monjalon <hidden>
Date: 2017-01-04 17:34:42
+Cc Sergio (maintainer of the secondary process thing) 2016-12-28 19:02, Yuanhan Liu:
quoted hunk ↗ jump to hunk
--- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c@@ -201,9 +201,6 @@ rte_eth_dev_allocate(const char *name) return NULL; } - if (rte_eth_dev_data == NULL) - rte_eth_dev_data_alloc(); -
It is dangerous to move this to rte_eth_dev_pci_probe. Please keep it here and duplicate it in eth_dev_attach. [...]
+/* + * Attach to a port already registered by the primary process, which + * makes sure that the same device would both have the same port id + * in the primary and secondary process. + */ +static struct rte_eth_dev * +eth_dev_attach(const char *name)
Maybe that the word "secondary" could help to differentiate of the function rte_eth_dev_attach().
+{
+ uint8_t i;
+ struct rte_eth_dev *eth_dev;
+
+ for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
+ if (strcmp(rte_eth_dev_data[i].name, name) == 0)
+ break;
+ }
+ if (i == RTE_MAX_ETHPORTS) {
+ RTE_PMD_DEBUG_TRACE(
+ "device %s is not driven by the primary process\n",
+ name);
+ return NULL;
+ }
+
+ RTE_ASSERT(eth_dev->data->port_id == i);
+
+ eth_dev = &rte_eth_devices[i];
+ eth_dev->data = &rte_eth_dev_data[i];
+ eth_dev->attached = DEV_ATTACHED;
+ nb_ports++;I am a bit nervous when I see these lines duplicated from rte_eth_dev_allocate. Not sure whether it deserves a common function or not. [...]
quoted hunk ↗ jump to hunk
@@ -246,9 +275,26 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv, - eth_dev = rte_eth_dev_allocate(ethdev_name); - if (eth_dev == NULL) - return -ENOMEM; + if (rte_eth_dev_data == NULL) + rte_eth_dev_data_alloc(); + + if (rte_eal_process_type() == RTE_PROC_PRIMARY) { + eth_dev = rte_eth_dev_allocate(ethdev_name); + if (eth_dev == NULL) + return -ENOMEM; + } else { + /* + * if we failed to attach a device, it means that + * device is skipped, due to some errors. Take + * virtio-net device as example, it could be the + * device is managed by virtio-net kernel driver. + * For such case, we return a positive value, to + * let EAL skip it as well. + */
This comment (a bit too long) should be placed between "if" and "return".
+ eth_dev = eth_dev_attach(ethdev_name); + if (eth_dev == NULL) + return 1; + }