[dpdk-stable] [PATCH 18.02 0/4] ethdev: fix iterator and probing notification

[dpdk-stable] [PATCH 18.02 0/4] ethdev: fix iterator and probing notification

[Thomas Monjalon]

These are the remaining patches from the series
  "ethdev: fix race conditions in iterator and notifications"
which were not yet backported in 18.02.

Please move the commit "net/failsafe: fix sub-device ownership race",
which was already backported in 18.02 branch,
on top of these patches, as there is a dependency on notification fix.
Thanks


Matan Azrad (1):
  ethdev: allow ownership operations on unused port

Thomas Monjalon (3):
  ethdev: add probing finish function
  ethdev: fix port visibility before initialization
  ethdev: fix port probing notification

 drivers/net/af_packet/rte_eth_af_packet.c |  1 +
 drivers/net/ark/ark_ethdev.c              |  2 ++
 drivers/net/bonding/rte_eth_bond_pmd.c    |  1 +
 drivers/net/cxgbe/cxgbe_ethdev.c          |  1 +
 drivers/net/cxgbe/cxgbe_main.c            |  5 +++
 drivers/net/dpaa/dpaa_ethdev.c            |  5 ++-
 drivers/net/dpaa2/dpaa2_ethdev.c          |  4 ++-
 drivers/net/failsafe/failsafe.c           |  1 +
 drivers/net/kni/rte_eth_kni.c             |  1 +
 drivers/net/mlx4/mlx4.c                   |  1 +
 drivers/net/mlx5/mlx5.c                   |  2 ++
 drivers/net/mrvl/mrvl_ethdev.c            |  1 +
 drivers/net/nfp/nfp_net.c                 |  2 ++
 drivers/net/null/rte_eth_null.c           |  1 +
 drivers/net/octeontx/octeontx_ethdev.c    |  2 ++
 drivers/net/pcap/rte_eth_pcap.c           |  1 +
 drivers/net/ring/rte_eth_ring.c           |  1 +
 drivers/net/softnic/rte_eth_softnic.c     |  2 ++
 drivers/net/tap/rte_eth_tap.c             |  1 +
 drivers/net/vhost/rte_eth_vhost.c         |  1 +
 drivers/net/virtio/virtio_user_ethdev.c   |  3 ++
 lib/librte_ether/rte_ethdev.c             | 57 ++++++++++++++++++++-----------
 lib/librte_ether/rte_ethdev_driver.h      | 14 ++++++++
 lib/librte_ether/rte_ethdev_pci.h         |  2 ++
 lib/librte_ether/rte_ethdev_version.map   |  1 +
 test/test/virtual_pmd.c                   |  2 ++
 26 files changed, 94 insertions(+), 21 deletions(-)

-- 
2.16.2

[dpdk-stable] [PATCH 18.02 1/4] ethdev: add probing finish function

[Thomas Monjalon]

A new hook function is added and called inside the PMDs at the end
of the device probing:
	- in primary process, after allocating, init and config
	- in secondary process, after attaching and local init

This new function is almost empty for now.
It will be used later to add some post-initialization processing.

For the PMDs calling the helpers rte_eth_dev_create() or
rte_eth_dev_pci_generic_probe(), the hook rte_eth_dev_probing_finish()
is called from here, and not in the PMD itself.

