* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode [not found] <1622775955.0233824-1-xuanzhuo@linux.alibaba.com> @ 2021-06-04 6:00 ` Jason Wang 0 siblings, 0 replies; 7+ messages in thread From: Jason Wang @ 2021-06-04 6:00 UTC (permalink / raw) To: Xuan Zhuo Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski, virtualization, Corentin Noël, netdev 在 2021/6/4 上午11:05, Xuan Zhuo 写道: > On Fri, 4 Jun 2021 11:00:25 +0800, Jason Wang <jasowang@redhat.com> wrote: >> 在 2021/6/4 上午10:30, Xuan Zhuo 写道: >>> On Fri, 4 Jun 2021 10:28:41 +0800, Jason Wang <jasowang@redhat.com> wrote: >>>> 在 2021/6/4 上午1:09, Xuan Zhuo 写道: >>>>> In virtio-net's large packet mode, there is a hole in the space behind >>>>> buf. >>>> before the buf actually or behind the vnet header? >>>> >>>> >>>>> hdr_padded_len - hdr_len >>>>> >>>>> We must take this into account when calculating tailroom. >>>>> >>>>> [ 44.544385] skb_put.cold (net/core/skbuff.c:5254 (discriminator 1) net/core/skbuff.c:5252 (discriminator 1)) >>>>> [ 44.544864] page_to_skb (drivers/net/virtio_net.c:485) [ 44.545361] receive_buf (drivers/net/virtio_net.c:849 drivers/net/virtio_net.c:1131) >>>>> [ 44.545870] ? netif_receive_skb_list_internal (net/core/dev.c:5714) >>>>> [ 44.546628] ? dev_gro_receive (net/core/dev.c:6103) >>>>> [ 44.547135] ? napi_complete_done (./include/linux/list.h:35 net/core/dev.c:5867 net/core/dev.c:5862 net/core/dev.c:6565) >>>>> [ 44.547672] virtnet_poll (drivers/net/virtio_net.c:1427 drivers/net/virtio_net.c:1525) >>>>> [ 44.548251] __napi_poll (net/core/dev.c:6985) >>>>> [ 44.548744] net_rx_action (net/core/dev.c:7054 net/core/dev.c:7139) >>>>> [ 44.549264] __do_softirq (./arch/x86/include/asm/jump_label.h:19 ./include/linux/jump_label.h:200 ./include/trace/events/irq.h:142 kernel/softirq.c:560) >>>>> [ 44.549762] irq_exit_rcu (kernel/softirq.c:433 kernel/softirq.c:637 kernel/softirq.c:649) >>>>> [ 44.551384] common_interrupt (arch/x86/kernel/irq.c:240 (discriminator 13)) >>>>> [ 44.551991] ? asm_common_interrupt (./arch/x86/include/asm/idtentry.h:638) >>>>> [ 44.552654] asm_common_interrupt (./arch/x86/include/asm/idtentry.h:638) >>>>> >>>>> Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") >>>>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> >>>>> Reported-by: Corentin Noël <corentin.noel@collabora.com> >>>>> Tested-by: Corentin Noël <corentin.noel@collabora.com> >>>>> --- >>>>> 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 fa407eb8b457..78a01c71a17c 100644 >>>>> --- a/drivers/net/virtio_net.c >>>>> +++ b/drivers/net/virtio_net.c >>>>> @@ -406,7 +406,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, >>>>> * add_recvbuf_mergeable() + get_mergeable_buf_len() >>>>> */ >>>>> truesize = headroom ? PAGE_SIZE : truesize; >>>>> - tailroom = truesize - len - headroom; >>>>> + tailroom = truesize - len - headroom - (hdr_padded_len - hdr_len); >>>> The patch looks correct and I saw it has been merged. >>>> >>>> But I prefer to do that in receive_big() instead of here. >>>> >>>> Thanks >>> How? >>> >>> change truesize or headroom? >>> >>> I didn't find a good way. Do you have a good way? >> >> Something like the following? The API is designed to let the caller to >> pass a correct headroom instead of figure it out by itself. >> >> struct sk_buff *skb = >> page_to_skb(vi, rq, page, 0, len, PAGE_SIZE, true, 0, >> hdr_padded_len - hdr_len); >> >> Thanks > > This line may be affected. > > buf = p - headroom; > > In my opinion, this changes the semantics of the original headroom. The meaning > of headroom in big mode and merge mode has become different. The more confusing > problem is that the parameters of page_to_skb() are getting more and more > chaotic. So I wrote the previous patch. Of course, I understand your concern. > This patch may bring Here are more questions, although I did a lot of tests. > > "virtio-net: Refactor the code related to page_to_skb" > > But I hope that our code development direction is as close to what this patch > realizes. I hope that the meaning of the parameters can be more clear. So I don't object to this method, but as I replied, it's better to do some benchmark to see if it introduces any regression > > Do you think this is ok? Looks ok, but if we decide to go with your approach, it can be squashed into that patch. Thanks > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 78a01c71a17c..6d62bb45a188 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -380,34 +380,20 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > struct page *page, unsigned int offset, > unsigned int len, unsigned int truesize, > bool hdr_valid, unsigned int metasize, > - unsigned int headroom) > + int tailroom, char *buf, > + unsigned int hdr_padded_len) > { > struct sk_buff *skb; > struct virtio_net_hdr_mrg_rxbuf *hdr; > - unsigned int copy, hdr_len, hdr_padded_len; > + unsigned int copy, hdr_len; > struct page *page_to_free = NULL; > - int tailroom, shinfo_size; > - char *p, *hdr_p, *buf; > + int shinfo_size; > + char *p, *hdr_p; > > p = page_address(page) + offset; > hdr_p = p; > > hdr_len = vi->hdr_len; > - if (vi->mergeable_rx_bufs) > - hdr_padded_len = sizeof(*hdr); > - else > - hdr_padded_len = sizeof(struct padded_vnet_hdr); > - > - /* If headroom is not 0, there is an offset between the beginning of the > - * data and the allocated space, otherwise the data and the allocated > - * space are aligned. > - * > - * Buffers with headroom use PAGE_SIZE as alloc size, see > - * add_recvbuf_mergeable() + get_mergeable_buf_len() > - */ > - truesize = headroom ? PAGE_SIZE : truesize; > - tailroom = truesize - len - headroom - (hdr_padded_len - hdr_len); > - buf = p - headroom; > > len -= hdr_len; > offset += hdr_padded_len; > @@ -492,6 +478,51 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > return skb; > } > > +static struct sk_buff *merge_page_to_skb(struct virtnet_info *vi, > + struct receive_queue *rq, > + struct page *page, unsigned int offset, > + unsigned int len, unsigned int truesize, > + bool hdr_valid, unsigned int metasize, > + unsigned int headroom) > +{ > + int tailroom; > + char *buf; > + > + /* If headroom is not 0, there is an offset between the beginning of the > + * data and the allocated space, otherwise the data and the allocated > + * space are aligned. > + * > + * Buffers with headroom use PAGE_SIZE as alloc size, see > + * add_recvbuf_mergeable() + get_mergeable_buf_len() > + */ > + truesize = headroom ? PAGE_SIZE : truesize; > + tailroom = truesize - len - headroom; > + buf = page_address(page) + offset - headroom; > + > + page_to_skb(vi, rq, page, offset, len, truesize, hdr_valid, metasize, > + tailroom, buf, sizeof(struct virtio_net_hdr_mrg_rxbuf)) > + > +} > + > +static struct sk_buff *big_page_to_skb(struct virtnet_info *vi, > + struct receive_queue *rq, > + struct page *page, unsigned int offset, > + unsigned int len, unsigned int truesize, > + bool hdr_valid, unsigned int metasize, > + unsigned int headroom) > +{ > + char *p = page_address(page); > + int hold; > + int tailroom; > + > + hold = sizeof(struct padded_vnet_hdr) - vi->hdr_len; > + > + tailroom = truesize - len - headroom - hold; > + > + page_to_skb(vi, rq, page, offset, len, truesize, hdr_valid, metasize, > + tailroom, p, sizeof(struct padded_vnet_hdr)); > +} > + > static int __virtnet_xdp_xmit_one(struct virtnet_info *vi, > struct send_queue *sq, > struct xdp_frame *xdpf) > > >> >>> Thanks. >>> >>>> >>>>> buf = p - headroom; >>>>> >>>>> len -= hdr_len; ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-10-09 9:17 Michael S. Tsirkin 2021-10-11 2:04 ` Jason Wang 0 siblings, 1 reply; 7+ messages in thread From: Michael S. Tsirkin @ 2021-10-09 9:17 UTC (permalink / raw) To: linux-kernel Cc: Xuan Zhuo, Greg KH, Corentin Noël, Jason Wang, David S. Miller, Jakub Kicinski, virtualization, netdev From: Xuan Zhuo <xuanzhuo@linux.alibaba.com> commit 126285651b7f ("Merge ra.kernel.org:/pub/scm/linux/kernel/git/netdev/net") accidentally reverted the effect of commit 1a8024239da ("virtio-net: fix for skb_over_panic inside big mode") on drivers/net/virtio_net.c As a result, users of crosvm (which is using large packet mode) are experiencing crashes with 5.14-rc1 and above that do not occur with 5.13. Crash trace: [ 61.346677] skbuff: skb_over_panic: text:ffffffff881ae2c7 len:3762 put:3762 head:ffff8a5ec8c22000 data:ffff8a5ec8c22010 tail:0xec2 end:0xec0 dev:<NULL> [ 61.369192] kernel BUG at net/core/skbuff.c:111! [ 61.372840] invalid opcode: 0000 [#1] SMP PTI [ 61.374892] CPU: 5 PID: 0 Comm: swapper/5 Not tainted 5.14.0-rc1 linux-v5.14-rc1-for-mesa-ci.tar.bz2 #1 [ 61.376450] Hardware name: ChromiumOS crosvm, BIOS 0 .. [ 61.393635] Call Trace: [ 61.394127] <IRQ> [ 61.394488] skb_put.cold+0x10/0x10 [ 61.395095] page_to_skb+0xf7/0x410 [ 61.395689] receive_buf+0x81/0x1660 [ 61.396228] ? netif_receive_skb_list_internal+0x1ad/0x2b0 [ 61.397180] ? napi_gro_flush+0x97/0xe0 [ 61.397896] ? detach_buf_split+0x67/0x120 [ 61.398573] virtnet_poll+0x2cf/0x420 [ 61.399197] __napi_poll+0x25/0x150 [ 61.399764] net_rx_action+0x22f/0x280 [ 61.400394] __do_softirq+0xba/0x257 [ 61.401012] irq_exit_rcu+0x8e/0xb0 [ 61.401618] common_interrupt+0x7b/0xa0 [ 61.402270] </IRQ> See https://lore.kernel.org/r/5edaa2b7c2fe4abd0347b8454b2ac032b6694e2c.camel%40collabora.com for the report. Apply the original 1a8024239da ("virtio-net: fix for skb_over_panic inside big mode") again, the original logic still holds: In virtio-net's large packet mode, there is a hole in the space behind buf. hdr_padded_len - hdr_len We must take this into account when calculating tailroom. Cc: Greg KH <gregkh@linuxfoundation.org> Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") Fixes: 126285651b7f ("Merge ra.kernel.org:/pub/scm/linux/kernel/git/netdev/net") Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> Reported-by: Corentin Noël <corentin.noel@collabora.com> Tested-by: Corentin Noël <corentin.noel@collabora.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- David, I think we need this in stable, too. 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 096c2ac6b7a6..6b0812f44bbf 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -406,7 +406,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, * add_recvbuf_mergeable() + get_mergeable_buf_len() */ truesize = headroom ? PAGE_SIZE : truesize; - tailroom = truesize - len - headroom; + tailroom = truesize - len - headroom - (hdr_padded_len - hdr_len); buf = p - headroom; len -= hdr_len; -- MST ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode 2021-10-09 9:17 Michael S. Tsirkin @ 2021-10-11 2:04 ` Jason Wang 0 siblings, 0 replies; 7+ messages in thread From: Jason Wang @ 2021-10-11 2:04 UTC (permalink / raw) To: Michael S. Tsirkin Cc: linux-kernel, Xuan Zhuo, Greg KH, Corentin Noël, David S. Miller, Jakub Kicinski, virtualization, netdev On Sat, Oct 9, 2021 at 5:18 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > commit 126285651b7f ("Merge ra.kernel.org:/pub/scm/linux/kernel/git/netdev/net") > accidentally reverted the effect of > commit 1a8024239da ("virtio-net: fix for skb_over_panic inside big mode") > on drivers/net/virtio_net.c > > As a result, users of crosvm (which is using large packet mode) > are experiencing crashes with 5.14-rc1 and above that do not > occur with 5.13. > > Crash trace: > > [ 61.346677] skbuff: skb_over_panic: text:ffffffff881ae2c7 len:3762 put:3762 head:ffff8a5ec8c22000 data:ffff8a5ec8c22010 tail:0xec2 end:0xec0 dev:<NULL> > [ 61.369192] kernel BUG at net/core/skbuff.c:111! > [ 61.372840] invalid opcode: 0000 [#1] SMP PTI > [ 61.374892] CPU: 5 PID: 0 Comm: swapper/5 Not tainted 5.14.0-rc1 linux-v5.14-rc1-for-mesa-ci.tar.bz2 #1 > [ 61.376450] Hardware name: ChromiumOS crosvm, BIOS 0 > > .. > > [ 61.393635] Call Trace: > [ 61.394127] <IRQ> > [ 61.394488] skb_put.cold+0x10/0x10 > [ 61.395095] page_to_skb+0xf7/0x410 > [ 61.395689] receive_buf+0x81/0x1660 > [ 61.396228] ? netif_receive_skb_list_internal+0x1ad/0x2b0 > [ 61.397180] ? napi_gro_flush+0x97/0xe0 > [ 61.397896] ? detach_buf_split+0x67/0x120 > [ 61.398573] virtnet_poll+0x2cf/0x420 > [ 61.399197] __napi_poll+0x25/0x150 > [ 61.399764] net_rx_action+0x22f/0x280 > [ 61.400394] __do_softirq+0xba/0x257 > [ 61.401012] irq_exit_rcu+0x8e/0xb0 > [ 61.401618] common_interrupt+0x7b/0xa0 > [ 61.402270] </IRQ> > > See > https://lore.kernel.org/r/5edaa2b7c2fe4abd0347b8454b2ac032b6694e2c.camel%40collabora.com > for the report. > > Apply the original 1a8024239da ("virtio-net: fix for skb_over_panic inside big mode") > again, the original logic still holds: > > In virtio-net's large packet mode, there is a hole in the space behind > buf. > > hdr_padded_len - hdr_len > > We must take this into account when calculating tailroom. > > Cc: Greg KH <gregkh@linuxfoundation.org> > Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") > Fixes: 126285651b7f ("Merge ra.kernel.org:/pub/scm/linux/kernel/git/netdev/net") > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > Reported-by: Corentin Noël <corentin.noel@collabora.com> > Tested-by: Corentin Noël <corentin.noel@collabora.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- Acked-by: Jason Wang <jasowang@redhat.com> > > David, I think we need this in stable, too. > > 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 096c2ac6b7a6..6b0812f44bbf 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -406,7 +406,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > * add_recvbuf_mergeable() + get_mergeable_buf_len() > */ > truesize = headroom ? PAGE_SIZE : truesize; > - tailroom = truesize - len - headroom; > + tailroom = truesize - len - headroom - (hdr_padded_len - hdr_len); > buf = p - headroom; > > len -= hdr_len; > -- > MST > ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <1622773823.5042562-1-xuanzhuo@linux.alibaba.com>]
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode [not found] <1622773823.5042562-1-xuanzhuo@linux.alibaba.com> @ 2021-06-04 3:00 ` Jason Wang 0 siblings, 0 replies; 7+ messages in thread From: Jason Wang @ 2021-06-04 3:00 UTC (permalink / raw) To: Xuan Zhuo Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski, virtualization, Corentin Noël, netdev 在 2021/6/4 上午10:30, Xuan Zhuo 写道: > On Fri, 4 Jun 2021 10:28:41 +0800, Jason Wang <jasowang@redhat.com> wrote: >> 在 2021/6/4 上午1:09, Xuan Zhuo 写道: >>> In virtio-net's large packet mode, there is a hole in the space behind >>> buf. >> >> before the buf actually or behind the vnet header? >> >> >>> hdr_padded_len - hdr_len >>> >>> We must take this into account when calculating tailroom. >>> >>> [ 44.544385] skb_put.cold (net/core/skbuff.c:5254 (discriminator 1) net/core/skbuff.c:5252 (discriminator 1)) >>> [ 44.544864] page_to_skb (drivers/net/virtio_net.c:485) [ 44.545361] receive_buf (drivers/net/virtio_net.c:849 drivers/net/virtio_net.c:1131) >>> [ 44.545870] ? netif_receive_skb_list_internal (net/core/dev.c:5714) >>> [ 44.546628] ? dev_gro_receive (net/core/dev.c:6103) >>> [ 44.547135] ? napi_complete_done (./include/linux/list.h:35 net/core/dev.c:5867 net/core/dev.c:5862 net/core/dev.c:6565) >>> [ 44.547672] virtnet_poll (drivers/net/virtio_net.c:1427 drivers/net/virtio_net.c:1525) >>> [ 44.548251] __napi_poll (net/core/dev.c:6985) >>> [ 44.548744] net_rx_action (net/core/dev.c:7054 net/core/dev.c:7139) >>> [ 44.549264] __do_softirq (./arch/x86/include/asm/jump_label.h:19 ./include/linux/jump_label.h:200 ./include/trace/events/irq.h:142 kernel/softirq.c:560) >>> [ 44.549762] irq_exit_rcu (kernel/softirq.c:433 kernel/softirq.c:637 kernel/softirq.c:649) >>> [ 44.551384] common_interrupt (arch/x86/kernel/irq.c:240 (discriminator 13)) >>> [ 44.551991] ? asm_common_interrupt (./arch/x86/include/asm/idtentry.h:638) >>> [ 44.552654] asm_common_interrupt (./arch/x86/include/asm/idtentry.h:638) >>> >>> Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") >>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> >>> Reported-by: Corentin Noël <corentin.noel@collabora.com> >>> Tested-by: Corentin Noël <corentin.noel@collabora.com> >>> --- >>> 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 fa407eb8b457..78a01c71a17c 100644 >>> --- a/drivers/net/virtio_net.c >>> +++ b/drivers/net/virtio_net.c >>> @@ -406,7 +406,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, >>> * add_recvbuf_mergeable() + get_mergeable_buf_len() >>> */ >>> truesize = headroom ? PAGE_SIZE : truesize; >>> - tailroom = truesize - len - headroom; >>> + tailroom = truesize - len - headroom - (hdr_padded_len - hdr_len); >> >> The patch looks correct and I saw it has been merged. >> >> But I prefer to do that in receive_big() instead of here. >> >> Thanks > How? > > change truesize or headroom? > > I didn't find a good way. Do you have a good way? Something like the following? The API is designed to let the caller to pass a correct headroom instead of figure it out by itself. struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len, PAGE_SIZE, true, 0, hdr_padded_len - hdr_len); Thanks > > Thanks. > >> >> >>> buf = p - headroom; >>> >>> len -= hdr_len; ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-06-03 17:09 Xuan Zhuo 2021-06-03 22:30 ` patchwork-bot+netdevbpf 2021-06-04 2:28 ` Jason Wang 0 siblings, 2 replies; 7+ messages in thread From: Xuan Zhuo @ 2021-06-03 17:09 UTC (permalink / raw) To: netdev Cc: Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski, Xuan Zhuo, virtualization, Corentin Noël In virtio-net's large packet mode, there is a hole in the space behind buf. hdr_padded_len - hdr_len We must take this into account when calculating tailroom. [ 44.544385] skb_put.cold (net/core/skbuff.c:5254 (discriminator 1) net/core/skbuff.c:5252 (discriminator 1)) [ 44.544864] page_to_skb (drivers/net/virtio_net.c:485) [ 44.545361] receive_buf (drivers/net/virtio_net.c:849 drivers/net/virtio_net.c:1131) [ 44.545870] ? netif_receive_skb_list_internal (net/core/dev.c:5714) [ 44.546628] ? dev_gro_receive (net/core/dev.c:6103) [ 44.547135] ? napi_complete_done (./include/linux/list.h:35 net/core/dev.c:5867 net/core/dev.c:5862 net/core/dev.c:6565) [ 44.547672] virtnet_poll (drivers/net/virtio_net.c:1427 drivers/net/virtio_net.c:1525) [ 44.548251] __napi_poll (net/core/dev.c:6985) [ 44.548744] net_rx_action (net/core/dev.c:7054 net/core/dev.c:7139) [ 44.549264] __do_softirq (./arch/x86/include/asm/jump_label.h:19 ./include/linux/jump_label.h:200 ./include/trace/events/irq.h:142 kernel/softirq.c:560) [ 44.549762] irq_exit_rcu (kernel/softirq.c:433 kernel/softirq.c:637 kernel/softirq.c:649) [ 44.551384] common_interrupt (arch/x86/kernel/irq.c:240 (discriminator 13)) [ 44.551991] ? asm_common_interrupt (./arch/x86/include/asm/idtentry.h:638) [ 44.552654] asm_common_interrupt (./arch/x86/include/asm/idtentry.h:638) Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> Reported-by: Corentin Noël <corentin.noel@collabora.com> Tested-by: Corentin Noël <corentin.noel@collabora.com> --- 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 fa407eb8b457..78a01c71a17c 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -406,7 +406,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, * add_recvbuf_mergeable() + get_mergeable_buf_len() */ truesize = headroom ? PAGE_SIZE : truesize; - tailroom = truesize - len - headroom; + tailroom = truesize - len - headroom - (hdr_padded_len - hdr_len); buf = p - headroom; len -= hdr_len; -- 2.31.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode 2021-06-03 17:09 Xuan Zhuo @ 2021-06-03 22:30 ` patchwork-bot+netdevbpf 2021-06-04 2:28 ` Jason Wang 1 sibling, 0 replies; 7+ messages in thread From: patchwork-bot+netdevbpf @ 2021-06-03 22:30 UTC (permalink / raw) To: Xuan Zhuo Cc: netdev, mst, jasowang, davem, kuba, virtualization, corentin.noel Hello: This patch was applied to netdev/net.git (refs/heads/master): On Fri, 4 Jun 2021 01:09:01 +0800 you wrote: > In virtio-net's large packet mode, there is a hole in the space behind > buf. > > hdr_padded_len - hdr_len > > We must take this into account when calculating tailroom. > > [...] Here is the summary with links: - [net] virtio-net: fix for skb_over_panic inside big mode https://git.kernel.org/netdev/net/c/1a8024239dac You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode 2021-06-03 17:09 Xuan Zhuo 2021-06-03 22:30 ` patchwork-bot+netdevbpf @ 2021-06-04 2:28 ` Jason Wang 1 sibling, 0 replies; 7+ messages in thread From: Jason Wang @ 2021-06-04 2:28 UTC (permalink / raw) To: Xuan Zhuo, netdev Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski, virtualization, Corentin Noël 在 2021/6/4 上午1:09, Xuan Zhuo 写道: > In virtio-net's large packet mode, there is a hole in the space behind > buf. before the buf actually or behind the vnet header? > > hdr_padded_len - hdr_len > > We must take this into account when calculating tailroom. > > [ 44.544385] skb_put.cold (net/core/skbuff.c:5254 (discriminator 1) net/core/skbuff.c:5252 (discriminator 1)) > [ 44.544864] page_to_skb (drivers/net/virtio_net.c:485) [ 44.545361] receive_buf (drivers/net/virtio_net.c:849 drivers/net/virtio_net.c:1131) > [ 44.545870] ? netif_receive_skb_list_internal (net/core/dev.c:5714) > [ 44.546628] ? dev_gro_receive (net/core/dev.c:6103) > [ 44.547135] ? napi_complete_done (./include/linux/list.h:35 net/core/dev.c:5867 net/core/dev.c:5862 net/core/dev.c:6565) > [ 44.547672] virtnet_poll (drivers/net/virtio_net.c:1427 drivers/net/virtio_net.c:1525) > [ 44.548251] __napi_poll (net/core/dev.c:6985) > [ 44.548744] net_rx_action (net/core/dev.c:7054 net/core/dev.c:7139) > [ 44.549264] __do_softirq (./arch/x86/include/asm/jump_label.h:19 ./include/linux/jump_label.h:200 ./include/trace/events/irq.h:142 kernel/softirq.c:560) > [ 44.549762] irq_exit_rcu (kernel/softirq.c:433 kernel/softirq.c:637 kernel/softirq.c:649) > [ 44.551384] common_interrupt (arch/x86/kernel/irq.c:240 (discriminator 13)) > [ 44.551991] ? asm_common_interrupt (./arch/x86/include/asm/idtentry.h:638) > [ 44.552654] asm_common_interrupt (./arch/x86/include/asm/idtentry.h:638) > > Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > Reported-by: Corentin Noël <corentin.noel@collabora.com> > Tested-by: Corentin Noël <corentin.noel@collabora.com> > --- > 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 fa407eb8b457..78a01c71a17c 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -406,7 +406,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > * add_recvbuf_mergeable() + get_mergeable_buf_len() > */ > truesize = headroom ? PAGE_SIZE : truesize; > - tailroom = truesize - len - headroom; > + tailroom = truesize - len - headroom - (hdr_padded_len - hdr_len); The patch looks correct and I saw it has been merged. But I prefer to do that in receive_big() instead of here. Thanks > buf = p - headroom; > > len -= hdr_len; ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-11 2:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1622775955.0233824-1-xuanzhuo@linux.alibaba.com> 2021-06-04 6:00 ` [PATCH net] virtio-net: fix for skb_over_panic inside big mode Jason Wang 2021-10-09 9:17 Michael S. Tsirkin 2021-10-11 2:04 ` Jason Wang [not found] <1622773823.5042562-1-xuanzhuo@linux.alibaba.com> 2021-06-04 3:00 ` Jason Wang -- strict thread matches above, loose matches on Subject: below -- 2021-06-03 17:09 Xuan Zhuo 2021-06-03 22:30 ` patchwork-bot+netdevbpf 2021-06-04 2:28 ` Jason Wang
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).