Re: [PATCH net 2/2] virtio_net: add checking sq is full inside xdp xmit

From: "Michael S. Tsirkin" <mst@redhat.com>
To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: netdev@vger.kernel.org, Jason Wang <jasowang@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	virtualization@lists.linux-foundation.org, bpf@vger.kernel.org,
	Yichun Zhang <yichun@openresty.com>
Subject: Re: [PATCH net 2/2] virtio_net: add checking sq is full inside xdp xmit
Date: Mon, 6 Mar 2023 12:57:34 -0500	[thread overview]
Message-ID: <20230306125344-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20230306041535.73319-3-xuanzhuo@linux.alibaba.com>

On Mon, Mar 06, 2023 at 12:15:35PM +0800, Xuan Zhuo wrote:
> If the queue of xdp xmit is not an independent queue, then when the xdp
> xmit used all the desc, the xmit from the __dev_queue_xmit() may encounter
> the following error.
> 
> net ens4: Unexpected TXQ (0) queue failure: -28
> 
> This patch adds a check whether sq is full in XDP Xmit.
> 
> Reported-by: Yichun Zhang <yichun@openresty.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  drivers/net/virtio_net.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 777de0ec0b1b..3001b9a548e5 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -302,6 +302,8 @@ struct padded_vnet_hdr {
>  
>  static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
>  static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf);
> +static void check_sq_full(struct virtnet_info *vi, struct net_device *dev,
> +			  struct send_queue *sq);
>  
>  static bool is_xdp_frame(void *ptr)
>  {
> @@ -341,6 +343,16 @@ static int rxq2vq(int rxq)
>  	return rxq * 2;
>  }
>  

I'd really rather we ordered functions reasonably so declarations
are not needed.

> +static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
> +{
> +	if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
> +		return false;
> +	else if (q < vi->curr_queue_pairs)
> +		return true;
> +	else
> +		return false;
> +}
> +
>  static inline struct virtio_net_hdr_mrg_rxbuf *skb_vnet_hdr(struct sk_buff *skb)
>  {
>  	return (struct virtio_net_hdr_mrg_rxbuf *)skb->cb;
> @@ -686,6 +698,9 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>  	}
>  	ret = nxmit;
>  
> +	if (!is_xdp_raw_buffer_queue(vi, sq - vi->sq))
> +		check_sq_full(vi, dev, sq);
> +
>  	if (flags & XDP_XMIT_FLUSH) {
>  		if (virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq))
>  			kicks = 1;
> @@ -1784,16 +1799,6 @@ static void check_sq_full(struct virtnet_info *vi, struct net_device *dev,
>  	}
>  }
>  
> -static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
> -{
> -	if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
> -		return false;
> -	else if (q < vi->curr_queue_pairs)
> -		return true;
> -	else
> -		return false;
> -}
> -
>  static void virtnet_poll_cleantx(struct receive_queue *rq)
>  {
>  	struct virtnet_info *vi = rq->vq->vdev->priv;
> -- 
> 2.32.0.3.g01195cf9f