Re: [PATCH net-next 1/3] net: Introduce netif_xmit_time_out_duration() helper

[Paolo Abeni]

On 11/25/25 8:12 AM, Tariq Toukan wrote:
> From: Shahar Shitrit <shshitrit@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> Reviewed-by: Yael Chemla <ychemla@xxxxxxxxxx>
> Signed-off-by: Tariq Toukan <tariqt@xxxxxxxxxx>
> ---
>  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