* [PATCH net] virtio-net: correct error handling of virtqueue_kick()
@ 2014-03-26 5:03 Jason Wang
2014-03-26 6:59 ` Michael S. Tsirkin
2014-03-27 17:14 ` David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Jason Wang @ 2014-03-26 5:03 UTC (permalink / raw)
To: rusty, mst, virtualization, netdev, linux-kernel
Current error handling of virtqueue_kick() was wrong in two places:
- The skb were freed immediately when virtqueue_kick() fail during
xmit. This may lead double free since the skb was not detached from
the virtqueue.
- try_fill_recv() returns false when virtqueue_kick() fail. This will
lead unnecessary rescheduling of refill work.
Actually, it's safe to just ignore the kick failure in those two
places. So this patch fixes this by partially revert commit
67975901183799af8e93ec60e322f9e2a1940b9b.
Fixes 67975901183799af8e93ec60e322f9e2a1940b9b
(virtio_net: verify if virtqueue_kick() succeeded).
Cc: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5632a99..841b608 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -671,8 +671,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
if (err)
break;
} while (rq->vq->num_free);
- if (unlikely(!virtqueue_kick(rq->vq)))
- return false;
+ virtqueue_kick(rq->vq);
return !oom;
}
@@ -877,7 +876,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
err = xmit_skb(sq, skb);
/* This should not happen! */
- if (unlikely(err) || unlikely(!virtqueue_kick(sq->vq))) {
+ if (unlikely(err)) {
dev->stats.tx_fifo_errors++;
if (net_ratelimit())
dev_warn(&dev->dev,
@@ -886,6 +885,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
kfree_skb(skb);
return NETDEV_TX_OK;
}
+ virtqueue_kick(sq->vq);
/* Don't wait up for transmitted skbs to be freed. */
skb_orphan(skb);
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] virtio-net: correct error handling of virtqueue_kick()
2014-03-26 5:03 [PATCH net] virtio-net: correct error handling of virtqueue_kick() Jason Wang
@ 2014-03-26 6:59 ` Michael S. Tsirkin
2014-03-27 17:14 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2014-03-26 6:59 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, virtualization
On Wed, Mar 26, 2014 at 01:03:00PM +0800, Jason Wang wrote:
> Current error handling of virtqueue_kick() was wrong in two places:
> - The skb were freed immediately when virtqueue_kick() fail during
> xmit. This may lead double free since the skb was not detached from
> the virtqueue.
> - try_fill_recv() returns false when virtqueue_kick() fail. This will
> lead unnecessary rescheduling of refill work.
>
> Actually, it's safe to just ignore the kick failure in those two
> places. So this patch fixes this by partially revert commit
> 67975901183799af8e93ec60e322f9e2a1940b9b.
>
> Fixes 67975901183799af8e93ec60e322f9e2a1940b9b
> (virtio_net: verify if virtqueue_kick() succeeded).
>
> Cc: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 5632a99..841b608 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -671,8 +671,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
> if (err)
> break;
> } while (rq->vq->num_free);
> - if (unlikely(!virtqueue_kick(rq->vq)))
> - return false;
> + virtqueue_kick(rq->vq);
> return !oom;
> }
>
> @@ -877,7 +876,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> err = xmit_skb(sq, skb);
>
> /* This should not happen! */
> - if (unlikely(err) || unlikely(!virtqueue_kick(sq->vq))) {
> + if (unlikely(err)) {
> dev->stats.tx_fifo_errors++;
> if (net_ratelimit())
> dev_warn(&dev->dev,
> @@ -886,6 +885,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> kfree_skb(skb);
> return NETDEV_TX_OK;
> }
> + virtqueue_kick(sq->vq);
>
> /* Don't wait up for transmitted skbs to be freed. */
> skb_orphan(skb);
> --
> 1.7.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] virtio-net: correct error handling of virtqueue_kick()
2014-03-26 5:03 [PATCH net] virtio-net: correct error handling of virtqueue_kick() Jason Wang
2014-03-26 6:59 ` Michael S. Tsirkin
@ 2014-03-27 17:14 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2014-03-27 17:14 UTC (permalink / raw)
To: jasowang; +Cc: mst, netdev, linux-kernel, virtualization
From: Jason Wang <jasowang@redhat.com>
Date: Wed, 26 Mar 2014 13:03:00 +0800
> Current error handling of virtqueue_kick() was wrong in two places:
> - The skb were freed immediately when virtqueue_kick() fail during
> xmit. This may lead double free since the skb was not detached from
> the virtqueue.
> - try_fill_recv() returns false when virtqueue_kick() fail. This will
> lead unnecessary rescheduling of refill work.
>
> Actually, it's safe to just ignore the kick failure in those two
> places. So this patch fixes this by partially revert commit
> 67975901183799af8e93ec60e322f9e2a1940b9b.
>
> Fixes 67975901183799af8e93ec60e322f9e2a1940b9b
> (virtio_net: verify if virtqueue_kick() succeeded).
>
> Cc: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-03-27 17:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-26 5:03 [PATCH net] virtio-net: correct error handling of virtqueue_kick() Jason Wang
2014-03-26 6:59 ` Michael S. Tsirkin
2014-03-27 17:14 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).