Messages in this thread | | | Date | Tue, 17 Jan 2023 17:33:55 +0100 | Subject | Re: [PATCH net V3] virtio-net: correctly enable callback during start_xmit | From | Laurent Vivier <> |
| |
On 1/17/23 04:47, Jason Wang wrote: > Commit a7766ef18b33("virtio_net: disable cb aggressively") enables > virtqueue callback via the following statement: > > do { > if (use_napi) > virtqueue_disable_cb(sq->vq); > > free_old_xmit_skbs(sq, false); > > } while (use_napi && kick && > unlikely(!virtqueue_enable_cb_delayed(sq->vq))); > > When NAPI is used and kick is false, the callback won't be enabled > here. And when the virtqueue is about to be full, the tx will be > disabled, but we still don't enable tx interrupt which will cause a TX > hang. This could be observed when using pktgen with burst enabled. > > TO be consistent with the logic that tries to disable cb only for > NAPI, fixing this by trying to enable delayed callback only when NAPI > is enabled when the queue is about to be full. > > Fixes: a7766ef18b33 ("virtio_net: disable cb aggressively") > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > The patch is needed for -stable. > Changes since V2: > - try to enabled delayed callback and schedule NAPI instead of try to > poll as when TX NAPI is disabled > Changes since V1: > - enable tx interrupt after we disable TX > --- > drivers/net/virtio_net.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7723b2a49d8e..18b3de854aeb 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -1877,8 +1877,10 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev) > */ > if (sq->vq->num_free < 2+MAX_SKB_FRAGS) { > netif_stop_subqueue(dev, qnum); > - if (!use_napi && > - unlikely(!virtqueue_enable_cb_delayed(sq->vq))) { > + if (use_napi) { > + if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) > + virtqueue_napi_schedule(&sq->napi, sq->vq); > + } else if (unlikely(!virtqueue_enable_cb_delayed(sq->vq))) { > /* More just got used, free them then recheck. */ > free_old_xmit_skbs(sq, false); > if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
This fix works fine with my test case (netdev stream + passt)
Tested-by: Laurent Vivier <lvivier@redhat.com>
Thanks, Laurent
| |