[dpdk-stable] [PATCH 1/3] net/virtio: fix crash for secondary process

[dpdk-stable] [PATCH 1/3] net/virtio: fix crash for secondary process

[Yuanhan Liu]

If a virtio net device is managed by kernel driver, the primary
process would be aware of it and skip it.

For secondary process, however, it isn't aware of that. Instead,
the 'hw' would be NULL. If we keep going on with the rx_func_get(),
a crash would happen. For such case, we simply return 1 to let
EAL skip this device. Thus, the crashed could be fixed.

Fixes: c1f86306a026 ("virtio: add new driver")

Cc: stable@dpdk.org
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 079fd6c..f2a803b 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1304,6 +1304,15 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
 
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		/*
+		 * if a virtio net device is managed by kernel driver,
+		 * the secondary process won't be aware of it. Instead,
+		 * it sees hw being set with NULL. For such case, here
+		 * we returns 1 to let EAL skip this device.
+		 */
+		if (!hw)
+			return 1;
+
 		rx_func_get(eth_dev);
 		return 0;
 	}
-- 
1.9.0

[dpdk-stable] [PATCH 2/3] net/virtio: fix multiple process support

[Yuanhan Liu]

The introduce of virtio 1.0 support brings yet another set of ops, badly,
it's not handled correctly, that it breaks the multiple process support.

The issue is the function pointer may vary from different processes, and
the current code just does one time set (for primary process only). That
said, the function pointer the secondary process saw is actually from the
primary process space. Accessing it could likely result to a crash.

The fix is somehow straightforward (at least, way more simpler than what
I have firstly thought): just reset the function pointers for the secondary
process on init stage.

Fixes: d5bbeefca826 ("virtio: introduce PCI implementation structure")

Cc: stable@dpdk.org
Reported-by: Yaron Illouz <yaroni@radcom.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 drivers/net/virtio/virtio_ethdev.c      | 12 ++++++++++++
 drivers/net/virtio/virtio_pci.c         |  4 ++--
 drivers/net/virtio/virtio_pci.h         |  4 ++++
 drivers/net/virtio/virtio_user_ethdev.c |  2 +-
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index f2a803b..a9f7ae4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1174,6 +1174,17 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 }
 
 static void
+set_vtpci_ops(struct rte_eth_dev *eth_dev, struct virtio_hw *hw)
+{
+	if (eth_dev->pci_dev == NULL)
+		hw->vtpci_ops = &virtio_user_ops;
+	else if (hw->modern)
+		hw->vtpci_ops = &modern_ops;
+	else
+		hw->vtpci_ops = &legacy_ops;
+}
+
+static void
 rx_func_get(struct rte_eth_dev *eth_dev)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
@@ -1313,6 +1324,7 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 		if (!hw)
 			return 1;
 
+		set_vtpci_ops(eth_dev, hw);
 		rx_func_get(eth_dev);
 		return 0;
 	}
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index 9b47165..e9e53a5 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -300,7 +300,7 @@
 	return 0;
 }
 
