From: Huisong Li <lihuisong@huawei.com>
To: <dev@dpdk.org>, <stephen@networkplumber.org>,
<thomas@monjalon.net>, <ferruh.yigit@amd.com>,
Ajit Khaparde <ajit.khaparde@broadcom.com>,
"Somnath Kotur" <somnath.kotur@broadcom.com>,
Praveen Shetty <praveen.shetty@intel.com>,
Andrew Boyer <andrew.boyer@amd.com>,
"Dariusz Sosnowski" <dsosnowski@nvidia.com>,
Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
Bing Zhao <bingz@nvidia.com>, Ori Kam <orika@nvidia.com>,
Suanming Mou <suanmingm@nvidia.com>,
Matan Azrad <matan@nvidia.com>,
Chaoyong He <chaoyong.he@corigine.com>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Cc: <fengchengwen@huawei.com>, <liuyonglong@huawei.com>,
<lihuisong@huawei.com>
Subject: [PATCH v1 2/2] ethdev: fix skip valid port in probing callback
Date: Mon, 13 Jan 2025 10:55:21 +0800 [thread overview]
Message-ID: <20250113025521.32703-3-lihuisong@huawei.com> (raw)
In-Reply-To: <20250113025521.32703-1-lihuisong@huawei.com>
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 | 2 +-
drivers/net/cpfl/cpfl_ethdev.h | 2 +-
drivers/net/ionic/ionic_ethdev.c | 2 +-
drivers/net/mlx5/mlx5.c | 2 +-
drivers/net/nfp/nfp_ethdev.c | 4 ++--
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 +
12 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index ef8a928c91..1441194b85 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -6706,7 +6706,7 @@ bnxt_dev_uninit(struct rte_eth_dev *eth_dev)
PMD_DRV_LOG_LINE(DEBUG, "Calling Device uninit");
- 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/cpfl/cpfl_ethdev.h b/drivers/net/cpfl/cpfl_ethdev.h
index 9a38a69194..aad05aafd6 100644
--- a/drivers/net/cpfl/cpfl_ethdev.h
+++ b/drivers/net/cpfl/cpfl_ethdev.h
@@ -328,7 +328,7 @@ cpfl_get_itf_by_port_id(uint16_t port_id)
}
dev = &rte_eth_devices[port_id];
- if (dev->state == RTE_ETH_DEV_UNUSED) {
+ if (!rte_eth_dev_is_used(dev->state)) {
PMD_DRV_LOG(ERR, "eth_dev[%d] is unused.", port_id);
return NULL;
}
diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index aa22b6a70d..2a4e565c4f 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -1109,7 +1109,7 @@ eth_ionic_dev_uninit(struct rte_eth_dev *eth_dev)
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
- if (eth_dev->state != RTE_ETH_DEV_UNUSED)
+ if (rte_eth_dev_is_used(eth_dev->state))
ionic_dev_close(eth_dev);
eth_dev->dev_ops = NULL;
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 6e4473e2f4..642e762868 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -3376,7 +3376,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/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index df5482f74a..dae4594e56 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -754,11 +754,11 @@ nfp_net_close(struct rte_eth_dev *dev)
/*
* In secondary process, a released eth device can be found by its name
* in shared memory.
- * If the state of the eth device is RTE_ETH_DEV_UNUSED, it means the
+ * If the state of the eth device isn't used, it means the
* eth device has been released.
*/
if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
- if (dev->state == RTE_ETH_DEV_UNUSED)
+ if (!rte_eth_dev_is_used(dev->state))
return 0;
nfp_pf_secondary_uninit(hw_priv);
diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
index 9afef06431..5537c2f7af 100644
--- a/lib/ethdev/ethdev_driver.c
+++ b/lib/ethdev/ethdev_driver.c
@@ -55,8 +55,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;
}
}
@@ -221,11 +221,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)
{
@@ -243,7 +250,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 1fd4562b40..dc496daf05 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1754,6 +1754,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 2229ffa252..1e62f30d8d 100644
--- a/lib/ethdev/ethdev_pci.h
+++ b/lib/ethdev/ethdev_pci.h
@@ -179,7 +179,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 a8d01e2595..f343c4b6eb 100644
--- a/lib/ethdev/rte_class_eth.c
+++ b/lib/ethdev/rte_class_eth.c
@@ -120,7 +120,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 6413c54e3b..3d7a3c39d3 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 1f71cad244..f9a72b9883 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2091,10 +2091,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 12f48c70a0..45b982e98d 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -351,6 +351,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;
--
2.22.0
next prev parent reply other threads:[~2025-01-13 3:07 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-13 2:55 [PATCH v1 0/2] " Huisong Li
2025-01-13 2:55 ` [PATCH v1 1/2] app/testpmd: check the validity of the port Huisong Li
2025-01-13 2:55 ` Huisong Li [this message]
2025-01-13 8:16 ` [PATCH v1 2/2] ethdev: fix skip valid port in probing callback Thomas Monjalon
2025-01-13 9:35 ` lihuisong (C)
2025-01-13 10:57 ` Thomas Monjalon
2025-01-13 11:23 ` lihuisong (C)
2025-01-13 12:05 ` lihuisong (C)
2025-01-13 12:30 ` Thomas Monjalon
2025-01-13 12:47 ` lihuisong (C)
2025-01-13 13:14 ` Thomas Monjalon
2025-01-14 1:50 ` lihuisong (C)
2025-01-14 11:13 ` Thomas Monjalon
2025-01-14 12:13 ` lihuisong (C)
2025-01-14 12:39 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250113025521.32703-3-lihuisong@huawei.com \
--to=lihuisong@huawei.com \
--cc=ajit.khaparde@broadcom.com \
--cc=andrew.boyer@amd.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=bingz@nvidia.com \
--cc=chaoyong.he@corigine.com \
--cc=dev@dpdk.org \
--cc=dsosnowski@nvidia.com \
--cc=fengchengwen@huawei.com \
--cc=ferruh.yigit@amd.com \
--cc=liuyonglong@huawei.com \
--cc=matan@nvidia.com \
--cc=orika@nvidia.com \
--cc=praveen.shetty@intel.com \
--cc=somnath.kotur@broadcom.com \
--cc=stephen@networkplumber.org \
--cc=suanmingm@nvidia.com \
--cc=thomas@monjalon.net \
--cc=viacheslavo@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).