Note that the helper rte_eth_dev_create() could be used more,
especially for vdevs, avoiding some code duplication in PMDs.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/af_packet/rte_eth_af_packet.c |  1 +
 drivers/net/ark/ark_ethdev.c              |  2 ++
 drivers/net/bonding/rte_eth_bond_pmd.c    |  1 +
 drivers/net/cxgbe/cxgbe_ethdev.c          |  1 +
 drivers/net/cxgbe/cxgbe_main.c            |  5 +++++
 drivers/net/dpaa/dpaa_ethdev.c            |  5 ++++-
 drivers/net/dpaa2/dpaa2_ethdev.c          |  4 +++-
 drivers/net/failsafe/failsafe.c           |  1 +
 drivers/net/kni/rte_eth_kni.c             |  1 +
 drivers/net/mlx4/mlx4.c                   |  1 +
 drivers/net/mlx5/mlx5.c                   |  2 ++
 drivers/net/mrvl/mrvl_ethdev.c            |  1 +
 drivers/net/nfp/nfp_net.c                 |  2 ++
 drivers/net/null/rte_eth_null.c           |  1 +
 drivers/net/octeontx/octeontx_ethdev.c    |  2 ++
 drivers/net/pcap/rte_eth_pcap.c           |  1 +
 drivers/net/ring/rte_eth_ring.c           |  1 +
 drivers/net/softnic/rte_eth_softnic.c     |  2 ++
 drivers/net/tap/rte_eth_tap.c             |  1 +
 drivers/net/vhost/rte_eth_vhost.c         |  1 +
 drivers/net/virtio/virtio_user_ethdev.c   |  3 +++
 lib/librte_ether/rte_ethdev.c             |  7 +++++++
 lib/librte_ether/rte_ethdev_driver.h      | 10 ++++++++++
 lib/librte_ether/rte_ethdev_pci.h         |  2 ++
 lib/librte_ether/rte_ethdev_version.map   |  1 +
 test/test/virtual_pmd.c                   |  2 ++
 26 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
index 7694b21ca..76e6ca21f 100644
--- a/drivers/net/af_packet/rte_eth_af_packet.c
+++ b/drivers/net/af_packet/rte_eth_af_packet.c
@@ -917,6 +917,7 @@ rte_eth_from_packet(struct rte_vdev_device *dev,
 	eth_dev->rx_pkt_burst = eth_af_packet_rx;
 	eth_dev->tx_pkt_burst = eth_af_packet_tx;
 
+	rte_eth_dev_probing_finish(eth_dev);
 	return 0;
 }
 
diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index ff87c20e2..9fdfce680 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/drivers/net/ark/ark_ethdev.c
@@ -422,6 +422,8 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
 			ark->user_data[eth_dev->data->port_id] =
 				ark->user_ext.dev_init(dev, ark->a_bar, p);
 		}
+
+		rte_eth_dev_probing_finish(eth_dev);
 	}
 
 	return ret;
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 324753484..7af5166b2 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3088,6 +3088,7 @@ bond_probe(struct rte_vdev_device *dev)
 		rte_eth_bond_8023ad_agg_selection_set(port_id, AGG_STABLE);
 	}
 
+	rte_eth_dev_probing_finish(&rte_eth_devices[port_id]);
 	RTE_LOG(INFO, EAL, "Create bonded device %s on port %d in mode %u on "
 			"socket %u.\n",	name, port_id, bonding_mode, socket_id);
 	return 0;
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 423a6957b..ed2f027c4 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -1027,6 +1027,7 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
 					eth_dev->rx_pkt_burst;
 				rest_eth_dev->tx_pkt_burst =
 					eth_dev->tx_pkt_burst;
+				rte_eth_dev_probing_finish(rest_eth_dev);
 			}
 		}
 		return 0;
diff --git a/drivers/net/cxgbe/cxgbe_main.c b/drivers/net/cxgbe/cxgbe_main.c
index 28db6c061..3fecc08c2 100644
--- a/drivers/net/cxgbe/cxgbe_main.c
+++ b/drivers/net/cxgbe/cxgbe_main.c
@@ -1318,6 +1318,11 @@ int cxgbe_probe(struct adapter *adapter)
 			err = -1;
 			goto out_free;
 		}
