All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net: mana: Add CQE polling as first-level TX timeout recovery
@ 2026-11-12 10:57 Sahil Chandna
  2026-11-12 12:07 ` Breno Leitao
  2026-11-13 10:58 ` sashiko-bot
  0 siblings, 2 replies; 6+ messages in thread
From: Sahil Chandna @ 2026-11-12 10:57 UTC (permalink / raw)
  To: haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet, kuba,
	pabeni, weh, kotaranov, horms, ernis, gargaditya, leitao,
	sahilchandna, linux-hyperv, netdev, linux-kernel, linux-rdma

The current mana_tx_timeout() handler unconditionally schedules a full
port queue reset (detach/attach) whenever the TX watchdog fires.
This is unnecessary for transient stalls where completions are
simply delayed or an interrupt was missed.

Implement a graduated recovery approach: on the first TX timeout (or
when the previous timeout was more than 2 minutes ago), schedule NAPI on
the stalled queue's TX CQ via napi_schedule(). This triggers
mana_poll_tx_cq() through the normal NAPI path, processing any pending
completions without relying on the interrupt and recovering
missed-interrupt scenarios without queue reset.

If any subsequent TX timeout fires on any queue, within 2 minutes of
the previous one, escalate to the existing port-level detach/attach
reset, as the CQE poll did not recover the queue.

This makes the TX timeout recovery three-tiered:

  Level 1: CQE poll via napi_schedule() (this patch)
  Level 2: Port queue reset (detach/attach)
  Level 3: device reset(FLR)

Co-developed-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Signed-off-by: Sahil Chandna <sahilchandna@linux.microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 31 ++++++++++++++++---
 .../ethernet/microsoft/mana/mana_ethtool.c    |  2 ++
 include/net/mana/mana.h                       |  4 +++
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index e6d494df22e4..0d8c8b8fbc83 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -336,6 +336,9 @@ static void mana_per_port_queue_reset_work_handler(struct work_struct *work)
 	if (err)
 		netdev_err(ndev, "mana_attach failed: %d\n", err);

+	/* Let the next TX timeout try a CQE poll recovery first */
+	WRITE_ONCE(apc->last_tx_timeout, 0);
+
 dealloc_pre_rxbufs:
 	mana_pre_dealloc_rxbufs(apc);
 clear_flag:
@@ -964,6 +967,7 @@ static void mana_tx_timeout(struct net_device *netdev, unsigned int txqueue)
 	struct mana_port_context *apc = netdev_priv(netdev);
 	struct mana_context *ac = apc->ac;
 	struct gdma_context *gc = ac->gdma_dev->gdma_context;
+	unsigned long last = READ_ONCE(apc->last_tx_timeout);

 	/* Debug knob for bringup/qualification: when set, log the timeout and
 	 * skip the reset so the failing state is preserved for telemetry.
@@ -980,11 +984,30 @@ static void mana_tx_timeout(struct net_device *netdev, unsigned int txqueue)
 	if (test_bit(GC_IN_SERVICE, &gc->flags))
 		return;

-	/* Note: If there are pending queue reset work for this port(apc),
-	 * subsequent request queued up from here are ignored. This is because
-	 * we are using the same work instance per port(apc).
+	if (!apc->port_is_up)
+		return;
+
+	/* If the previous timeout fired less than
+	 * MANA_TX_TIMEOUT_RESET_THRESHOLD ago, the earlier CQE poll did not
+	 * help and queue stalled again	: escalate to a port reset.
+	 * Note: If there is pending queue reset work for this port (apc),
+	 * subsequent requests queued up from here are ignored, because the
+	 * same work instance is used per port (apc).
 	 */
