Re: [PATCH net 2/4] net: ntb_netdev: Fix TX busy and drop handling

[Dave Jiang]

On 8/16/26 10:35 PM, Koichiro Den wrote:
> Currently, ntb_netdev returns NETDEV_TX_BUSY for every enqueue error. It
> also increments the drop and error counters while leaving the skb owned
> by the qdisc, and may return BUSY with the subqueue still awake.
> Retrying a permanent error cannot succeed either.
> 
> The unconditional BUSY return and premature accounting date back to the
> initial driver. The error-path queue stop was later removed without
> changing that return value. The current flow-control code includes a
> resource check, but ntb_netdev does not honor its result before enqueue.
> 
> Honor the resource check before enqueue. For -EAGAIN and -EBUSY, stop
> the subqueue, arm the existing reaper timer, and return BUSY without
> touching the skb. For other errors, free the skb, increment tx_dropped,
> and return NETDEV_TX_OK.
> 
> Fixes: 548c237c0a99 ("net: Add support for NTB virtual ethernet device")
> Fixes: d723485cb4ca ("ntb_netdev: remove tx timeout")

This is probably the only Fixes tag we need.

> Fixes: e74bfeedad08 ("NTB: Add flow control to the ntb_netdev")
> Cc: stable@xxxxxxxxxxxxxxx> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>

Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>

> ---
>  drivers/net/ntb_netdev.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
> index 029a4a532a10..02b35cf53a62 100644
> --- a/drivers/net/ntb_netdev.c
> +++ b/drivers/net/ntb_netdev.c
> @@ -199,8 +199,10 @@ static int __ntb_netdev_maybe_stop_tx(struct net_device *netdev,
>  static int ntb_netdev_maybe_stop_tx(struct net_device *ndev,
>  				    struct ntb_netdev_queue *q, int size)
>  {
> -	if (__netif_subqueue_stopped(ndev, q->qid) ||
> -	    (ntb_transport_tx_free_entry(q->qp) >= size))
> +	if (__netif_subqueue_stopped(ndev, q->qid))
> +		return -EBUSY;
> +
> +	if (ntb_transport_tx_free_entry(q->qp) >= size)
>  		return 0;
>  
>  	return __ntb_netdev_maybe_stop_tx(ndev, q, size);
> @@ -256,21 +258,30 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
>  
>  	q = &dev->queues[qid];
>  
> -	ntb_netdev_maybe_stop_tx(ndev, q, tx_stop);
> +	if (unlikely(ntb_netdev_maybe_stop_tx(ndev, q, tx_stop)))
> +		return NETDEV_TX_BUSY;
>  
>  	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len);
> -	if (rc)
> -		goto err;
> +	if (rc) {
> +		if (rc == -EAGAIN || rc == -EBUSY) {
> +			netif_stop_subqueue(ndev, q->qid);
> +			mod_timer(&q->tx_timer,
> +				  jiffies + usecs_to_jiffies(tx_time));
> +			return NETDEV_TX_BUSY;
> +		}
> +
> +		goto drop;
> +	}
>  
>  	/* check for next submit */
>  	ntb_netdev_maybe_stop_tx(ndev, q, tx_stop);
>  
>  	return NETDEV_TX_OK;
>  
> -err:
> +drop:
> +	dev_kfree_skb_any(skb);
>  	ndev->stats.tx_dropped++;
> -	ndev->stats.tx_errors++;
> -	return NETDEV_TX_BUSY;
> +	return NETDEV_TX_OK;
>  }
>  
>  static void ntb_netdev_tx_timer(struct timer_list *t)