-static const struct virtio_pci_ops legacy_ops = {
+const struct virtio_pci_ops legacy_ops = {
 	.read_dev_cfg	= legacy_read_dev_config,
 	.write_dev_cfg	= legacy_write_dev_config,
 	.reset		= legacy_reset,
@@ -516,7 +516,7 @@
 	io_write16(1, vq->notify_addr);
 }
 
-static const struct virtio_pci_ops modern_ops = {
+const struct virtio_pci_ops modern_ops = {
 	.read_dev_cfg	= modern_read_dev_config,
 	.write_dev_cfg	= modern_write_dev_config,
 	.reset		= modern_reset,
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index de271bf..3e1e911 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -317,4 +317,8 @@ int vtpci_init(struct rte_pci_device *, struct virtio_hw *,
 
 uint16_t vtpci_irq_config(struct virtio_hw *, uint16_t);
 
+extern const struct virtio_pci_ops legacy_ops;
+extern const struct virtio_pci_ops modern_ops;
+extern const struct virtio_pci_ops virtio_user_ops;
+
 #endif /* _VIRTIO_PCI_H_ */
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 406beea..26eca37 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -212,7 +212,7 @@
 			    strerror(errno));
 }
 
-static const struct virtio_pci_ops virtio_user_ops = {
+const struct virtio_pci_ops virtio_user_ops = {
 	.read_dev_cfg	= virtio_user_read_dev_config,
 	.write_dev_cfg	= virtio_user_write_dev_config,
 	.reset		= virtio_user_reset,
-- 
1.9.0

[dpdk-stable] [PATCH 3/3] net/virtio: fix wrong Rx/Tx method for secondary process

[Yuanhan Liu]

If the primary enables the vector Rx/Tx path, the current code would
let the secondary always choose the non vector Rx/Tx path. This results
to a Rx/Tx method mismatch between primary and secondary process. Werid
errors then may happen, something like:

    PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14

Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
That is, use vector path if it's given.

Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")

Cc: stable@dpdk.org
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index a9f7ae4..04d93de 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1325,7 +1325,11 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 			return 1;
 
 		set_vtpci_ops(eth_dev, hw);
-		rx_func_get(eth_dev);
+		if (hw->use_simple_rxtx) {
+			eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple;
+			eth_dev->rx_pkt_burst = virtio_recv_pkts_vec;
+		} else
+			rx_func_get(eth_dev);
 		return 0;
 	}
 
-- 
1.9.0

[dpdk-stable] [PATCH v2 1/6] ethdev: fix port data mismatched in multiple process model

[Yuanhan Liu]

Assume we have two virtio ports, 00:03.0 and 00:04.0. The first one is
managed by the kernel driver, while the later one is managed by DPDK.

Now we start the primary process. 00:03.0 will be skipped by DPDK virtio
PMD driver (since it's being used by the kernel). 00:04.0 would be
successfully initiated by DPDK virtio PMD (if nothing abnormal happens).
After that, we would get a port id 0, and all the related info needed
by virtio (virtio_hw) is stored at rte_eth_dev_data[0].

Then we start the secondary process. As usual, 00:03.0 will be firstly
probed. It firstly tries to get a local eth_dev structure for it (by
rte_eth_dev_allocate):

        port_id = rte_eth_dev_find_free_port();
        ...

        eth_dev = &rte_eth_devices[port_id];
        eth_dev->data = &rte_eth_dev_data[port_id];
        ...

        return eth_dev;

Since it's a first PCI device, port_id will be 0. eth_dev->data would
then point to rte_eth_dev_data[0]. And here things start going wrong,
as rte_eth_dev_data[0] actually stores the virtio_hw for 00:04.0.

That said, in the secondary process, DPDK will continue to drive PCI
device 00.03.0 (despite the fact it's been managed by kernel), with
the info from PCI device 00:04.0. Which is wrong.

The fix is to attach the port already registered by the primary process:
iterate the rte_eth_dev_data[], and get the port id who's PCI ID matches
the current PCI device.

This would let us maintain same port ID for the same PCI device, keeping
the chance of referencing to wrong data minimal.

Fixes: af75078fece3 ("first public release")

Cc: stable@dpdk.org
Cc: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_ether/rte_ethdev.c | 58 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..c10eb9c 100644
--- 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();
-
 	if (rte_eth_dev_allocated(name) != NULL) {
 		RTE_PMD_DEBUG_TRACE("Ethernet Device with name %s already allocated!\n",
 				name);
@@ -231,6 +228,38 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
+/*
+ * 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)
+{
+	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++;
+
+	return eth_dev;
+}
+
 int
 rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
 		      struct rte_pci_device *pci_dev)
@@ -246,9 +275,26 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
 	rte_eal_pci_device_name(&pci_dev->addr, ethdev_name,
 			sizeof(ethdev_name));
 
-	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.
+		 */
+		eth_dev = eth_dev_attach(ethdev_name);
+		if (eth_dev == NULL)
+			return 1;
+	}
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		eth_dev->data->dev_private = rte_zmalloc("ethdev private structure",
-- 
2.8.1

[dpdk-stable] [PATCH v2 2/6] net/virtio: fix wrong Rx/Tx method for secondary process

[Yuanhan Liu]

If the primary enables the vector Rx/Tx path, the current code would
let the secondary always choose the non vector Rx/Tx path. This results
to a Rx/Tx method mismatch between primary and secondary process. Werid
errors then may happen, something like:

    PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14

Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
That is, use vector path if it's given.

Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")

Cc: stable@dpdk.org
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---

v2: fix a checkpatch warning: use {} consistently
---
 drivers/net/virtio/virtio_ethdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 079fd6c..ef37ad1 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1304,7 +1304,12 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
 
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
-		rx_func_get(eth_dev);
+		if (hw->use_simple_rxtx) {
+			eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple;
+			eth_dev->rx_pkt_burst = virtio_recv_pkts_vec;
+		} else {
+			rx_func_get(eth_dev);
+		}
 		return 0;
 	}
 
-- 
2.8.1

Re: [dpdk-stable] [PATCH v2 5/6] net/virtio: fix multiple process support

[Yuanhan Liu]

On Wed, Dec 28, 2016 at 07:02:41PM +0800, Yuanhan Liu wrote:
...
> Cc: stable@kernel.org

I knew I would make a mistake like this some day :/

Not my first time typing wrong, but it's the first time sending it out.
Sorry for that. Fixed in this reply.

	--yliu

[dpdk-stable] [PATCH v3 1/6] ethdev: fix port data mismatched in multiple process model

[Yuanhan Liu]

Assume we have two virtio ports, 00:03.0 and 00:04.0. The first one is
managed by the kernel driver, while the later one is managed by DPDK.

Now we start the primary process. 00:03.0 will be skipped by DPDK virtio
PMD driver (since it's being used by the kernel). 00:04.0 would be
successfully initiated by DPDK virtio PMD (if nothing abnormal happens).
After that, we would get a port id 0, and all the related info needed
by virtio (virtio_hw) is stored at rte_eth_dev_data[0].

Then we start the secondary process. As usual, 00:03.0 will be firstly
probed. It firstly tries to get a local eth_dev structure for it (by
rte_eth_dev_allocate):

        port_id = rte_eth_dev_find_free_port();
        ...

        eth_dev = &rte_eth_devices[port_id];
        eth_dev->data = &rte_eth_dev_data[port_id];
        ...

        return eth_dev;

Since it's a first PCI device, port_id will be 0. eth_dev->data would
then point to rte_eth_dev_data[0]. And here things start going wrong,
as rte_eth_dev_data[0] actually stores the virtio_hw for 00:04.0.

That said, in the secondary process, DPDK will continue to drive PCI
device 00.03.0 (despite the fact it's been managed by kernel), with
the info from PCI device 00:04.0. Which is wrong.

The fix is to attach the port already registered by the primary process:
iterate the rte_eth_dev_data[], and get the port id who's PCI ID matches
the current PCI device.

This would let us maintain same port ID for the same PCI device, keeping
the chance of referencing to wrong data minimal.

Fixes: af75078fece3 ("first public release")

Cc: stable@dpdk.org
Cc: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---

v3: - do not move rte_eth_dev_data_alloc to pci_probe
    - rename eth_dev_attach to eth_dev_attach_secondary
    - introduce eth_dev_init() for common eth_dev struct initiation
    - move comment block inside the "if" block
---
 lib/librte_ether/rte_ethdev.c | 77 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 68 insertions(+), 9 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..c3e65f1 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -189,6 +189,21 @@ struct rte_eth_dev *
 	return RTE_MAX_ETHPORTS;
 }
 
+static void
+eth_dev_init(struct rte_eth_dev *eth_dev, uint8_t port_id, const char *name)
+{
+	eth_dev->data = &rte_eth_dev_data[port_id];
+	eth_dev->attached = DEV_ATTACHED;
+	eth_dev_last_created_port = port_id;
+	nb_ports++;
+
+	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;
+	}
+}
+
 struct rte_eth_dev *
 rte_eth_dev_allocate(const char *name)
 {
@@ -211,12 +226,41 @@ struct rte_eth_dev *
 	}
 
 	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;
-	eth_dev->attached = DEV_ATTACHED;
-	eth_dev_last_created_port = port_id;
-	nb_ports++;
+	eth_dev_init(eth_dev, port_id, name);
+
+	return eth_dev;
+}
+
+/*
+ * Attach to a port already registered by the primary process, which
+ * makes sure that the same device would have the same port id both
+ * in the primary and secondary process.
+ */
+static struct rte_eth_dev *
+eth_dev_attach_secondary(const char *name)
+{
+	uint8_t i;
+	struct rte_eth_dev *eth_dev;
+
+	if (rte_eth_dev_data == NULL)
+		rte_eth_dev_data_alloc();
+
+	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_init(eth_dev, i, NULL);
+
 	return eth_dev;
 }
 
@@ -246,9 +290,24 @@ struct rte_eth_dev *
 	rte_eal_pci_device_name(&pci_dev->addr, ethdev_name,
 			sizeof(ethdev_name));
 
-	eth_dev = rte_eth_dev_allocate(ethdev_name);
-	if (eth_dev == NULL)
-		return -ENOMEM;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		eth_dev = rte_eth_dev_allocate(ethdev_name);
+		if (eth_dev == NULL)
+			return -ENOMEM;
+	} else {
+		eth_dev = eth_dev_attach_secondary(ethdev_name);
+		if (eth_dev == NULL) {
+			/*
+			 * if we failed to attach a device, it means
+			 * the device is skipped, due to some errors.
+			 * Take virtio-net device as example, it could
+			 * due to the device is managed by virtio-net
+			 * kernel driver.  For such case, we return a
+			 * positive value, to let EAL skip it as well.
+			 */
+			return 1;
+		}
+	}
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		eth_dev->data->dev_private = rte_zmalloc("ethdev private structure",
-- 
1.9.0

[dpdk-stable] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process

[Yuanhan Liu]

If the primary enables the vector Rx/Tx path, the current code would
let the secondary always choose the non vector Rx/Tx path. This results
to a Rx/Tx method mismatch between primary and secondary process. Werid
errors then may happen, something like:

    PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14

Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
That is, use vector path if it's given.

Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")

Cc: stable@dpdk.org
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---

v2: fix a checkpatch warning: use {} consistently
---
 drivers/net/virtio/virtio_ethdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 079fd6c..ef37ad1 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1304,7 +1304,12 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
 
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
-		rx_func_get(eth_dev);
+		if (hw->use_simple_rxtx) {
+			eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple;
+			eth_dev->rx_pkt_burst = virtio_recv_pkts_vec;
+		} else {
+			rx_func_get(eth_dev);
+		}
 		return 0;
 	}
 
-- 
1.9.0

[dpdk-stable] [PATCH v3 5/6] net/virtio: fix multiple process support

[Yuanhan Liu]

The introduce of virtio 1.0 support brings yet another set of ops, badly,
it's not handled correctly, that it breaks the multiple process support.

The issue is the data/function pointer may vary from different processes,
and the old used to do one time set (for primary process only). That
said, the function pointer the secondary process saw is actually from the
primary process space. Accessing it could likely result to a crash.

Kudos to the last patches, we now be able to maintain those info that may
vary among different process locally, meaning every process could have its
own copy for each of them, with the correct value set. And this is what
this patch does:

- remap the PCI (IO port for legacy device and memory map for modern
  device)

- set vtpci_ops correctly

After that, multiple process would work like a charm. (At least, it
passed my fuzzy test)

Fixes: b8f04520ad71 ("virtio: use PCI ioport API")
Fixes: d5bbeefca826 ("virtio: introduce PCI implementation structure")
Fixes: 6ba1f63b5ab0 ("virtio: support specification 1.0")

Cc: stable@dpdk.org
Cc: Juho Snellman <jsnell@iki.fi>
Cc: Yaron Illouz <yaroni@radcom.com>
Reported-by: Juho Snellman <jsnell@iki.fi>
Reported-by: Yaron Illouz <yaroni@radcom.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---

v3: - update release note and nic matrix

v2: - fixed PCI remap, so that virtio 1.0 also works
---
 doc/guides/nics/features/virtio.ini     |  1 +
 doc/guides/rel_notes/release_17_02.rst  |  5 ++++
 drivers/net/virtio/virtio_ethdev.c      | 53 +++++++++++++++++++++++++++++++--
 drivers/net/virtio/virtio_pci.c         |  4 +--
 drivers/net/virtio/virtio_pci.h         |  4 +++
 drivers/net/virtio/virtio_user_ethdev.c |  2 +-
 6 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
index 41830c1..f5de291 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -22,3 +22,4 @@ ARMv8                = Y
 x86-32               = Y
 x86-64               = Y
 Usage doc            = Y
+Multiprocess aware   = Y
diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 3b65038..a4260de 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -54,6 +54,11 @@ Resolved Issues
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+   * **fixed virtio multiple process support.**
+
+     Fixed few regressions introduced in recent releases that break the virtio
+     multiple process support.
+
 
 EAL
 ~~~
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 5567aa2..19d4348 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1289,6 +1289,49 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 }
 
 /*
+ * Remap the PCI device again (IO port map for legacy device and
+ * memory map for modern device), so that the secondary process
+ * could have the PCI initiated correctly.
+ */
+static int
+virtio_remap_pci(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
+{
+	if (hw->modern) {
+		/*
+		 * We don't have to re-parse the PCI config space, since
+		 * rte_eal_pci_map_device() makes sure the mapped address
+		 * in secondary process would equal to the one mapped in
+		 * the primary process: error will be returned if that
+		 * requirement is not met.
+		 *
+		 * That said, we could simply reuse all cap pointers
+		 * (such as dev_cfg, common_cfg, etc.) parsed from the
+		 * primary process, which is stored in shared memory.
+		 */
+		if (rte_eal_pci_map_device(pci_dev)) {
+			PMD_INIT_LOG(DEBUG, "failed to map pci device!");
+			return -1;
+		}
+	} else {
+		if (rte_eal_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0)
+			return -1;
+	}
+
+	return 0;
+}
+
+static void
+virtio_set_vtpci_ops(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
+{
+	if (pci_dev == NULL)
+		VTPCI_OPS(hw) = &virtio_user_ops;
+	else if (hw->modern)
+		VTPCI_OPS(hw) = &modern_ops;
+	else
+		VTPCI_OPS(hw) = &legacy_ops;
+}
+
+/*
  * This function is based on probe() function in virtio_pci.c
  * It returns 0 on success.
  */
@@ -1296,7 +1339,7 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 {
 	struct virtio_hw *hw = eth_dev->data->dev_private;
-	struct rte_pci_device *pci_dev;
+	struct rte_pci_device *pci_dev = eth_dev->pci_dev;
 	uint32_t dev_flags = RTE_ETH_DEV_DETACHABLE;
 	int ret;
 
@@ -1306,6 +1349,13 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 	eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
 
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		if (pci_dev) {
+			ret = virtio_remap_pci(pci_dev, hw);
+			if (ret)
+				return ret;
+		}
+
+		virtio_set_vtpci_ops(pci_dev, hw);
 		if (hw->use_simple_rxtx) {
 			eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple;
 			eth_dev->rx_pkt_burst = virtio_recv_pkts_vec;
@@ -1325,7 +1375,6 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
 	}
 
 	hw->port_id = eth_dev->data->port_id;
-	pci_dev = eth_dev->pci_dev;
 
 	if (pci_dev) {
 		ret = vtpci_init(pci_dev, hw, &dev_flags);
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index d1e9c05..f5754e5 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -303,7 +303,7 @@
 	return 0;
 }
 
-static const struct virtio_pci_ops legacy_ops = {
+const struct virtio_pci_ops legacy_ops = {
 	.read_dev_cfg	= legacy_read_dev_config,
 	.write_dev_cfg	= legacy_write_dev_config,
 	.reset		= legacy_reset,
@@ -519,7 +519,7 @@
 	io_write16(1, vq->notify_addr);
 }
 
-static const struct virtio_pci_ops modern_ops = {
+const struct virtio_pci_ops modern_ops = {
 	.read_dev_cfg	= modern_read_dev_config,
 	.write_dev_cfg	= modern_write_dev_config,
 	.reset		= modern_reset,
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 6b9aecf..511a1c8 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -333,4 +333,8 @@ int vtpci_init(struct rte_pci_device *, struct virtio_hw *,
 
 uint16_t vtpci_irq_config(struct virtio_hw *, uint16_t);
 
+extern const struct virtio_pci_ops legacy_ops;
+extern const struct virtio_pci_ops modern_ops;
+extern const struct virtio_pci_ops virtio_user_ops;
+
 #endif /* _VIRTIO_PCI_H_ */
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 7d2a9d9..3563952 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -212,7 +212,7 @@
 			    strerror(errno));
 }
 
-static const struct virtio_pci_ops virtio_user_ops = {
+const struct virtio_pci_ops virtio_user_ops = {
 	.read_dev_cfg	= virtio_user_read_dev_config,
 	.write_dev_cfg	= virtio_user_write_dev_config,
 	.reset		= virtio_user_reset,
-- 
1.9.0

Re: [dpdk-stable] [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process

[Stephen Hemminger]

On Fri,  6 Jan 2017 18:16:16 +0800
Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:

> If the primary enables the vector Rx/Tx path, the current code would
> let the secondary always choose the non vector Rx/Tx path. This results
> to a Rx/Tx method mismatch between primary and secondary process. Werid
> errors then may happen, something like:
> 
>     PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14
> 
> Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
> That is, use vector path if it's given.
> 
> Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")
> 
> Cc: stable@dpdk.org
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

This is failing on intel compile tests.


http://dpdk.org/patch/18975

_Compilation issues_

Submitter: Yuanhan Liu <yuanhan.liu at linux.intel.com>
Date: Fri,  6 Jan 2017 18:16:18 +0800
DPDK git baseline: Repo:dpdk-next-virtio, Branch:master, CommitID:2b2669fc4c792f9a3ab73490bb93f7810a71c089

Patch18974-18975 --> compile error
Build Summary: 18 Builds Done, 0 Successful, 18 Failures

Test environment and configuration as below:
OS: RHEL7.2_64
    Kernel Version:3.10.0-327.el7.x86_64
    CPU info:Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
    GCC Version:gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-4)
    Clang Version:3.4.2
    i686-native-linuxapp-gcc
    x86_64-native-linuxapp-gcc
    x86_64-native-linuxapp-gcc-shared
OS: FreeBSD10.3_64
    Kernel Version:10.3-RELEASE
    CPU info: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz (2194.97-MHz K8-class CPU)
    GCC Version:gcc (FreeBSD Ports Collection) 4.8.5
    Clang Version:3.4.1
    x86_64-native-bsdapp-clang
    x86_64-native-bsdapp-gcc
OS: FC24_64
    Kernel Version:4.8.6-201.fc24.x86_64
    CPU info:Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz
    GCC Version:gcc (GCC) 6.2.1 20160916 (Red Hat 6.2.1-2)
    Clang Version:3.8.0
    x86_64-native-linuxapp-gcc-debug
    i686-native-linuxapp-gcc
    x86_64-native-linuxapp-gcc
    x86_64-native-linuxapp-gcc-shared
    x86_64-native-linuxapp-clang
OS: UB1604_64
    Kernel Version:4.4.0-53-generic
    CPU info:Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz
    GCC Version:gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
    Clang Version:3.8.0
    i686-native-linuxapp-gcc
    x86_64-native-linuxapp-gcc
    x86_64-native-linuxapp-gcc-shared
    x86_64-native-linuxapp-clang
OS: CentOS7_64
    Kernel Version:3.10.0-327.el7.x86_64
    CPU info:Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz
    GCC Version:gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-4)
    Clang Version:3.4.2
    i686-native-linuxapp-gcc
    x86_64-native-linuxapp-clang
    x86_64-native-linuxapp-gcc-shared
    x86_64-native-linuxapp-gcc

Failed Build #1:
OS: RHEL7.2_64
Target: i686-native-linuxapp-gcc
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/i686-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x930): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status


Failed Build #2:
OS: RHEL7.2_64
Target: x86_64-native-linuxapp-gcc
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x9c4): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status


Failed Build #3:
OS: RHEL7.2_64
Target: x86_64-native-linuxapp-gcc-shared

/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c: In function ‘rte_eth_dev_pci_probe’:
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:3: warning: implicit declaration of function ‘eth_dev_attach_secondary’ [-Wimplicit-function-declaration]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
   ^
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:3: warning: nested extern declaration of ‘eth_dev_attach_secondary’ [-Wnested-externs]
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:11: warning: assignment makes pointer from integer without a cast [enabled by default]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
           ^
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c: At top level:
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:193:1: warning: ‘eth_dev_init’ defined but not used [-Wunused-function]
 eth_dev_init(struct rte_eth_dev *eth_dev, uint8_t port_id, const char *name)
 ^  LD librte_ethdev.so.5.1
rte_ethdev.o: In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0xa37): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status


Failed Build #4:
OS: FreeBSD10.3_64
Target: x86_64-native-bsdapp-clang
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-bsdapp-clang/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:(.text+0x295): undefined reference to `eth_dev_attach_secondary'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
/home/patchWorkOrg/compilation/mk/rte.app.mk:231: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'app' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #5:
OS: FreeBSD10.3_64
Target: x86_64-native-bsdapp-gcc
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-bsdapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x9c4): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status
/home/patchWorkOrg/compilation/mk/rte.app.mk:231: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'app' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #6:
OS: FC24_64
Target: x86_64-native-linuxapp-gcc-debug

MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0xba6): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status
/home/patchWorkOrg/compilation/mk/rte.app.mk:231: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'app' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #7:
OS: FC24_64
Target: i686-native-linuxapp-gcc
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/i686-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x89d): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status
/home/patchWorkOrg/compilation/mk/rte.app.mk:231: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'app' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #8:
OS: FC24_64
Target: x86_64-native-linuxapp-gcc
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x916): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status
/home/patchWorkOrg/compilation/mk/rte.app.mk:231: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'app' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #9:
OS: FC24_64
Target: x86_64-native-linuxapp-gcc-shared

/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c: In function ‘rte_eth_dev_pci_probe’:
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:13: warning: implicit declaration of function ‘eth_dev_attach_secondary’ [-Wimplicit-function-declaration]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
             ^~~~~~~~~~~~~~~~~~~~~~~~
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:3: warning: nested extern declaration of ‘eth_dev_attach_secondary’ [-Wnested-externs]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
   ^~~~~~~
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:11: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
           ^
At top level:
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:193:1: warning: ‘eth_dev_init’ defined but not used [-Wunused-function]
 eth_dev_init(struct rte_eth_dev *eth_dev, uint8_t port_id, const char *name)
 ^~~~~~~~~~~~  LD librte_ethdev.so.5.1
rte_ethdev.o: In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x98d): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status
/home/patchWorkOrg/compilation/mk/rte.lib.mk:120: recipe for target 'librte_ethdev.so.5.1' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'librte_ether' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'lib' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #10:
OS: FC24_64
Target: x86_64-native-linuxapp-clang
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-linuxapp-clang/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:(.text+0x346): undefined reference to `eth_dev_attach_secondary'
clang-3.8: error: linker command failed with exit code 1 (use -v to see invocation)
/home/patchWorkOrg/compilation/mk/rte.app.mk:231: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'app' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #11:
OS: UB1604_64
Target: i686-native-linuxapp-gcc
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/i686-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x901): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status
/home/patchWorkOrg/compilation/mk/rte.app.mk:231: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'app' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #12:
OS: UB1604_64
Target: x86_64-native-linuxapp-gcc
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x974): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status
/home/patchWorkOrg/compilation/mk/rte.app.mk:231: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'app' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #13:
OS: UB1604_64
Target: x86_64-native-linuxapp-gcc-shared

/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c: In function ‘rte_eth_dev_pci_probe’:
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:13: warning: implicit declaration of function ‘eth_dev_attach_secondary’ [-Wimplicit-function-declaration]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
             ^
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:3: warning: nested extern declaration of ‘eth_dev_attach_secondary’ [-Wnested-externs]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
   ^
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:11: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
           ^
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c: At top level:
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:193:1: warning: ‘eth_dev_init’ defined but not used [-Wunused-function]
 eth_dev_init(struct rte_eth_dev *eth_dev, uint8_t port_id, const char *name)
 ^  LD librte_ethdev.so.5.1
rte_ethdev.o: In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x9ba): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status
/home/patchWorkOrg/compilation/mk/rte.lib.mk:120: recipe for target 'librte_ethdev.so.5.1' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'librte_ether' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'lib' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #14:
OS: UB1604_64
Target: x86_64-native-linuxapp-clang
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-linuxapp-clang/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:(.text+0x346): undefined reference to `eth_dev_attach_secondary'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
/home/patchWorkOrg/compilation/mk/rte.app.mk:231: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.subdir.mk:61: recipe for target 'test' failed
/home/patchWorkOrg/compilation/mk/rte.sdkbuild.mk:78: recipe for target 'app' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:126: recipe for target 'all' failed
/home/patchWorkOrg/compilation/mk/rte.sdkinstall.mk:85: recipe for target 'pre_install' failed
/home/patchWorkOrg/compilation/mk/rte.sdkroot.mk:101: recipe for target 'install' failed


Failed Build #15:
OS: CentOS7_64
Target: i686-native-linuxapp-gcc
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/i686-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x930): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status


Failed Build #16:
OS: CentOS7_64
Target: x86_64-native-linuxapp-clang
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-linuxapp-clang/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:(.text+0x273): undefined reference to `eth_dev_attach_secondary'
clang: error: linker command failed with exit code 1 (use -v to see invocation)


Failed Build #17:
OS: CentOS7_64
Target: x86_64-native-linuxapp-gcc-shared

/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c: In function ‘rte_eth_dev_pci_probe’:
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:3: warning: implicit declaration of function ‘eth_dev_attach_secondary’ [-Wimplicit-function-declaration]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
   ^
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:3: warning: nested extern declaration of ‘eth_dev_attach_secondary’ [-Wnested-externs]
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:273:11: warning: assignment makes pointer from integer without a cast [enabled by default]
   eth_dev = eth_dev_attach_secondary(ethdev_name);
           ^
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c: At top level:
/home/patchWorkOrg/compilation/lib/librte_ether/rte_ethdev.c:193:1: warning: ‘eth_dev_init’ defined but not used [-Wunused-function]
 eth_dev_init(struct rte_eth_dev *eth_dev, uint8_t port_id, const char *name)
 ^  LD librte_ethdev.so.5.1
rte_ethdev.o: In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0xa37): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status


Failed Build #18:
OS: CentOS7_64
Target: x86_64-native-linuxapp-gcc
MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
rte_ethdev.c:(.text+0x9c4): undefined reference to `eth_dev_attach_secondary'
collect2: error: ld returned 1 exit status

Re: [dpdk-stable] [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process

[Yuanhan Liu]

On Sun, Jan 08, 2017 at 03:15:00PM -0800, Stephen Hemminger wrote:
> On Fri,  6 Jan 2017 18:16:16 +0800
> Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> 
> > If the primary enables the vector Rx/Tx path, the current code would
> > let the secondary always choose the non vector Rx/Tx path. This results
> > to a Rx/Tx method mismatch between primary and secondary process. Werid
> > errors then may happen, something like:
> > 
> >     PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14
> > 
> > Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
> > That is, use vector path if it's given.
> > 
> > Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")
> > 
> > Cc: stable@dpdk.org
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> 
> This is failing on intel compile tests.
> 
> 
> http://dpdk.org/patch/18975

Thanks, but it looks like a false alarm to me, for reasons below.

> Failed Build #2:
> OS: RHEL7.2_64
> Target: x86_64-native-linuxapp-gcc
> MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function `rte_eth_dev_pci_probe':
> rte_ethdev.c:(.text+0x9c4): undefined reference to `eth_dev_attach_secondary'
> collect2: error: ld returned 1 exit status

- eth_dev_attach_secondary is not defined in this patch, it's defined
  (and used) in the first patch.

- eth_dev_attach_secondary is actually defined; The report even shows
  it fails to build with gcc: the gcc build passes on my dev box.

Honestly, I seldom trusted the build reports from STV.

	--yliu

[dpdk-stable] [PATCH v4] ethdev: fix port data mismatched in multiple process model

[Yuanhan Liu]

Assume we have two virtio ports, 00:03.0 and 00:04.0. The first one is
managed by the kernel driver, while the later one is managed by DPDK.

Now we start the primary process. 00:03.0 will be skipped by DPDK virtio
PMD driver (since it's being used by the kernel). 00:04.0 would be
successfully initiated by DPDK virtio PMD (if nothing abnormal happens).
After that, we would get a port id 0, and all the related info needed
by virtio (virtio_hw) is stored at rte_eth_dev_data[0].

Then we start the secondary process. As usual, 00:03.0 will be firstly
probed. It firstly tries to get a local eth_dev structure for it (by
rte_eth_dev_allocate):

        port_id = rte_eth_dev_find_free_port();
        ...

        eth_dev = &rte_eth_devices[port_id];
        eth_dev->data = &rte_eth_dev_data[port_id];
        ...

        return eth_dev;

Since it's a first PCI device, port_id will be 0. eth_dev->data would
then point to rte_eth_dev_data[0]. And here things start going wrong,
as rte_eth_dev_data[0] actually stores the virtio_hw for 00:04.0.

That said, in the secondary process, DPDK will continue to drive PCI
device 00.03.0 (despite the fact it's been managed by kernel), with
the info from PCI device 00:04.0. Which is wrong.

The fix is to attach the port already registered by the primary process:
iterate the rte_eth_dev_data[], and get the port id who's PCI ID matches
the current PCI device.

This would let us maintain same port ID for the same PCI device, keeping
the chance of referencing to wrong data minimal.

Fixes: af75078fece3 ("first public release")

Cc: stable@dpdk.org
Cc: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---

v4: - assign eth_dev in the common eth_dev init help function
      it also renamed to eth_dev_get, to not confuse with the
      eth_dev_init callback.
    - move primoary process specific assignments to rte_eth_dev_allocate
    - drop the virtio example in comment
    - combine two code block for primary into one

v3: - do not move rte_eth_dev_data_alloc to pci_probe
    - rename eth_dev_attach to eth_dev_attach_secondary
    - introduce eth_dev_init() for common eth_dev struct initiation
    - move comment block inside the "if" block
---
 lib/librte_ether/rte_ethdev.c | 71 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 9 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..1453df1 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -189,6 +189,19 @@ struct rte_eth_dev *
 	return RTE_MAX_ETHPORTS;
 }
 
+static struct rte_eth_dev *
+eth_dev_get(uint8_t port_id)
+{
+	struct rte_eth_dev *eth_dev = &rte_eth_devices[port_id];
+
+	eth_dev->data = &rte_eth_dev_data[port_id];
+	eth_dev->attached = DEV_ATTACHED;
+	eth_dev_last_created_port = port_id;
+	nb_ports++;
+
+	return eth_dev;
+}
+
 struct rte_eth_dev *
 rte_eth_dev_allocate(const char *name)
 {
@@ -210,13 +223,41 @@ struct rte_eth_dev *
 		return NULL;
 	}
 
-	eth_dev = &rte_eth_devices[port_id];
-	eth_dev->data = &rte_eth_dev_data[port_id];
+	eth_dev = eth_dev_get(port_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;
-	eth_dev_last_created_port = port_id;
-	nb_ports++;
+
+	return eth_dev;
+}
+
+/*
+ * Attach to a port already registered by the primary process, which
+ * makes sure that the same device would have the same port id both
+ * in the primary and secondary process.
+ */
+static struct rte_eth_dev *
+eth_dev_attach_secondary(const char *name)
+{
+	uint8_t i;
+	struct rte_eth_dev *eth_dev;
+
+	if (rte_eth_dev_data == NULL)
+		rte_eth_dev_data_alloc();
+
+	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;
+	}
+
+	eth_dev = eth_dev_get(i);
+	RTE_ASSERT(eth_dev->data->port_id == i);
+
 	return eth_dev;
 }
 
@@ -246,16 +287,28 @@ struct rte_eth_dev *
 	rte_eal_pci_device_name(&pci_dev->addr, ethdev_name,
 			sizeof(ethdev_name));
 
-	eth_dev = rte_eth_dev_allocate(ethdev_name);
-	if (eth_dev == NULL)
-		return -ENOMEM;
-
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		eth_dev = rte_eth_dev_allocate(ethdev_name);
+		if (eth_dev == NULL)
+			return -ENOMEM;
+
 		eth_dev->data->dev_private = rte_zmalloc("ethdev private structure",
 				  eth_drv->dev_private_size,
 				  RTE_CACHE_LINE_SIZE);
 		if (eth_dev->data->dev_private == NULL)
 			rte_panic("Cannot allocate memzone for private port data\n");
+	} else {
+		eth_dev = eth_dev_attach_secondary(ethdev_name);
+		if (eth_dev == NULL) {
+			/*
+			 * if we failed to attach a device, it means the
+			 * device is skipped in primary process, due to
+			 * some errors. If so, we return a positive value,
+			 * to let EAL skip it for the secondary process
+			 * as well.
+			 */
+			return 1;
+		}
 	}
 	eth_dev->pci_dev = pci_dev;
 	eth_dev->driver = eth_drv;
-- 
1.9.0

Re: [dpdk-stable] [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process

[Xu, Qian Q]

Fangfang, 
Could you help double confirm below error and the patchset ? Thanks. 
http://dpdk.org/patch/18975

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, January 9, 2017 1:19 PM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: dev@dpdk.org; stable@dpdk.org; Xu, Qian Q <qian.q.xu@intel.com>; Liu,
> Yong <yong.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for
> secondary process
> 
> On Sun, Jan 08, 2017 at 03:15:00PM -0800, Stephen Hemminger wrote:
> > On Fri,  6 Jan 2017 18:16:16 +0800
> > Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> >
> > > If the primary enables the vector Rx/Tx path, the current code would
> > > let the secondary always choose the non vector Rx/Tx path. This
> > > results to a Rx/Tx method mismatch between primary and secondary
> > > process. Werid errors then may happen, something like:
> > >
> > >     PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14
> > >
> > > Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
> > > That is, use vector path if it's given.
> > >
> > > Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")
> > >
> > > Cc: stable@dpdk.org
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >
> > This is failing on intel compile tests.
> >
> >
> > http://dpdk.org/patch/18975
> 
> Thanks, but it looks like a false alarm to me, for reasons below.
> 
> > Failed Build #2:
> > OS: RHEL7.2_64
> > Target: x86_64-native-linuxapp-gcc
> > MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-
> native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function
> `rte_eth_dev_pci_probe':
> > rte_ethdev.c:(.text+0x9c4): undefined reference to
> `eth_dev_attach_secondary'
> > collect2: error: ld returned 1 exit status
> 
> - eth_dev_attach_secondary is not defined in this patch, it's defined
>   (and used) in the first patch.
> 
> - eth_dev_attach_secondary is actually defined; The report even shows
>   it fails to build with gcc: the gcc build passes on my dev box.
> 
> Honestly, I seldom trusted the build reports from STV.
> 
> 	--yliu

Re: [dpdk-stable] [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process

[Wei, FangfangX]

I've re-build it manually, it passed with this patch.


-----Original Message-----
From: Xu, Qian Q 
Sent: Monday, January 9, 2017 4:02 PM
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Stephen Hemminger <stephen@networkplumber.org>; Wei, FangfangX <fangfangx.wei@intel.com>
Cc: dev@dpdk.org; stable@dpdk.org; Liu, Yong <yong.liu@intel.com>
Subject: RE: [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process

Fangfang,
Could you help double confirm below error and the patchset ? Thanks. 
http://dpdk.org/patch/18975

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, January 9, 2017 1:19 PM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: dev@dpdk.org; stable@dpdk.org; Xu, Qian Q <qian.q.xu@intel.com>; 
> Liu, Yong <yong.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx 
> method for secondary process
> 
> On Sun, Jan 08, 2017 at 03:15:00PM -0800, Stephen Hemminger wrote:
> > On Fri,  6 Jan 2017 18:16:16 +0800
> > Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> >
> > > If the primary enables the vector Rx/Tx path, the current code 
> > > would let the secondary always choose the non vector Rx/Tx path. 
> > > This results to a Rx/Tx method mismatch between primary and 
> > > secondary process. Werid errors then may happen, something like:
> > >
> > >     PMD: virtio_xmit_pkts() tx: virtqueue_enqueue error: -14
> > >
> > > Fix it by choosing the correct Rx/Tx callbacks for the secondary process.
> > > That is, use vector path if it's given.
> > >
> > > Fixes: 8d8393fb1861 ("virtio: pick simple Rx/Tx")
> > >
> > > Cc: stable@dpdk.org
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >
> > This is failing on intel compile tests.
> >
> >
> > http://dpdk.org/patch/18975
> 
> Thanks, but it looks like a false alarm to me, for reasons below.
> 
> > Failed Build #2:
> > OS: RHEL7.2_64
> > Target: x86_64-native-linuxapp-gcc
> > MKRES test_resource_c.res.o  /home/patchWorkOrg/compilation/x86_64-
> native-linuxapp-gcc/lib/librte_ethdev.a(rte_ethdev.o): In function
> `rte_eth_dev_pci_probe':
> > rte_ethdev.c:(.text+0x9c4): undefined reference to
> `eth_dev_attach_secondary'
> > collect2: error: ld returned 1 exit status
> 
> - eth_dev_attach_secondary is not defined in this patch, it's defined
>   (and used) in the first patch.
> 
> - eth_dev_attach_secondary is actually defined; The report even shows
>   it fails to build with gcc: the gcc build passes on my dev box.
> 
> Honestly, I seldom trusted the build reports from STV.
> 
> 	--yliu

Re: [dpdk-stable] [PATCH v4] ethdev: fix port data mismatched in multiple process model

[Thomas Monjalon]

Hi Yuanhan,

Nit: the title should be "v4 1/6"
Except that, good patch :)

> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Re: [dpdk-stable] [PATCH v4] ethdev: fix port data mismatched in multiple process model

[Yuanhan Liu]

On Mon, Jan 09, 2017 at 06:08:04PM +0100, Thomas Monjalon wrote:
> Hi Yuanhan,
> 
> Nit: the title should be "v4 1/6"

Oops, my bad.

> Except that, good patch :)
> 
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> 
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Thanks for review! Mind if I apply it to the next-virtio tree?

	--yliu

Re: [dpdk-stable] [PATCH v4] ethdev: fix port data mismatched in multiple process model

[Thomas Monjalon]

2017-01-10 22:33, Yuanhan Liu:
> On Mon, Jan 09, 2017 at 06:08:04PM +0100, Thomas Monjalon wrote:
> > Hi Yuanhan,
> > 
> > Nit: the title should be "v4 1/6"
> 
> Oops, my bad.
> 
> > Except that, good patch :)
> > 
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > 
> > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Thanks for review! Mind if I apply it to the next-virtio tree?

Yes, please do.

Re: [dpdk-stable] [PATCH v4] ethdev: fix port data mismatched in multiple process model

[Yuanhan Liu]

On Wed, Jan 11, 2017 at 02:32:03PM +0100, Thomas Monjalon wrote:
> 2017-01-10 22:33, Yuanhan Liu:
> > On Mon, Jan 09, 2017 at 06:08:04PM +0100, Thomas Monjalon wrote:
> > > Hi Yuanhan,
> > > 
> > > Nit: the title should be "v4 1/6"
> > 
> > Oops, my bad.
> > 
> > > Except that, good patch :)
> > > 
> > > > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > > 
> > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > 
> > Thanks for review! Mind if I apply it to the next-virtio tree?
> 
> Yes, please do.

Thanks!

Series applied to dpdk-next-virtio.

	--yliu

Re: [dpdk-stable] [PATCH v4] ethdev: fix port data mismatched in multiple process model

[Ferruh Yigit]

On 1/9/2017 7:50 AM, Yuanhan Liu wrote:
> Assume we have two virtio ports, 00:03.0 and 00:04.0. The first one is
> managed by the kernel driver, while the later one is managed by DPDK.
> 
> Now we start the primary process. 00:03.0 will be skipped by DPDK virtio
> PMD driver (since it's being used by the kernel). 00:04.0 would be
> successfully initiated by DPDK virtio PMD (if nothing abnormal happens).
> After that, we would get a port id 0, and all the related info needed
> by virtio (virtio_hw) is stored at rte_eth_dev_data[0].
> 
> Then we start the secondary process. As usual, 00:03.0 will be firstly
> probed. It firstly tries to get a local eth_dev structure for it (by
> rte_eth_dev_allocate):
> 
>         port_id = rte_eth_dev_find_free_port();
>         ...
> 
>         eth_dev = &rte_eth_devices[port_id];
>         eth_dev->data = &rte_eth_dev_data[port_id];
>         ...
> 
>         return eth_dev;
> 
> Since it's a first PCI device, port_id will be 0. eth_dev->data would
> then point to rte_eth_dev_data[0]. And here things start going wrong,
> as rte_eth_dev_data[0] actually stores the virtio_hw for 00:04.0.
> 
> That said, in the secondary process, DPDK will continue to drive PCI
> device 00.03.0 (despite the fact it's been managed by kernel), with
> the info from PCI device 00:04.0. Which is wrong.
> 
> The fix is to attach the port already registered by the primary process:
> iterate the rte_eth_dev_data[], and get the port id who's PCI ID matches
> the current PCI device.
> 
> This would let us maintain same port ID for the same PCI device, keeping
> the chance of referencing to wrong data minimal.
> 
> Fixes: af75078fece3 ("first public release")
> 
> Cc: stable@dpdk.org
> Cc: Thomas Monjalon <thomas.monjalon@6wind.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
> 
> v4: - assign eth_dev in the common eth_dev init help function
>       it also renamed to eth_dev_get, to not confuse with the
>       eth_dev_init callback.
>     - move primoary process specific assignments to rte_eth_dev_allocate
>     - drop the virtio example in comment
>     - combine two code block for primary into one
> 
> v3: - do not move rte_eth_dev_data_alloc to pci_probe
>     - rename eth_dev_attach to eth_dev_attach_secondary
>     - introduce eth_dev_init() for common eth_dev struct initiation
>     - move comment block inside the "if" block
> ---
>  lib/librte_ether/rte_ethdev.c | 71 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 62 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index fde8112..1453df1 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -189,6 +189,19 @@ struct rte_eth_dev *
>  	return RTE_MAX_ETHPORTS;
>  }
>  
> +static struct rte_eth_dev *
> +eth_dev_get(uint8_t port_id)
> +{
> +	struct rte_eth_dev *eth_dev = &rte_eth_devices[port_id];
> +
> +	eth_dev->data = &rte_eth_dev_data[port_id];
> +	eth_dev->attached = DEV_ATTACHED;
> +	eth_dev_last_created_port = port_id;
> +	nb_ports++;
> +
> +	return eth_dev;
> +}
> +
>  struct rte_eth_dev *
>  rte_eth_dev_allocate(const char *name)
>  {
> @@ -210,13 +223,41 @@ struct rte_eth_dev *
>  		return NULL;
>  	}
>  
> -	eth_dev = &rte_eth_devices[port_id];
> -	eth_dev->data = &rte_eth_dev_data[port_id];
> +	eth_dev = eth_dev_get(port_id);

There can be a merge issue here, please help me understand.

In repo, different from seen here, this patch does this here:
  -       eth_dev = &rte_eth_devices[port_id];
  -       eth_dev->data = &rte_eth_dev_data[port_id];
  -       memset(eth_dev->data, 0, sizeof(*eth_dev->data));
  +       memset(&rte_eth_devices[port_id], 0, sizeof(*eth_dev->data));
  +       eth_dev = eth_dev_get(port_id);

Which no more resets the eth_dev->data, but rte_eth_devices[port_id]
(with sizeof(*eth_dev->data))

memset(eth_dev->data) added by Jan Blunck on comment:
7f95f78a8aea ("ethdev: clear data when allocating device")

most probably it should stay as "memset(eth_dev->data)", but if not,
please aware that commit 7f95f78a8aea removed some assignment from
drivers relying this memset, they needs to be added back.

>  	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_last_created_port = port_id;
> -	nb_ports++;
> +
> +	return eth_dev;
> +}
> +
> +/*
> + * Attach to a port already registered by the primary process, which
> + * makes sure that the same device would have the same port id both
> + * in the primary and secondary process.
> + */
> +static struct rte_eth_dev *
> +eth_dev_attach_secondary(const char *name)
> +{
> +	uint8_t i;
> +	struct rte_eth_dev *eth_dev;
> +
> +	if (rte_eth_dev_data == NULL)
> +		rte_eth_dev_data_alloc();
> +
> +	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;
> +	}
> +
> +	eth_dev = eth_dev_get(i);
> +	RTE_ASSERT(eth_dev->data->port_id == i);
> +
>  	return eth_dev;
>  }
>  
> @@ -246,16 +287,28 @@ struct rte_eth_dev *
>  	rte_eal_pci_device_name(&pci_dev->addr, ethdev_name,
>  			sizeof(ethdev_name));
>  
> -	eth_dev = rte_eth_dev_allocate(ethdev_name);
> -	if (eth_dev == NULL)
> -		return -ENOMEM;
> -
>  	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +		eth_dev = rte_eth_dev_allocate(ethdev_name);
> +		if (eth_dev == NULL)
> +			return -ENOMEM;
> +
>  		eth_dev->data->dev_private = rte_zmalloc("ethdev private structure",
>  				  eth_drv->dev_private_size,
>  				  RTE_CACHE_LINE_SIZE);
>  		if (eth_dev->data->dev_private == NULL)
>  			rte_panic("Cannot allocate memzone for private port data\n");
> +	} else {
> +		eth_dev = eth_dev_attach_secondary(ethdev_name);
> +		if (eth_dev == NULL) {
> +			/*
> +			 * if we failed to attach a device, it means the
> +			 * device is skipped in primary process, due to
> +			 * some errors. If so, we return a positive value,
> +			 * to let EAL skip it for the secondary process
> +			 * as well.
> +			 */
> +			return 1;
> +		}
>  	}
>  	eth_dev->pci_dev = pci_dev;
>  	eth_dev->driver = eth_drv;
> 

Re: [dpdk-stable] [PATCH v4] ethdev: fix port data mismatched in multiple process model

[Yuanhan Liu]

On Thu, Jan 19, 2017 at 06:39:13PM +0000, Ferruh Yigit wrote:
> >  struct rte_eth_dev *
> >  rte_eth_dev_allocate(const char *name)
> >  {
> > @@ -210,13 +223,41 @@ struct rte_eth_dev *
> >  		return NULL;
> >  	}
> >  
> > -	eth_dev = &rte_eth_devices[port_id];
> > -	eth_dev->data = &rte_eth_dev_data[port_id];
> > +	eth_dev = eth_dev_get(port_id);
> 
> There can be a merge issue here, please help me understand.
> 
> In repo, different from seen here, this patch does this here:
>   -       eth_dev = &rte_eth_devices[port_id];
>   -       eth_dev->data = &rte_eth_dev_data[port_id];
>   -       memset(eth_dev->data, 0, sizeof(*eth_dev->data));
>   +       memset(&rte_eth_devices[port_id], 0, sizeof(*eth_dev->data));
>   +       eth_dev = eth_dev_get(port_id);
> 
> Which no more resets the eth_dev->data, but rte_eth_devices[port_id]
> (with sizeof(*eth_dev->data))

Nice catch! Sorry, it's a silly error by auto-complete :/
As you said, it should be:

    memset(&rte_eth_dev_data[port_id], 0, sizeof(struct rte_eth_dev_data));

Will make a patch soon. Thanks.

	--yliu

> 
> memset(eth_dev->data) added by Jan Blunck on comment:
> 7f95f78a8aea ("ethdev: clear data when allocating device")
> 
> most probably it should stay as "memset(eth_dev->data)", but if not,
> please aware that commit 7f95f78a8aea removed some assignment from
> drivers relying this memset, they needs to be added back.