On Mon, Jan 11, 2021, at 04:31, Shay Agroskin wrote: > Is this addition needed ? Seems like we don't set VIRTIO_XDP_TX > bit in case of virtnet_xdp_xmit() failure, so the surrounding 'if' > won't be taken.
Good catch, it looks like you're right. I'm happy to remove that extra branch although I would like to add a comment explaining that assumption: diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index ed08998765e0..3ae7cd2f1e72 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1472,8 +1472,10 @@ static int virtnet_poll(struct napi_struct *napi, int budget) xdp_do_flush(); if (xdp_xmit & VIRTIO_XDP_TX) { + /* VIRTIO_XDP_TX only set on successful virtnet_xdp_xmit, + * implies sq != NULL */ sq = virtnet_xdp_sq(vi); - if (sq && virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq)) { + if (virtqueue_kick_prepare(sq->vq) && virtqueue_notify(sq->vq)) { u64_stats_update_begin(&sq->stats.syncp); sq->stats.kicks++; u64_stats_update_end(&sq->stats.syncp);