* [PATCH net v2 0/2] virtio-net: fix for build_skb() @ 2021-06-01 6:39 ` Xuan Zhuo 0 siblings, 0 replies; 18+ messages in thread From: Xuan Zhuo @ 2021-06-01 6:39 UTC (permalink / raw) To: netdev, Jason Wang Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Xuan Zhuo, virtualization, bpf #1 Fixed a serious error. #2 Fixed a logical error, but this error did not cause any serious consequences. The logic of this piece is really messy. Fortunately, my refactored patch can be completed with a small amount of testing. Thanks. Xuan Zhuo (2): virtio-net: fix for unable to handle page fault for address virtio_net: get build_skb() buf by data ptr drivers/net/virtio_net.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) -- 2.31.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net v2 0/2] virtio-net: fix for build_skb() @ 2021-06-01 6:39 ` Xuan Zhuo 0 siblings, 0 replies; 18+ messages in thread From: Xuan Zhuo @ 2021-06-01 6:39 UTC (permalink / raw) To: netdev, Jason Wang Cc: Jesper Dangaard Brouer, Daniel Borkmann, Michael S. Tsirkin, John Fastabend, Alexei Starovoitov, virtualization, Jakub Kicinski, bpf, David S. Miller #1 Fixed a serious error. #2 Fixed a logical error, but this error did not cause any serious consequences. The logic of this piece is really messy. Fortunately, my refactored patch can be completed with a small amount of testing. Thanks. Xuan Zhuo (2): virtio-net: fix for unable to handle page fault for address virtio_net: get build_skb() buf by data ptr drivers/net/virtio_net.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) -- 2.31.0 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net v2 1/2] virtio-net: fix for unable to handle page fault for address 2021-06-01 6:39 ` Xuan Zhuo @ 2021-06-01 6:39 ` Xuan Zhuo -1 siblings, 0 replies; 18+ messages in thread From: Xuan Zhuo @ 2021-06-01 6:39 UTC (permalink / raw) To: netdev, Jason Wang Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Xuan Zhuo, virtualization, bpf In merge mode, when xdp is enabled, if the headroom of buf is smaller than virtnet_get_headroom(), xdp_linearize_page() will be called but the variable of "headroom" is still 0, which leads to wrong logic after entering page_to_skb(). [ 16.600944] BUG: unable to handle page fault for address: ffffecbfff7b43c8[ 16.602175] #PF: supervisor read access in kernel mode [ 16.603350] #PF: error_code(0x0000) - not-present page [ 16.604200] PGD 0 P4D 0 [ 16.604686] Oops: 0000 [#1] SMP PTI [ 16.605306] CPU: 4 PID: 715 Comm: sh Tainted: G B 5.12.0+ #312 [ 16.606429] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/04 [ 16.608217] RIP: 0010:unmap_page_range+0x947/0xde0 [ 16.609014] Code: 00 00 08 00 48 83 f8 01 45 19 e4 41 f7 d4 41 83 e4 03 e9 a4 fd ff ff e8 b7 63 ed ff 4c 89 e0 48 c1 e0 065 [ 16.611863] RSP: 0018:ffffc90002503c58 EFLAGS: 00010286 [ 16.612720] RAX: ffffecbfff7b43c0 RBX: 00007f19f7203000 RCX: ffffffff812ff359 [ 16.613853] RDX: ffff888107778000 RSI: 0000000000000000 RDI: 0000000000000005 [ 16.614976] RBP: ffffea000425e000 R08: 0000000000000000 R09: 3030303030303030 [ 16.616124] R10: ffffffff82ed7d94 R11: 6637303030302052 R12: 7c00000afffded0f [ 16.617276] R13: 0000000000000001 R14: ffff888119ee7010 R15: 00007f19f7202000 [ 16.618423] FS: 0000000000000000(0000) GS:ffff88842fd00000(0000) knlGS:0000000000000000 [ 16.619738] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 16.620670] CR2: ffffecbfff7b43c8 CR3: 0000000103220005 CR4: 0000000000370ee0 [ 16.621792] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 16.622920] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 16.624047] Call Trace: [ 16.624525] ? release_pages+0x24d/0x730 [ 16.625209] unmap_single_vma+0xa9/0x130 [ 16.625885] unmap_vmas+0x76/0xf0 [ 16.626480] exit_mmap+0xa0/0x210 [ 16.627129] mmput+0x67/0x180 [ 16.627673] do_exit+0x3d1/0xf10 [ 16.628259] ? do_user_addr_fault+0x231/0x840 [ 16.629000] do_group_exit+0x53/0xd0 [ 16.629631] __x64_sys_exit_group+0x1d/0x20 [ 16.630354] do_syscall_64+0x3c/0x80 [ 16.630988] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 16.631828] RIP: 0033:0x7f1a043d0191 [ 16.632464] Code: Unable to access opcode bytes at RIP 0x7f1a043d0167. [ 16.633502] RSP: 002b:00007ffe3d993308 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7 [ 16.634737] RAX: ffffffffffffffda RBX: 00007f1a044c9490 RCX: 00007f1a043d0191 [ 16.635857] RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000 [ 16.636986] RBP: 0000000000000000 R08: ffffffffffffff88 R09: 0000000000000001 [ 16.638120] R10: 0000000000000008 R11: 0000000000000246 R12: 00007f1a044c9490 [ 16.639245] R13: 0000000000000001 R14: 00007f1a044c9968 R15: 0000000000000000 [ 16.640408] Modules linked in: [ 16.640958] CR2: ffffecbfff7b43c8 [ 16.641557] ---[ end trace bc4891c6ce46354c ]--- [ 16.642335] RIP: 0010:unmap_page_range+0x947/0xde0 [ 16.643135] Code: 00 00 08 00 48 83 f8 01 45 19 e4 41 f7 d4 41 83 e4 03 e9 a4 fd ff ff e8 b7 63 ed ff 4c 89 e0 48 c1 e0 065 [ 16.645983] RSP: 0018:ffffc90002503c58 EFLAGS: 00010286 [ 16.646845] RAX: ffffecbfff7b43c0 RBX: 00007f19f7203000 RCX: ffffffff812ff359 [ 16.647970] RDX: ffff888107778000 RSI: 0000000000000000 RDI: 0000000000000005 [ 16.649091] RBP: ffffea000425e000 R08: 0000000000000000 R09: 3030303030303030 [ 16.650250] R10: ffffffff82ed7d94 R11: 6637303030302052 R12: 7c00000afffded0f [ 16.651394] R13: 0000000000000001 R14: ffff888119ee7010 R15: 00007f19f7202000 [ 16.652529] FS: 0000000000000000(0000) GS:ffff88842fd00000(0000) knlGS:0000000000000000 [ 16.653887] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 16.654841] CR2: ffffecbfff7b43c8 CR3: 0000000103220005 CR4: 0000000000370ee0 [ 16.655992] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 16.657150] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 16.658290] Kernel panic - not syncing: Fatal exception [ 16.659613] Kernel Offset: disabled [ 16.660234] ---[ end Kernel panic - not syncing: Fatal exception ]--- Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- drivers/net/virtio_net.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 9b6a4a875c55..6b929aca155a 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -958,7 +958,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, put_page(page); head_skb = page_to_skb(vi, rq, xdp_page, offset, len, PAGE_SIZE, false, - metasize, headroom); + metasize, + VIRTIO_XDP_HEADROOM); return head_skb; } break; -- 2.31.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH net v2 1/2] virtio-net: fix for unable to handle page fault for address @ 2021-06-01 6:39 ` Xuan Zhuo 0 siblings, 0 replies; 18+ messages in thread From: Xuan Zhuo @ 2021-06-01 6:39 UTC (permalink / raw) To: netdev, Jason Wang Cc: Jesper Dangaard Brouer, Daniel Borkmann, Michael S. Tsirkin, John Fastabend, Alexei Starovoitov, virtualization, Jakub Kicinski, bpf, David S. Miller In merge mode, when xdp is enabled, if the headroom of buf is smaller than virtnet_get_headroom(), xdp_linearize_page() will be called but the variable of "headroom" is still 0, which leads to wrong logic after entering page_to_skb(). [ 16.600944] BUG: unable to handle page fault for address: ffffecbfff7b43c8[ 16.602175] #PF: supervisor read access in kernel mode [ 16.603350] #PF: error_code(0x0000) - not-present page [ 16.604200] PGD 0 P4D 0 [ 16.604686] Oops: 0000 [#1] SMP PTI [ 16.605306] CPU: 4 PID: 715 Comm: sh Tainted: G B 5.12.0+ #312 [ 16.606429] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/04 [ 16.608217] RIP: 0010:unmap_page_range+0x947/0xde0 [ 16.609014] Code: 00 00 08 00 48 83 f8 01 45 19 e4 41 f7 d4 41 83 e4 03 e9 a4 fd ff ff e8 b7 63 ed ff 4c 89 e0 48 c1 e0 065 [ 16.611863] RSP: 0018:ffffc90002503c58 EFLAGS: 00010286 [ 16.612720] RAX: ffffecbfff7b43c0 RBX: 00007f19f7203000 RCX: ffffffff812ff359 [ 16.613853] RDX: ffff888107778000 RSI: 0000000000000000 RDI: 0000000000000005 [ 16.614976] RBP: ffffea000425e000 R08: 0000000000000000 R09: 3030303030303030 [ 16.616124] R10: ffffffff82ed7d94 R11: 6637303030302052 R12: 7c00000afffded0f [ 16.617276] R13: 0000000000000001 R14: ffff888119ee7010 R15: 00007f19f7202000 [ 16.618423] FS: 0000000000000000(0000) GS:ffff88842fd00000(0000) knlGS:0000000000000000 [ 16.619738] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 16.620670] CR2: ffffecbfff7b43c8 CR3: 0000000103220005 CR4: 0000000000370ee0 [ 16.621792] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 16.622920] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 16.624047] Call Trace: [ 16.624525] ? release_pages+0x24d/0x730 [ 16.625209] unmap_single_vma+0xa9/0x130 [ 16.625885] unmap_vmas+0x76/0xf0 [ 16.626480] exit_mmap+0xa0/0x210 [ 16.627129] mmput+0x67/0x180 [ 16.627673] do_exit+0x3d1/0xf10 [ 16.628259] ? do_user_addr_fault+0x231/0x840 [ 16.629000] do_group_exit+0x53/0xd0 [ 16.629631] __x64_sys_exit_group+0x1d/0x20 [ 16.630354] do_syscall_64+0x3c/0x80 [ 16.630988] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 16.631828] RIP: 0033:0x7f1a043d0191 [ 16.632464] Code: Unable to access opcode bytes at RIP 0x7f1a043d0167. [ 16.633502] RSP: 002b:00007ffe3d993308 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7 [ 16.634737] RAX: ffffffffffffffda RBX: 00007f1a044c9490 RCX: 00007f1a043d0191 [ 16.635857] RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000 [ 16.636986] RBP: 0000000000000000 R08: ffffffffffffff88 R09: 0000000000000001 [ 16.638120] R10: 0000000000000008 R11: 0000000000000246 R12: 00007f1a044c9490 [ 16.639245] R13: 0000000000000001 R14: 00007f1a044c9968 R15: 0000000000000000 [ 16.640408] Modules linked in: [ 16.640958] CR2: ffffecbfff7b43c8 [ 16.641557] ---[ end trace bc4891c6ce46354c ]--- [ 16.642335] RIP: 0010:unmap_page_range+0x947/0xde0 [ 16.643135] Code: 00 00 08 00 48 83 f8 01 45 19 e4 41 f7 d4 41 83 e4 03 e9 a4 fd ff ff e8 b7 63 ed ff 4c 89 e0 48 c1 e0 065 [ 16.645983] RSP: 0018:ffffc90002503c58 EFLAGS: 00010286 [ 16.646845] RAX: ffffecbfff7b43c0 RBX: 00007f19f7203000 RCX: ffffffff812ff359 [ 16.647970] RDX: ffff888107778000 RSI: 0000000000000000 RDI: 0000000000000005 [ 16.649091] RBP: ffffea000425e000 R08: 0000000000000000 R09: 3030303030303030 [ 16.650250] R10: ffffffff82ed7d94 R11: 6637303030302052 R12: 7c00000afffded0f [ 16.651394] R13: 0000000000000001 R14: ffff888119ee7010 R15: 00007f19f7202000 [ 16.652529] FS: 0000000000000000(0000) GS:ffff88842fd00000(0000) knlGS:0000000000000000 [ 16.653887] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 16.654841] CR2: ffffecbfff7b43c8 CR3: 0000000103220005 CR4: 0000000000370ee0 [ 16.655992] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 16.657150] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 16.658290] Kernel panic - not syncing: Fatal exception [ 16.659613] Kernel Offset: disabled [ 16.660234] ---[ end Kernel panic - not syncing: Fatal exception ]--- Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- drivers/net/virtio_net.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 9b6a4a875c55..6b929aca155a 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -958,7 +958,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, put_page(page); head_skb = page_to_skb(vi, rq, xdp_page, offset, len, PAGE_SIZE, false, - metasize, headroom); + metasize, + VIRTIO_XDP_HEADROOM); return head_skb; } break; -- 2.31.0 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 1/2] virtio-net: fix for unable to handle page fault for address 2021-06-01 6:39 ` Xuan Zhuo @ 2021-06-01 8:53 ` Jason Wang -1 siblings, 0 replies; 18+ messages in thread From: Jason Wang @ 2021-06-01 8:53 UTC (permalink / raw) To: Xuan Zhuo, netdev Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, virtualization, bpf 在 2021/6/1 下午2:39, Xuan Zhuo 写道: > In merge mode, when xdp is enabled, if the headroom of buf is smaller > than virtnet_get_headroom(), xdp_linearize_page() will be called but the > variable of "headroom" is still 0, which leads to wrong logic after > entering page_to_skb(). > > [ 16.600944] BUG: unable to handle page fault for address: ffffecbfff7b43c8[ 16.602175] #PF: supervisor read access in kernel mode > [ 16.603350] #PF: error_code(0x0000) - not-present page > [ 16.604200] PGD 0 P4D 0 > [ 16.604686] Oops: 0000 [#1] SMP PTI > [ 16.605306] CPU: 4 PID: 715 Comm: sh Tainted: G B 5.12.0+ #312 > [ 16.606429] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/04 > [ 16.608217] RIP: 0010:unmap_page_range+0x947/0xde0 > [ 16.609014] Code: 00 00 08 00 48 83 f8 01 45 19 e4 41 f7 d4 41 83 e4 03 e9 a4 fd ff ff e8 b7 63 ed ff 4c 89 e0 48 c1 e0 065 > [ 16.611863] RSP: 0018:ffffc90002503c58 EFLAGS: 00010286 > [ 16.612720] RAX: ffffecbfff7b43c0 RBX: 00007f19f7203000 RCX: ffffffff812ff359 > [ 16.613853] RDX: ffff888107778000 RSI: 0000000000000000 RDI: 0000000000000005 > [ 16.614976] RBP: ffffea000425e000 R08: 0000000000000000 R09: 3030303030303030 > [ 16.616124] R10: ffffffff82ed7d94 R11: 6637303030302052 R12: 7c00000afffded0f > [ 16.617276] R13: 0000000000000001 R14: ffff888119ee7010 R15: 00007f19f7202000 > [ 16.618423] FS: 0000000000000000(0000) GS:ffff88842fd00000(0000) knlGS:0000000000000000 > [ 16.619738] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 16.620670] CR2: ffffecbfff7b43c8 CR3: 0000000103220005 CR4: 0000000000370ee0 > [ 16.621792] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > [ 16.622920] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > [ 16.624047] Call Trace: > [ 16.624525] ? release_pages+0x24d/0x730 > [ 16.625209] unmap_single_vma+0xa9/0x130 > [ 16.625885] unmap_vmas+0x76/0xf0 > [ 16.626480] exit_mmap+0xa0/0x210 > [ 16.627129] mmput+0x67/0x180 > [ 16.627673] do_exit+0x3d1/0xf10 > [ 16.628259] ? do_user_addr_fault+0x231/0x840 > [ 16.629000] do_group_exit+0x53/0xd0 > [ 16.629631] __x64_sys_exit_group+0x1d/0x20 > [ 16.630354] do_syscall_64+0x3c/0x80 > [ 16.630988] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 16.631828] RIP: 0033:0x7f1a043d0191 > [ 16.632464] Code: Unable to access opcode bytes at RIP 0x7f1a043d0167. > [ 16.633502] RSP: 002b:00007ffe3d993308 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7 > [ 16.634737] RAX: ffffffffffffffda RBX: 00007f1a044c9490 RCX: 00007f1a043d0191 > [ 16.635857] RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000 > [ 16.636986] RBP: 0000000000000000 R08: ffffffffffffff88 R09: 0000000000000001 > [ 16.638120] R10: 0000000000000008 R11: 0000000000000246 R12: 00007f1a044c9490 > [ 16.639245] R13: 0000000000000001 R14: 00007f1a044c9968 R15: 0000000000000000 > [ 16.640408] Modules linked in: > [ 16.640958] CR2: ffffecbfff7b43c8 > [ 16.641557] ---[ end trace bc4891c6ce46354c ]--- > [ 16.642335] RIP: 0010:unmap_page_range+0x947/0xde0 > [ 16.643135] Code: 00 00 08 00 48 83 f8 01 45 19 e4 41 f7 d4 41 83 e4 03 e9 a4 fd ff ff e8 b7 63 ed ff 4c 89 e0 48 c1 e0 065 > [ 16.645983] RSP: 0018:ffffc90002503c58 EFLAGS: 00010286 > [ 16.646845] RAX: ffffecbfff7b43c0 RBX: 00007f19f7203000 RCX: ffffffff812ff359 > [ 16.647970] RDX: ffff888107778000 RSI: 0000000000000000 RDI: 0000000000000005 > [ 16.649091] RBP: ffffea000425e000 R08: 0000000000000000 R09: 3030303030303030 > [ 16.650250] R10: ffffffff82ed7d94 R11: 6637303030302052 R12: 7c00000afffded0f > [ 16.651394] R13: 0000000000000001 R14: ffff888119ee7010 R15: 00007f19f7202000 > [ 16.652529] FS: 0000000000000000(0000) GS:ffff88842fd00000(0000) knlGS:0000000000000000 > [ 16.653887] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 16.654841] CR2: ffffecbfff7b43c8 CR3: 0000000103220005 CR4: 0000000000370ee0 > [ 16.655992] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > [ 16.657150] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > [ 16.658290] Kernel panic - not syncing: Fatal exception > [ 16.659613] Kernel Offset: disabled > [ 16.660234] ---[ end Kernel panic - not syncing: Fatal exception ]--- > > Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> Acked-by: Jason Wang <jasowang@redhat.com> > --- > drivers/net/virtio_net.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 9b6a4a875c55..6b929aca155a 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -958,7 +958,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > put_page(page); > head_skb = page_to_skb(vi, rq, xdp_page, offset, > len, PAGE_SIZE, false, > - metasize, headroom); > + metasize, > + VIRTIO_XDP_HEADROOM); > return head_skb; > } > break; ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 1/2] virtio-net: fix for unable to handle page fault for address @ 2021-06-01 8:53 ` Jason Wang 0 siblings, 0 replies; 18+ messages in thread From: Jason Wang @ 2021-06-01 8:53 UTC (permalink / raw) To: Xuan Zhuo, netdev Cc: Jesper Dangaard Brouer, Daniel Borkmann, Michael S. Tsirkin, John Fastabend, Alexei Starovoitov, virtualization, Jakub Kicinski, bpf, David S. Miller 在 2021/6/1 下午2:39, Xuan Zhuo 写道: > In merge mode, when xdp is enabled, if the headroom of buf is smaller > than virtnet_get_headroom(), xdp_linearize_page() will be called but the > variable of "headroom" is still 0, which leads to wrong logic after > entering page_to_skb(). > > [ 16.600944] BUG: unable to handle page fault for address: ffffecbfff7b43c8[ 16.602175] #PF: supervisor read access in kernel mode > [ 16.603350] #PF: error_code(0x0000) - not-present page > [ 16.604200] PGD 0 P4D 0 > [ 16.604686] Oops: 0000 [#1] SMP PTI > [ 16.605306] CPU: 4 PID: 715 Comm: sh Tainted: G B 5.12.0+ #312 > [ 16.606429] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/04 > [ 16.608217] RIP: 0010:unmap_page_range+0x947/0xde0 > [ 16.609014] Code: 00 00 08 00 48 83 f8 01 45 19 e4 41 f7 d4 41 83 e4 03 e9 a4 fd ff ff e8 b7 63 ed ff 4c 89 e0 48 c1 e0 065 > [ 16.611863] RSP: 0018:ffffc90002503c58 EFLAGS: 00010286 > [ 16.612720] RAX: ffffecbfff7b43c0 RBX: 00007f19f7203000 RCX: ffffffff812ff359 > [ 16.613853] RDX: ffff888107778000 RSI: 0000000000000000 RDI: 0000000000000005 > [ 16.614976] RBP: ffffea000425e000 R08: 0000000000000000 R09: 3030303030303030 > [ 16.616124] R10: ffffffff82ed7d94 R11: 6637303030302052 R12: 7c00000afffded0f > [ 16.617276] R13: 0000000000000001 R14: ffff888119ee7010 R15: 00007f19f7202000 > [ 16.618423] FS: 0000000000000000(0000) GS:ffff88842fd00000(0000) knlGS:0000000000000000 > [ 16.619738] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 16.620670] CR2: ffffecbfff7b43c8 CR3: 0000000103220005 CR4: 0000000000370ee0 > [ 16.621792] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > [ 16.622920] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > [ 16.624047] Call Trace: > [ 16.624525] ? release_pages+0x24d/0x730 > [ 16.625209] unmap_single_vma+0xa9/0x130 > [ 16.625885] unmap_vmas+0x76/0xf0 > [ 16.626480] exit_mmap+0xa0/0x210 > [ 16.627129] mmput+0x67/0x180 > [ 16.627673] do_exit+0x3d1/0xf10 > [ 16.628259] ? do_user_addr_fault+0x231/0x840 > [ 16.629000] do_group_exit+0x53/0xd0 > [ 16.629631] __x64_sys_exit_group+0x1d/0x20 > [ 16.630354] do_syscall_64+0x3c/0x80 > [ 16.630988] entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 16.631828] RIP: 0033:0x7f1a043d0191 > [ 16.632464] Code: Unable to access opcode bytes at RIP 0x7f1a043d0167. > [ 16.633502] RSP: 002b:00007ffe3d993308 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7 > [ 16.634737] RAX: ffffffffffffffda RBX: 00007f1a044c9490 RCX: 00007f1a043d0191 > [ 16.635857] RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000 > [ 16.636986] RBP: 0000000000000000 R08: ffffffffffffff88 R09: 0000000000000001 > [ 16.638120] R10: 0000000000000008 R11: 0000000000000246 R12: 00007f1a044c9490 > [ 16.639245] R13: 0000000000000001 R14: 00007f1a044c9968 R15: 0000000000000000 > [ 16.640408] Modules linked in: > [ 16.640958] CR2: ffffecbfff7b43c8 > [ 16.641557] ---[ end trace bc4891c6ce46354c ]--- > [ 16.642335] RIP: 0010:unmap_page_range+0x947/0xde0 > [ 16.643135] Code: 00 00 08 00 48 83 f8 01 45 19 e4 41 f7 d4 41 83 e4 03 e9 a4 fd ff ff e8 b7 63 ed ff 4c 89 e0 48 c1 e0 065 > [ 16.645983] RSP: 0018:ffffc90002503c58 EFLAGS: 00010286 > [ 16.646845] RAX: ffffecbfff7b43c0 RBX: 00007f19f7203000 RCX: ffffffff812ff359 > [ 16.647970] RDX: ffff888107778000 RSI: 0000000000000000 RDI: 0000000000000005 > [ 16.649091] RBP: ffffea000425e000 R08: 0000000000000000 R09: 3030303030303030 > [ 16.650250] R10: ffffffff82ed7d94 R11: 6637303030302052 R12: 7c00000afffded0f > [ 16.651394] R13: 0000000000000001 R14: ffff888119ee7010 R15: 00007f19f7202000 > [ 16.652529] FS: 0000000000000000(0000) GS:ffff88842fd00000(0000) knlGS:0000000000000000 > [ 16.653887] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 16.654841] CR2: ffffecbfff7b43c8 CR3: 0000000103220005 CR4: 0000000000370ee0 > [ 16.655992] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > [ 16.657150] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > [ 16.658290] Kernel panic - not syncing: Fatal exception > [ 16.659613] Kernel Offset: disabled > [ 16.660234] ---[ end Kernel panic - not syncing: Fatal exception ]--- > > Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom") > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> Acked-by: Jason Wang <jasowang@redhat.com> > --- > drivers/net/virtio_net.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 9b6a4a875c55..6b929aca155a 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -958,7 +958,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > put_page(page); > head_skb = page_to_skb(vi, rq, xdp_page, offset, > len, PAGE_SIZE, false, > - metasize, headroom); > + metasize, > + VIRTIO_XDP_HEADROOM); > return head_skb; > } > break; _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net v2 2/2] virtio_net: get build_skb() buf by data ptr 2021-06-01 6:39 ` Xuan Zhuo @ 2021-06-01 6:40 ` Xuan Zhuo -1 siblings, 0 replies; 18+ messages in thread From: Xuan Zhuo @ 2021-06-01 6:40 UTC (permalink / raw) To: netdev, Jason Wang Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Xuan Zhuo, virtualization, bpf In the case of merge, the page passed into page_to_skb() may be a head page, not the page where the current data is located. So when trying to get the buf where the data is located, we should get buf based on headroom instead of offset. This patch solves this problem. But if you don't use this patch, the original code can also run, because if the page is not the page of the current data, the calculated tailroom will be less than 0, and will not enter the logic of build_skb() . The significance of this patch is to modify this logical problem, allowing more situations to use build_skb(). Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- drivers/net/virtio_net.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 6b929aca155a..fa407eb8b457 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -401,18 +401,13 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, /* 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() */ - if (headroom) { - /* Buffers with headroom use PAGE_SIZE as alloc size, - * see add_recvbuf_mergeable() + get_mergeable_buf_len() - */ - truesize = PAGE_SIZE; - tailroom = truesize - len - offset; - buf = page_address(page); - } else { - tailroom = truesize - len; - buf = p; - } + truesize = headroom ? PAGE_SIZE : truesize; + tailroom = truesize - len - headroom; + buf = p - headroom; len -= hdr_len; offset += hdr_padded_len; -- 2.31.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH net v2 2/2] virtio_net: get build_skb() buf by data ptr @ 2021-06-01 6:40 ` Xuan Zhuo 0 siblings, 0 replies; 18+ messages in thread From: Xuan Zhuo @ 2021-06-01 6:40 UTC (permalink / raw) To: netdev, Jason Wang Cc: Jesper Dangaard Brouer, Daniel Borkmann, Michael S. Tsirkin, John Fastabend, Alexei Starovoitov, virtualization, Jakub Kicinski, bpf, David S. Miller In the case of merge, the page passed into page_to_skb() may be a head page, not the page where the current data is located. So when trying to get the buf where the data is located, we should get buf based on headroom instead of offset. This patch solves this problem. But if you don't use this patch, the original code can also run, because if the page is not the page of the current data, the calculated tailroom will be less than 0, and will not enter the logic of build_skb() . The significance of this patch is to modify this logical problem, allowing more situations to use build_skb(). Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- drivers/net/virtio_net.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 6b929aca155a..fa407eb8b457 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -401,18 +401,13 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, /* 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() */ - if (headroom) { - /* Buffers with headroom use PAGE_SIZE as alloc size, - * see add_recvbuf_mergeable() + get_mergeable_buf_len() - */ - truesize = PAGE_SIZE; - tailroom = truesize - len - offset; - buf = page_address(page); - } else { - tailroom = truesize - len; - buf = p; - } + truesize = headroom ? PAGE_SIZE : truesize; + tailroom = truesize - len - headroom; + buf = p - headroom; len -= hdr_len; offset += hdr_padded_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] 18+ messages in thread
* Re: [PATCH net v2 2/2] virtio_net: get build_skb() buf by data ptr 2021-06-01 6:40 ` Xuan Zhuo @ 2021-06-01 8:55 ` Jason Wang -1 siblings, 0 replies; 18+ messages in thread From: Jason Wang @ 2021-06-01 8:55 UTC (permalink / raw) To: Xuan Zhuo, netdev Cc: Michael S. Tsirkin, David S. Miller, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, virtualization, bpf 在 2021/6/1 下午2:40, Xuan Zhuo 写道: > In the case of merge, the page passed into page_to_skb() may be a head > page, not the page where the current data is located. So when trying to > get the buf where the data is located, we should get buf based on > headroom instead of offset. > > This patch solves this problem. But if you don't use this patch, the > original code can also run, because if the page is not the page of the > current data, the calculated tailroom will be less than 0, and will not > enter the logic of build_skb() . The significance of this patch is to > modify this logical problem, allowing more situations to use > build_skb(). > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > --- > drivers/net/virtio_net.c | 17 ++++++----------- > 1 file changed, 6 insertions(+), 11 deletions(-) Acked-by: Jason Wang <jasowang@redhat.com> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 6b929aca155a..fa407eb8b457 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -401,18 +401,13 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > /* 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() > */ > - if (headroom) { > - /* Buffers with headroom use PAGE_SIZE as alloc size, > - * see add_recvbuf_mergeable() + get_mergeable_buf_len() > - */ > - truesize = PAGE_SIZE; > - tailroom = truesize - len - offset; > - buf = page_address(page); > - } else { > - tailroom = truesize - len; > - buf = p; > - } > + truesize = headroom ? PAGE_SIZE : truesize; > + tailroom = truesize - len - headroom; > + buf = p - headroom; > > len -= hdr_len; > offset += hdr_padded_len; ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 2/2] virtio_net: get build_skb() buf by data ptr @ 2021-06-01 8:55 ` Jason Wang 0 siblings, 0 replies; 18+ messages in thread From: Jason Wang @ 2021-06-01 8:55 UTC (permalink / raw) To: Xuan Zhuo, netdev Cc: Jesper Dangaard Brouer, Daniel Borkmann, Michael S. Tsirkin, John Fastabend, Alexei Starovoitov, virtualization, Jakub Kicinski, bpf, David S. Miller 在 2021/6/1 下午2:40, Xuan Zhuo 写道: > In the case of merge, the page passed into page_to_skb() may be a head > page, not the page where the current data is located. So when trying to > get the buf where the data is located, we should get buf based on > headroom instead of offset. > > This patch solves this problem. But if you don't use this patch, the > original code can also run, because if the page is not the page of the > current data, the calculated tailroom will be less than 0, and will not > enter the logic of build_skb() . The significance of this patch is to > modify this logical problem, allowing more situations to use > build_skb(). > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > --- > drivers/net/virtio_net.c | 17 ++++++----------- > 1 file changed, 6 insertions(+), 11 deletions(-) Acked-by: Jason Wang <jasowang@redhat.com> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 6b929aca155a..fa407eb8b457 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -401,18 +401,13 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > /* 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() > */ > - if (headroom) { > - /* Buffers with headroom use PAGE_SIZE as alloc size, > - * see add_recvbuf_mergeable() + get_mergeable_buf_len() > - */ > - truesize = PAGE_SIZE; > - tailroom = truesize - len - offset; > - buf = page_address(page); > - } else { > - tailroom = truesize - len; > - buf = p; > - } > + truesize = headroom ? PAGE_SIZE : truesize; > + tailroom = truesize - len - headroom; > + buf = p - headroom; > > len -= hdr_len; > offset += hdr_padded_len; _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 0/2] virtio-net: fix for build_skb() 2021-06-01 6:39 ` Xuan Zhuo @ 2021-06-01 11:06 ` Michael S. Tsirkin -1 siblings, 0 replies; 18+ messages in thread From: Michael S. Tsirkin @ 2021-06-01 11:06 UTC (permalink / raw) To: Xuan Zhuo Cc: netdev, Jason Wang, David S. Miller, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, virtualization, bpf On Tue, Jun 01, 2021 at 02:39:58PM +0800, Xuan Zhuo wrote: > #1 Fixed a serious error. > #2 Fixed a logical error, but this error did not cause any serious consequences. > > The logic of this piece is really messy. Fortunately, my refactored patch can be > completed with a small amount of testing. Looks good, thanks! Also needed for stable I think. Acked-by: Michael S. Tsirkin <mst@redhat.com> > Thanks. > > Xuan Zhuo (2): > virtio-net: fix for unable to handle page fault for address > virtio_net: get build_skb() buf by data ptr > > drivers/net/virtio_net.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > -- > 2.31.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 0/2] virtio-net: fix for build_skb() @ 2021-06-01 11:06 ` Michael S. Tsirkin 0 siblings, 0 replies; 18+ messages in thread From: Michael S. Tsirkin @ 2021-06-01 11:06 UTC (permalink / raw) To: Xuan Zhuo Cc: Jesper Dangaard Brouer, Daniel Borkmann, netdev, John Fastabend, Alexei Starovoitov, virtualization, Jakub Kicinski, bpf, David S. Miller On Tue, Jun 01, 2021 at 02:39:58PM +0800, Xuan Zhuo wrote: > #1 Fixed a serious error. > #2 Fixed a logical error, but this error did not cause any serious consequences. > > The logic of this piece is really messy. Fortunately, my refactored patch can be > completed with a small amount of testing. Looks good, thanks! Also needed for stable I think. Acked-by: Michael S. Tsirkin <mst@redhat.com> > Thanks. > > Xuan Zhuo (2): > virtio-net: fix for unable to handle page fault for address > virtio_net: get build_skb() buf by data ptr > > drivers/net/virtio_net.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > -- > 2.31.0 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 0/2] virtio-net: fix for build_skb() 2021-06-01 11:06 ` Michael S. Tsirkin (?) @ 2021-07-30 12:16 ` Jakub Kicinski 2021-08-02 4:45 ` Jason Wang 2021-08-02 16:58 ` Michael S. Tsirkin -1 siblings, 2 replies; 18+ messages in thread From: Jakub Kicinski @ 2021-07-30 12:16 UTC (permalink / raw) To: Michael S. Tsirkin, Xuan Zhuo, Jason Wang Cc: netdev, David S. Miller, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, virtualization, bpf On Tue, 1 Jun 2021 07:06:43 -0400 Michael S. Tsirkin wrote: > On Tue, Jun 01, 2021 at 02:39:58PM +0800, Xuan Zhuo wrote: > > #1 Fixed a serious error. > > #2 Fixed a logical error, but this error did not cause any serious consequences. > > > > The logic of this piece is really messy. Fortunately, my refactored patch can be > > completed with a small amount of testing. > > Looks good, thanks! > Also needed for stable I think. > > Acked-by: Michael S. Tsirkin <mst@redhat.com> Just a heads up folks, looks like we ended up merging both this and the net-next version of the patch set: 8fb7da9e9907 virtio_net: get build_skb() buf by data ptr 5c37711d9f27 virtio-net: fix for unable to handle page fault for address and 7bf64460e3b2 virtio-net: get build_skb() buf by data ptr 6c66c147b9a4 virtio-net: fix for unable to handle page fault for address Are you okay with the code as is or should we commit something like: --- diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 56c3f8519093..74482a52f076 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -380,7 +380,7 @@ 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, - bool whole_page) + unsigned int headroom) { struct sk_buff *skb; struct virtio_net_hdr_mrg_rxbuf *hdr; @@ -398,28 +398,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, else hdr_padded_len = sizeof(struct padded_vnet_hdr); - /* If whole_page, there is an offset between the beginning of the + /* 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() */ - if (whole_page) { - /* Buffers with whole_page use PAGE_SIZE as alloc size, - * see add_recvbuf_mergeable() + get_mergeable_buf_len() - */ - truesize = PAGE_SIZE; - - /* page maybe head page, so we should get the buf by p, not the - * page - */ - tailroom = truesize - len - offset_in_page(p); - buf = (char *)((unsigned long)p & PAGE_MASK); - } else { - tailroom = truesize - len; - buf = p; - } + truesize = headroom ? PAGE_SIZE : truesize; + tailroom = truesize - len - headroom; + buf = p - headroom; len -= hdr_len; offset += hdr_padded_len; @@ -978,7 +966,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, put_page(page); head_skb = page_to_skb(vi, rq, xdp_page, offset, len, PAGE_SIZE, false, - metasize, true); + metasize, + VIRTIO_XDP_HEADROOM); return head_skb; } break; @@ -1029,7 +1018,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, rcu_read_unlock(); head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog, - metasize, !!headroom); + metasize, headroom); curr_skb = head_skb; if (unlikely(!curr_skb)) ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 0/2] virtio-net: fix for build_skb() 2021-07-30 12:16 ` Jakub Kicinski @ 2021-08-02 4:45 ` Jason Wang 2021-08-02 16:58 ` Michael S. Tsirkin 1 sibling, 0 replies; 18+ messages in thread From: Jason Wang @ 2021-08-02 4:45 UTC (permalink / raw) To: Jakub Kicinski, Michael S. Tsirkin, Xuan Zhuo Cc: netdev, David S. Miller, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, virtualization, bpf 在 2021/7/30 下午8:16, Jakub Kicinski 写道: > On Tue, 1 Jun 2021 07:06:43 -0400 Michael S. Tsirkin wrote: >> On Tue, Jun 01, 2021 at 02:39:58PM +0800, Xuan Zhuo wrote: >>> #1 Fixed a serious error. >>> #2 Fixed a logical error, but this error did not cause any serious consequences. >>> >>> The logic of this piece is really messy. Fortunately, my refactored patch can be >>> completed with a small amount of testing. >> Looks good, thanks! >> Also needed for stable I think. >> >> Acked-by: Michael S. Tsirkin <mst@redhat.com> > Just a heads up folks, looks like we ended up merging both this and the > net-next version of the patch set: > > 8fb7da9e9907 virtio_net: get build_skb() buf by data ptr > 5c37711d9f27 virtio-net: fix for unable to handle page fault for address > > and > > 7bf64460e3b2 virtio-net: get build_skb() buf by data ptr > 6c66c147b9a4 virtio-net: fix for unable to handle page fault for address > > Are you okay with the code as is or should we commit something like: I think we need commit the following codes since it's more easier to be understood. Thanks > > --- > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 56c3f8519093..74482a52f076 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -380,7 +380,7 @@ 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, > - bool whole_page) > + unsigned int headroom) > { > struct sk_buff *skb; > struct virtio_net_hdr_mrg_rxbuf *hdr; > @@ -398,28 +398,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > else > hdr_padded_len = sizeof(struct padded_vnet_hdr); > > - /* If whole_page, there is an offset between the beginning of the > + /* 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() > */ > - if (whole_page) { > - /* Buffers with whole_page use PAGE_SIZE as alloc size, > - * see add_recvbuf_mergeable() + get_mergeable_buf_len() > - */ > - truesize = PAGE_SIZE; > - > - /* page maybe head page, so we should get the buf by p, not the > - * page > - */ > - tailroom = truesize - len - offset_in_page(p); > - buf = (char *)((unsigned long)p & PAGE_MASK); > - } else { > - tailroom = truesize - len; > - buf = p; > - } > + truesize = headroom ? PAGE_SIZE : truesize; > + tailroom = truesize - len - headroom; > + buf = p - headroom; > > len -= hdr_len; > offset += hdr_padded_len; > @@ -978,7 +966,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > put_page(page); > head_skb = page_to_skb(vi, rq, xdp_page, offset, > len, PAGE_SIZE, false, > - metasize, true); > + metasize, > + VIRTIO_XDP_HEADROOM); > return head_skb; > } > break; > @@ -1029,7 +1018,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > rcu_read_unlock(); > > head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog, > - metasize, !!headroom); > + metasize, headroom); > curr_skb = head_skb; > > if (unlikely(!curr_skb)) > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 0/2] virtio-net: fix for build_skb() @ 2021-08-02 4:45 ` Jason Wang 0 siblings, 0 replies; 18+ messages in thread From: Jason Wang @ 2021-08-02 4:45 UTC (permalink / raw) To: Jakub Kicinski, Michael S. Tsirkin, Xuan Zhuo Cc: Jesper Dangaard Brouer, Daniel Borkmann, netdev, John Fastabend, Alexei Starovoitov, virtualization, bpf, David S. Miller 在 2021/7/30 下午8:16, Jakub Kicinski 写道: > On Tue, 1 Jun 2021 07:06:43 -0400 Michael S. Tsirkin wrote: >> On Tue, Jun 01, 2021 at 02:39:58PM +0800, Xuan Zhuo wrote: >>> #1 Fixed a serious error. >>> #2 Fixed a logical error, but this error did not cause any serious consequences. >>> >>> The logic of this piece is really messy. Fortunately, my refactored patch can be >>> completed with a small amount of testing. >> Looks good, thanks! >> Also needed for stable I think. >> >> Acked-by: Michael S. Tsirkin <mst@redhat.com> > Just a heads up folks, looks like we ended up merging both this and the > net-next version of the patch set: > > 8fb7da9e9907 virtio_net: get build_skb() buf by data ptr > 5c37711d9f27 virtio-net: fix for unable to handle page fault for address > > and > > 7bf64460e3b2 virtio-net: get build_skb() buf by data ptr > 6c66c147b9a4 virtio-net: fix for unable to handle page fault for address > > Are you okay with the code as is or should we commit something like: I think we need commit the following codes since it's more easier to be understood. Thanks > > --- > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 56c3f8519093..74482a52f076 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -380,7 +380,7 @@ 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, > - bool whole_page) > + unsigned int headroom) > { > struct sk_buff *skb; > struct virtio_net_hdr_mrg_rxbuf *hdr; > @@ -398,28 +398,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > else > hdr_padded_len = sizeof(struct padded_vnet_hdr); > > - /* If whole_page, there is an offset between the beginning of the > + /* 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() > */ > - if (whole_page) { > - /* Buffers with whole_page use PAGE_SIZE as alloc size, > - * see add_recvbuf_mergeable() + get_mergeable_buf_len() > - */ > - truesize = PAGE_SIZE; > - > - /* page maybe head page, so we should get the buf by p, not the > - * page > - */ > - tailroom = truesize - len - offset_in_page(p); > - buf = (char *)((unsigned long)p & PAGE_MASK); > - } else { > - tailroom = truesize - len; > - buf = p; > - } > + truesize = headroom ? PAGE_SIZE : truesize; > + tailroom = truesize - len - headroom; > + buf = p - headroom; > > len -= hdr_len; > offset += hdr_padded_len; > @@ -978,7 +966,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > put_page(page); > head_skb = page_to_skb(vi, rq, xdp_page, offset, > len, PAGE_SIZE, false, > - metasize, true); > + metasize, > + VIRTIO_XDP_HEADROOM); > return head_skb; > } > break; > @@ -1029,7 +1018,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > rcu_read_unlock(); > > head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog, > - metasize, !!headroom); > + metasize, headroom); > curr_skb = head_skb; > > if (unlikely(!curr_skb)) > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 0/2] virtio-net: fix for build_skb() 2021-07-30 12:16 ` Jakub Kicinski @ 2021-08-02 16:58 ` Michael S. Tsirkin 2021-08-02 16:58 ` Michael S. Tsirkin 1 sibling, 0 replies; 18+ messages in thread From: Michael S. Tsirkin @ 2021-08-02 16:58 UTC (permalink / raw) To: Jakub Kicinski Cc: Xuan Zhuo, Jason Wang, netdev, David S. Miller, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, virtualization, bpf On Fri, Jul 30, 2021 at 05:16:43AM -0700, Jakub Kicinski wrote: > On Tue, 1 Jun 2021 07:06:43 -0400 Michael S. Tsirkin wrote: > > On Tue, Jun 01, 2021 at 02:39:58PM +0800, Xuan Zhuo wrote: > > > #1 Fixed a serious error. > > > #2 Fixed a logical error, but this error did not cause any serious consequences. > > > > > > The logic of this piece is really messy. Fortunately, my refactored patch can be > > > completed with a small amount of testing. > > > > Looks good, thanks! > > Also needed for stable I think. > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > Just a heads up folks, looks like we ended up merging both this and the > net-next version of the patch set: > > 8fb7da9e9907 virtio_net: get build_skb() buf by data ptr > 5c37711d9f27 virtio-net: fix for unable to handle page fault for address > > and > > 7bf64460e3b2 virtio-net: get build_skb() buf by data ptr > 6c66c147b9a4 virtio-net: fix for unable to handle page fault for address > > Are you okay with the code as is or should we commit something like: Yes: Acked-by: Michael S. Tsirkin <mst@redhat.com> Will you post this one then? > --- > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 56c3f8519093..74482a52f076 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -380,7 +380,7 @@ 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, > - bool whole_page) > + unsigned int headroom) > { > struct sk_buff *skb; > struct virtio_net_hdr_mrg_rxbuf *hdr; > @@ -398,28 +398,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > else > hdr_padded_len = sizeof(struct padded_vnet_hdr); > > - /* If whole_page, there is an offset between the beginning of the > + /* 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() > */ > - if (whole_page) { > - /* Buffers with whole_page use PAGE_SIZE as alloc size, > - * see add_recvbuf_mergeable() + get_mergeable_buf_len() > - */ > - truesize = PAGE_SIZE; > - > - /* page maybe head page, so we should get the buf by p, not the > - * page > - */ > - tailroom = truesize - len - offset_in_page(p); > - buf = (char *)((unsigned long)p & PAGE_MASK); > - } else { > - tailroom = truesize - len; > - buf = p; > - } > + truesize = headroom ? PAGE_SIZE : truesize; > + tailroom = truesize - len - headroom; > + buf = p - headroom; > > len -= hdr_len; > offset += hdr_padded_len; > @@ -978,7 +966,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > put_page(page); > head_skb = page_to_skb(vi, rq, xdp_page, offset, > len, PAGE_SIZE, false, > - metasize, true); > + metasize, > + VIRTIO_XDP_HEADROOM); > return head_skb; > } > break; > @@ -1029,7 +1018,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > rcu_read_unlock(); > > head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog, > - metasize, !!headroom); > + metasize, headroom); > curr_skb = head_skb; > > if (unlikely(!curr_skb)) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 0/2] virtio-net: fix for build_skb() @ 2021-08-02 16:58 ` Michael S. Tsirkin 0 siblings, 0 replies; 18+ messages in thread From: Michael S. Tsirkin @ 2021-08-02 16:58 UTC (permalink / raw) To: Jakub Kicinski Cc: Jesper Dangaard Brouer, Daniel Borkmann, netdev, John Fastabend, Alexei Starovoitov, virtualization, bpf, David S. Miller On Fri, Jul 30, 2021 at 05:16:43AM -0700, Jakub Kicinski wrote: > On Tue, 1 Jun 2021 07:06:43 -0400 Michael S. Tsirkin wrote: > > On Tue, Jun 01, 2021 at 02:39:58PM +0800, Xuan Zhuo wrote: > > > #1 Fixed a serious error. > > > #2 Fixed a logical error, but this error did not cause any serious consequences. > > > > > > The logic of this piece is really messy. Fortunately, my refactored patch can be > > > completed with a small amount of testing. > > > > Looks good, thanks! > > Also needed for stable I think. > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > Just a heads up folks, looks like we ended up merging both this and the > net-next version of the patch set: > > 8fb7da9e9907 virtio_net: get build_skb() buf by data ptr > 5c37711d9f27 virtio-net: fix for unable to handle page fault for address > > and > > 7bf64460e3b2 virtio-net: get build_skb() buf by data ptr > 6c66c147b9a4 virtio-net: fix for unable to handle page fault for address > > Are you okay with the code as is or should we commit something like: Yes: Acked-by: Michael S. Tsirkin <mst@redhat.com> Will you post this one then? > --- > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 56c3f8519093..74482a52f076 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -380,7 +380,7 @@ 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, > - bool whole_page) > + unsigned int headroom) > { > struct sk_buff *skb; > struct virtio_net_hdr_mrg_rxbuf *hdr; > @@ -398,28 +398,16 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi, > else > hdr_padded_len = sizeof(struct padded_vnet_hdr); > > - /* If whole_page, there is an offset between the beginning of the > + /* 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() > */ > - if (whole_page) { > - /* Buffers with whole_page use PAGE_SIZE as alloc size, > - * see add_recvbuf_mergeable() + get_mergeable_buf_len() > - */ > - truesize = PAGE_SIZE; > - > - /* page maybe head page, so we should get the buf by p, not the > - * page > - */ > - tailroom = truesize - len - offset_in_page(p); > - buf = (char *)((unsigned long)p & PAGE_MASK); > - } else { > - tailroom = truesize - len; > - buf = p; > - } > + truesize = headroom ? PAGE_SIZE : truesize; > + tailroom = truesize - len - headroom; > + buf = p - headroom; > > len -= hdr_len; > offset += hdr_padded_len; > @@ -978,7 +966,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > put_page(page); > head_skb = page_to_skb(vi, rq, xdp_page, offset, > len, PAGE_SIZE, false, > - metasize, true); > + metasize, > + VIRTIO_XDP_HEADROOM); > return head_skb; > } > break; > @@ -1029,7 +1018,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > rcu_read_unlock(); > > head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog, > - metasize, !!headroom); > + metasize, headroom); > curr_skb = head_skb; > > if (unlikely(!curr_skb)) _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v2 0/2] virtio-net: fix for build_skb() 2021-06-01 6:39 ` Xuan Zhuo ` (3 preceding siblings ...) (?) @ 2021-06-01 22:30 ` patchwork-bot+netdevbpf -1 siblings, 0 replies; 18+ messages in thread From: patchwork-bot+netdevbpf @ 2021-06-01 22:30 UTC (permalink / raw) To: Xuan Zhuo Cc: netdev, jasowang, mst, davem, kuba, ast, daniel, hawk, john.fastabend, virtualization, bpf Hello: This series was applied to netdev/net.git (refs/heads/master): On Tue, 1 Jun 2021 14:39:58 +0800 you wrote: > #1 Fixed a serious error. > #2 Fixed a logical error, but this error did not cause any serious consequences. > > The logic of this piece is really messy. Fortunately, my refactored patch can be > completed with a small amount of testing. > > Thanks. > > [...] Here is the summary with links: - [net,v2,1/2] virtio-net: fix for unable to handle page fault for address https://git.kernel.org/netdev/net/c/5c37711d9f27 - [net,v2,2/2] virtio_net: get build_skb() buf by data ptr https://git.kernel.org/netdev/net/c/8fb7da9e9907 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] 18+ messages in thread
end of thread, other threads:[~2021-08-02 16:58 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-01 6:39 [PATCH net v2 0/2] virtio-net: fix for build_skb() Xuan Zhuo 2021-06-01 6:39 ` Xuan Zhuo 2021-06-01 6:39 ` [PATCH net v2 1/2] virtio-net: fix for unable to handle page fault for address Xuan Zhuo 2021-06-01 6:39 ` Xuan Zhuo 2021-06-01 8:53 ` Jason Wang 2021-06-01 8:53 ` Jason Wang 2021-06-01 6:40 ` [PATCH net v2 2/2] virtio_net: get build_skb() buf by data ptr Xuan Zhuo 2021-06-01 6:40 ` Xuan Zhuo 2021-06-01 8:55 ` Jason Wang 2021-06-01 8:55 ` Jason Wang 2021-06-01 11:06 ` [PATCH net v2 0/2] virtio-net: fix for build_skb() Michael S. Tsirkin 2021-06-01 11:06 ` Michael S. Tsirkin 2021-07-30 12:16 ` Jakub Kicinski 2021-08-02 4:45 ` Jason Wang 2021-08-02 4:45 ` Jason Wang 2021-08-02 16:58 ` Michael S. Tsirkin 2021-08-02 16:58 ` Michael S. Tsirkin 2021-06-01 22:30 ` patchwork-bot+netdevbpf
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.