* [Patch 22.11] net/mana: fix multi-process tracking
@ 2025-03-12 22:33 longli
2025-03-13 0:06 ` Luca Boccassi
0 siblings, 1 reply; 2+ messages in thread
From: longli @ 2025-03-12 22:33 UTC (permalink / raw)
To: stable; +Cc: Long Li
From: Long Li <longli@microsoft.com>
[ upstream commit 57aa3ec91ecf13ab2f11e4dc0dc74c50a2afa0cc ]
The driver uses mana_shared_data for tracking usage count for primary
process. This is not correct as the mana_shared_data is allocated
by the primary and is meant to track usage of secondary process by the
primary process. And it creates a race condition when the device is
removed because the counter is no longer available if this shared
memory is freed.
Move the usage count tracking to mana_local_data and fix the race
condition in mana_pci_remove().
Fixes: 517ed6e2d590 ("net/mana: add basic driver with build environment")
Cc: stable@dpdk.org
Signed-off-by: Long Li <longli@microsoft.com>
---
drivers/net/mana/mana.c | 100 ++++++++++++++++++++++------------------
drivers/net/mana/mana.h | 6 +--
drivers/net/mana/mp.c | 2 +-
3 files changed, 58 insertions(+), 50 deletions(-)
diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index eb3b734949..57325eb528 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -20,9 +20,14 @@
#include "mana.h"
/* Shared memory between primary/secondary processes, per driver */
-/* Data to track primary/secondary usage */
struct mana_shared_data *mana_shared_data;
-static struct mana_shared_data mana_local_data;
+
+/* Local data to track device instance usage for primary/secondary processes */
+static struct mana_local_data {
+ int init_done;
+ unsigned int primary_cnt;
+ unsigned int secondary_cnt;
+} mana_local_data;
/* The memory region for the above data */
static const struct rte_memzone *mana_shared_mz;
@@ -1068,8 +1073,12 @@ mana_init_shared_data(void)
rte_spinlock_lock(&mana_shared_data_lock);
/* Skip if shared data is already initialized */
- if (mana_shared_data)
+ if (mana_shared_data) {
+ DRV_LOG(INFO, "shared data is already initialized");
goto exit;
+ }
+
+ memset(&mana_local_data, 0, sizeof(mana_local_data));
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
mana_shared_mz = rte_memzone_reserve(MZ_MANA_SHARED_DATA,
@@ -1082,8 +1091,7 @@ mana_init_shared_data(void)
}
mana_shared_data = mana_shared_mz->addr;
- memset(mana_shared_data, 0, sizeof(*mana_shared_data));
- rte_spinlock_init(&mana_shared_data->lock);
+ rte_atomic32_set(&mana_shared_data->secondary_cnt, 0);
} else {
secondary_mz = rte_memzone_lookup(MZ_MANA_SHARED_DATA);
if (!secondary_mz) {
@@ -1093,7 +1101,6 @@ mana_init_shared_data(void)
}
mana_shared_data = secondary_mz->addr;
- memset(&mana_local_data, 0, sizeof(mana_local_data));
}
exit:
@@ -1114,11 +1121,11 @@ mana_init_once(void)
if (ret)
return ret;
- rte_spinlock_lock(&mana_shared_data->lock);
+ rte_spinlock_lock(&mana_shared_data_lock);
switch (rte_eal_process_type()) {
case RTE_PROC_PRIMARY:
- if (mana_shared_data->init_done)
+ if (mana_local_data.init_done)
break;
ret = mana_mp_init_primary();
@@ -1126,7 +1133,7 @@ mana_init_once(void)
break;
DRV_LOG(ERR, "MP INIT PRIMARY");
- mana_shared_data->init_done = 1;
+ mana_local_data.init_done = 1;
break;
case RTE_PROC_SECONDARY:
@@ -1149,7 +1156,7 @@ mana_init_once(void)
break;
}
- rte_spinlock_unlock(&mana_shared_data->lock);
+ rte_spinlock_unlock(&mana_shared_data_lock);
return ret;
}
@@ -1220,11 +1227,6 @@ mana_probe_port(struct ibv_device *ibdev, struct ibv_device_attr_ex *dev_attr,
eth_dev->tx_pkt_burst = mana_tx_burst_removed;
eth_dev->rx_pkt_burst = mana_rx_burst_removed;
- rte_spinlock_lock(&mana_shared_data->lock);
- mana_shared_data->secondary_cnt++;
- mana_local_data.secondary_cnt++;
- rte_spinlock_unlock(&mana_shared_data->lock);
-
rte_eth_copy_pci_info(eth_dev, pci_dev);
rte_eth_dev_probing_finish(eth_dev);
@@ -1307,10 +1309,6 @@ mana_probe_port(struct ibv_device *ibdev, struct ibv_device_attr_ex *dev_attr,
goto failed;
}
- rte_spinlock_lock(&mana_shared_data->lock);
- mana_shared_data->primary_cnt++;
- rte_spinlock_unlock(&mana_shared_data->lock);
-
eth_dev->device = &pci_dev->device;
DRV_LOG(INFO, "device %s at port %u", name, eth_dev->data->port_id);
@@ -1456,13 +1454,37 @@ mana_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
count = mana_pci_probe_mac(pci_dev, NULL);
}
+ /* If no device is found, clean up resources if this is the last one */
if (!count) {
- rte_memzone_free(mana_shared_mz);
- mana_shared_mz = NULL;
- ret = -ENODEV;
+ rte_spinlock_lock(&mana_shared_data_lock);
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ if (!mana_local_data.primary_cnt) {
+ mana_mp_uninit_primary();
+ rte_memzone_free(mana_shared_mz);
+ mana_shared_mz = NULL;
+ mana_shared_data = NULL;
+ }
+ } else {
+ if (!mana_local_data.secondary_cnt) {
+ mana_mp_uninit_secondary();
+ mana_shared_data = NULL;
+ }
+ }
+ rte_spinlock_unlock(&mana_shared_data_lock);
+ return -ENODEV;
}
- return ret;
+ /* At least one eth_dev is probed, increase counter for shared data */
+ rte_spinlock_lock(&mana_shared_data_lock);
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ mana_local_data.primary_cnt++;
+ } else {
+ rte_atomic32_add(&mana_shared_data->secondary_cnt, 1);
+ mana_local_data.secondary_cnt++;
+ }
+ rte_spinlock_unlock(&mana_shared_data_lock);
+
+ return 0;
}
static int
@@ -1477,45 +1499,35 @@ mana_dev_uninit(struct rte_eth_dev *dev)
static int
mana_pci_remove(struct rte_pci_device *pci_dev)
{
+ rte_spinlock_lock(&mana_shared_data_lock);
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
- rte_spinlock_lock(&mana_shared_data_lock);
-
- rte_spinlock_lock(&mana_shared_data->lock);
+ RTE_VERIFY(mana_local_data.primary_cnt > 0);
+ mana_local_data.primary_cnt--;
- RTE_VERIFY(mana_shared_data->primary_cnt > 0);
- mana_shared_data->primary_cnt--;
- if (!mana_shared_data->primary_cnt) {
+ if (!mana_local_data.primary_cnt) {
DRV_LOG(DEBUG, "mp uninit primary");
mana_mp_uninit_primary();
- }
-
- rte_spinlock_unlock(&mana_shared_data->lock);
- /* Also free the shared memory if this is the last */
- if (!mana_shared_data->primary_cnt) {
+ /* Also free the shared memory if this is the last */
DRV_LOG(DEBUG, "free shared memezone data");
rte_memzone_free(mana_shared_mz);
mana_shared_mz = NULL;
+ mana_shared_data = NULL;
}
-
- rte_spinlock_unlock(&mana_shared_data_lock);
} else {
- rte_spinlock_lock(&mana_shared_data_lock);
-
- rte_spinlock_lock(&mana_shared_data->lock);
- RTE_VERIFY(mana_shared_data->secondary_cnt > 0);
- mana_shared_data->secondary_cnt--;
- rte_spinlock_unlock(&mana_shared_data->lock);
+ RTE_VERIFY(rte_atomic32_read(&mana_shared_data->secondary_cnt)
+ > 0);
+ rte_atomic32_sub(&mana_shared_data->secondary_cnt, 1);
RTE_VERIFY(mana_local_data.secondary_cnt > 0);
mana_local_data.secondary_cnt--;
if (!mana_local_data.secondary_cnt) {
DRV_LOG(DEBUG, "mp uninit secondary");
mana_mp_uninit_secondary();
+ mana_shared_data = NULL;
}
-
- rte_spinlock_unlock(&mana_shared_data_lock);
}
+ rte_spinlock_unlock(&mana_shared_data_lock);
return rte_eth_dev_pci_generic_remove(pci_dev, mana_dev_uninit);
}
diff --git a/drivers/net/mana/mana.h b/drivers/net/mana/mana.h
index 57576c62e4..579e6063ab 100644
--- a/drivers/net/mana/mana.h
+++ b/drivers/net/mana/mana.h
@@ -8,12 +8,8 @@
#define PCI_VENDOR_ID_MICROSOFT 0x1414
#define PCI_DEVICE_ID_MICROSOFT_MANA 0x00ba
-/* Shared data between primary/secondary processes */
struct mana_shared_data {
- rte_spinlock_t lock;
- int init_done;
- unsigned int primary_cnt;
- unsigned int secondary_cnt;
+ rte_atomic32_t secondary_cnt;
};
#define MIN_RX_BUF_SIZE 1024
diff --git a/drivers/net/mana/mp.c b/drivers/net/mana/mp.c
index 738487f65a..c748fc6159 100644
--- a/drivers/net/mana/mp.c
+++ b/drivers/net/mana/mp.c
@@ -305,7 +305,7 @@ mana_mp_req_on_rxtx(struct rte_eth_dev *dev, enum mana_mp_req_type type)
return;
}
- if (!mana_shared_data->secondary_cnt)
+ if (rte_atomic32_read(&mana_shared_data->secondary_cnt) == 0)
return;
mp_init_msg(&mp_req, type, dev->data->port_id);
--
2.34.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Patch 22.11] net/mana: fix multi-process tracking
2025-03-12 22:33 [Patch 22.11] net/mana: fix multi-process tracking longli
@ 2025-03-13 0:06 ` Luca Boccassi
0 siblings, 0 replies; 2+ messages in thread
From: Luca Boccassi @ 2025-03-13 0:06 UTC (permalink / raw)
To: longli; +Cc: stable
On Wed, 12 Mar 2025 at 22:33, <longli@linuxonhyperv.com> wrote:
>
> From: Long Li <longli@microsoft.com>
>
> [ upstream commit 57aa3ec91ecf13ab2f11e4dc0dc74c50a2afa0cc ]
>
> The driver uses mana_shared_data for tracking usage count for primary
> process. This is not correct as the mana_shared_data is allocated
> by the primary and is meant to track usage of secondary process by the
> primary process. And it creates a race condition when the device is
> removed because the counter is no longer available if this shared
> memory is freed.
>
> Move the usage count tracking to mana_local_data and fix the race
> condition in mana_pci_remove().
>
> Fixes: 517ed6e2d590 ("net/mana: add basic driver with build environment")
> Cc: stable@dpdk.org
>
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> drivers/net/mana/mana.c | 100 ++++++++++++++++++++++------------------
> drivers/net/mana/mana.h | 6 +--
> drivers/net/mana/mp.c | 2 +-
> 3 files changed, 58 insertions(+), 50 deletions(-)
Thanks, applied
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-03-13 0:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-12 22:33 [Patch 22.11] net/mana: fix multi-process tracking longli
2025-03-13 0:06 ` Luca Boccassi
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).