Jason Wang
2013-Nov-20 09:07 UTC
[PATCH net 1/3] virtio-net: drop the rest of buffers when we can't allocate skb
When mergeable buffer were used, we only put the first page buf leave the rest of buffers in the virt queue. This will cause the driver could not get the correct head buffer any more. Fix this by dropping the rest of buffers for this packet. The bug was introduced by commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399 (virtio_net: Defer skb allocation in receive path). Cc: Rusty Russell <rusty at rustcorp.com.au> Cc: Michael S. Tsirkin <mst at redhat.com> Cc: Michael Dalton <mwdalton at google.com> Cc: Eric Dumazet <edumazet at google.com> Cc: Shirley Ma <xma at us.ibm.com> Signed-off-by: Jason Wang <jasowang at redhat.com> --- This patch was needed for stable --- drivers/net/virtio_net.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 7bab4de..24fd502 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -222,6 +222,17 @@ static void skb_xmit_done(struct virtqueue *vq) netif_wake_subqueue(vi->dev, vq2txq(vq)); } +static void drop_mergeable_buffer(struct receive_queue *rq, int num_buf) +{ + char *buf; + int len; + + while (--num_buf && (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) { + --rq->num; + put_page(virt_to_head_page(buf)); + } +} + /* Called from bottom half context */ static struct sk_buff *page_to_skb(struct receive_queue *rq, struct page *page, unsigned int offset, @@ -237,8 +248,13 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq, /* copy small packet so we can reuse these pages for small data */ skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN); - if (unlikely(!skb)) + if (unlikely(!skb)) { + if (vi->mergeable_rx_bufs) { + hdr = (struct skb_vnet_hdr *)p; + drop_mergeable_buffer(rq, hdr->mhdr.num_buffers); + } return NULL; + } hdr = skb_vnet_hdr(skb); -- 1.8.3.2
Jason Wang
2013-Nov-20 09:07 UTC
[PATCH net 2/3] virtio-net: fix num calculation on frag skb allocation failure
We need decrease the rq->num after we can get a buf through virtqueue_get_buf() even if we could not allocate frag skb. Otherwise, the refill routine won't be triggered under heavy memory stress since the driver may still think there's enough room. This bug was introduced by commit 2613af0ed18a11d5c566a81f9a6510b73180660a (virtio_net: migrate mergeable rx buffers to page frag allocators). Cc: Rusty Russell <rusty at rustcorp.com.au> Cc: Michael S. Tsirkin <mst at redhat.com> Cc: Michael Dalton <mwdalton at google.com> Cc: Eric Dumazet <edumazet at google.com> Signed-off-by: Jason Wang <jasowang at redhat.com> --- The patch was needed for 3.12 stable. --- drivers/net/virtio_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 24fd502..de1d6ca 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -333,6 +333,7 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb) head_skb->dev->stats.rx_length_errors++; return -EINVAL; } + --rq->num; if (unlikely(len > MERGE_BUFFER_LEN)) { pr_debug("%s: rx error: merge buffer too long\n", head_skb->dev->name); @@ -367,7 +368,6 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb) skb_add_rx_frag(curr_skb, num_skb_frags, page, offset, len, MERGE_BUFFER_LEN); } - --rq->num; } return 0; } -- 1.8.3.2
Jason Wang
2013-Nov-20 09:07 UTC
[PATCH net 3/3] virtio-net: fix resources leaking when fail to allocate frag skb
When we fail to allocate a frag skb, we need put the refcnt of the page and drop the rest of the buffers. Otherwise the page was leaked and driver won't get a correct head buffer after this failure. This bug was introduced by commit 2613af0ed18a11d5c566a81f9a6510b73180660a (virtio_net: migrate mergeable rx buffers to page frag allocators). Cc: Rusty Russell <rusty at rustcorp.com.au> Cc: Michael S. Tsirkin <mst at redhat.com> Cc: Michael Dalton <mwdalton at google.com> Cc: Eric Dumazet <edumazet at google.com> Signed-off-by: Jason Wang <jasowang at redhat.com> --- The patch was needed for 3.12 stable. --- drivers/net/virtio_net.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index de1d6ca..f6f1e20 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -339,9 +339,12 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb) head_skb->dev->name); len = MERGE_BUFFER_LEN; } + page = virt_to_head_page(buf); if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) { struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC); if (unlikely(!nskb)) { + put_page(page); + drop_mergeable_buffer(rq, num_buf); head_skb->dev->stats.rx_dropped++; return -ENOMEM; } @@ -358,7 +361,6 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb) head_skb->len += len; head_skb->truesize += MERGE_BUFFER_LEN; } - page = virt_to_head_page(buf); offset = buf - (char *)page_address(page); if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) { put_page(page); -- 1.8.3.2
Michael S. Tsirkin
2013-Nov-20 10:34 UTC
[PATCH net 1/3] virtio-net: drop the rest of buffers when we can't allocate skb
On Wed, Nov 20, 2013 at 05:07:25PM +0800, Jason Wang wrote:> When mergeable buffer were used, we only put the first page buf leave the rest > of buffers in the virt queue. This will cause the driver could not get the > correct head buffer any more. Fix this by dropping the rest of buffers for this > packet. > > The bug was introduced by commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399 > (virtio_net: Defer skb allocation in receive path). > > Cc: Rusty Russell <rusty at rustcorp.com.au> > Cc: Michael S. Tsirkin <mst at redhat.com> > Cc: Michael Dalton <mwdalton at google.com> > Cc: Eric Dumazet <edumazet at google.com> > Cc: Shirley Ma <xma at us.ibm.com> > Signed-off-by: Jason Wang <jasowang at redhat.com> > --- > This patch was needed for stable > --- > drivers/net/virtio_net.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7bab4de..24fd502 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -222,6 +222,17 @@ static void skb_xmit_done(struct virtqueue *vq) > netif_wake_subqueue(vi->dev, vq2txq(vq)); > } > > +static void drop_mergeable_buffer(struct receive_queue *rq, int num_buf) > +{ > + char *buf; > + int len; > + > + while (--num_buf && (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) { > + --rq->num; > + put_page(virt_to_head_page(buf)); > + } > +} > +This is the same code we have in receive_mergeable anyway. So let's reuse that.> /* Called from bottom half context */ > static struct sk_buff *page_to_skb(struct receive_queue *rq, > struct page *page, unsigned int offset, > @@ -237,8 +248,13 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq, > > /* copy small packet so we can reuse these pages for small data */ > skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN); > - if (unlikely(!skb)) > + if (unlikely(!skb)) { > + if (vi->mergeable_rx_bufs) { > + hdr = (struct skb_vnet_hdr *)p; > + drop_mergeable_buffer(rq, hdr->mhdr.num_buffers); > + } > return NULL; > + } > > hdr = skb_vnet_hdr(skb); > > -- > 1.8.3.2
Michael S. Tsirkin
2013-Nov-20 10:37 UTC
[PATCH net 2/3] virtio-net: fix num calculation on frag skb allocation failure
On Wed, Nov 20, 2013 at 05:07:26PM +0800, Jason Wang wrote:> We need decrease the rq->num after we can get a buf through > virtqueue_get_buf() even if we could not allocate frag skb. Otherwise, the > refill routine won't be triggered under heavy memory stress since the driver may > still think there's enough room. > > This bug was introduced by commit 2613af0ed18a11d5c566a81f9a6510b73180660a > (virtio_net: migrate mergeable rx buffers to page frag allocators). > > Cc: Rusty Russell <rusty at rustcorp.com.au> > Cc: Michael S. Tsirkin <mst at redhat.com> > Cc: Michael Dalton <mwdalton at google.com> > Cc: Eric Dumazet <edumazet at google.com> > Signed-off-by: Jason Wang <jasowang at redhat.com>So let's wrap virtqueue_get_buf to make sure we get it right?> --- > The patch was needed for 3.12 stable. > --- > drivers/net/virtio_net.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 24fd502..de1d6ca 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -333,6 +333,7 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb) > head_skb->dev->stats.rx_length_errors++; > return -EINVAL; > } > + --rq->num; > if (unlikely(len > MERGE_BUFFER_LEN)) { > pr_debug("%s: rx error: merge buffer too long\n", > head_skb->dev->name); > @@ -367,7 +368,6 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb) > skb_add_rx_frag(curr_skb, num_skb_frags, page, > offset, len, MERGE_BUFFER_LEN); > } > - --rq->num; > } > return 0; > } > -- > 1.8.3.2
Jason Wang
2013-Nov-20 12:08 UTC
[PATCH net 1/3] virtio-net: drop the rest of buffers when we can't allocate skb
----- ???? -----> On Wed, Nov 20, 2013 at 05:07:25PM +0800, Jason Wang wrote: > > When mergeable buffer were used, we only put the first page buf leave the > > rest > > of buffers in the virt queue. This will cause the driver could not get the > > correct head buffer any more. Fix this by dropping the rest of buffers for > > this > > packet. > > > > The bug was introduced by commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399 > > (virtio_net: Defer skb allocation in receive path). > > > > Cc: Rusty Russell <rusty at rustcorp.com.au> > > Cc: Michael S. Tsirkin <mst at redhat.com> > > Cc: Michael Dalton <mwdalton at google.com> > > Cc: Eric Dumazet <edumazet at google.com> > > Cc: Shirley Ma <xma at us.ibm.com> > > Signed-off-by: Jason Wang <jasowang at redhat.com> > > --- > > This patch was needed for stable > > --- > > drivers/net/virtio_net.c | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > index 7bab4de..24fd502 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -222,6 +222,17 @@ static void skb_xmit_done(struct virtqueue *vq) > > netif_wake_subqueue(vi->dev, vq2txq(vq)); > > } > > > > +static void drop_mergeable_buffer(struct receive_queue *rq, int num_buf) > > +{ > > + char *buf; > > + int len; > > + > > + while (--num_buf && (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) { > > + --rq->num; > > + put_page(virt_to_head_page(buf)); > > + } > > +} > > + > > This is the same code we have in receive_mergeable anyway. > So let's reuse that. > >receive_mergeable() was called after page_to_skb() was called and there's lots of conditions check there. I'm not sure how could we reuse them.> > /* Called from bottom half context */ > > static struct sk_buff *page_to_skb(struct receive_queue *rq, > > struct page *page, unsigned int offset, > > @@ -237,8 +248,13 @@ static struct sk_buff *page_to_skb(struct > > receive_queue *rq, > > > > /* copy small packet so we can reuse these pages for small data */ > > skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN); > > - if (unlikely(!skb)) > > + if (unlikely(!skb)) { > > + if (vi->mergeable_rx_bufs) { > > + hdr = (struct skb_vnet_hdr *)p; > > + drop_mergeable_buffer(rq, hdr->mhdr.num_buffers); > > + } > > return NULL; > > + } > > > > hdr = skb_vnet_hdr(skb); > > > > -- > > 1.8.3.2 > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
Michael S. Tsirkin
2013-Nov-20 13:26 UTC
[PATCH RFC] virtio_net: fix error handling for mergeable buffers
On Wed, Nov 20, 2013 at 05:07:25PM +0800, Jason Wang wrote:> When mergeable buffer were used, we only put the first page buf leave the rest > of buffers in the virt queue. This will cause the driver could not get the > correct head buffer any more. Fix this by dropping the rest of buffers for this > packet. > > The bug was introduced by commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399 > (virtio_net: Defer skb allocation in receive path). > > Cc: Rusty Russell <rusty at rustcorp.com.au> > Cc: Michael S. Tsirkin <mst at redhat.com> > Cc: Michael Dalton <mwdalton at google.com> > Cc: Eric Dumazet <edumazet at google.com> > Cc: Shirley Ma <xma at us.ibm.com> > Signed-off-by: Jason Wang <jasowang at redhat.com>Just to clarify my previous comment: it was not about the idea of adding drop_mergeable_buffer - rather, I think that adding knowledge about mergeable buffers into page_to_skb creates an ugly internal API. Let's move the call to page_to_skb within receive_mergeable instead: it's also nice that int offset = buf - page_address(page) logic is not spread around like it was. Also, it's not nice that we ignore length errors when we drop packets because of OOM. So I came up with the following - it seems to work but I didn't stress test yet. commit ebffb3fe4335ffe07124e4518e76d6e05844fa18 Author: Michael S. Tsirkin <mst at redhat.com> Date: Wed Nov 20 14:41:29 2013 +0200 virtio_net: fix error handling for mergeable buffers 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. Jason Wang noticed that we also leak a page and that we don't decrement the rq buf count, so we won't repost buffers (a resource leak). Cc: Rusty Russell <rusty at rustcorp.com.au> Cc: Michael Dalton <mwdalton at google.com> Reported-by: Eric Dumazet <edumazet at google.com> Reported-by: Jason Wang <jasowang at redhat.com> Signed-off-by: Michael S. Tsirkin <mst at redhat.com> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 01f4eb5..42f6a1e 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -296,41 +296,53 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq, return skb; } -static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_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(head_skb); + struct skb_vnet_hdr *hdr = buf; + int num_buf = hdr->mhdr.num_buffers; + struct page *page = virt_to_head_page(buf); + int offset = buf - page_address(page); + struct sk_buff *head_skb = page_to_skb(rq, page, offset, len, + MAX_PACKET_LEN); struct sk_buff *curr_skb = head_skb; - char *buf; - struct page *page; - int num_buf, len, offset; - num_buf = hdr->mhdr.num_buffers; - while (--num_buf) { - int num_skb_frags = skb_shinfo(curr_skb)->nr_frags; + if (unlikely(!curr_skb)) + goto err_skb; + + while (--num_buf) { + int num_skb_frags; + buf = virtqueue_get_buf(rq->vq, &len); if (unlikely(!buf)) { - pr_debug("%s: rx error: %d buffers missing\n", - head_skb->dev->name, hdr->mhdr.num_buffers); - head_skb->dev->stats.rx_length_errors++; - return -EINVAL; + pr_debug("%s: rx error: %d buffers out of %d missing\n", + dev->name, num_buf, hdr->mhdr.num_buffers); + dev->stats.rx_length_errors++; + goto err_buf; } if (unlikely(len > MAX_PACKET_LEN)) { pr_debug("%s: rx error: merge buffer too long\n", - head_skb->dev->name); + dev->name); len = MAX_PACKET_LEN; } + + page = virt_to_head_page(buf); + --rq->num; + + num_skb_frags = skb_shinfo(curr_skb)->nr_frags; if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) { struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC); - if (unlikely(!nskb)) { - head_skb->dev->stats.rx_dropped++; - return -ENOMEM; - } + + if (unlikely(!nskb)) + goto err_skb; if (curr_skb == head_skb) skb_shinfo(curr_skb)->frag_list = nskb; else curr_skb->next = nskb; - curr_skb = nskb; head_skb->truesize += nskb->truesize; + curr_skb = nskb; num_skb_frags = 0; } if (curr_skb != head_skb) { @@ -338,8 +350,7 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb) head_skb->len += len; head_skb->truesize += MAX_PACKET_LEN; } - page = virt_to_head_page(buf); - offset = buf - (char *)page_address(page); + offset = buf - page_address(page); if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) { put_page(page); skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1, @@ -349,9 +360,28 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *head_skb) offset, len, MAX_PACKET_LEN); } + } + + return head_skb; + +err_skb: + put_page(page); +err_buf: + dev->stats.rx_dropped++; + dev_kfree_skb(head_skb); + 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 = virt_to_head_page(buf); + put_page(page); --rq->num; } - return 0; + return NULL; } static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) @@ -380,19 +410,9 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) len -= sizeof(struct virtio_net_hdr); skb_trim(skb, len); } else if (vi->mergeable_rx_bufs) { - struct page *page = virt_to_head_page(buf); - skb = page_to_skb(rq, page, - (char *)buf - (char *)page_address(page), - len, MAX_PACKET_LEN); - if (unlikely(!skb)) { - dev->stats.rx_dropped++; - put_page(page); + skb = receive_mergeable(dev, rq, buf, len); + if (unlikely(!skb)) return; - } - if (receive_mergeable(rq, skb)) { - dev_kfree_skb(skb); - return; - } } else { page = buf; skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
Jason Wang
2013-Nov-20 13:54 UTC
[PATCH RFC] virtio_net: fix error handling for mergeable buffers
----- ???? -----> On Wed, Nov 20, 2013 at 05:07:25PM +0800, Jason Wang wrote: > > When mergeable buffer were used, we only put the first page buf leave the > > rest > > of buffers in the virt queue. This will cause the driver could not get the > > correct head buffer any more. Fix this by dropping the rest of buffers for > > this > > packet. > > > > The bug was introduced by commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399 > > (virtio_net: Defer skb allocation in receive path). > > > > Cc: Rusty Russell <rusty at rustcorp.com.au> > > Cc: Michael S. Tsirkin <mst at redhat.com> > > Cc: Michael Dalton <mwdalton at google.com> > > Cc: Eric Dumazet <edumazet at google.com> > > Cc: Shirley Ma <xma at us.ibm.com> > > Signed-off-by: Jason Wang <jasowang at redhat.com> > > Just to clarify my previous comment: it was not about the > idea of adding drop_mergeable_buffer - rather, I think that > adding knowledge about mergeable buffers into page_to_skb creates an > ugly internal API. > > Let's move the call to page_to_skb within receive_mergeable instead: > it's also nice that int offset = buf - page_address(page) logic > is not spread around like it was. > > Also, it's not nice that we ignore length errors when we drop > packets because of OOM. > > So I came up with the following - it seems to work but I didn't > stress test yet.I've no objection on this. But I've rather like my small and direct patch to be applied to -net first. It has lower risk and was much more easier to be backported to stable trees. Then we can do the re-factor like this in net-next.> > commit ebffb3fe4335ffe07124e4518e76d6e05844fa18 > Author: Michael S. Tsirkin <mst at redhat.com> > Date: Wed Nov 20 14:41:29 2013 +0200 > > virtio_net: fix error handling for mergeable buffers > > 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. > > Jason Wang noticed that we also leak a page and that we don't decrement > the rq buf count, so we won't repost buffers (a resource leak). > > Cc: Rusty Russell <rusty at rustcorp.com.au> > Cc: Michael Dalton <mwdalton at google.com> > Reported-by: Eric Dumazet <edumazet at google.com> > Reported-by: Jason Wang <jasowang at redhat.com> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 01f4eb5..42f6a1e 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -296,41 +296,53 @@ static struct sk_buff *page_to_skb(struct receive_queue > *rq, > return skb; > } > > -static int receive_mergeable(struct receive_queue *rq, struct sk_buff > *head_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(head_skb); > + struct skb_vnet_hdr *hdr = buf; > + int num_buf = hdr->mhdr.num_buffers; > + struct page *page = virt_to_head_page(buf); > + int offset = buf - page_address(page); > + struct sk_buff *head_skb = page_to_skb(rq, page, offset, len, > + MAX_PACKET_LEN); > struct sk_buff *curr_skb = head_skb; > - char *buf; > - struct page *page; > - int num_buf, len, offset; > > - num_buf = hdr->mhdr.num_buffers; > - while (--num_buf) { > - int num_skb_frags = skb_shinfo(curr_skb)->nr_frags; > + if (unlikely(!curr_skb)) > + goto err_skb; > + > + while (--num_buf) { > + int num_skb_frags; > + > buf = virtqueue_get_buf(rq->vq, &len); > if (unlikely(!buf)) { > - pr_debug("%s: rx error: %d buffers missing\n", > - head_skb->dev->name, hdr->mhdr.num_buffers); > - head_skb->dev->stats.rx_length_errors++; > - return -EINVAL; > + pr_debug("%s: rx error: %d buffers out of %d missing\n", > + dev->name, num_buf, hdr->mhdr.num_buffers); > + dev->stats.rx_length_errors++; > + goto err_buf; > } > if (unlikely(len > MAX_PACKET_LEN)) { > pr_debug("%s: rx error: merge buffer too long\n", > - head_skb->dev->name); > + dev->name); > len = MAX_PACKET_LEN; > } > + > + page = virt_to_head_page(buf); > + --rq->num; > + > + num_skb_frags = skb_shinfo(curr_skb)->nr_frags; > if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) { > struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC); > - if (unlikely(!nskb)) { > - head_skb->dev->stats.rx_dropped++; > - return -ENOMEM; > - } > + > + if (unlikely(!nskb)) > + goto err_skb; > if (curr_skb == head_skb) > skb_shinfo(curr_skb)->frag_list = nskb; > else > curr_skb->next = nskb; > - curr_skb = nskb; > head_skb->truesize += nskb->truesize; > + curr_skb = nskb; > num_skb_frags = 0; > } > if (curr_skb != head_skb) { > @@ -338,8 +350,7 @@ static int receive_mergeable(struct receive_queue *rq, > struct sk_buff *head_skb) > head_skb->len += len; > head_skb->truesize += MAX_PACKET_LEN; > } > - page = virt_to_head_page(buf); > - offset = buf - (char *)page_address(page); > + offset = buf - page_address(page); > if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) { > put_page(page); > skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1, > @@ -349,9 +360,28 @@ static int receive_mergeable(struct receive_queue *rq, > struct sk_buff *head_skb) > offset, len, > MAX_PACKET_LEN); > } > + } > + > + return head_skb; > + > +err_skb: > + put_page(page); > +err_buf: > + dev->stats.rx_dropped++; > + dev_kfree_skb(head_skb); > + 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 = virt_to_head_page(buf); > + put_page(page); > --rq->num; > } > - return 0; > + return NULL; > } > > static void receive_buf(struct receive_queue *rq, void *buf, unsigned int > len) > @@ -380,19 +410,9 @@ static void receive_buf(struct receive_queue *rq, void > *buf, unsigned int len) > len -= sizeof(struct virtio_net_hdr); > skb_trim(skb, len); > } else if (vi->mergeable_rx_bufs) { > - struct page *page = virt_to_head_page(buf); > - skb = page_to_skb(rq, page, > - (char *)buf - (char *)page_address(page), > - len, MAX_PACKET_LEN); > - if (unlikely(!skb)) { > - dev->stats.rx_dropped++; > - put_page(page); > + skb = receive_mergeable(dev, rq, buf, len); > + if (unlikely(!skb)) > return; > - } > - if (receive_mergeable(rq, skb)) { > - dev_kfree_skb(skb); > - return; > - } > } else { > page = buf; > skb = page_to_skb(rq, page, 0, len, PAGE_SIZE); > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Reasonably Related Threads
- [PATCH net 1/3] virtio-net: drop the rest of buffers when we can't allocate skb
- [PATCH net 1/3] virtio-net: drop the rest of buffers when we can't allocate skb
- [PATCH net 1/3] virtio-net: drop the rest of buffers when we can't allocate skb
- [PATCH net 1/3] virtio-net: drop the rest of buffers when we can't allocate skb
- [PATCH RFC] virtio_net: fix error handling for mergeable buffers