| Message ID | 20230306041535.73319-3-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | add checking sq is full inside xdp xmit | expand |
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
On Mon, 6 Mar 2023 12:57:34 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote: > 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. Yes, but this function depends on some other functions. If we put this function above, then these dependent functions must be processed. The current method should be a relatively simple way. Thanks. > > > +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 >
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; } +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;
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(-)