[dpdk-dev] [PATCH v6 3/3] net/failsafe: fix hotplug races
Matan Azrad
matan at mellanox.com
Sun Feb 11 18:24:32 CET 2018
Fail-safe uses periodic alarm mechanism, running from the host thread,
to manage the hot-plug events of its sub-devices. This management
requires a lot of sub-devices PMDs operations (stop,close,start,etc).
While the hot-plug alarm runs in the host thread, the application may
call fail-safe operations which directly trigger the sub-devices PMDs
operations too, This call may occur from any thread decided by the
application (probably the master thread).
So, more than one operation can execute to a sub-device in same time
what can cause a lot of races in the sub-PMDs.
Moreover, some control operations update the fail-safe internal
databases which can be used by the alarm mechanism in the same
time, what also can cause to races and crashes.
Fail-safe is the owner of its sub-devices and must to synchronize their
use according to the ETHDEV ownership rules.
Synchronize hot-plug management by a new lock mechanism uses a mutex to
atomically defend each critical section in the fail-safe hot-plug
mechanism and control operations to prevent any races between them.
Fixes: a46f8d5 ("net/failsafe: add fail-safe PMD")
Cc: stable at dpdk.org
Signed-off-by: Matan Azrad <matan at mellanox.com>
---
drivers/net/failsafe/Makefile | 1 +
drivers/net/failsafe/failsafe.c | 35 ++++++++
drivers/net/failsafe/failsafe_ether.c | 6 +-
drivers/net/failsafe/failsafe_flow.c | 20 ++++-
drivers/net/failsafe/failsafe_ops.c | 148 ++++++++++++++++++++++++++------
drivers/net/failsafe/failsafe_private.h | 62 +++++++++++--
6 files changed, 239 insertions(+), 33 deletions(-)
diff --git a/drivers/net/failsafe/Makefile b/drivers/net/failsafe/Makefile
index d1ae899..bd2f019 100644
--- a/drivers/net/failsafe/Makefile
+++ b/drivers/net/failsafe/Makefile
@@ -68,5 +68,6 @@ CFLAGS += -pedantic
LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs
LDLIBS += -lrte_bus_vdev
+LDLIBS += -lpthread
include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 7b2cdbb..c499bfb 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -113,17 +113,46 @@
break;
/* if we have non-probed device */
if (i != PRIV(dev)->subs_tail) {
+ if (fs_lock(dev, 1) != 0)
+ goto reinstall;
ret = failsafe_eth_dev_state_sync(dev);
+ fs_unlock(dev, 1);
if (ret)
ERROR("Unable to synchronize sub_device state");
}
failsafe_dev_remove(dev);
+reinstall:
ret = failsafe_hotplug_alarm_install(dev);
if (ret)
ERROR("Unable to set up next alarm");
}
static int
+fs_mutex_init(struct fs_priv *priv)
+{
+ int ret;
+ pthread_mutexattr_t attr;
+
+ ret = pthread_mutexattr_init(&attr);
+ if (ret) {
+ ERROR("Cannot initiate mutex attributes - %s", strerror(ret));
+ return ret;
+ }
+ /* Allow mutex relocks for the thread holding the mutex. */
+ ret = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
+ if (ret) {
+ ERROR("Cannot set mutex type - %s", strerror(ret));
+ return ret;
+ }
+ ret = pthread_mutex_init(&priv->hotplug_mutex, &attr);
+ if (ret) {
+ ERROR("Cannot initiate mutex - %s", strerror(ret));
+ return ret;
+ }
+ return 0;
+}
+
+static int
fs_eth_dev_create(struct rte_vdev_device *vdev)
{
struct rte_eth_dev *dev;
@@ -176,6 +205,9 @@
ret = failsafe_eal_init(dev);
if (ret)
goto free_args;
+ ret = fs_mutex_init(priv);
+ if (ret)
+ goto free_args;
ret = failsafe_hotplug_alarm_install(dev);
if (ret) {
ERROR("Could not set up plug-in event detection");
@@ -250,6 +282,9 @@
ERROR("Error while uninitializing sub-EAL");
failsafe_args_free(dev);
fs_sub_device_free(dev);
+ ret = pthread_mutex_destroy(&PRIV(dev)->hotplug_mutex);
+ if (ret)
+ ERROR("Error while destroying hotplug mutex");
rte_free(PRIV(dev));
rte_eth_dev_release_port(dev);
return ret;
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index d820faf..8672819 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -328,8 +328,11 @@
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
if (sdev->remove && fs_rxtx_clean(sdev)) {
+ if (fs_lock(dev, 1) != 0)
+ return;
fs_dev_stats_save(sdev);
fs_dev_remove(sdev);
+ fs_unlock(dev, 1);
}
}
@@ -428,7 +431,7 @@
void *cb_arg, void *out __rte_unused)
{
struct sub_device *sdev = cb_arg;
-
+ fs_lock(sdev->fs_dev, 0);
/* Switch as soon as possible tx_dev. */
fs_switch_dev(sdev->fs_dev, sdev);
/* Use safe bursts in any case. */
@@ -438,6 +441,7 @@
* the callback at the source of the current thread context.
*/
sdev->remove = 1;
+ fs_unlock(sdev->fs_dev, 0);
return 0;
}
diff --git a/drivers/net/failsafe/failsafe_flow.c b/drivers/net/failsafe/failsafe_flow.c
index 4d18e8e..ec8c909 100644
--- a/drivers/net/failsafe/failsafe_flow.c
+++ b/drivers/net/failsafe/failsafe_flow.c
@@ -55,6 +55,7 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_flow_validate on sub_device %d", i);
ret = rte_flow_validate(PORT_ID(sdev),
@@ -62,9 +63,11 @@
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_flow_validate failed for sub_device %d"
" with error %d", i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
+ fs_unlock(dev, 0);
return 0;
}
@@ -79,6 +82,7 @@
struct rte_flow *flow;
uint8_t i;
+ fs_lock(dev, 0);
flow = fs_flow_allocate(attr, patterns, actions);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
flow->flows[i] = rte_flow_create(PORT_ID(sdev),
@@ -90,6 +94,7 @@
}
}
TAILQ_INSERT_TAIL(&PRIV(dev)->flow_list, flow, next);
+ fs_unlock(dev, 0);
return flow;
err:
FOREACH_SUBDEV(sdev, i, dev) {
@@ -98,6 +103,7 @@
flow->flows[i], error);
}
fs_flow_release(&flow);
+ fs_unlock(dev, 0);
return NULL;
}
@@ -115,6 +121,7 @@
return -EINVAL;
}
ret = 0;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
int local_ret;
@@ -131,6 +138,7 @@
}
TAILQ_REMOVE(&PRIV(dev)->flow_list, flow, next);
fs_flow_release(&flow);
+ fs_unlock(dev, 0);
return ret;
}
@@ -144,12 +152,14 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_flow_flush on sub_device %d", i);
ret = rte_flow_flush(PORT_ID(sdev), error);
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_flow_flush failed for sub_device %d"
" with error %d", i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
@@ -157,6 +167,7 @@
TAILQ_REMOVE(&PRIV(dev)->flow_list, flow, next);
fs_flow_release(&flow);
}
+ fs_unlock(dev, 0);
return 0;
}
@@ -169,15 +180,19 @@
{
struct sub_device *sdev;
+ fs_lock(dev, 0);
sdev = TX_SUBDEV(dev);
if (sdev != NULL) {
int ret = rte_flow_query(PORT_ID(sdev),
flow->flows[SUB_ID(sdev)],
type, arg, error);
- if ((ret = fs_err(sdev, ret)))
+ if ((ret = fs_err(sdev, ret))) {
+ fs_unlock(dev, 0);
return ret;
+ }
}
+ fs_unlock(dev, 0);
WARN("No active sub_device to query about its flow");
return -1;
}
@@ -191,6 +206,7 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
FOREACH_SUBDEV(sdev, i, dev) {
if (sdev->state < DEV_PROBED)
continue;
@@ -202,11 +218,13 @@
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_flow_isolate failed for sub_device %d"
" with error %d", i, ret);
+ fs_unlock(dev, 0);
return ret;
}
sdev->flow_isolated = set;
}
PRIV(dev)->flow_isolated = set;
+ fs_unlock(dev, 0);
return 0;
}
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index f0e48c1..fe64c68 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -94,6 +94,7 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
supp_tx_offloads = PRIV(dev)->infos.tx_offload_capa;
tx_offloads = dev->data->dev_conf.txmode.offloads;
if ((tx_offloads & supp_tx_offloads) != tx_offloads) {
@@ -101,6 +102,7 @@
ERROR("Some Tx offloads are not supported, "
"requested 0x%" PRIx64 " supported 0x%" PRIx64,
tx_offloads, supp_tx_offloads);
+ fs_unlock(dev, 0);
return -rte_errno;
}
FOREACH_SUBDEV(sdev, i, dev) {
@@ -139,6 +141,7 @@
if (!fs_err(sdev, ret))
continue;
ERROR("Could not configure sub_device %d", i);
+ fs_unlock(dev, 0);
return ret;
}
if (rmv_interrupt) {
@@ -165,6 +168,7 @@
}
if (PRIV(dev)->state < DEV_ACTIVE)
PRIV(dev)->state = DEV_ACTIVE;
+ fs_unlock(dev, 0);
return 0;
}
@@ -175,9 +179,12 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
ret = failsafe_rx_intr_install(dev);
- if (ret)
+ if (ret) {
+ fs_unlock(dev, 0);
return ret;
+ }
FOREACH_SUBDEV(sdev, i, dev) {
if (sdev->state != DEV_ACTIVE)
continue;
@@ -186,6 +193,7 @@
if (ret) {
if (!fs_err(sdev, ret))
continue;
+ fs_unlock(dev, 0);
return ret;
}
ret = failsafe_rx_intr_install_subdevice(sdev);
@@ -193,6 +201,7 @@
if (!fs_err(sdev, ret))
continue;
rte_eth_dev_stop(PORT_ID(sdev));
+ fs_unlock(dev, 0);
return ret;
}
sdev->state = DEV_STARTED;
@@ -200,6 +209,7 @@
if (PRIV(dev)->state < DEV_STARTED)
PRIV(dev)->state = DEV_STARTED;
fs_switch_dev(dev, NULL);
+ fs_unlock(dev, 0);
return 0;
}
@@ -209,6 +219,7 @@
struct sub_device *sdev;
uint8_t i;
+ fs_lock(dev, 0);
PRIV(dev)->state = DEV_STARTED - 1;
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_STARTED) {
rte_eth_dev_stop(PORT_ID(sdev));
@@ -216,6 +227,7 @@
sdev->state = DEV_STARTED - 1;
}
failsafe_rx_intr_uninstall(dev);
+ fs_unlock(dev, 0);
}
static int
@@ -225,15 +237,18 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_set_link_up on sub_device %d", i);
ret = rte_eth_dev_set_link_up(PORT_ID(sdev));
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_eth_dev_set_link_up failed for sub_device %d"
" with error %d", i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
+ fs_unlock(dev, 0);
return 0;
}
@@ -244,15 +259,18 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_set_link_down on sub_device %d", i);
ret = rte_eth_dev_set_link_down(PORT_ID(sdev));
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_eth_dev_set_link_down failed for sub_device %d"
" with error %d", i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
+ fs_unlock(dev, 0);
return 0;
}
@@ -263,6 +281,7 @@
struct sub_device *sdev;
uint8_t i;
+ fs_lock(dev, 0);
failsafe_hotplug_alarm_cancel(dev);
if (PRIV(dev)->state == DEV_STARTED)
dev->dev_ops->dev_stop(dev);
@@ -273,6 +292,7 @@
sdev->state = DEV_ACTIVE - 1;
}
fs_dev_free_queues(dev);
+ fs_unlock(dev, 0);
}
static bool
@@ -305,14 +325,16 @@
if (queue == NULL)
return;
rxq = queue;
+ dev = rxq->priv->dev;
+ fs_lock(dev, 0);
if (rxq->event_fd > 0)
close(rxq->event_fd);
- dev = rxq->priv->dev;
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
SUBOPS(sdev, rx_queue_release)
(ETH(sdev)->data->rx_queues[rxq->qid]);
dev->data->rx_queues[rxq->qid] = NULL;
rte_free(rxq);
+ fs_unlock(dev, 0);
}
static int
@@ -338,6 +360,7 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
rxq = dev->data->rx_queues[rx_queue_id];
if (rxq != NULL) {
fs_rx_queue_release(rxq);
@@ -353,14 +376,17 @@
dev->data->dev_conf.rxmode.offloads,
PRIV(dev)->infos.rx_offload_capa |
PRIV(dev)->infos.rx_queue_offload_capa);
+ fs_unlock(dev, 0);
return -rte_errno;
}
rxq = rte_zmalloc(NULL,
sizeof(*rxq) +
sizeof(rte_atomic64_t) * PRIV(dev)->subs_tail,
RTE_CACHE_LINE_SIZE);
- if (rxq == NULL)
+ if (rxq == NULL) {
+ fs_unlock(dev, 0);
return -ENOMEM;
+ }
FOREACH_SUBDEV(sdev, i, dev)
rte_atomic64_init(&rxq->refcnt[i]);
rxq->qid = rx_queue_id;
@@ -371,8 +397,10 @@
rxq->priv = PRIV(dev);
rxq->sdev = PRIV(dev)->subs;
ret = rte_intr_efd_enable(&intr_handle, 1);
- if (ret < 0)
+ if (ret < 0) {
+ fs_unlock(dev, 0);
return ret;
+ }
rxq->event_fd = intr_handle.efds[0];
dev->data->rx_queues[rx_queue_id] = rxq;
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
@@ -385,9 +413,11 @@
goto free_rxq;
}
}
+ fs_unlock(dev, 0);
return 0;
free_rxq:
fs_rx_queue_release(rxq);
+ fs_unlock(dev, 0);
return ret;
}
@@ -400,20 +430,21 @@
int ret;
int rc = 0;
+ fs_lock(dev, 0);
if (idx >= dev->data->nb_rx_queues) {
- rte_errno = EINVAL;
- return -rte_errno;
+ rc = -EINVAL;
+ goto unlock;
}
rxq = dev->data->rx_queues[idx];
if (rxq == NULL || rxq->event_fd <= 0) {
- rte_errno = EINVAL;
- return -rte_errno;
+ rc = -EINVAL;
+ goto unlock;
}
/* Fail if proxy service is nor running. */
if (PRIV(dev)->rxp.sstate != SS_RUNNING) {
ERROR("failsafe interrupt services are not running");
- rte_errno = EAGAIN;
- return -rte_errno;
+ rc = -EAGAIN;
+ goto unlock;
}
rxq->enable_events = 1;
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
@@ -422,6 +453,8 @@
if (ret)
rc = ret;
}
+unlock:
+ fs_unlock(dev, 0);
if (rc)
rte_errno = -rc;
return rc;
@@ -437,14 +470,15 @@
int rc = 0;
int ret;
+ fs_lock(dev, 0);
if (idx >= dev->data->nb_rx_queues) {
- rte_errno = EINVAL;
- return -rte_errno;
+ rc = -EINVAL;
+ goto unlock;
}
rxq = dev->data->rx_queues[idx];
if (rxq == NULL || rxq->event_fd <= 0) {
- rte_errno = EINVAL;
- return -rte_errno;
+ rc = -EINVAL;
+ goto unlock;
}
rxq->enable_events = 0;
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
@@ -456,6 +490,8 @@
/* Clear pending events */
while (read(rxq->event_fd, &u64, sizeof(uint64_t)) > 0)
;
+unlock:
+ fs_unlock(dev, 0);
if (rc)
rte_errno = -rc;
return rc;
@@ -492,11 +528,13 @@
return;
txq = queue;
dev = txq->priv->dev;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
SUBOPS(sdev, tx_queue_release)
(ETH(sdev)->data->tx_queues[txq->qid]);
dev->data->tx_queues[txq->qid] = NULL;
rte_free(txq);
+ fs_unlock(dev, 0);
}
static int
@@ -511,6 +549,7 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
txq = dev->data->tx_queues[tx_queue_id];
if (txq != NULL) {
fs_tx_queue_release(txq);
@@ -531,14 +570,17 @@
dev->data->dev_conf.txmode.offloads,
PRIV(dev)->infos.tx_offload_capa |
PRIV(dev)->infos.tx_queue_offload_capa);
+ fs_unlock(dev, 0);
return -rte_errno;
}
txq = rte_zmalloc("ethdev TX queue",
sizeof(*txq) +
sizeof(rte_atomic64_t) * PRIV(dev)->subs_tail,
RTE_CACHE_LINE_SIZE);
- if (txq == NULL)
+ if (txq == NULL) {
+ fs_unlock(dev, 0);
return -ENOMEM;
+ }
FOREACH_SUBDEV(sdev, i, dev)
rte_atomic64_init(&txq->refcnt[i]);
txq->qid = tx_queue_id;
@@ -557,9 +599,11 @@
goto free_txq;
}
}
+ fs_unlock(dev, 0);
return 0;
free_txq:
fs_tx_queue_release(txq);
+ fs_unlock(dev, 0);
return ret;
}
@@ -586,8 +630,10 @@
struct sub_device *sdev;
uint8_t i;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
rte_eth_promiscuous_enable(PORT_ID(sdev));
+ fs_unlock(dev, 0);
}
static void
@@ -596,8 +642,10 @@
struct sub_device *sdev;
uint8_t i;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
rte_eth_promiscuous_disable(PORT_ID(sdev));
+ fs_unlock(dev, 0);
}
static void
@@ -606,8 +654,10 @@
struct sub_device *sdev;
uint8_t i;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
rte_eth_allmulticast_enable(PORT_ID(sdev));
+ fs_unlock(dev, 0);
}
static void
@@ -616,8 +666,10 @@
struct sub_device *sdev;
uint8_t i;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
rte_eth_allmulticast_disable(PORT_ID(sdev));
+ fs_unlock(dev, 0);
}
static int
@@ -628,6 +680,7 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling link_update on sub_device %d", i);
ret = (SUBOPS(sdev, link_update))(ETH(sdev), wait_to_complete);
@@ -635,6 +688,7 @@
rte_eth_dev_is_removed(PORT_ID(sdev)) == 0) {
ERROR("Link update failed for sub_device %d with error %d",
i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
@@ -646,9 +700,11 @@
l2 = Ð(TX_SUBDEV(dev))->data->dev_link;
if (memcmp(l1, l2, sizeof(*l1))) {
*l1 = *l2;
+ fs_unlock(dev, 0);
return 0;
}
}
+ fs_unlock(dev, 0);
return -1;
}
@@ -661,6 +717,7 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
rte_memcpy(stats, &PRIV(dev)->stats_accumulator, sizeof(*stats));
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
struct rte_eth_stats *snapshot = &sdev->stats_snapshot.stats;
@@ -676,12 +733,14 @@
ERROR("Operation rte_eth_stats_get failed for sub_device %d with error %d",
i, ret);
*timestamp = 0;
+ fs_unlock(dev, 0);
return ret;
}
*timestamp = rte_rdtsc();
inc:
failsafe_stats_increment(stats, snapshot);
}
+ fs_unlock(dev, 0);
return 0;
}
@@ -691,11 +750,13 @@
struct sub_device *sdev;
uint8_t i;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
rte_eth_stats_reset(PORT_ID(sdev));
memset(&sdev->stats_snapshot, 0, sizeof(struct rte_eth_stats));
}
memset(&PRIV(dev)->stats_accumulator, 0, sizeof(struct rte_eth_stats));
+ fs_unlock(dev, 0);
}
/**
@@ -771,14 +832,20 @@
{
struct sub_device *sdev;
struct rte_eth_dev *edev;
+ const uint32_t *ret;
+ fs_lock(dev, 0);
sdev = TX_SUBDEV(dev);
- if (sdev == NULL)
- return NULL;
+ if (sdev == NULL) {
+ ret = NULL;
+ goto unlock;
+ }
edev = ETH(sdev);
/* ENOTSUP: counts as no supported ptypes */
- if (SUBOPS(sdev, dev_supported_ptypes_get) == NULL)
- return NULL;
+ if (SUBOPS(sdev, dev_supported_ptypes_get) == NULL) {
+ ret = NULL;
+ goto unlock;
+ }
/*
* The API does not permit to do a clean AND of all ptypes,
* It is also incomplete by design and we do not really care
@@ -786,7 +853,10 @@
* We just return the ptypes of the device of highest
* priority, usually the PREFERRED device.
*/
- return SUBOPS(sdev, dev_supported_ptypes_get)(edev);
+ ret = SUBOPS(sdev, dev_supported_ptypes_get)(edev);
+unlock:
+ fs_unlock(dev, 0);
+ return ret;
}
static int
@@ -796,15 +866,18 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_set_mtu on sub_device %d", i);
ret = rte_eth_dev_set_mtu(PORT_ID(sdev), mtu);
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_eth_dev_set_mtu failed for sub_device %d with error %d",
i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
+ fs_unlock(dev, 0);
return 0;
}
@@ -815,15 +888,18 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_vlan_filter on sub_device %d", i);
ret = rte_eth_dev_vlan_filter(PORT_ID(sdev), vlan_id, on);
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_eth_dev_vlan_filter failed for sub_device %d"
" with error %d", i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
+ fs_unlock(dev, 0);
return 0;
}
@@ -832,13 +908,22 @@
struct rte_eth_fc_conf *fc_conf)
{
struct sub_device *sdev;
+ int ret;
+ fs_lock(dev, 0);
sdev = TX_SUBDEV(dev);
- if (sdev == NULL)
- return 0;
- if (SUBOPS(sdev, flow_ctrl_get) == NULL)
- return -ENOTSUP;
- return SUBOPS(sdev, flow_ctrl_get)(ETH(sdev), fc_conf);
+ if (sdev == NULL) {
+ ret = 0;
+ goto unlock;
+ }
+ if (SUBOPS(sdev, flow_ctrl_get) == NULL) {
+ ret = -ENOTSUP;
+ goto unlock;
+ }
+ ret = SUBOPS(sdev, flow_ctrl_get)(ETH(sdev), fc_conf);
+unlock:
+ fs_unlock(dev, 0);
+ return ret;
}
static int
@@ -849,15 +934,18 @@
uint8_t i;
int ret;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_flow_ctrl_set on sub_device %d", i);
ret = rte_eth_dev_flow_ctrl_set(PORT_ID(sdev), fc_conf);
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_eth_dev_flow_ctrl_set failed for sub_device %d"
" with error %d", i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
+ fs_unlock(dev, 0);
return 0;
}
@@ -867,6 +955,7 @@
struct sub_device *sdev;
uint8_t i;
+ fs_lock(dev, 0);
/* No check: already done within the rte_eth_dev_mac_addr_remove
* call for the fail-safe device.
*/
@@ -874,6 +963,7 @@
rte_eth_dev_mac_addr_remove(PORT_ID(sdev),
&dev->data->mac_addrs[index]);
PRIV(dev)->mac_addr_pool[index] = 0;
+ fs_unlock(dev, 0);
}
static int
@@ -887,11 +977,13 @@
uint8_t i;
RTE_ASSERT(index < FAILSAFE_MAX_ETHADDR);
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
ret = rte_eth_dev_mac_addr_add(PORT_ID(sdev), mac_addr, vmdq);
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_eth_dev_mac_addr_add failed for sub_device %"
PRIu8 " with error %d", i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
@@ -900,6 +992,7 @@
PRIV(dev)->nb_mac_addr = index;
}
PRIV(dev)->mac_addr_pool[index] = vmdq;
+ fs_unlock(dev, 0);
return 0;
}
@@ -909,8 +1002,10 @@
struct sub_device *sdev;
uint8_t i;
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
rte_eth_dev_default_mac_addr_set(PORT_ID(sdev), mac_addr);
+ fs_unlock(dev, 0);
}
static int
@@ -928,15 +1023,18 @@
*(const void **)arg = &fs_flow_ops;
return 0;
}
+ fs_lock(dev, 0);
FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
DEBUG("Calling rte_eth_dev_filter_ctrl on sub_device %d", i);
ret = rte_eth_dev_filter_ctrl(PORT_ID(sdev), type, op, arg);
if ((ret = fs_err(sdev, ret))) {
ERROR("Operation rte_eth_dev_filter_ctrl failed for sub_device %d"
" with error %d", i, ret);
+ fs_unlock(dev, 0);
return ret;
}
}
+ fs_unlock(dev, 0);
return 0;
}
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index f3be152..ef1f63b 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -7,6 +7,7 @@
#define _RTE_ETH_FAILSAFE_PRIVATE_H_
#include <sys/queue.h>
+#include <pthread.h>
#include <rte_atomic.h>
#include <rte_dev.h>
@@ -161,6 +162,9 @@ struct fs_priv {
* appropriate failsafe Rx queue.
*/
struct rx_proxy rxp;
+ pthread_mutex_t hotplug_mutex;
+ /* Hot-plug mutex is locked by the alarm mechanism. */
+ volatile unsigned int alarm_lock:1;
unsigned int pending_alarm:1; /* An alarm is pending */
/* flow isolation state */
int flow_isolated:1;
@@ -255,12 +259,6 @@ int failsafe_eth_lsc_event_callback(uint16_t port_id,
s != NULL; \
s = fs_find_next((dev), i + 1, state, &i))
-/**
- * Iterator construct over fail-safe sub-devices:
- * s: (struct sub_device *), iterator
- * i: (uint8_t), increment
- * dev: (struct rte_eth_dev *), fail-safe ethdev
- */
#define FOREACH_SUBDEV(s, i, dev) \
FOREACH_SUBDEV_STATE(s, i, dev, DEV_UNDEFINED)
@@ -347,6 +345,58 @@ int failsafe_eth_lsc_event_callback(uint16_t port_id,
}
/*
+ * Lock hot-plug mutex.
+ * is_alarm means that the caller is, for sure, the hot-plug alarm mechanism.
+ */
+static inline int
+fs_lock(struct rte_eth_dev *dev, unsigned int is_alarm)
+{
+ int ret;
+
+ if (is_alarm) {
+ ret = pthread_mutex_trylock(&PRIV(dev)->hotplug_mutex);
+ if (ret) {
+ DEBUG("Hot-plug mutex lock trying failed(%s), will try"
+ " again later...", strerror(ret));
+ return ret;
+ }
+ PRIV(dev)->alarm_lock = 1;
+ } else {
+ ret = pthread_mutex_lock(&PRIV(dev)->hotplug_mutex);
+ if (ret) {
+ ERROR("Cannot lock mutex(%s)", strerror(ret));
+ return ret;
+ }
+ }
+ DEBUG("Hot-plug mutex was locked by thread %lu%s", pthread_self(),
+ PRIV(dev)->alarm_lock ? " by the hot-plug alarm" : "");
+ return ret;
+}
+
+/*
+ * Unlock hot-plug mutex.
+ * is_alarm means that the caller is, for sure, the hot-plug alarm mechanism.
+ */
+static inline void
+fs_unlock(struct rte_eth_dev *dev, unsigned int is_alarm)
+{
+ int ret;
+ unsigned int prev_alarm_lock = PRIV(dev)->alarm_lock;
+
+ if (is_alarm) {
+ RTE_ASSERT(PRIV(dev)->alarm_lock == 1);
+ PRIV(dev)->alarm_lock = 0;
+ }
+ ret = pthread_mutex_unlock(&PRIV(dev)->hotplug_mutex);
+ if (ret)
+ ERROR("Cannot unlock hot-plug mutex(%s)", strerror(ret));
+ else
+ DEBUG("Hot-plug mutex was unlocked by thread %lu%s",
+ pthread_self(),
+ prev_alarm_lock ? " by the hot-plug alarm" : "");
+}
+
+/*
* Switch emitting device.
* If banned is set, banned must not be considered for
* the role of emitting device.
--
1.8.3.1
More information about the dev
mailing list