+
+		if (i > 0) {
+			/* First port will be notified by upper layer */
+			rte_eth_dev_probing_finish(eth_dev);
+		}
 	}
 
 	if (adapter->flags & FW_OK) {
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 7f841bd0c..b15c13d8b 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -1321,6 +1321,7 @@ rte_dpaa_probe(struct rte_dpaa_driver *dpaa_drv,
 		eth_dev = rte_eth_dev_attach_secondary(dpaa_dev->name);
 		if (!eth_dev)
 			return -ENOMEM;
+		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
 
@@ -1370,8 +1371,10 @@ rte_dpaa_probe(struct rte_dpaa_driver *dpaa_drv,
 
 	/* Invoke PMD device initialization function */
 	diag = dpaa_dev_init(eth_dev);
-	if (diag == 0)
+	if (diag == 0) {
+		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
+	}
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_free(eth_dev->data->dev_private);
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 182fcf329..59b526c6e 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -2016,8 +2016,10 @@ rte_dpaa2_probe(struct rte_dpaa2_driver *dpaa2_drv,
 
 	/* Invoke PMD device initialization function */
 	diag = dpaa2_dev_init(eth_dev);
-	if (diag == 0)
+	if (diag == 0) {
+		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
+	}
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_free(eth_dev->data->dev_private);
diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index da2c7f07a..785e373f3 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -266,6 +266,7 @@ fs_eth_dev_create(struct rte_vdev_device *vdev)
 		.fd = -1,
 		.type = RTE_INTR_HANDLE_EXT,
 	};
+	rte_eth_dev_probing_finish(dev);
 	return 0;
 cancel_alarm:
 	failsafe_hotplug_alarm_cancel(dev);
diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index 57f3c50e3..5b53b920e 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -429,6 +429,7 @@ eth_kni_probe(struct rte_vdev_device *vdev)
 	eth_dev->rx_pkt_burst = eth_kni_rx;
 	eth_dev->tx_pkt_burst = eth_kni_tx;
 
+	rte_eth_dev_probing_finish(eth_dev);
 	return 0;
 
 kni_uninit:
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index bc12b6030..7b3616e3c 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -733,6 +733,7 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
 		/* Update link status once if waiting for LSC. */
 		if (eth_dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)
 			mlx4_link_update(eth_dev, 0);
+		rte_eth_dev_probing_finish(eth_dev);
 		continue;
 port_error:
 		rte_free(priv);
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 81057f438..f1f8006b7 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -757,6 +757,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 				mlx5_select_rx_function(eth_dev);
 			eth_dev->tx_pkt_burst =
 				mlx5_select_tx_function(eth_dev);
+			rte_eth_dev_probing_finish(eth_dev);
 			continue;
 		}
 		DEBUG("using port %u (%08" PRIx32 ")", port, test);
@@ -948,6 +949,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		mlx5_link_update(eth_dev, 0);
 		/* Store device configuration on private structure. */
 		priv->config = config;
+		rte_eth_dev_probing_finish(eth_dev);
 		continue;
 port_error:
 		if (priv)
diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c
index 4226f83a4..917dc3581 100644
--- a/drivers/net/mrvl/mrvl_ethdev.c
+++ b/drivers/net/mrvl/mrvl_ethdev.c
@@ -2281,6 +2281,7 @@ mrvl_eth_dev_create(struct rte_vdev_device *vdev, const char *name)
 	eth_dev->device = &vdev->device;
 	eth_dev->dev_ops = &mrvl_ops;
 
+	rte_eth_dev_probing_finish(eth_dev);
 	return 0;
 out_free_mac:
 	rte_free(eth_dev->data->mac_addrs);
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 16adbb29e..a010f223c 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -3008,6 +3008,8 @@ nfp_pf_create_dev(struct rte_pci_device *dev, int port, int ports,
 
 	if (ret)
 		rte_eth_dev_release_port(eth_dev);
+	else
+		rte_eth_dev_probing_finish(eth_dev);
 
 	rte_free(port_name);
 
diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index 181785b48..1ba5ca473 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -564,6 +564,7 @@ eth_dev_null_create(struct rte_vdev_device *dev,
 		eth_dev->tx_pkt_burst = eth_null_tx;
 	}
 
+	rte_eth_dev_probing_finish(eth_dev);
 	return 0;
 }
 
diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c
index 311e9ce4c..93d57e3e4 100644
--- a/drivers/net/octeontx/octeontx_ethdev.c
+++ b/drivers/net/octeontx/octeontx_ethdev.c
@@ -1051,6 +1051,7 @@ octeontx_create(struct rte_vdev_device *dev, int port, uint8_t evdev,
 
 		eth_dev->tx_pkt_burst = octeontx_xmit_pkts;
 		eth_dev->rx_pkt_burst = octeontx_recv_pkts;
+		rte_eth_dev_probing_finish(eth_dev);
 		return 0;
 	}
 
@@ -1145,6 +1146,7 @@ octeontx_create(struct rte_vdev_device *dev, int port, uint8_t evdev,
 	rte_octeontx_pchan_map[(nic->base_ochan >> 8) & 0x7]
 		[(nic->base_ochan >> 4) & 0xF] = data->port_id;
 
+	rte_eth_dev_probing_finish(eth_dev);
 	return data->port_id;
 
 err:
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 814beedc8..026d29574 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -899,6 +899,7 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 	else
 		eth_dev->tx_pkt_burst = eth_pcap_tx;
 
+	rte_eth_dev_probing_finish(eth_dev);
 	return 0;
 }
 
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 311ec3d5a..947c8f13c 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -335,6 +335,7 @@ do_eth_dev_ring_create(const char *name,
 	eth_dev->rx_pkt_burst = eth_ring_rx;
 	eth_dev->tx_pkt_burst = eth_ring_tx;
 
+	rte_eth_dev_probing_finish(eth_dev);
 	*eth_dev_p = eth_dev;
 
 	return data->port_id;
diff --git a/drivers/net/softnic/rte_eth_softnic.c b/drivers/net/softnic/rte_eth_softnic.c
index d8ecfc2f0..a1ae81f73 100644
--- a/drivers/net/softnic/rte_eth_softnic.c
+++ b/drivers/net/softnic/rte_eth_softnic.c
@@ -529,6 +529,8 @@ pmd_ethdev_register(struct rte_vdev_device *vdev,
 	soft_dev->data->kdrv = RTE_KDRV_NONE;
 	soft_dev->data->numa_node = numa_node;
 
+	rte_eth_dev_probing_finish(soft_dev);
+
 	return 0;
 }
 
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 7f85d4133..6ce02c9b6 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1506,6 +1506,7 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
 		}
 	}
 
+	rte_eth_dev_probing_finish(dev);
 	return 0;
 
 disable_rte_flow:
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 1a1d532e7..8a9653875 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1130,6 +1130,7 @@ eth_dev_vhost_create(struct rte_vdev_device *dev, char *iface_name,
 		goto error;
 	}
 
+	rte_eth_dev_probing_finish(eth_dev);
 	return data->port_id;
 
 error:
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 263649006..d8ee70528 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -482,6 +482,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
 			virtio_user_eth_dev_free(eth_dev);
 			goto end;
 		}
