* [PATCH net-next 1/3] net: Introduce netif_xmit_time_out_duration() helper
2025-11-25 7:12 [PATCH net-next 0/3] Introduce and use netif_xmit_timeout_ms() helper Tariq Toukan
@ 2025-11-25 7:12 ` Tariq Toukan
2025-11-27 10:53 ` Paolo Abeni
2025-11-28 1:34 ` Jakub Kicinski
2025-11-25 7:12 ` [PATCH net-next 2/3] net: hns3: Use netif_xmit_timeout_ms() helper Tariq Toukan
2025-11-25 7:12 ` [PATCH net-next 3/3] net/mlx5e: Refine TX timeout handling to skip non-timed-out SQ Tariq Toukan
2 siblings, 2 replies; 7+ messages in thread
From: Tariq Toukan @ 2025-11-25 7:12 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jian Shen, Salil Mehta, Jijie Shao, Saeed Mahameed, Tariq Toukan,
Mark Bloch, Leon Romanovsky, Jamal Hadi Salim, Cong Wang,
Jiri Pirko, netdev, linux-kernel, linux-rdma, Gal Pressman,
Moshe Shemesh, Yael Chemla, Shahar Shitrit
From: Shahar Shitrit <shshitrit@nvidia.com>
Introduce a new helper function netif_xmit_time_out_duration() to
check if a TX queue has timed out and report the timeout duration.
This helper consolidates the logic that is duplicated in several
locations and also encapsulates the check for whether the TX queue
is stopped.
As the first user, convert dev_watchdog() to use this helper.
Signed-off-by: Shahar Shitrit <shshitrit@nvidia.com>
Reviewed-by: Yael Chemla <ychemla@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
include/linux/netdevice.h | 15 +++++++++++++++
net/sched/sch_generic.c | 7 +++----
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e808071dbb7d..3cd73769fcfa 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3680,6 +3680,21 @@ static inline bool netif_xmit_stopped(const struct netdev_queue *dev_queue)
return dev_queue->state & QUEUE_STATE_ANY_XOFF;
}
+static inline unsigned int
+netif_xmit_timeout_ms(struct netdev_queue *txq, unsigned long *trans_start)
+{
+ unsigned long txq_trans_start = READ_ONCE(txq->trans_start);
+
+ if (trans_start)
+ *trans_start = txq_trans_start;
+
+ if (netif_xmit_stopped(txq) &&
+ time_after(jiffies, txq_trans_start + txq->dev->watchdog_timeo))
+ return jiffies_to_msecs(jiffies - txq_trans_start);
+
+ return 0;
+}
+
static inline bool
netif_xmit_frozen_or_stopped(const struct netdev_queue *dev_queue)
{
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 852e603c1755..aa6192781a24 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -523,10 +523,9 @@ static void dev_watchdog(struct timer_list *t)
* netdev_tx_sent_queue() and netif_tx_stop_queue().
*/
smp_mb();
- trans_start = READ_ONCE(txq->trans_start);
-
- if (time_after(jiffies, trans_start + dev->watchdog_timeo)) {
- timedout_ms = jiffies_to_msecs(jiffies - trans_start);
+ timedout_ms = netif_xmit_timeout_ms(txq,
+ &trans_start);
+ if (timedout_ms) {
atomic_long_inc(&txq->trans_timeout);
break;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net-next 1/3] net: Introduce netif_xmit_time_out_duration() helper
2025-11-25 7:12 ` [PATCH net-next 1/3] net: Introduce netif_xmit_time_out_duration() helper Tariq Toukan
@ 2025-11-27 10:53 ` Paolo Abeni
2025-11-27 13:53 ` Shahar Shitrit
2025-11-28 1:34 ` Jakub Kicinski
1 sibling, 1 reply; 7+ messages in thread
From: Paolo Abeni @ 2025-11-27 10:53 UTC (permalink / raw)
To: Tariq Toukan, Eric Dumazet, Jakub Kicinski, Andrew Lunn,
David S. Miller
Cc: Jian Shen, Salil Mehta, Jijie Shao, Saeed Mahameed, Mark Bloch,
Leon Romanovsky, Jamal Hadi Salim, Cong Wang, Jiri Pirko, netdev,
linux-kernel, linux-rdma, Gal Pressman, Moshe Shemesh,
Yael Chemla, Shahar Shitrit
On 11/25/25 8:12 AM, Tariq Toukan wrote:
> From: Shahar Shitrit <shshitrit@nvidia.com>
>
> Introduce a new helper function netif_xmit_time_out_duration() to
> check if a TX queue has timed out and report the timeout duration.
> This helper consolidates the logic that is duplicated in several
> locations and also encapsulates the check for whether the TX queue
> is stopped.
>
> As the first user, convert dev_watchdog() to use this helper.
>
> Signed-off-by: Shahar Shitrit <shshitrit@nvidia.com>
> Reviewed-by: Yael Chemla <ychemla@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> include/linux/netdevice.h | 15 +++++++++++++++
> net/sched/sch_generic.c | 7 +++----
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index e808071dbb7d..3cd73769fcfa 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3680,6 +3680,21 @@ static inline bool netif_xmit_stopped(const struct netdev_queue *dev_queue)
> return dev_queue->state & QUEUE_STATE_ANY_XOFF;
> }
>
> +static inline unsigned int
> +netif_xmit_timeout_ms(struct netdev_queue *txq, unsigned long *trans_start)
> +{
> + unsigned long txq_trans_start = READ_ONCE(txq->trans_start);
> +
> + if (trans_start)
> + *trans_start = txq_trans_start;
What about making this argument mandatory?
> +
> + if (netif_xmit_stopped(txq) &&
Why restricting to the <queue stopped> case? AFAICS the watchdog is
intended to additionally catch the scenarios where the rx ring is not
full but the H/W is stuck for whatever reasons, and this change will not
catch them anymore.
/P
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net-next 1/3] net: Introduce netif_xmit_time_out_duration() helper
2025-11-27 10:53 ` Paolo Abeni
@ 2025-11-27 13:53 ` Shahar Shitrit
0 siblings, 0 replies; 7+ messages in thread
From: Shahar Shitrit @ 2025-11-27 13:53 UTC (permalink / raw)
To: Paolo Abeni, Tariq Toukan, Eric Dumazet, Jakub Kicinski,
Andrew Lunn, David S. Miller
Cc: Jian Shen, Salil Mehta, Jijie Shao, Saeed Mahameed, Mark Bloch,
Leon Romanovsky, Jamal Hadi Salim, Cong Wang, Jiri Pirko, netdev,
linux-kernel, linux-rdma, Gal Pressman, Moshe Shemesh,
Yael Chemla
On 27/11/2025 12:53, Paolo Abeni wrote:
> On 11/25/25 8:12 AM, Tariq Toukan wrote:
>> From: Shahar Shitrit <shshitrit@nvidia.com>
>>
>> Introduce a new helper function netif_xmit_time_out_duration() to
>> check if a TX queue has timed out and report the timeout duration.
>> This helper consolidates the logic that is duplicated in several
>> locations and also encapsulates the check for whether the TX queue
>> is stopped.
>>
>> As the first user, convert dev_watchdog() to use this helper.
>>
>> Signed-off-by: Shahar Shitrit <shshitrit@nvidia.com>
>> Reviewed-by: Yael Chemla <ychemla@nvidia.com>
>> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
>> ---
>> include/linux/netdevice.h | 15 +++++++++++++++
>> net/sched/sch_generic.c | 7 +++----
>> 2 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index e808071dbb7d..3cd73769fcfa 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -3680,6 +3680,21 @@ static inline bool netif_xmit_stopped(const struct netdev_queue *dev_queue)
>> return dev_queue->state & QUEUE_STATE_ANY_XOFF;
>> }
>>
>> +static inline unsigned int
>> +netif_xmit_timeout_ms(struct netdev_queue *txq, unsigned long *trans_start)
>> +{
>> + unsigned long txq_trans_start = READ_ONCE(txq->trans_start);
>> +
>> + if (trans_start)
>> + *trans_start = txq_trans_start;
>
> What about making this argument mandatory?
Since not all callers are interested in this return value, as in the
case of mlx5, it would be nice to allow them pass NULL.>
>> +
>> + if (netif_xmit_stopped(txq) &&
>
> Why restricting to the <queue stopped> case? AFAICS the watchdog is
> intended to additionally catch the scenarios where the rx ring is not
> full but the H/W is stuck for whatever reasons, and this change will not
> catch them anymore.
>
> /P
>
dev_watchdog catches only the cases where tx queue is both full (queue
stopped) and timed out (last transmit timestamp was longer ago than the
watchdog timeout period). We wanted to preserve the same conditions.
By the way, I notice now that the new helper name in the title doesn't
match the one in the code. We will fix.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/3] net: Introduce netif_xmit_time_out_duration() helper
2025-11-25 7:12 ` [PATCH net-next 1/3] net: Introduce netif_xmit_time_out_duration() helper Tariq Toukan
2025-11-27 10:53 ` Paolo Abeni
@ 2025-11-28 1:34 ` Jakub Kicinski
1 sibling, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2025-11-28 1:34 UTC (permalink / raw)
To: Tariq Toukan
Cc: Eric Dumazet, Paolo Abeni, Andrew Lunn, David S. Miller,
Jian Shen, Salil Mehta, Jijie Shao, Saeed Mahameed, Mark Bloch,
Leon Romanovsky, Jamal Hadi Salim, Cong Wang, Jiri Pirko, netdev,
linux-kernel, linux-rdma, Gal Pressman, Moshe Shemesh,
Yael Chemla, Shahar Shitrit
On Tue, 25 Nov 2025 09:12:54 +0200 Tariq Toukan wrote:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index e808071dbb7d..3cd73769fcfa 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
include/net/net_queue.h seems like a better place for new code
> @@ -3680,6 +3680,21 @@ static inline bool netif_xmit_stopped(const struct netdev_queue *dev_queue)
> return dev_queue->state & QUEUE_STATE_ANY_XOFF;
> }
>
> +static inline unsigned int
> +netif_xmit_timeout_ms(struct netdev_queue *txq, unsigned long *trans_start)
> +{
> + unsigned long txq_trans_start = READ_ONCE(txq->trans_start);
> +
> + if (trans_start)
> + *trans_start = txq_trans_start;
The drivers don't really care about this, AFAICT hns3 uses this
to calculate the stall length (return value of this func.
> + if (netif_xmit_stopped(txq) &&
> + time_after(jiffies, txq_trans_start + txq->dev->watchdog_timeo))
> + return jiffies_to_msecs(jiffies - txq_trans_start);
> +
> + return 0;
> +}
> +
> static inline bool
> netif_xmit_frozen_or_stopped(const struct netdev_queue *dev_queue)
> {
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 852e603c1755..aa6192781a24 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -523,10 +523,9 @@ static void dev_watchdog(struct timer_list *t)
> * netdev_tx_sent_queue() and netif_tx_stop_queue().
> */
> smp_mb();
> - trans_start = READ_ONCE(txq->trans_start);
> -
> - if (time_after(jiffies, trans_start + dev->watchdog_timeo)) {
> - timedout_ms = jiffies_to_msecs(jiffies - trans_start);
> + timedout_ms = netif_xmit_timeout_ms(txq,
> + &trans_start);
> + if (timedout_ms) {
The use of the new helper in the core feels a bit forced, I'd leave
the core as is. Otherwise you need the awkward output param, and
core now duplicates the netif_xmit_stopped(txq) check
> atomic_long_inc(&txq->trans_timeout);
> break;
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 2/3] net: hns3: Use netif_xmit_timeout_ms() helper
2025-11-25 7:12 [PATCH net-next 0/3] Introduce and use netif_xmit_timeout_ms() helper Tariq Toukan
2025-11-25 7:12 ` [PATCH net-next 1/3] net: Introduce netif_xmit_time_out_duration() helper Tariq Toukan
@ 2025-11-25 7:12 ` Tariq Toukan
2025-11-25 7:12 ` [PATCH net-next 3/3] net/mlx5e: Refine TX timeout handling to skip non-timed-out SQ Tariq Toukan
2 siblings, 0 replies; 7+ messages in thread
From: Tariq Toukan @ 2025-11-25 7:12 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jian Shen, Salil Mehta, Jijie Shao, Saeed Mahameed, Tariq Toukan,
Mark Bloch, Leon Romanovsky, Jamal Hadi Salim, Cong Wang,
Jiri Pirko, netdev, linux-kernel, linux-rdma, Gal Pressman,
Moshe Shemesh, Yael Chemla, Shahar Shitrit
From: Shahar Shitrit <shshitrit@nvidia.com>
Replace the open-coded TX queue timeout check in
hns3_get_timeout_queue() with a call to netif_xmit_timeout_ms() helper.
Signed-off-by: Shahar Shitrit <shshitrit@nvidia.com>
Reviewed-by: Yael Chemla <ychemla@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 7a0654e2d3dd..3e8fe3b5d32b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2811,10 +2811,7 @@ static int hns3_get_timeout_queue(struct net_device *ndev)
unsigned long trans_start;
q = netdev_get_tx_queue(ndev, i);
- trans_start = READ_ONCE(q->trans_start);
- if (netif_xmit_stopped(q) &&
- time_after(jiffies,
- (trans_start + ndev->watchdog_timeo))) {
+ if (netif_xmit_timeout_ms(q, &trans_start)) {
#ifdef CONFIG_BQL
struct dql *dql = &q->dql;
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net-next 3/3] net/mlx5e: Refine TX timeout handling to skip non-timed-out SQ
2025-11-25 7:12 [PATCH net-next 0/3] Introduce and use netif_xmit_timeout_ms() helper Tariq Toukan
2025-11-25 7:12 ` [PATCH net-next 1/3] net: Introduce netif_xmit_time_out_duration() helper Tariq Toukan
2025-11-25 7:12 ` [PATCH net-next 2/3] net: hns3: Use netif_xmit_timeout_ms() helper Tariq Toukan
@ 2025-11-25 7:12 ` Tariq Toukan
2 siblings, 0 replies; 7+ messages in thread
From: Tariq Toukan @ 2025-11-25 7:12 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jian Shen, Salil Mehta, Jijie Shao, Saeed Mahameed, Tariq Toukan,
Mark Bloch, Leon Romanovsky, Jamal Hadi Salim, Cong Wang,
Jiri Pirko, netdev, linux-kernel, linux-rdma, Gal Pressman,
Moshe Shemesh, Yael Chemla, Shahar Shitrit
From: Shahar Shitrit <shshitrit@nvidia.com>
mlx5e_tx_timeout_work() is invoked when the dev_watchdog reports a
timed-out TX queue. Currently, the recovery flow is triggered for all
stopped SQs, which is not always correct — some SQs may be temporarily
stopped without actually timing out. Attempting to recover such SQs
results in no EQE being polled (since no real timeout occurred), which
the driver misinterprets as a recovery failure, unnecessarily causing
channel reopening.
Improve the logic to initiate recovery only for SQs that are both
stopped and timed out. Utilize the helper introduced in the previous
patch to determine whether the netdevice watchdog timeout period has
elapsed since the SQ’s last transmit timestamp.
Signed-off-by: Shahar Shitrit <shshitrit@nvidia.com>
Reviewed-by: Yael Chemla <ychemla@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index e537df670758..cd146df29ada 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5139,7 +5139,7 @@ static void mlx5e_tx_timeout_work(struct work_struct *work)
netdev_get_tx_queue(netdev, i);
struct mlx5e_txqsq *sq = priv->txq2sq[i];
- if (!netif_xmit_stopped(dev_queue))
+ if (!netif_xmit_timeout_ms(dev_queue, NULL))
continue;
if (mlx5e_reporter_tx_timeout(sq))
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread