* [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-06-03 17:09 ` Xuan Zhuo 0 siblings, 0 replies; 16+ 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] 16+ messages in thread
* [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-06-03 17:09 ` Xuan Zhuo 0 siblings, 0 replies; 16+ messages in thread From: Xuan Zhuo @ 2021-06-03 17:09 UTC (permalink / raw) To: netdev Cc: Michael S. Tsirkin, Corentin Noël, virtualization, Jakub Kicinski, David S. Miller 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 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 16+ 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 -1 siblings, 0 replies; 16+ 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] 16+ 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-04 2:28 ` Jason Wang -1 siblings, 0 replies; 16+ 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] 16+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-06-04 2:28 ` Jason Wang 0 siblings, 0 replies; 16+ messages in thread From: Jason Wang @ 2021-06-04 2:28 UTC (permalink / raw) To: Xuan Zhuo, netdev Cc: Jakub Kicinski, virtualization, David S. Miller, Corentin Noël, Michael S. Tsirkin 在 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; _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode 2021-06-04 2:28 ` Jason Wang (?) @ 2021-06-04 2:30 ` Xuan Zhuo 2021-06-04 3:00 ` Jason Wang -1 siblings, 1 reply; 16+ messages in thread From: Xuan Zhuo @ 2021-06-04 2:30 UTC (permalink / raw) To: Jason Wang Cc: Michael S. Tsirkin, netdev, Corentin Noël, virtualization, Jakub Kicinski, David S. Miller 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? Thanks. > > > > > buf = p - headroom; > > > > len -= hdr_len; > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode 2021-06-04 2:30 ` Xuan Zhuo @ 2021-06-04 3:00 ` Jason Wang 0 siblings, 0 replies; 16+ 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] 16+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-06-04 3:00 ` Jason Wang 0 siblings, 0 replies; 16+ messages in thread From: Jason Wang @ 2021-06-04 3:00 UTC (permalink / raw) To: Xuan Zhuo Cc: Michael S. Tsirkin, netdev, Corentin Noël, virtualization, Jakub Kicinski, David S. Miller 在 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; _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode 2021-06-04 3:00 ` Jason Wang (?) @ 2021-06-04 3:05 ` Xuan Zhuo 2021-06-04 6:00 ` Jason Wang -1 siblings, 1 reply; 16+ messages in thread From: Xuan Zhuo @ 2021-06-04 3:05 UTC (permalink / raw) To: Jason Wang Cc: Michael S. Tsirkin, netdev, Corentin Noël, virtualization, Jakub Kicinski, David S. Miller 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. Do you think this is ok? 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; > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode 2021-06-04 3:05 ` Xuan Zhuo @ 2021-06-04 6:00 ` Jason Wang 0 siblings, 0 replies; 16+ 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] 16+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-06-04 6:00 ` Jason Wang 0 siblings, 0 replies; 16+ messages in thread From: Jason Wang @ 2021-06-04 6:00 UTC (permalink / raw) To: Xuan Zhuo Cc: Michael S. Tsirkin, netdev, Corentin Noël, virtualization, Jakub Kicinski, David S. Miller 在 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; _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-10-09 9:17 ` Michael S. Tsirkin 0 siblings, 0 replies; 16+ 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] 16+ messages in thread
* [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-10-09 9:17 ` Michael S. Tsirkin 0 siblings, 0 replies; 16+ messages in thread From: Michael S. Tsirkin @ 2021-10-09 9:17 UTC (permalink / raw) To: linux-kernel Cc: Greg KH, Corentin Noël, virtualization, netdev, Jakub Kicinski, David S. Miller 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 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 16+ 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-09 9:29 ` Xuan Zhuo -1 siblings, 0 replies; 16+ messages in thread From: Xuan Zhuo @ 2021-10-09 9:29 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Greg KH, Corentin Noël, virtualization, netdev, Jakub Kicinski, David S. Miller, linux-kernel On Sat, 9 Oct 2021 05:17:53 -0400, 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> LGTM Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.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 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 16+ 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 -1 siblings, 0 replies; 16+ 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] 16+ messages in thread
* Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode @ 2021-10-11 2:04 ` Jason Wang 0 siblings, 0 replies; 16+ messages in thread From: Jason Wang @ 2021-10-11 2:04 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Greg KH, linux-kernel, Corentin Noël, netdev, Jakub Kicinski, virtualization, David S. Miller 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 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-10-11 2:05 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-03 17:09 [PATCH net] virtio-net: fix for skb_over_panic inside big mode Xuan Zhuo 2021-06-03 17:09 ` Xuan Zhuo 2021-06-03 22:30 ` patchwork-bot+netdevbpf 2021-06-04 2:28 ` Jason Wang 2021-06-04 2:28 ` Jason Wang 2021-06-04 2:30 ` Xuan Zhuo 2021-06-04 3:00 ` Jason Wang 2021-06-04 3:00 ` Jason Wang 2021-06-04 3:05 ` Xuan Zhuo 2021-06-04 6:00 ` Jason Wang 2021-06-04 6:00 ` Jason Wang 2021-10-09 9:17 Michael S. Tsirkin 2021-10-09 9:17 ` Michael S. Tsirkin 2021-10-09 9:29 ` Xuan Zhuo 2021-10-11 2:04 ` Jason Wang 2021-10-11 2:04 ` Jason Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.