+
 	} else {
 		eth_dev = rte_eth_dev_attach_secondary(rte_vdev_device_name(dev));
 		if (!eth_dev)
@@ -494,6 +495,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
 		virtio_user_eth_dev_free(eth_dev);
 		goto end;
 	}
+
+	rte_eth_dev_probing_finish(eth_dev);
 	ret = 0;
 
 end:
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 25fd3bc46..365540d3f 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3375,6 +3375,13 @@ _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
 	return rc;
 }
 
+void
+rte_eth_dev_probing_finish(struct rte_eth_dev *dev)
+{
+	if (dev == NULL)
+		return;
+}
+
 int
 rte_eth_dev_rx_intr_ctl(uint16_t port_id, int epfd, int op, void *data)
 {
diff --git a/lib/librte_ether/rte_ethdev_driver.h b/lib/librte_ether/rte_ethdev_driver.h
index 45f08c65e..77297f331 100644
--- a/lib/librte_ether/rte_ethdev_driver.h
+++ b/lib/librte_ether/rte_ethdev_driver.h
@@ -101,6 +101,16 @@ void _rte_eth_dev_reset(struct rte_eth_dev *dev);
 int _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
 		enum rte_eth_event_type event, void *ret_param);
 
+/**
+ * @internal
+ * This is the last step of device probing.
+ * It must be called after a port is allocated and initialized successfully.
+ *
+ * @param dev
+ *  New ethdev port.
+ */
+void rte_eth_dev_probing_finish(struct rte_eth_dev *dev);
+
 /**
  * Create memzone for HW rings.
  * malloc can't be used as the physical address is needed.
diff --git a/lib/librte_ether/rte_ethdev_pci.h b/lib/librte_ether/rte_ethdev_pci.h
index 6565ae7d3..6ea401da4 100644
--- a/lib/librte_ether/rte_ethdev_pci.h
+++ b/lib/librte_ether/rte_ethdev_pci.h
@@ -163,6 +163,8 @@ rte_eth_dev_pci_generic_probe(struct rte_pci_device *pci_dev,
 	ret = dev_init(eth_dev);
 	if (ret)
 		rte_eth_dev_pci_release(eth_dev);
+	else
+		rte_eth_dev_probing_finish(eth_dev);
 
 	return ret;
 }
diff --git a/lib/librte_ether/rte_ethdev_version.map b/lib/librte_ether/rte_ethdev_version.map
index 87f02fb74..d7143a0a7 100644
--- a/lib/librte_ether/rte_ethdev_version.map
+++ b/lib/librte_ether/rte_ethdev_version.map
@@ -201,6 +201,7 @@ DPDK_18.02 {
 	global:
 
 	rte_eth_dev_filter_ctrl;
+	rte_eth_dev_probing_finish;
 
 } DPDK_17.11;
 
diff --git a/test/test/virtual_pmd.c b/test/test/virtual_pmd.c
index 2f5b31dba..3d16c7197 100644
--- a/test/test/virtual_pmd.c
+++ b/test/test/virtual_pmd.c
@@ -589,6 +589,8 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
 	eth_dev->rx_pkt_burst = virtual_ethdev_rx_burst_success;
 	eth_dev->tx_pkt_burst = virtual_ethdev_tx_burst_success;
 
+	rte_eth_dev_probing_finish(eth_dev);
+
 	return eth_dev->data->port_id;
 
 err:
-- 
2.16.2

[dpdk-stable] [PATCH 18.02 2/4] ethdev: allow ownership operations on unused port

[Thomas Monjalon]

From: Matan Azrad <matan@mellanox.com>

When the state will be updated later than in allocation,
we may need to update the ownership of a port which is
still in state unused.

It will be used to take ownership of a port before it is
declared as available for other entities.

Signed-off-by: Matan Azrad <matan@mellanox.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ether/rte_ethdev.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 365540d3f..4ce990776 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -10,6 +10,7 @@
 #include <string.h>
 #include <stdarg.h>
 #include <errno.h>
+#include <stdbool.h>
 #include <stdint.h>
 #include <inttypes.h>
 #include <netinet/in.h>
@@ -222,6 +223,12 @@ rte_eth_dev_shared_data_prepare(void)
 	rte_spinlock_unlock(&rte_eth_shared_data_lock);
 }
 
+static bool
+is_allocated(const struct rte_eth_dev *ethdev)
+{
+	return ethdev->data->name[0] != '\0';
+}
+
 static struct rte_eth_dev *
 _rte_eth_dev_allocated(const char *name)
 {
@@ -424,10 +431,14 @@ static int
 _rte_eth_dev_owner_set(const uint16_t port_id, const uint64_t old_owner_id,
 		       const struct rte_eth_dev_owner *new_owner)
 {
+	struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
 	struct rte_eth_dev_owner *port_owner;
 	int sret;
 
-	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	if (port_id >= RTE_MAX_ETHPORTS || !is_allocated(ethdev)) {
+		RTE_PMD_DEBUG_TRACE("Port id %"PRIu16" is not allocated.\n", port_id);
+		return -ENODEV;
+	}
 
 	if (!rte_eth_is_valid_owner_id(new_owner->id) &&
 	    !rte_eth_is_valid_owner_id(old_owner_id))
@@ -498,9 +509,10 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
 
 	if (rte_eth_is_valid_owner_id(owner_id)) {
-		RTE_ETH_FOREACH_DEV_OWNED_BY(port_id, owner_id)
-			memset(&rte_eth_devices[port_id].data->owner, 0,
-			       sizeof(struct rte_eth_dev_owner));
+		for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++)
+			if (rte_eth_devices[port_id].data->owner.id == owner_id)
+				memset(&rte_eth_devices[port_id].data->owner, 0,
+				       sizeof(struct rte_eth_dev_owner));
 		RTE_PMD_DEBUG_TRACE("All port owners owned by %016"PRIX64
 				" identifier have removed.\n", owner_id);
 	}
@@ -512,17 +524,17 @@ int __rte_experimental
 rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
 {
 	int ret = 0;
+	struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
 
 	rte_eth_dev_shared_data_prepare();
 
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
 
-	if (!rte_eth_dev_is_valid_port(port_id)) {
-		RTE_PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+	if (port_id >= RTE_MAX_ETHPORTS || !is_allocated(ethdev)) {
+		RTE_PMD_DEBUG_TRACE("Port id %"PRIu16" is not allocated.\n", port_id);
 		ret = -ENODEV;
 	} else {
-		rte_memcpy(owner, &rte_eth_devices[port_id].data->owner,
-			   sizeof(*owner));
+		rte_memcpy(owner, &ethdev->data->owner, sizeof(*owner));
 	}
 
 	rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
-- 
2.16.2

[dpdk-stable] [PATCH 18.02 3/4] ethdev: fix port visibility before initialization

[Thomas Monjalon]

The port was set to the state ATTACHED during allocation.
The consequence was to iterate over ports which are not initialized.

The state ATTACHED is now set as the last step of probing.

The uniqueness of port name is now checked before the availability
of a port id for allocation (order reversed).

As the state is not set on allocation anymore, it is also not checked
in the function telling whether a port is allocated or not.
The name of the port is set on allocation, so it is enough as a check.

Fixes: 5588909af21b ("ethdev: add device iterator")

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Signed-off-by: Matan Azrad <matan@mellanox.com>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ether/rte_ethdev.c        | 17 +++++++++--------
 lib/librte_ether/rte_ethdev_driver.h |  2 ++
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 4ce990776..04a1e474e 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -235,7 +235,7 @@ _rte_eth_dev_allocated(const char *name)
 	unsigned i;
 
 	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
-		if ((rte_eth_devices[i].state == RTE_ETH_DEV_ATTACHED) &&
+		if (rte_eth_devices[i].data != NULL &&
 		    strcmp(rte_eth_devices[i].data->name, name) == 0)
 			return &rte_eth_devices[i];
 	}
@@ -280,7 +280,6 @@ eth_dev_get(uint16_t port_id)
 	struct rte_eth_dev *eth_dev = &rte_eth_devices[port_id];
 
 	eth_dev->data = &rte_eth_dev_shared_data->data[port_id];
-	eth_dev->state = RTE_ETH_DEV_ATTACHED;
 
 	eth_dev_last_created_port = port_id;
 
@@ -298,18 +297,18 @@ rte_eth_dev_allocate(const char *name)
 	/* Synchronize port creation between primary and secondary threads. */
 	rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
 