-	queue_work(ac->per_port_queue_reset_wq, &apc->queue_reset_work);
+	if (last &&
+	    time_after(last, jiffies - MANA_TX_TIMEOUT_RESET_THRESHOLD)) {
+		queue_work(ac->per_port_queue_reset_wq, &apc->queue_reset_work);
+		return;
+	}
+
+	/* First timeout, or the previous one was long ago: try to recover by
+	 * polling the TX CQ for completions before escalating to a reset.
+	 */
+	if (txqueue < apc->num_queues && apc->tx_qp && apc->tx_qp[txqueue]) {
+		WRITE_ONCE(apc->last_tx_timeout, jiffies);
+		apc->eth_stats.tx_cqe_poll_recovery++;
+		napi_schedule(&apc->tx_qp[txqueue]->tx_cq.napi);
+	}
 }

 static int mana_shaper_set(struct net_shaper_binding *binding,
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index a91eb12badca..ff4799c61a6c 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -22,6 +22,8 @@ static const struct mana_stats_desc mana_eth_stats[] = {
 				       tx_linear_pkt_cnt)},
 	{"rx_cqe_unknown_type", offsetof(struct mana_ethtool_stats,
 					rx_cqe_unknown_type)},
+	{"tx_cqe_poll_recovery", offsetof(struct mana_ethtool_stats,
+					tx_cqe_poll_recovery)},
 };

 static const struct mana_stats_desc mana_hc_stats[] = {
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 09725e3256fa..5de6eaa02102 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -94,6 +94,8 @@ enum mana_priv_flag_bits {

 #define MANA_RX_FRAG_ALIGNMENT 64

+#define MANA_TX_TIMEOUT_RESET_THRESHOLD (2 * 60 * HZ)
+
 struct mana_stats_rx {
 	u64 packets;
 	u64 bytes;
@@ -447,6 +449,7 @@ struct mana_tx_qp {
 struct mana_ethtool_stats {
 	u64 stop_queue;
 	u64 wake_queue;
+	u64 tx_cqe_poll_recovery;
 	u64 tx_cqe_err;
 	u64 tx_cqe_unknown_type;
 	u64 tx_linear_pkt_cnt;
@@ -566,6 +569,7 @@ struct mana_port_context {
 	struct net_device *ndev;
 	struct work_struct queue_reset_work;

+	unsigned long last_tx_timeout;
 	/* Debug knob to log TX timeout but skip recovery reset */
 	bool tx_timeout_skip_reset;

--
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] net: mana: Add CQE polling as first-level TX timeout recovery
  2026-11-12 10:57 [PATCH net-next] net: mana: Add CQE polling as first-level TX timeout recovery Sahil Chandna
@ 2026-11-12 12:07 ` Breno Leitao
  2026-11-13  8:34   ` Sahil Chandna
  2026-11-13 10:58 ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Breno Leitao @ 2026-11-12 12:07 UTC (permalink / raw)
  To: Sahil Chandna
  Cc: haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet, kuba,
	pabeni, weh, kotaranov, horms, ernis, gargaditya, linux-hyperv,
	netdev, linux-kernel, linux-rdma

Hello Sahil,

On Thu, Nov 12, 2026 at 04:57:18AM -0600, Sahil Chandna wrote:
> Implement a graduated recovery approach: on the first TX timeout (or
> when the previous timeout was more than 2 minutes ago), schedule NAPI on
> the stalled queue's TX CQ via napi_schedule(). This triggers
> mana_poll_tx_cq() through the normal NAPI path, processing any pending
> completions without relying on the interrupt and recovering
> missed-interrupt scenarios without queue reset.

Which interrupt is being missed here?

> If any subsequent TX timeout fires on any queue, within 2 minutes of
> the previous one, escalate to the existing port-level detach/attach
> reset, as the CQE poll did not recover the queue.

How was 2 minutes chosen? mana sets watchdog_timeo to 15 * HZ, no?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] net: mana: Add CQE polling as first-level TX timeout recovery
  2026-11-12 12:07 ` Breno Leitao
@ 2026-11-13  8:34   ` Sahil Chandna
  2026-11-13 10:16     ` Breno Leitao
  0 siblings, 1 reply; 6+ messages in thread
From: Sahil Chandna @ 2026-11-13  8:34 UTC (permalink / raw)
  To: Breno Leitao
  Cc: haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet, kuba,
	pabeni, weh, kotaranov, horms, ernis, gargaditya, linux-hyperv,
	netdev, linux-kernel, linux-rdma

On 12-11-2026 17:37, Breno Leitao wrote:
> Hello Sahil,
> 
> On Thu, Nov 12, 2026 at 04:57:18AM -0600, Sahil Chandna wrote:
>> Implement a graduated recovery approach: on the first TX timeout (or
>> when the previous timeout was more than 2 minutes ago), schedule NAPI on
>> the stalled queue's TX CQ via napi_schedule(). This triggers
>> mana_poll_tx_cq() through the normal NAPI path, processing any pending
>> completions without relying on the interrupt and recovering
>> missed-interrupt scenarios without queue reset.
> 
> Which interrupt is being missed here?
> 
Hi Breno,

It is the TX CQ completion interrupt. After NAPI finishes, it re-arms
the TX CQ. When a new CQE arrives, the hardware post EQE, which raises
the MSI-X and schedules NAPI. If that notification for the TX CQ is lost
, NAPI never runs for the TX CQ and pending_sends never drains and the
queue stays stopped,  so napi_schedule() polls the CQ, reaps the CQEs
and re-arms it.
>> If any subsequent TX timeout fires on any queue, within 2 minutes of
>> the previous one, escalate to the existing port-level detach/attach
>> reset, as the CQE poll did not recover the queue.
> 
> How was 2 minutes chosen? mana sets watchdog_timeo to 15 * HZ, no?
The watchdog_timeo is TX stall timeout setting i.e if a queue remains
stalled for this much time, it trigger watchdog timeout. Here, 2 minute
is not timeout/wait. The 2 minutes here is for deciding whether a new
queue stall event is a repeat of the previous incident or a new,
unrelated one that should get the cheap poll first. For e.g. if a queue
gets stalled, 15 * HZ sec later, timeout occurs and this patch trigger
manual NAPI, if another stall occurs (it can be on any queue) within 2
minutes of last one, the existing recovery mechanism of mana
detach/attach will be tried.The patch is adding a first level less
destructive recovery first.
For selecting 2 minutes, I do not have any hardware basis for it afaik,
but I can confirm and check with our host team as well.
Hope this helps !
Regards,
Sahil

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] net: mana: Add CQE polling as first-level TX timeout recovery
  2026-11-13  8:34   ` Sahil Chandna
@ 2026-11-13 10:16     ` Breno Leitao
  0 siblings, 0 replies; 6+ messages in thread
From: Breno Leitao @ 2026-11-13 10:16 UTC (permalink / raw)
  To: Sahil Chandna
  Cc: haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet, kuba,
	pabeni, weh, kotaranov, horms, ernis, gargaditya, linux-hyperv,
	netdev, linux-kernel, linux-rdma

On Fri, Nov 13, 2026 at 02:04:08PM +0530, Sahil Chandna wrote:
> On 12-11-2026 17:37, Breno Leitao wrote:
> > Hello Sahil,
> > 
> > On Thu, Nov 12, 2026 at 04:57:18AM -0600, Sahil Chandna wrote:
> >> Implement a graduated recovery approach: on the first TX timeout (or
> >> when the previous timeout was more than 2 minutes ago), schedule NAPI on
> >> the stalled queue's TX CQ via napi_schedule(). This triggers
> >> mana_poll_tx_cq() through the normal NAPI path, processing any pending
> >> completions without relying on the interrupt and recovering
> >> missed-interrupt scenarios without queue reset.
> > 
> > Which interrupt is being missed here?
> > 
> Hi Breno,
> 
> It is the TX CQ completion interrupt. After NAPI finishes, it re-arms
> the TX CQ. When a new CQE arrives, the hardware post EQE, which raises
> the MSI-X and schedules NAPI. If that notification for the TX CQ is lost
> , NAPI never runs for the TX CQ and pending_sends never drains and the
> queue stays stopped,  so napi_schedule() polls the CQ, reaps the CQEs
> and re-arms it.

Thanks.  Which one gets lost, the EQE or the MSI-X?

> >> If any subsequent TX timeout fires on any queue, within 2 minutes of
> >> the previous one, escalate to the existing port-level detach/attach
> >> reset, as the CQE poll did not recover the queue.
> > 
> > How was 2 minutes chosen? mana sets watchdog_timeo to 15 * HZ, no?
> The watchdog_timeo is TX stall timeout setting i.e if a queue remains
> stalled for this much time, it trigger watchdog timeout. Here, 2 minute
> is not timeout/wait. The 2 minutes here is for deciding whether a new
> queue stall event is a repeat of the previous incident or a new,
> unrelated one that should get the cheap poll first. For e.g. if a queue
> gets stalled, 15 * HZ sec later, timeout occurs and this patch trigger
> manual NAPI, if another stall occurs (it can be on any queue) within 2
> minutes of last one, the existing recovery mechanism of mana
> detach/attach will be tried.The patch is adding a first level less
> destructive recovery first.
> For selecting 2 minutes, I do not have any hardware basis for it afaik,
> but I can confirm and check with our host team as well.

So the goal is to tell an isolated stall from a wider problem.

A port-wide timestamp only approximates that. It cannot tell the same
queue from another one, and since dev_watchdog() reports one queue per
run, two queues that stall together show up ~15s apart: the first gets
a kick, the second gets the port reset anyway, no?

Could mana_tx_timeout() check this directly? Look at the other TX queues
the way dev_watchdog() does (stopped, with trans_start older than
watchdog_timeo). If more than one is stalled, reset right away.

Otherwise kick this queue, and reset if the same queue times out again
within a couple of watchdog periods, as gve_tx_timeout_try_q_kick()
does. That also gets rid of the 2 minutes.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] net: mana: Add CQE polling as first-level TX timeout recovery
  2026-11-12 10:57 [PATCH net-next] net: mana: Add CQE polling as first-level TX timeout recovery Sahil Chandna
  2026-11-12 12:07 ` Breno Leitao
@ 2026-11-13 10:58 ` sashiko-bot
  2026-11-13 12:11   ` Sahil Chandna
  1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-11-13 10:58 UTC (permalink / raw)
  To: Sahil Chandna, linux-hyperv, linux-rdma

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] net: mana: missing netdev_lock() in mana_detach() callers
  [1]
--

commit 6f4e15a96596e896188b6e028bb49a0d4c04269b
Author: Sahil Chandna <sahilchandna@linux.microsoft.com>

net: mana: Add CQE polling as first-level TX timeout recovery

This commit adds a first-level TX timeout recovery mechanism that polls
the stalled queue's TX CQ via napi_schedule() before escalating to a
port-level queue reset.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index e6d494df22e4d..0d8c8b8fbc83f 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -336,6 +336,9 @@ static void mana_per_port_queue_reset_work_handler(struct work_struct *work)
>  	if (err)
>  		netdev_err(ndev, "mana_attach failed: %d\n", err);
>  
> +	/* Let the next TX timeout try a CQE poll recovery first */
> +	WRITE_ONCE(apc->last_tx_timeout, 0);

[Severity: Critical]
This problem wasn't introduced by this patch, but should
mana_per_port_queue_reset_work_handler() (along with mana_remove(),
mana_probe() on resume, and mana_xdp_set()) hold netdev_lock(ndev) around
mana_detach() and mana_attach() [1]?

Because mana_devops sets .net_shaper_ops, netdev_need_ops_lock() is
enabled on MANA net_device instances.  Queue teardown in mana_detach()
calls napi_disable_locked() and netif_napi_del_locked() via
mana_destroy_txq() and mana_destroy_rxq(), while mana_attach() calls
netif_napi_add_locked(), netif_napi_add_weight_locked(), and
napi_enable_locked().  Calling these under rtnl_lock() alone violates
netdev_assert_locked(), leaves NAPI state unserialized against netdev-genl
operations, and allows ops-locked ethtool callbacks (such as
mana_get_ethtool_stats() or mana_set_coalesce()) running under
netdev_lock() without rtnl_lock() to race with apc->rxqs and apc->tx_qp
teardown, leading to a use-after-free or NULL pointer dereference.

[1] https://sashiko.dev/#/bug/linux-2946b7e7-6855-49c0-921d-a6daa482c10c

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112105752.978446-1-sahilchandna@linux.microsoft.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] net: mana: Add CQE polling as first-level TX timeout recovery
  2026-11-13 10:58 ` sashiko-bot
@ 2026-11-13 12:11   ` Sahil Chandna
  0 siblings, 0 replies; 6+ messages in thread
From: Sahil Chandna @ 2026-11-13 12:11 UTC (permalink / raw)
  To: sashiko-reviews, linux-hyperv, linux-rdma

On 13-11-2026 16:28, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found no new issues
> in this patch, but noticed 1 related pre-existing issue it does not
> introduce:
> - [Critical] net: mana: missing netdev_lock() in mana_detach() callers
>   [1]
> --
> 
> commit 6f4e15a96596e896188b6e028bb49a0d4c04269b
> Author: Sahil Chandna <sahilchandna@linux.microsoft.com>
> 
> net: mana: Add CQE polling as first-level TX timeout recovery
> 
> This commit adds a first-level TX timeout recovery mechanism that polls
> the stalled queue's TX CQ via napi_schedule() before escalating to a
> port-level queue reset.
> 
>> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
>> index e6d494df22e4d..0d8c8b8fbc83f 100644
>> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
>> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
>> @@ -336,6 +336,9 @@ static void mana_per_port_queue_reset_work_handler(struct work_struct *work)
>>  	if (err)
>>  		netdev_err(ndev, "mana_attach failed: %d\n", err);
>>  
>> +	/* Let the next TX timeout try a CQE poll recovery first */
>> +	WRITE_ONCE(apc->last_tx_timeout, 0);
> 
> [Severity: Critical]
> This problem wasn't introduced by this patch, but should
> mana_per_port_queue_reset_work_handler() (along with mana_remove(),
> mana_probe() on resume, and mana_xdp_set()) hold netdev_lock(ndev) around
> mana_detach() and mana_attach() [1]?
> 
> Because mana_devops sets .net_shaper_ops, netdev_need_ops_lock() is
> enabled on MANA net_device instances.  Queue teardown in mana_detach()
> calls napi_disable_locked() and netif_napi_del_locked() via
> mana_destroy_txq() and mana_destroy_rxq(), while mana_attach() calls
> netif_napi_add_locked(), netif_napi_add_weight_locked(), and
> napi_enable_locked().  Calling these under rtnl_lock() alone violates
> netdev_assert_locked(), leaves NAPI state unserialized against netdev-genl
> operations, and allows ops-locked ethtool callbacks (such as
> mana_get_ethtool_stats() or mana_set_coalesce()) running under
> netdev_lock() without rtnl_lock() to race with apc->rxqs and apc->tx_qp
> teardown, leading to a use-after-free or NULL pointer dereference.
> 
> [1] https://sashiko.dev/#/bug/linux-2946b7e7-6855-49c0-921d-a6daa482c10c
> 
Thanks or reporting this, the issue looks real and I will work on this
in a separate patch.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-13 12:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-12 10:57 [PATCH net-next] net: mana: Add CQE polling as first-level TX timeout recovery Sahil Chandna
2026-11-12 12:07 ` Breno Leitao
2026-11-13  8:34   ` Sahil Chandna
2026-11-13 10:16     ` Breno Leitao
2026-11-13 10:58 ` sashiko-bot
2026-11-13 12:11   ` Sahil Chandna

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.