virtio_net: fix error handling for mergeable buffers
authorMichael S. Tsirkin <mst@redhat.com>
Thu, 26 Dec 2013 13:32:47 +0000 (15:32 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 15 Jan 2014 23:28:49 +0000 (15:28 -0800)
Eric Dumazet noticed that if we encounter an error
when processing a mergeable buffer, we don't
dequeue all of the buffers from this packet,
the result is almost sure to be loss of networking.

Fix this issue.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael Dalton <mwdalton@google.com>
Acked-by: Michael Dalton <mwdalton@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
(cherry picked from commit 8fc3b9e9a229778e5af3aa453c44f1a3857ba769)
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/net/virtio_net.c

index 5055eb7527c63d53aa7e2be08bf6360aaf674d0e..ad9b385e1f3d6cd4feee09c565b360a0bebae3a3 100644 (file)
@@ -294,26 +294,33 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq,
        return skb;
 }
 
-static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb)
+static struct sk_buff *receive_mergeable(struct net_device *dev,
+                                        struct receive_queue *rq,
+                                        void *buf,
+                                        unsigned int len)
 {
-       struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb);
-       struct page *page;
-       int num_buf, i, len;
+       struct skb_vnet_hdr *hdr = page_address(buf);
+       int num_buf = hdr->mhdr.num_buffers;
+       struct page *page = buf;
+       struct sk_buff *skb = page_to_skb(rq, page, len);
+       int i;
+
+       if (unlikely(!skb))
+               goto err_skb;
 
-       num_buf = hdr->mhdr.num_buffers;
        while (--num_buf) {
                i = skb_shinfo(skb)->nr_frags;
                if (i >= MAX_SKB_FRAGS) {
                        pr_debug("%s: packet too long\n", skb->dev->name);
                        skb->dev->stats.rx_length_errors++;
-                       return -EINVAL;
+                       return NULL;
                }
                page = virtqueue_get_buf(rq->vq, &len);
                if (!page) {
-                       pr_debug("%s: rx error: %d buffers missing\n",
-                                skb->dev->name, hdr->mhdr.num_buffers);
-                       skb->dev->stats.rx_length_errors++;
-                       return -EINVAL;
+                       pr_debug("%s: rx error: %d buffers %d missing\n",
+                                dev->name, hdr->mhdr.num_buffers, num_buf);
+                       dev->stats.rx_length_errors++;
+                       goto err_buf;
                }
 
                if (len > PAGE_SIZE)
@@ -323,7 +330,25 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb)
 
                --rq->num;
        }
-       return 0;
+       return skb;
+err_skb:
+       give_pages(rq, page);
+       while (--num_buf) {
+               buf = virtqueue_get_buf(rq->vq, &len);
+               if (unlikely(!buf)) {
+                       pr_debug("%s: rx error: %d buffers missing\n",
+                                dev->name, num_buf);
+                       dev->stats.rx_length_errors++;
+                       break;
+               }
+               page = buf;
+               give_pages(rq, page);
+               --rq->num;
+       }
+err_buf:
+       dev->stats.rx_dropped++;
+       dev_kfree_skb(skb);
+       return NULL;
 }
 
 static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
@@ -351,17 +376,18 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
                skb_trim(skb, len);
        } else {
                page = buf;
-               skb = page_to_skb(rq, page, len);
-               if (unlikely(!skb)) {
-                       dev->stats.rx_dropped++;
-                       give_pages(rq, page);
-                       return;
-               }
-               if (vi->mergeable_rx_bufs)
-                       if (receive_mergeable(rq, skb)) {
-                               dev_kfree_skb(skb);
+               if (vi->mergeable_rx_bufs) {
+                       skb = receive_mergeable(dev, rq, page, len);
+                       if (unlikely(!skb))
+                               return;
+               } else {
+                       skb = page_to_skb(rq, page, len);
+                       if (unlikely(!skb)) {
+                               dev->stats.rx_dropped++;
+                               give_pages(rq, page);
                                return;
                        }
+               }
        }
 
        hdr = skb_vnet_hdr(skb);