-	port_id = rte_eth_dev_find_free_port();
-	if (port_id == RTE_MAX_ETHPORTS) {
-		RTE_LOG(ERR, EAL, "Reached maximum number of Ethernet ports\n");
-		goto unlock;
-	}
-
 	if (_rte_eth_dev_allocated(name) != NULL) {
 		RTE_LOG(ERR, EAL, "Ethernet Device with name %s already allocated!\n",
 				name);
 		goto unlock;
 	}
 
+	port_id = rte_eth_dev_find_free_port();
+	if (port_id == RTE_MAX_ETHPORTS) {
+		RTE_LOG(ERR, EAL, "Reached maximum number of Ethernet ports\n");
+		goto unlock;
+	}
+
 	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;
@@ -3392,6 +3391,8 @@ rte_eth_dev_probing_finish(struct rte_eth_dev *dev)
 {
 	if (dev == NULL)
 		return;
+
+	dev->state = RTE_ETH_DEV_ATTACHED;
 }
 
 int
diff --git a/lib/librte_ether/rte_ethdev_driver.h b/lib/librte_ether/rte_ethdev_driver.h
index 77297f331..a0780cdcc 100644
--- a/lib/librte_ether/rte_ethdev_driver.h
+++ b/lib/librte_ether/rte_ethdev_driver.h
@@ -106,6 +106,8 @@ int _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
  * This is the last step of device probing.
  * It must be called after a port is allocated and initialized successfully.
  *
