[PATCH 2/2] net/mana: fix Rx interrupt enable failing on lock contention

[Orgad Shaneh <orgads@gmail.com>]

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: stable@dpdk.org

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
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