[v7,2/5] ethdev: fix skip valid port in probing callback

Message ID 20240130063648.32131-3-lihuisong@huawei.com (mailing list archive)
State New
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: support multiple process attach and detach port |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

lihuisong (C) Jan. 30, 2024, 6:36 a.m. UTC
  The event callback in application may use the macro RTE_ETH_FOREACH_DEV to
iterate over all enabled ports to do something(like, verifying the port id
validity) when receive a probing event. If the ethdev state of a port is
not RTE_ETH_DEV_UNUSED, this port will be considered as a valid port.

However, this state is set to RTE_ETH_DEV_ATTACHED after pushing probing
event. It means that probing callback will skip this port. But this
assignment can not move to front of probing notification. See
commit be8cd210379a ("ethdev: fix port probing notification")

So this patch has to add a new state, RTE_ETH_DEV_ALLOCATED. Set the ethdev
state to RTE_ETH_DEV_ALLOCATED before pushing probing event and set it to
RTE_ETH_DEV_ATTACHED after definitely probed. And this port is valid if its
device state is 'ALLOCATED' or 'ATTACHED'.

In addition, the new state has to be placed behind 'REMOVED' to avoid ABI
break. Fortunately, this ethdev state is internal and applications can not
access it directly. So this patch encapsulates an API, rte_eth_dev_is_used,
for ethdev or PMD to call and eliminate concerns about using this state
enum value comparison.

Fixes: be8cd210379a ("ethdev: fix port probing notification")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/net/bnxt/bnxt_ethdev.c |  3 ++-
 drivers/net/mlx5/mlx5.c        |  2 +-
 lib/ethdev/ethdev_driver.c     | 13 ++++++++++---
 lib/ethdev/ethdev_driver.h     | 12 ++++++++++++
 lib/ethdev/ethdev_pci.h        |  2 +-
 lib/ethdev/rte_class_eth.c     |  2 +-
 lib/ethdev/rte_ethdev.c        |  4 ++--
 lib/ethdev/rte_ethdev.h        |  4 +++-
 lib/ethdev/version.map         |  1 +
 9 files changed, 33 insertions(+), 10 deletions(-)
  

Patch

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index acf7e6e46e..cb4c4cfb0e 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -6109,7 +6109,8 @@  bnxt_dev_uninit(struct rte_eth_dev *eth_dev)
 
 	PMD_DRV_LOG(DEBUG, "Calling Device uninit\n");
 
