Message ID | 1454689748-29095-1-git-send-email-vkuznets@redhat.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Vitaly Kuznetsov <vkuznets@redhat.com> writes: > Commit c0eb454034aa ("hv_netvsc: Don't ask for additional head room in the > skb") got rid of needed_headroom setting for the driver. With the change I > hit the following issue trying to use ptkgen module: > > [ 57.522021] kernel BUG at net/core/skbuff.c:1128! > [ 57.522021] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC > ... > [ 58.721068] Call Trace: > [ 58.721068] [<ffffffffa0144e86>] netvsc_start_xmit+0x4c6/0x8e0 [hv_netvsc] > ... > [ 58.721068] [<ffffffffa02f87fc>] ? pktgen_finalize_skb+0x25c/0x2a0 [pktgen] > [ 58.721068] [<ffffffff814f5760>] ? __netdev_alloc_skb+0xc0/0x100 > [ 58.721068] [<ffffffffa02f9907>] pktgen_thread_worker+0x257/0x1920 [pktgen] > > Basically, we're calling skb_cow_head(skb, RNDIS_AND_PPI_SIZE) and crash on > if (skb_shared(skb)) > BUG(); > > We probably need to restore needed_headroom setting (but shrunk to > RNDIS_AND_PPI_SIZE as we don't need more) to request the required headroom > space. In theory, it should not give us performance penalty. I'm sorry for the ping but this is kind of a regression and it would be nice to have it fixed in 4.5. Here is a script to trigger kernel crash: #! /bin/sh modprobe pktgen echo "rem_device_all" > /proc/net/pktgen/kpktgend_0 echo "add_device eth0" > /proc/net/pktgen/kpktgend_0 echo "pkt_size 60" > /proc/net/pktgen/eth0 echo "clone_skb 1000000" > /proc/net/pktgen/eth0 echo "count 100000" > /proc/net/pktgen/eth0 echo "dst <SOME_IP>" > /proc/net/pktgen/eth0 echo "dst_mac <SOME_MAC>" > /proc/net/pktgen/eth0 echo "start" > /proc/net/pktgen/pgctrl cat /proc/net/pktgen/eth0 Please review. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > drivers/net/hyperv/netvsc_drv.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index 1d3a665..98e34fe 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -1089,6 +1089,9 @@ static int netvsc_probe(struct hv_device *dev, > net->ethtool_ops = ðtool_ops; > SET_NETDEV_DEV(net, &dev->device); > > + /* We always need headroom for rndis header */ > + net->needed_headroom = RNDIS_AND_PPI_SIZE; > + > /* Notify the netvsc driver of the new device */ > memset(&device_info, 0, sizeof(device_info)); > device_info.ring_size = ring_size;
From: Vitaly Kuznetsov <vkuznets@redhat.com> Date: Wed, 10 Feb 2016 11:05:50 +0100 > I'm sorry for the ping but this is kind of a regression and it would be > nice to have it fixed in 4.5. In case you can't figure it out, I'm several days backlogged and busy conferencing, travelling, etc. so there will be up to another week of latency in dealing with any patch or request on my part.
From: Vitaly Kuznetsov <vkuznets@redhat.com> Date: Fri, 5 Feb 2016 17:29:08 +0100 > Commit c0eb454034aa ("hv_netvsc: Don't ask for additional head room in the > skb") got rid of needed_headroom setting for the driver. With the change I > hit the following issue trying to use ptkgen module: > > [ 57.522021] kernel BUG at net/core/skbuff.c:1128! > [ 57.522021] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC > ... > [ 58.721068] Call Trace: > [ 58.721068] [<ffffffffa0144e86>] netvsc_start_xmit+0x4c6/0x8e0 [hv_netvsc] > ... > [ 58.721068] [<ffffffffa02f87fc>] ? pktgen_finalize_skb+0x25c/0x2a0 [pktgen] > [ 58.721068] [<ffffffff814f5760>] ? __netdev_alloc_skb+0xc0/0x100 > [ 58.721068] [<ffffffffa02f9907>] pktgen_thread_worker+0x257/0x1920 [pktgen] > > Basically, we're calling skb_cow_head(skb, RNDIS_AND_PPI_SIZE) and crash on > if (skb_shared(skb)) > BUG(); > > We probably need to restore needed_headroom setting (but shrunk to > RNDIS_AND_PPI_SIZE as we don't need more) to request the required headroom > space. In theory, it should not give us performance penalty. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Applied, thanks.
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 1d3a665..98e34fe 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -1089,6 +1089,9 @@ static int netvsc_probe(struct hv_device *dev, net->ethtool_ops = ðtool_ops; SET_NETDEV_DEV(net, &dev->device); + /* We always need headroom for rndis header */ + net->needed_headroom = RNDIS_AND_PPI_SIZE; + /* Notify the netvsc driver of the new device */ memset(&device_info, 0, sizeof(device_info)); device_info.ring_size = ring_size;
Commit c0eb454034aa ("hv_netvsc: Don't ask for additional head room in the skb") got rid of needed_headroom setting for the driver. With the change I hit the following issue trying to use ptkgen module: [ 57.522021] kernel BUG at net/core/skbuff.c:1128! [ 57.522021] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC ... [ 58.721068] Call Trace: [ 58.721068] [<ffffffffa0144e86>] netvsc_start_xmit+0x4c6/0x8e0 [hv_netvsc] ... [ 58.721068] [<ffffffffa02f87fc>] ? pktgen_finalize_skb+0x25c/0x2a0 [pktgen] [ 58.721068] [<ffffffff814f5760>] ? __netdev_alloc_skb+0xc0/0x100 [ 58.721068] [<ffffffffa02f9907>] pktgen_thread_worker+0x257/0x1920 [pktgen] Basically, we're calling skb_cow_head(skb, RNDIS_AND_PPI_SIZE) and crash on if (skb_shared(skb)) BUG(); We probably need to restore needed_headroom setting (but shrunk to RNDIS_AND_PPI_SIZE as we don't need more) to request the required headroom space. In theory, it should not give us performance penalty. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- drivers/net/hyperv/netvsc_drv.c | 3 +++ 1 file changed, 3 insertions(+)