+ * The state is set as RTE_ETH_DEV_ATTACHED.
+ *
  * @param dev
  *  New ethdev port.
  */
-- 
2.16.2

[dpdk-stable] [PATCH 18.02 4/4] ethdev: fix port probing notification

[Thomas Monjalon]

The new device was notified as soon as it was allocated.
It leads to use a device which is not yet initialized.

The notification must be published after the initialization is done
by the PMD, but before the state is changed, in order to let
notified entities taking ownership before general availability.

Fixes: 29aa41e36de7 ("ethdev: add notifications for probing and removal")

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ether/rte_ethdev.c        | 5 ++---
 lib/librte_ether/rte_ethdev_driver.h | 2 ++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 04a1e474e..8ce5ec999 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -317,9 +317,6 @@ rte_eth_dev_allocate(const char *name)
 unlock:
 	rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
 
-	if (eth_dev != NULL)
-		_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_NEW, NULL);
-
 	return eth_dev;
 }
 
@@ -3392,6 +3389,8 @@ rte_eth_dev_probing_finish(struct rte_eth_dev *dev)
 	if (dev == NULL)
 		return;
 
+	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
+
 	dev->state = RTE_ETH_DEV_ATTACHED;
 }
 
diff --git a/lib/librte_ether/rte_ethdev_driver.h b/lib/librte_ether/rte_ethdev_driver.h
index a0780cdcc..710850dd2 100644
--- a/lib/librte_ether/rte_ethdev_driver.h
+++ b/lib/librte_ether/rte_ethdev_driver.h
@@ -106,6 +106,8 @@ int _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
  * This is the last step of device probing.
  * It must be called after a port is allocated and initialized successfully.
  *
