Rx interrupt enable and disable take the per-device reset_ops_lock
with a trylock and return -EBUSY when it is held. Every Rx queue
takes that lock on each enable and disable, so with two or more
queues on different lcores the calls collide routinely. A failed
enable leaves the CQ unarmed, and a thread that then sleeps in
rte_epoll_wait() is not woken by the next packet; it sleeps until
its timeout with packets in the ring. A failed disable leaves the
event unconsumed. Applications do not expect -EBUSY from these
calls outside a reset, and l3fwd-power ignores the return value.
Seen on Azure (MANA VF) with 2 queues and a 10 ms epoll timeout:
about 550 lost wakeups per queue in one run, p99 latency 9.9 ms,
and 22.7% packet loss at the higher rate tested. With one queue
there was no loss.
The section under the lock is a doorbell write or a non-blocking
read of the completion channel, so wait for the lock instead.
The device state check is kept, so the calls still return -EBUSY
while a reset is in progress; a caller that arrives while the reset
thread holds the lock waits for that step to finish first.
Fixes: 077ca514d641 ("net/mana: support device reset")
Cc: [email protected]
Signed-off-by: Orgad Shaneh <[email protected]>
---
Builds on main (meson -Dwerror=true, gcc 14, rdma-core 56).
Tested on Azure D2s_v6 (MANA VF, Debian 13, kernel 7.2.11), main + 1/2
with and without this patch, 2 Rx queues, 10 ms epoll timeout, no
retry in the application, one 12 s run per rate:
pps each way without: loss / p99 / lost wakeups / -EBUSY with
25k 0 % / 8.3 ms / 300 / 913 0 % / 78 us / 0 / 0
100k 0.047 % / 9.9 ms / 2232 / 7733 0 % / 88 us / 0 / 0
200k 22.8 % / 10.0 ms / 2472 / 10093 0 % / 87 us / 0 / 0
With the patch, 400k pps on 2 queues and 100k on 1 queue are also clean.
An alternative that never blocks is to guard these two calls with the
per-queue burst_state gate instead of the device mutex (the reset path
already blocks and drains it before teardown). It is a larger change,
and the dev_state check would still be needed after a failed reset,
which clears burst_state. Happy to switch if you prefer it.
drivers/net/mana/mana.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index 0ea6d9a54a..a25c7590ef 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -449,6 +449,24 @@ mana_reset_trylock(struct mana_priv *priv)
return 0;
}
+/*
+ * Like mana_reset_trylock(), but wait for the lock. Used by Rx interrupt
+ * enable/disable, which every queue calls on each sleep and wakeup: they
+ * must not fail just because another queue holds the lock for a moment.
+ */
+static int
+mana_reset_lock(struct mana_priv *priv)
+{
+ pthread_mutex_lock(&priv->reset_ops_lock);
+
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_ACTIVE) {
+ pthread_mutex_unlock(&priv->reset_ops_lock);
+ return -EBUSY;
+ }
+ return 0;
+}
+
static int
mana_dev_info_get_lock(struct rte_eth_dev *dev,
struct rte_eth_dev_info *dev_info)
@@ -1126,7 +1144,7 @@ mana_rx_intr_enable_lock(struct rte_eth_dev *dev,
uint16_t rx_queue_id)
struct mana_priv *priv = dev->data->dev_private;
int ret;
- if (mana_reset_trylock(priv))
+ if (mana_reset_lock(priv))
return -EBUSY;
ret = mana_rx_intr_enable(dev, rx_queue_id);
pthread_mutex_unlock(&priv->reset_ops_lock);
@@ -1139,7 +1157,7 @@ mana_rx_intr_disable_lock(struct rte_eth_dev *dev,
uint16_t rx_queue_id)
struct mana_priv *priv = dev->data->dev_private;
int ret;
- if (mana_reset_trylock(priv))
+ if (mana_reset_lock(priv))
return -EBUSY;
ret = mana_rx_intr_disable(dev, rx_queue_id);
pthread_mutex_unlock(&priv->reset_ops_lock);
--
2.53.0
