+ Aditya - [email protected] Hi Orgad,
> -----Original Message----- > From: Orgad Shaneh <[email protected]> > Sent: Monday, November 2, 2026 3:08 PM > To: [email protected] > Cc: Wei Hu <[email protected]>; Long Li <[email protected]>; Stephen > Hemminger <[email protected]>; [email protected] > Subject: [EXTERNAL] [PATCH 2/2] net/mana: fix Rx interrupt enable failing on > lock contention > > [You don't often get email from [email protected]. Learn why this is > important at https://aka.ms/LearnAboutSenderIdentification ] > > 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; > +} > + App threads could get stuck as long as 15 seconds waiting for the lock when the device is in reset. Is it better to add following? static int mana_reset_lock(struct mana_priv *priv) { + if (rte_atomic_load_explicit(&priv->dev_state, + rte_memory_order_relaxed) != MANA_DEV_ACTIVE) + return -EBUSY; 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; } Thanks, Wei
