For each sub-device, there's a remove flag that's used to indicate if
the sub-device has been removed or not. This flag is currently a
volatile bit field. Even if it's marked as volatile, a bit field is not
thread-safe according to the C standard.
The remove flag is now an unsigned int rather than a bit field. We also
now use the atomic built-ins to read/write the value of the remove flag
to ensure thread-safety.
Fixes: 598fb8aec6f6 ("net/failsafe: support device removal")
Cc: Gaetan Rivet <gr...@u256.net>
Cc: sta...@dpdk.org
Signed-off-by: Luc Pelletier <lucp.at.w...@gmail.com>
---
drivers/net/failsafe/failsafe_ether.c | 8 ++++----
drivers/net/failsafe/failsafe_ops.c | 2 +-
drivers/net/failsafe/failsafe_private.h | 4 ++--
drivers/net/failsafe/failsafe_rxtx.c | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/failsafe/failsafe_ether.c
b/drivers/net/failsafe/failsafe_ether.c
index 3bfc446638..9014777e52 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -311,7 +311,7 @@ fs_dev_remove(struct sub_device *sdev)
/* the end */
break;
}
- sdev->remove = 0;
+ __atomic_store_n(&sdev->remove, 0, __ATOMIC_RELEASE);
failsafe_hotplug_alarm_install(fs_dev(sdev));
}
@@ -388,7 +388,7 @@ failsafe_dev_remove(struct rte_eth_dev *dev)
uint8_t i;
FOREACH_SUBDEV(sdev, i, dev) {
- if (!sdev->remove)
+ if (!__atomic_load_n(&sdev->remove, __ATOMIC_ACQUIRE))
continue;
/* Active devices must have finished their burst and
@@ -556,7 +556,7 @@ failsafe_eth_dev_state_sync(struct rte_eth_dev *dev)
err_remove:
FOREACH_SUBDEV(sdev, i, dev)
if (sdev->state != PRIV(dev)->state)
- sdev->remove = 1;
+ __atomic_store_n(&sdev->remove, 1, __ATOMIC_RELEASE);
return ret;
}
@@ -597,7 +597,7 @@ failsafe_eth_rmv_event_callback(uint16_t port_id
__rte_unused,
* Async removal, the sub-PMD will try to unregister
* the callback at the source of the current thread context.
*/
- sdev->remove = 1;
+ __atomic_store_n(&sdev->remove, 1, __ATOMIC_RELEASE);
fs_unlock(fs_dev(sdev), 0);
return 0;
}
diff --git a/drivers/net/failsafe/failsafe_ops.c
b/drivers/net/failsafe/failsafe_ops.c
index b66dfa269b..e65c8a261a 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -837,7 +837,7 @@ fs_link_update(struct rte_eth_dev *dev,
if (likely(sdev_link_ret == 0)) {
if (TX_SUBDEV(dev) == sdev)
ret = rte_eth_linkstatus_set(dev, &link_info);
- } else if (sdev->remove == 0 &&
+ } else if (__atomic_load_n(&sdev->remove, __ATOMIC_ACQUIRE) ==
0 &&
rte_eth_dev_is_removed(PORT_ID(sdev)) == 0) {
ERROR("Link get failed for sub_device %d with error %d",
i, sdev_link_ret);
diff --git a/drivers/net/failsafe/failsafe_private.h
b/drivers/net/failsafe/failsafe_private.h
index 3865f2fc34..0577a62ab2 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -131,7 +131,7 @@ struct sub_device {
/* sub device port id*/
uint16_t sdev_port_id; /* shared between processes */
/* flag calling for recollection */
- volatile unsigned int remove:1;
+ unsigned int remove;
/* flow isolation state */
int flow_isolated:1;
/* RMV callback registration state */
@@ -490,7 +490,7 @@ static inline int
fs_err(struct sub_device *sdev, int err)
{
/* A device removal shouldn't be reported as an error. */
- if (sdev->remove == 1 || err == -EIO)
+ if (__atomic_load_n(&sdev->remove, __ATOMIC_ACQUIRE) == 1 || err ==
-EIO)
return rte_errno = 0;
return err;
}
diff --git a/drivers/net/failsafe/failsafe_rxtx.c
b/drivers/net/failsafe/failsafe_rxtx.c
index 6d8ab0a6e7..c5caab4204 100644
--- a/drivers/net/failsafe/failsafe_rxtx.c
+++ b/drivers/net/failsafe/failsafe_rxtx.c
@@ -16,7 +16,7 @@ fs_rx_unsafe(struct sub_device *sdev)
return (ETH(sdev) == NULL) ||
(ETH(sdev)->rx_pkt_burst == NULL) ||
(sdev->state != DEV_STARTED) ||
- (sdev->remove != 0);
+ (__atomic_load_n(&sdev->remove, __ATOMIC_ACQUIRE) != 0);
}
static inline int
--
2.38.1
