| Message ID | 1431451117-70051-9-git-send-email-joao.martins@neclab.eu |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, May 12, 2015 at 07:18:32PM +0200, Joao Martins wrote: > On xenvif_start_xmit() we have an additional queue to the netback RX > kthread that will sends the packet. When using burst>1 pktgen sets > skb->xmit_more to tell the driver that there more skbs in the queue. > However, pktgen transmits the same skb <burst> times, which leads to > the BUG below. Long story short adding the same skb in the rx_queue > queue leads to crash. Specifically, having pktgen running with burst=2 > what happens is: when we queue the second skb (that is the same as > the first queued skb), the list will have the tail element with skb->prev > which is the skb itself. On skb_unlink (i.e. when dequeueing the skb) > skb->prev will become NULL, but still having list->next pointing to the > unlinked skb. Because of this skb_peek will still return an skb, which > will redo the skb_unlink trying to set (skb->prev)->next where skb->prev > is now NULL, thus leading to the crash (trace below). > From your description this doesn't sound Xen specific. Sounds like pktgen breaks in any driver that has an internal queue, which is plenty. > I'm not sure what the best way to fix this but since it's only happening > when we use pktgen with burst>1: I chose doing an skb_clone when we don't > use persistent grants and skb->xmit_more flag is set, and when > CONFIG_NET_PKTGEN is compiled builtin. > I don't think we should do this. Wei. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19 May 2015, at 17:36, Wei Liu <wei.liu2@citrix.com> wrote: > On Tue, May 12, 2015 at 07:18:32PM +0200, Joao Martins wrote: >> On xenvif_start_xmit() we have an additional queue to the netback RX >> kthread that will sends the packet. When using burst>1 pktgen sets >> skb->xmit_more to tell the driver that there more skbs in the queue. >> However, pktgen transmits the same skb <burst> times, which leads to >> the BUG below. Long story short adding the same skb in the rx_queue >> queue leads to crash. Specifically, having pktgen running with burst=2 >> what happens is: when we queue the second skb (that is the same as >> the first queued skb), the list will have the tail element with skb->prev >> which is the skb itself. On skb_unlink (i.e. when dequeueing the skb) >> skb->prev will become NULL, but still having list->next pointing to the >> unlinked skb. Because of this skb_peek will still return an skb, which >> will redo the skb_unlink trying to set (skb->prev)->next where skb->prev >> is now NULL, thus leading to the crash (trace below). >> > > From your description this doesn't sound Xen specific. Sounds like > pktgen breaks in any driver that has an internal queue, which is plenty. Yes, it’s only on drivers with an internal queue. >> I'm not sure what the best way to fix this but since it's only happening >> when we use pktgen with burst>1: I chose doing an skb_clone when we don't >> use persistent grants and skb->xmit_more flag is set, and when >> CONFIG_NET_PKTGEN is compiled builtin. >> > > I don't think we should do this. Ok, I will remove this one out then. Part of the reason I submitted it was to make you aware of the crash. Joao -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c index dfe2b7b..5748ba5 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -170,6 +170,15 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev) cb->expires = jiffies + vif->drain_timeout; if (!queue->vif->persistent_grants) { +#ifdef CONFIG_NET_PKTGEN + if (skb->xmit_more) { + struct sk_buff *nskb; + + nskb = skb_clone(skb, GFP_ATOMIC | __GFP_NOWARN); + dev_kfree_skb(skb); + skb = nskb; + } +#endif xenvif_rx_queue_tail(queue, skb); xenvif_kick_thread(queue); } else if (xenvif_rx_map(queue, skb)) {
On xenvif_start_xmit() we have an additional queue to the netback RX kthread that will sends the packet. When using burst>1 pktgen sets skb->xmit_more to tell the driver that there more skbs in the queue. However, pktgen transmits the same skb <burst> times, which leads to the BUG below. Long story short adding the same skb in the rx_queue queue leads to crash. Specifically, having pktgen running with burst=2 what happens is: when we queue the second skb (that is the same as the first queued skb), the list will have the tail element with skb->prev which is the skb itself. On skb_unlink (i.e. when dequeueing the skb) skb->prev will become NULL, but still having list->next pointing to the unlinked skb. Because of this skb_peek will still return an skb, which will redo the skb_unlink trying to set (skb->prev)->next where skb->prev is now NULL, thus leading to the crash (trace below). I'm not sure what the best way to fix this but since it's only happening when we use pktgen with burst>1: I chose doing an skb_clone when we don't use persistent grants and skb->xmit_more flag is set, and when CONFIG_NET_PKTGEN is compiled builtin. BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 IP: [<ffffffffa01dbcdc>] xenvif_rx_dequeue+0x7c/0x120 [xen_netback] PGD 0 Oops: 0002 [#1] SMP CPU: 1 PID: 10391 Comm: vif510.1-q0-gue Not tainted 4.0.0-rc2-net-next+ task: ffff88003b0ce400 ti: ffff880008538000 task.ti: ffff880008538000 RIP: e030:[<ffffffffa01dbcdc>] [<ffffffffa01dbcdc>] xenvif_rx_dequeue+0x7c/0x120 [xen_netback] RSP: e02b:ffff88000853bde8 EFLAGS: 00010006 RAX: 0000000000000000 RBX: ffffc9000212e000 RCX: 00000000000000e4 RDX: 0000000000000000 RSI: ffff88003b0c0200 RDI: ffffc90002139a24 RBP: ffff88000853bdf8 R08: ffff880008538000 R09: 0000000000000000 R10: aaaaaaaaaaaaaaaa R11: 0000000000000000 R12: ffff8800089a6400 R13: ffffc9000212e000 R14: ffffc90002139a10 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff88003f700000(0000) knlGS:ffff88003f700000 CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000008 CR3: 000000000260b000 CR4: 0000000000042660 Stack: ffff88000853be48 ffff88000853be30 ffff88000853beb8 ffffffffa01e19ea ffff88000853be60 ffff88003b0ce400 ffff88003ba418c0 ffffc900021399c0 0000000000000000 ffff88000853be30 ffff88000853be30 ffff000000000000 Call Trace: [<ffffffffa01e19ea>] xenvif_kthread_guest_rx+0x26a/0x6e0 [xen_netback] [<ffffffffa01e1780>] ? xenvif_map_frontend_rings+0x110/0x110 [xen_netback] [<ffffffff8111ae9b>] kthread+0x11b/0x150 [<ffffffff81120000>] ? clean_sort_range+0x170/0x2f0 [<ffffffff8111ad80>] ? kthread_stop+0x230/0x230 [<ffffffff81d6957c>] ret_from_fork+0x7c/0xb0 [<ffffffff8111ad80>] ? kthread_stop+0x230/0x230 Code: 01 48 83 05 9e f5 00 00 01 49 8b 44 24 08 49 8b 14 24 49 c7 44 24 08 00 00 00 00 49 c7 04 24 00 00 00 00 48 83 05 84 f5 00 00 01 <48> 89 42 08 48 89 10 41 8b 84 24 80 00 00 00 29 83 2c ba 00 00 RIP [<ffffffffa01dbcdc>] xenvif_rx_dequeue+0x7c/0x120 [xen_netback] RSP <ffff88000853bde8> CR2: 0000000000000008 ---[ end trace b3caaf6875c8a975 ]--- Signed-off-by: Joao Martins <joao.martins@neclab.eu> --- drivers/net/xen-netback/interface.c | 9 +++++++++ 1 file changed, 9 insertions(+)