-	if (eth_dev->state != RTE_ETH_DEV_UNUSED)
+
+	if (rte_eth_dev_is_used(eth_dev->state))
 		bnxt_dev_close_op(eth_dev);
 
 	return 0;
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 3a182de248..1f1d9b7d96 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -3258,7 +3258,7 @@  mlx5_eth_find_next(uint16_t port_id, struct rte_device *odev)
 	while (port_id < RTE_MAX_ETHPORTS) {
 		struct rte_eth_dev *dev = &rte_eth_devices[port_id];
 
-		if (dev->state != RTE_ETH_DEV_UNUSED &&
+		if (rte_eth_dev_is_used(dev->state) &&
 		    dev->device &&
 		    (dev->device == odev ||
 		     (dev->device->driver &&
diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
index bd917a15fc..2b4a6e2b2c 100644
--- a/lib/ethdev/ethdev_driver.c
+++ b/lib/ethdev/ethdev_driver.c
@@ -52,8 +52,8 @@  eth_dev_find_free_port(void)
 	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
 		/* Using shared name field to find a free port. */
 		if (eth_dev_shared_data->data[i].name[0] == '\0') {
-			RTE_ASSERT(rte_eth_devices[i].state ==
-				   RTE_ETH_DEV_UNUSED);
+			RTE_ASSERT(!rte_eth_dev_is_used(
+					rte_eth_devices[i].state));
 			return i;
 		}
 	}
@@ -217,11 +217,18 @@  rte_eth_dev_probing_finish(struct rte_eth_dev *dev)
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
 		eth_dev_fp_ops_setup(rte_eth_fp_ops + dev->data->port_id, dev);
 
+	dev->state = RTE_ETH_DEV_ALLOCATED;
 	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
 
 	dev->state = RTE_ETH_DEV_ATTACHED;
 }
 
+bool rte_eth_dev_is_used(uint16_t dev_state)
+{
+	return dev_state == RTE_ETH_DEV_ALLOCATED ||
+		dev_state == RTE_ETH_DEV_ATTACHED;
+}
+
 int
 rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
 {
@@ -239,7 +246,7 @@  rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
 	if (ret != 0)
 		return ret;
 
-	if (eth_dev->state != RTE_ETH_DEV_UNUSED)
+	if (rte_eth_dev_is_used(eth_dev->state))
 		rte_eth_dev_callback_process(eth_dev,
 				RTE_ETH_EVENT_DESTROY, NULL);
 
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index f05f68a67c..238bd72ab0 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1596,6 +1596,18 @@  int rte_eth_dev_callback_process(struct rte_eth_dev *dev,
 __rte_internal
 void rte_eth_dev_probing_finish(struct rte_eth_dev *dev);
 
+/**
+ * Check if a Ethernet device state is used or not
+ *
+ * @param dev_state
+ *   The state of the Ethernet device
+ * @return
+ *   - true if the state of the Ethernet device is allocated or attached
+ *   - false if this state is neither allocated nor attached
+ */
+__rte_internal
+bool rte_eth_dev_is_used(uint16_t dev_state);
+
 /**
  * Create memzone for HW rings.
  * malloc can't be used as the physical address is needed.
diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
index 737fff1833..c07a98e04f 100644
--- a/lib/ethdev/ethdev_pci.h
+++ b/lib/ethdev/ethdev_pci.h
@@ -165,7 +165,7 @@  rte_eth_dev_pci_generic_remove(struct rte_pci_device *pci_dev,
 	 * eth device has been released.
 	 */
 	if (rte_eal_process_type() == RTE_PROC_SECONDARY &&
-	    eth_dev->state == RTE_ETH_DEV_UNUSED)
+	    !rte_eth_dev_is_used(eth_dev->state))
 		return 0;
 
 	if (dev_uninit) {
diff --git a/lib/ethdev/rte_class_eth.c b/lib/ethdev/rte_class_eth.c
index bc003db8af..31cabee525 100644
--- a/lib/ethdev/rte_class_eth.c
+++ b/lib/ethdev/rte_class_eth.c
@@ -118,7 +118,7 @@  eth_dev_match(const struct rte_eth_dev *edev,
 	const struct rte_kvargs *kvlist = arg->kvlist;
 	unsigned int pair;
 
-	if (edev->state == RTE_ETH_DEV_UNUSED)
+	if (!rte_eth_dev_is_used(edev->state))
 		return -1;
 	if (arg->device != NULL && arg->device != edev->device)
 		return -1;
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f61dc7dd7b..b6b7a20e35 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -349,7 +349,7 @@  uint16_t
 rte_eth_find_next(uint16_t port_id)
 {
 	while (port_id < RTE_MAX_ETHPORTS &&
-			rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED)
+	       !rte_eth_dev_is_used(rte_eth_devices[port_id].state))
 		port_id++;
 
 	if (port_id >= RTE_MAX_ETHPORTS)
@@ -408,7 +408,7 @@  rte_eth_dev_is_valid_port(uint16_t port_id)
 	int is_valid;
 
 	if (port_id >= RTE_MAX_ETHPORTS ||
-	    (rte_eth_devices[port_id].state == RTE_ETH_DEV_UNUSED))
+	    !rte_eth_dev_is_used(rte_eth_devices[port_id].state))
 		is_valid = 0;
 	else
 		is_valid = 1;
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 2687c23fa6..5079fea8a6 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2068,10 +2068,12 @@  typedef uint16_t (*rte_tx_callback_fn)(uint16_t port_id, uint16_t queue,
 enum rte_eth_dev_state {
 	/** Device is unused before being probed. */
 	RTE_ETH_DEV_UNUSED = 0,
-	/** Device is attached when allocated in probing. */
+	/** Device is attached when definitely probed. */
 	RTE_ETH_DEV_ATTACHED,
 	/** Device is in removed state when plug-out is detected. */
 	RTE_ETH_DEV_REMOVED,
+	/** Device is allocated and is set before reporting new event. */
+	RTE_ETH_DEV_ALLOCATED,
 };
 
 struct rte_eth_dev_sriov {
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index a050baab0f..26867eb6e5 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -333,6 +333,7 @@  INTERNAL {
 	rte_eth_dev_get_by_name;
 	rte_eth_dev_is_rx_hairpin_queue;
 	rte_eth_dev_is_tx_hairpin_queue;
+	rte_eth_dev_is_used;
 	rte_eth_dev_probing_finish;
 	rte_eth_dev_release_port;
 	rte_eth_dev_internal_reset;