+ * The notification RTE_ETH_EVENT_NEW is sent to other entities
+ * (libraries and applications).
  * The state is set as RTE_ETH_DEV_ATTACHED.
  *
  * @param dev
-- 
2.16.2

Re: [dpdk-stable] [PATCH 18.02 0/4] ethdev: fix iterator and probing notification

[Luca Boccassi]

On Sun, 2018-05-20 at 13:00 +0200, Thomas Monjalon wrote:
> These are the remaining patches from the series
>   "ethdev: fix race conditions in iterator and notifications"
> which were not yet backported in 18.02.
> 
> Please move the commit "net/failsafe: fix sub-device ownership race",
> which was already backported in 18.02 branch,
> on top of these patches, as there is a dependency on notification
> fix.
> Thanks
> 
> 
> Matan Azrad (1):
>   ethdev: allow ownership operations on unused port
> 
> Thomas Monjalon (3):
>   ethdev: add probing finish function
>   ethdev: fix port visibility before initialization
>   ethdev: fix port probing notification
> 
>  drivers/net/af_packet/rte_eth_af_packet.c |  1 +
>  drivers/net/ark/ark_ethdev.c              |  2 ++
>  drivers/net/bonding/rte_eth_bond_pmd.c    |  1 +
>  drivers/net/cxgbe/cxgbe_ethdev.c          |  1 +
>  drivers/net/cxgbe/cxgbe_main.c            |  5 +++
>  drivers/net/dpaa/dpaa_ethdev.c            |  5 ++-
>  drivers/net/dpaa2/dpaa2_ethdev.c          |  4 ++-
>  drivers/net/failsafe/failsafe.c           |  1 +
>  drivers/net/kni/rte_eth_kni.c             |  1 +
>  drivers/net/mlx4/mlx4.c                   |  1 +
>  drivers/net/mlx5/mlx5.c                   |  2 ++
>  drivers/net/mrvl/mrvl_ethdev.c            |  1 +
>  drivers/net/nfp/nfp_net.c                 |  2 ++
>  drivers/net/null/rte_eth_null.c           |  1 +
>  drivers/net/octeontx/octeontx_ethdev.c    |  2 ++
>  drivers/net/pcap/rte_eth_pcap.c           |  1 +
>  drivers/net/ring/rte_eth_ring.c           |  1 +
>  drivers/net/softnic/rte_eth_softnic.c     |  2 ++
>  drivers/net/tap/rte_eth_tap.c             |  1 +
>  drivers/net/vhost/rte_eth_vhost.c         |  1 +
>  drivers/net/virtio/virtio_user_ethdev.c   |  3 ++
>  lib/librte_ether/rte_ethdev.c             | 57 ++++++++++++++++++++-
> ----------
>  lib/librte_ether/rte_ethdev_driver.h      | 14 ++++++++
>  lib/librte_ether/rte_ethdev_pci.h         |  2 ++
>  lib/librte_ether/rte_ethdev_version.map   |  1 +
>  test/test/virtual_pmd.c                   |  2 ++
>  26 files changed, 94 insertions(+), 21 deletions(-)

Thanks, applied and pushed to dpdk-stable/18.02 with minor correction
to the first patch (build issue)

-- 
Kind regards,
Luca Boccassi