Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] brcmfmac: unlink URB when request timed out
From: Arend van Spriel @ 2014-11-10 11:18 UTC (permalink / raw)
  To: Mathy Vanhoef, brudley, frankyl, meuleman, linville, pieterpg,
	linux-wireless, brcm80211-dev-list, netdev, Oliver Neukum
In-Reply-To: <545FAE05.2030701@gmail.com>

On 09-11-14 19:10, Mathy Vanhoef wrote:
> From: Mathy Vanhoef <vanhoefm@gmail.com>
> 
> Unlink the submitted URB in brcmf_usb_dl_cmd if the request timed out. This
> assures the URB is never submitted twice, preventing a driver crash.

Hi Mathy,

What driver crash are you referring to? The log only shows the WARNING
ending in a USB disconnect but no actual crash. Does your patch get the
driver running properly or does it only avoid the warning.

With that said, it seems there is some need for improvement, but I also
notice you are running this on a virtual machine so could that affect
the timeout to kick in before completion. Could you try to increase the
timeout. Still when a timeout occurs this needs to be handled properly.
Could you also try the following patch?

Regards,
Arend
---
 drivers/net/wireless/brcm80211/brcmfmac/usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
b/drivers/net/wireles
index dc13591..786c40b 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
@@ -640,7 +640,7 @@ static int brcmf_usb_dl_cmd(struct brcmf_usbdev_info
*devinf
        char *tmpbuf;
        u16 size;

-       if ((!devinfo) || (devinfo->ctl_urb == NULL))
+       if (!devinfo || !devinfo->ctl_urb || !devinfo->ctl_completed)
                return -EINVAL;

        tmpbuf = kmalloc(buflen, GFP_ATOMIC);

> Signed-off-by: Mathy Vanhoef <vanhoefm@gmail.com>
> ---
> Currently brcmfmac may crash when a USB device is attached (tested with a LG
> TWFM-B003D). In particular it fails on the second call to brcmf_usb_dl_cmd in
> the while loop of brcmf_usb_resetcfg. The problem is that an URB is being
> submitted twice:
> 
> [  169.861800] brcmfmac: brcmf_usb_dl_writeimage Enter, fw f14db000, len 348160
> [  171.787791] brcmfmac: brcmf_usb_dl_writeimage Exit, err=0
> [  171.787797] brcmfmac: brcmf_usb_dlstart Exit, err=0
> [  171.787799] brcmfmac: brcmf_usb_dlrun Enter
> [  171.791794] brcmfmac: brcmf_usb_resetcfg Enter
> [  173.988072] ------------[ cut here ]------------
> [  173.988083] WARNING: CPU: 0 PID: 369 at drivers/usb/core/urb.c:339 usb_submit_urb+0x4e6/0x500()
> [  173.988085] URB eaf45f00 submitted while active
> [  173.988086] Modules linked in: brcmfmac brcmutil vmw_pvscsi pcnet32 mptspi mptscsih mptbase
> [  173.988100] CPU: 0 PID: 369 Comm: kworker/0:2 Not tainted 3.18.0-rc3-wl #1
> [  173.988102] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/31/2013
> [  173.988106] Workqueue: events request_firmware_work_func
> [  173.988108]  00000000 00000000 ee747db8 c1711f4a ee747df8 ee747de8 c103edaf c18d1e10
> [  173.988112]  ee747e14 00000171 c18a8b29 00000153 c1490556 c1490556 eaf45f00 eafdc660
> [  173.988115]  f14b8fa0 ee747e00 c103ee4e 00000009 ee747df8 c18d1e10 ee747e14 ee747e50
> [  173.988119] Call Trace:
> [  173.988129]  [<c1711f4a>] dump_stack+0x41/0x52
> [  173.988136]  [<c103edaf>] warn_slowpath_common+0x7f/0xa0
> [  173.988139]  [<c1490556>] ? usb_submit_urb+0x4e6/0x500
> [  173.988141]  [<c1490556>] ? usb_submit_urb+0x4e6/0x500
> [  173.988147]  [<f14b8fa0>] ? brcmf_usb_ioctl_resp_wake+0x40/0x40 [brcmfmac]
> [  173.988150]  [<c103ee4e>] warn_slowpath_fmt+0x2e/0x30
> [  173.988152]  [<c1490556>] usb_submit_urb+0x4e6/0x500
> [  173.988156]  [<c1123de1>] ? __kmalloc+0x21/0x140
> [  173.988161]  [<f14b91c3>] ? brcmf_usb_dl_cmd+0x33/0x120 [brcmfmac]
> [  173.988166]  [<f14b9243>] brcmf_usb_dl_cmd+0xb3/0x120 [brcmfmac]
> [  173.988170]  [<f14ba6c4>] brcmf_usb_probe_phase2+0x4e4/0x640 [brcmfmac]
> [  173.988176]  [<f14b4900>] brcmf_fw_request_code_done+0xd0/0xf0 [brcmfmac]
> [  173.988178]  [<c1400876>] request_firmware_work_func+0x26/0x50
> [  173.988182]  [<c10513ee>] process_one_work+0x11e/0x360
> [  173.988184]  [<c1051750>] worker_thread+0xf0/0x3c0
> [  173.988205]  [<c106e14a>] ? __wake_up_locked+0x1a/0x20
> [  173.988208]  [<c1051660>] ? process_scheduled_works+0x30/0x30
> [  173.988211]  [<c1055b56>] kthread+0x96/0xb0
> [  173.988214]  [<c1719c81>] ret_from_kernel_thread+0x21/0x30
> [  173.988217]  [<c1055ac0>] ? kthread_worker_fn+0x110/0x110
> [  173.988219] ---[ end trace 0c88bf46801de083 ]---
> [  173.988221] brcmf_usb_dl_cmd: usb_submit_urb failed -16
> [  173.988396] brcmfmac: brcmf_usb_probe_phase2 failed: dev=1-1, err=-19
> [  173.989503] brcmfmac: brcmf_usb_disconnect Enter
> 
> This patch fixes the brcmf_usb_dl_cmd function to prevent an URB from being
> submitted twice. Tested using a LG TWFM-B003D, which now works properly.
> 
> 
>  drivers/net/wireless/brcm80211/brcmfmac/usb.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
> index 5265aa7..1bc7858 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
> @@ -738,10 +738,12 @@ static int brcmf_usb_dl_cmd(struct brcmf_usbdev_info *devinfo, u8 cmd,
>  		goto finalize;
>  	}
>  
> -	if (!brcmf_usb_ioctl_resp_wait(devinfo))
> +	if (!brcmf_usb_ioctl_resp_wait(devinfo)) {
> +		usb_unlink_urb(devinfo->ctl_urb);
>  		ret = -ETIMEDOUT;
> -	else
> +	} else {
>  		memcpy(buffer, tmpbuf, buflen);
> +	}
>  
>  finalize:
>  	kfree(tmpbuf);
> 

^ permalink raw reply related

* skbuff_fclone_cache poison overwritten
From: Sitsofe Wheeler @ 2014-11-10 11:32 UTC (permalink / raw)
  To: David S. Miller
  Cc: K. Y. Srinivasan, Haiyang Zhang, Long Li, netdev, linux-kernel

While using 3.18.0-rc3.x86_64-00116-g6ac94d3 on a Hyper-V 2012 R2 the
poison in skbuff_fclone_cache was overwritten:

[39099.484435] sd 7:0:0:0: [sdi] Attached SCSI disk
[39099.484688] sd 6:0:0:0: [sdh] Attached SCSI disk
[45285.786640] =============================================================================
[45285.787543] BUG skbuff_fclone_cache (Not tainted): Poison overwritten
[45285.787543] -----------------------------------------------------------------------------

[45285.787543] Disabling lock debugging due to kernel taint
[45285.787543] INFO: 0xffff8800d144c056-0xffff8800d144c056. First byte 0x6f instead of 0x6b
[45285.787543] INFO: Allocated in __alloc_skb+0x4e/0x240 age=11 cpu=1 pid=17444
[45285.787543] 	__slab_alloc+0x50a/0x563
[45285.787543] 	kmem_cache_alloc_node+0xfe/0x2a0
[45285.787543] 	__alloc_skb+0x4e/0x240
[45285.787543] 	sk_stream_alloc_skb+0x3d/0x110
[45285.787543] 	tcp_sendmsg+0x36d/0xc60
[45285.787543] 	inet_sendmsg+0xd7/0xf0
[45285.787543] 	sock_sendmsg+0x90/0xb0
[45285.787543] 	SYSC_sendto+0x10e/0x150
[45285.787543] 	SyS_sendto+0xe/0x10
[45285.787543] 	system_call_fastpath+0x12/0x17
[45285.787543] INFO: Freed in kfree_skbmem+0x6f/0xa0 age=21 cpu=1 pid=17444
[45285.787543] 	__slab_free+0x39/0x2a0
[45285.787543] 	kmem_cache_free+0x1ce/0x280
[45285.787543] 	kfree_skbmem+0x6f/0xa0
[45285.787543] 	__kfree_skb+0x1e/0x30
[45285.787543] 	tcp_ack+0x66e/0x11f0
[45285.787543] 	tcp_rcv_established+0x514/0x6e0
[45285.787543] 	tcp_v4_do_rcv+0xb4/0x330
[45285.787543] 	release_sock+0xfd/0x1f0
[45285.787543] 	tcp_sendmsg+0xa65/0xc60
[45285.787543] 	inet_sendmsg+0xd7/0xf0
[45285.787543] 	sock_sendmsg+0x90/0xb0
[45285.787543] 	SYSC_sendto+0x10e/0x150
[45285.787543] 	SyS_sendto+0xe/0x10
[45285.787543] 	system_call_fastpath+0x12/0x17
[45285.787543] INFO: Slab 0xffffea0003451200 objects=42 used=42 fp=0x          (null) flags=0x3ffe0000004080
[45285.787543] INFO: Object 0xffff8800d144bf00 @offset=16128 fp=0xffff8800d1448f00

[45285.787543] Bytes b4 ffff8800d144bef0: 88 3d ad 02 01 00 00 00 5a 5a 5a 5a 5a 5a 5a 5a  .=......ZZZZZZZZ
[45285.787543] Object ffff8800d144bf00: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bf10: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bf20: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bf30: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bf40: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bf50: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bf60: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bf70: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bf80: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bf90: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bfa0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bfb0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bfc0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bfd0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bfe0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144bff0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c020: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c040: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c050: 6b 6b 6b 6b 6b 6b 6f 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkokkkkkkkkk
[45285.787543] Object ffff8800d144c060: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c070: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c080: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c090: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c0a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[45285.787543] Object ffff8800d144c0b0: 6b 6b 6b 6b 6b 6b 6b a5                          kkkkkkk.
[45285.787543] Redzone ffff8800d144c0b8: bb bb bb bb bb bb bb bb                          ........
[45285.787543] Padding ffff8800d144c1f8: 5a 5a 5a 5a 5a 5a 5a 5a                          ZZZZZZZZ
[45285.787543] CPU: 7 PID: 16678 Comm: phantomjs Tainted: G    B          3.18.0-rc3.x86_64-00116-g6ac94d3 #160
[45285.787543] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS 090006  05/23/2012
[45285.787543]  ffff8800d144bf00 ffff8801eb41b928 ffffffff816db38f 0000000000000000
[45285.787543]  ffff8801fbd34e00 ffff8801eb41b968 ffffffff811a6187 0000000000000008
[45285.787543]  ffff880000000001 ffff8800d144c057 ffff8801fbd34e00 000000000000006b
[45285.787543] Call Trace:
[45285.787543]  [<ffffffff816db38f>] dump_stack+0x4e/0x68
[45285.787543]  [<ffffffff811a6187>] print_trailer+0x1c7/0x1e0
[45285.787543]  [<ffffffff811a6b7b>] check_bytes_and_report+0xbb/0x110
[45285.787543]  [<ffffffff811a76ee>] check_object+0x10e/0x240
[45285.787543]  [<ffffffff815fb11e>] ? __alloc_skb+0x4e/0x240
[45285.787543]  [<ffffffff816d8b65>] alloc_debug_processing+0x76/0x118
[45285.787543]  [<ffffffff816d981b>] __slab_alloc+0x50a/0x563
[45285.787543]  [<ffffffff815fb11e>] ? __alloc_skb+0x4e/0x240
[45285.787543]  [<ffffffff810b7ad8>] ? mark_held_locks+0x88/0xa0
[45285.787543]  [<ffffffff811a9dbe>] kmem_cache_alloc_node+0xfe/0x2a0
[45285.787543]  [<ffffffff815fb11e>] __alloc_skb+0x4e/0x240
[45285.787543]  [<ffffffff81655c3d>] sk_stream_alloc_skb+0x3d/0x110
[45285.787543]  [<ffffffff8165666d>] tcp_sendmsg+0x36d/0xc60
[45285.787543]  [<ffffffff81683847>] inet_sendmsg+0xd7/0xf0
[45285.787543]  [<ffffffff81683775>] ? inet_sendmsg+0x5/0xf0
[45285.787543]  [<ffffffff815f2980>] sock_sendmsg+0x90/0xb0
[45285.787543]  [<ffffffff811e4541>] ? __fget_light+0x61/0x80
[45285.787543]  [<ffffffff811e4ee3>] ? __fdget+0x13/0x20
[45285.787543]  [<ffffffff815f2aae>] SYSC_sendto+0x10e/0x150
[45285.787543]  [<ffffffff811cab6f>] ? SYSC_newstat+0x2f/0x40
[45285.787543]  [<ffffffff816e5a5c>] ? retint_swapgs+0x13/0x1b
[45285.787543]  [<ffffffff813aa1fe>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[45285.787543]  [<ffffffff815f3afe>] SyS_sendto+0xe/0x10
[45285.787543]  [<ffffffff816e4e29>] system_call_fastpath+0x12/0x17
[45285.787543] FIX skbuff_fclone_cache: Restoring 0xffff8800d144c056-0xffff8800d144c056=0x6b

[45285.787543] FIX skbuff_fclone_cache: Marking all objects used
[46810.070997] =============================================================================
[46810.071289] BUG skbuff_fclone_cache (Tainted: G    B         ): Poison overwritten
[46810.071289] -----------------------------------------------------------------------------

[46810.071289] INFO: 0xffff8801c48fe756-0xffff8801c48fe756. First byte 0x6f instead of 0x6b
[46810.071289] INFO: Allocated in __alloc_skb+0x4e/0x240 age=9 cpu=6 pid=1220
[46810.071289] 	__slab_alloc+0x50a/0x563
[46810.071289] 	kmem_cache_alloc_node+0xfe/0x2a0
[46810.071289] 	__alloc_skb+0x4e/0x240
[46810.071289] 	sk_stream_alloc_skb+0x3d/0x110
[46810.071289] 	tcp_sendmsg+0x36d/0xc60
[46810.071289] 	inet_sendmsg+0xd7/0xf0
[46810.071289] 	sock_sendmsg+0x90/0xb0
[46810.071289] 	SYSC_sendto+0x10e/0x150
[46810.071289] 	SyS_sendto+0xe/0x10
[46810.071289] 	system_call_fastpath+0x12/0x17
[46810.071289] INFO: Freed in kfree_skbmem+0x6f/0xa0 age=23 cpu=6 pid=1220
[46810.071289] 	__slab_free+0x39/0x2a0
[46810.071289] 	kmem_cache_free+0x1ce/0x280
[46810.071289] 	kfree_skbmem+0x6f/0xa0
[46810.071289] 	__kfree_skb+0x1e/0x30
[46810.071289] 	tcp_ack+0x66e/0x11f0
[46810.071289] 	tcp_rcv_established+0x107/0x6e0
[46810.071289] 	tcp_v4_do_rcv+0xb4/0x330
[46810.071289] 	release_sock+0xfd/0x1f0
[46810.071289] 	tcp_sendmsg+0xa65/0xc60
[46810.071289] 	inet_sendmsg+0xd7/0xf0
[46810.071289] 	sock_sendmsg+0x90/0xb0
[46810.071289] 	SYSC_sendto+0x10e/0x150
[46810.071289] 	SyS_sendto+0xe/0x10
[46810.071289] 	system_call_fastpath+0x12/0x17
[46810.071289] INFO: Slab 0xffffea0007123e00 objects=42 used=42 fp=0x          (null) flags=0x5ffe0000004080
[46810.071289] INFO: Object 0xffff8801c48fe600 @offset=26112 fp=0xffff8801c48fd700

[46810.071289] Bytes b4 ffff8801c48fe5f0: c2 89 c4 02 01 00 00 00 5a 5a 5a 5a 5a 5a 5a 5a  ........ZZZZZZZZ
[46810.071289] Object ffff8801c48fe600: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe610: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe620: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe630: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe640: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe650: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe660: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe670: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe680: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe690: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe6a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe6b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe6c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe6d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe6e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe6f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.071289] Object ffff8801c48fe700: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.228303] Object ffff8801c48fe710: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.228303] Object ffff8801c48fe720: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.240204] Object ffff8801c48fe730: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.240204] Object ffff8801c48fe740: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.240204] Object ffff8801c48fe750: 6b 6b 6b 6b 6b 6b 6f 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkokkkkkkkkk
[46810.240204] Object ffff8801c48fe760: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.240204] Object ffff8801c48fe770: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.240204] Object ffff8801c48fe780: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.240204] Object ffff8801c48fe790: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.240204] Object ffff8801c48fe7a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46810.291135] Object ffff8801c48fe7b0: 6b 6b 6b 6b 6b 6b 6b a5                          kkkkkkk.
[46810.291135] Redzone ffff8801c48fe7b8: bb bb bb bb bb bb bb bb                          ........
[46810.291135] Padding ffff8801c48fe8f8: 5a 5a 5a 5a 5a 5a 5a 5a                          ZZZZZZZZ
[46810.291135] CPU: 6 PID: 1233 Comm: phantomjs Tainted: G    B          3.18.0-rc3.x86_64-00116-g6ac94d3 #160
[46810.291135] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS 090006  05/23/2012
[46810.291135]  ffff8801c48fe600 ffff8801f2ebbc08 ffffffff816db38f ffff8801b6b59350
[46810.291135]  ffff8801fbd34e00 ffff8801f2ebbc48 ffffffff811a6187 0000000000000008
[46810.291135]  ffff880100000001 ffff8801c48fe757 ffff8801fbd34e00 000000000000006b
[46810.291135] Call Trace:
[46810.291135]  [<ffffffff816db38f>] dump_stack+0x4e/0x68
[46810.291135]  [<ffffffff811a6187>] print_trailer+0x1c7/0x1e0
[46810.291135]  [<ffffffff811a6b7b>] check_bytes_and_report+0xbb/0x110
[46810.291135]  [<ffffffff811a76ee>] check_object+0x10e/0x240
[46810.291135]  [<ffffffff815fb11e>] ? __alloc_skb+0x4e/0x240
[46810.291135]  [<ffffffff816d8b65>] alloc_debug_processing+0x76/0x118
[46810.291135]  [<ffffffff816d981b>] __slab_alloc+0x50a/0x563
[46810.291135]  [<ffffffff810b7f4d>] ? trace_hardirqs_on+0xd/0x10
[46810.291135]  [<ffffffff815fb11e>] ? __alloc_skb+0x4e/0x240
[46810.291135]  [<ffffffff811a9dbe>] kmem_cache_alloc_node+0xfe/0x2a0
[46810.291135]  [<ffffffff815fb11e>] __alloc_skb+0x4e/0x240
[46810.291135]  [<ffffffff81666caa>] tcp_send_fin+0x7a/0x1a0
[46810.291135]  [<ffffffff81657fe6>] tcp_shutdown+0x46/0x60
[46810.291135]  [<ffffffff81682125>] inet_shutdown+0xb5/0x110
[46810.291135]  [<ffffffff815f3d17>] SyS_shutdown+0x47/0x70
[46810.291135]  [<ffffffff816e4e29>] system_call_fastpath+0x12/0x17
[46810.291135] FIX skbuff_fclone_cache: Restoring 0xffff8801c48fe756-0xffff8801c48fe756=0x6b

[46810.291135] FIX skbuff_fclone_cache: Marking all objects used
[46994.744143] =============================================================================
[46994.744548] BUG skbuff_fclone_cache (Tainted: G    B         ): Poison overwritten
[46994.744548] -----------------------------------------------------------------------------

[46994.744548] INFO: 0xffff8801eb7df056-0xffff8801eb7df056. First byte 0x6f instead of 0x6b
[46994.744548] INFO: Allocated in __alloc_skb+0x4e/0x240 age=10 cpu=0 pid=17426
[46994.744548] 	__slab_alloc+0x50a/0x563
[46994.744548] 	kmem_cache_alloc_node+0xfe/0x2a0
[46994.744548] 	__alloc_skb+0x4e/0x240
[46994.744548] 	sk_stream_alloc_skb+0x3d/0x110
[46994.744548] 	tcp_sendmsg+0x36d/0xc60
[46994.744548] 	inet_sendmsg+0xd7/0xf0
[46994.744548] 	sock_sendmsg+0x90/0xb0
[46994.744548] 	SYSC_sendto+0x10e/0x150
[46994.744548] 	SyS_sendto+0xe/0x10
[46994.744548] 	system_call_fastpath+0x12/0x17
[46994.744548] INFO: Freed in kfree_skbmem+0x6f/0xa0 age=21 cpu=0 pid=17426
[46994.744548] 	__slab_free+0x39/0x2a0
[46994.744548] 	kmem_cache_free+0x1ce/0x280
[46994.744548] 	kfree_skbmem+0x6f/0xa0
[46994.744548] 	__kfree_skb+0x1e/0x30
[46994.744548] 	tcp_ack+0x66e/0x11f0
[46994.744548] 	tcp_rcv_established+0x514/0x6e0
[46994.744548] 	tcp_v4_do_rcv+0xb4/0x330
[46994.744548] 	release_sock+0xfd/0x1f0
[46994.744548] 	tcp_sendmsg+0xa65/0xc60
[46994.744548] 	inet_sendmsg+0xd7/0xf0
[46994.744548] 	sock_sendmsg+0x90/0xb0
[46994.744548] 	SYSC_sendto+0x10e/0x150
[46994.744548] 	SyS_sendto+0xe/0x10
[46994.744548] 	system_call_fastpath+0x12/0x17
[46994.744548] INFO: Slab 0xffffea0007adf600 objects=42 used=42 fp=0x          (null) flags=0x5ffe0000004080
[46994.744548] INFO: Object 0xffff8801eb7def00 @offset=28416 fp=0xffff8801eb7db900

[46994.744548] Bytes b4 ffff8801eb7deef0: f0 cd b1 02 01 00 00 00 5a 5a 5a 5a 5a 5a 5a 5a  ........ZZZZZZZZ
[46994.744548] Object ffff8801eb7def00: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7def10: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7def20: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7def30: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7def40: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7def50: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7def60: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7def70: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7def80: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7def90: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7defa0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7defb0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7defc0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7defd0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7defe0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7deff0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df020: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df030: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df040: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df050: 6b 6b 6b 6b 6b 6b 6f 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkokkkkkkkkk
[46994.744548] Object ffff8801eb7df060: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df070: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df080: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df090: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df0a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[46994.744548] Object ffff8801eb7df0b0: 6b 6b 6b 6b 6b 6b 6b a5                          kkkkkkk.
[46994.744548] Redzone ffff8801eb7df0b8: bb bb bb bb bb bb bb bb                          ........
[46994.744548] Padding ffff8801eb7df1f8: 5a 5a 5a 5a 5a 5a 5a 5a                          ZZZZZZZZ
[46994.744548] CPU: 4 PID: 24686 Comm: phantomjs Tainted: G    B          3.18.0-rc3.x86_64-00116-g6ac94d3 #160
[46994.744548] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS 090006  05/23/2012
[46994.744548]  ffff8801eb7def00 ffff8801c4823928 ffffffff816db38f 0000000000000000
[46994.744548]  ffff8801fbd34e00 ffff8801c4823968 ffffffff811a6187 0000000000000008
[46994.744548]  ffff880100000001 ffff8801eb7df057 ffff8801fbd34e00 000000000000006b
[46994.744548] Call Trace:
[46994.744548]  [<ffffffff816db38f>] dump_stack+0x4e/0x68
[46994.744548]  [<ffffffff811a6187>] print_trailer+0x1c7/0x1e0
[46994.744548]  [<ffffffff811a6b7b>] check_bytes_and_report+0xbb/0x110
[46994.744548]  [<ffffffff811a76ee>] check_object+0x10e/0x240
[46994.744548]  [<ffffffff815fb11e>] ? __alloc_skb+0x4e/0x240
[46994.744548]  [<ffffffff816d8b65>] alloc_debug_processing+0x76/0x118
[46994.744548]  [<ffffffff816d981b>] __slab_alloc+0x50a/0x563
[46994.744548]  [<ffffffff815fb11e>] ? __alloc_skb+0x4e/0x240
[46994.744548]  [<ffffffff8100611b>] ? print_context_stack+0xdb/0x100
[46994.744548]  [<ffffffff811a9dbe>] kmem_cache_alloc_node+0xfe/0x2a0
[46994.744548]  [<ffffffff815fb11e>] __alloc_skb+0x4e/0x240
[46994.744548]  [<ffffffff81655c3d>] sk_stream_alloc_skb+0x3d/0x110
[46994.744548]  [<ffffffff8165666d>] tcp_sendmsg+0x36d/0xc60
[46994.744548]  [<ffffffff81683847>] inet_sendmsg+0xd7/0xf0
[46994.744548]  [<ffffffff81683775>] ? inet_sendmsg+0x5/0xf0
[46994.744548]  [<ffffffff815f2980>] sock_sendmsg+0x90/0xb0
[46994.744548]  [<ffffffff811e4541>] ? __fget_light+0x61/0x80
[46994.744548]  [<ffffffff811e4ee3>] ? __fdget+0x13/0x20
[46994.744548]  [<ffffffff815f2aae>] SYSC_sendto+0x10e/0x150
[46994.744548]  [<ffffffff811cab6f>] ? SYSC_newstat+0x2f/0x40
[46994.744548]  [<ffffffff810db34e>] ? getnstimeofday64+0xe/0x30
[46994.744548]  [<ffffffff813aa1fe>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[46994.744548]  [<ffffffff815f3afe>] SyS_sendto+0xe/0x10
[46994.744548]  [<ffffffff816e4e29>] system_call_fastpath+0x12/0x17
[46994.744548] FIX skbuff_fclone_cache: Restoring 0xffff8801eb7df056-0xffff8801eb7df056=0x6b

[46994.744548] FIX skbuff_fclone_cache: Marking all objects used
[71820.156136] =============================================================================
[71820.156461] BUG skbuff_fclone_cache (Tainted: G    B         ): Poison overwritten
[71820.156461] -----------------------------------------------------------------------------

[71820.156461] INFO: 0xffff8801eb42e756-0xffff8801eb42e756. First byte 0x6f instead of 0x6b
[71820.156461] INFO: Allocated in __alloc_skb+0x4e/0x240 age=11 cpu=3 pid=4181
[71820.156461] 	__slab_alloc+0x50a/0x563
[71820.156461] 	kmem_cache_alloc_node+0xfe/0x2a0
[71820.156461] 	__alloc_skb+0x4e/0x240
[71820.156461] 	sk_stream_alloc_skb+0x3d/0x110
[71820.156461] 	tcp_sendmsg+0x36d/0xc60
[71820.156461] 	inet_sendmsg+0xd7/0xf0
[71820.156461] 	sock_sendmsg+0x90/0xb0
[71820.156461] 	SYSC_sendto+0x10e/0x150
[71820.156461] 	SyS_sendto+0xe/0x10
[71820.156461] 	system_call_fastpath+0x12/0x17
[71820.156461] INFO: Freed in kfree_skbmem+0x6f/0xa0 age=30 cpu=3 pid=4181
[71820.156461] 	__slab_free+0x39/0x2a0
[71820.156461] 	kmem_cache_free+0x1ce/0x280
[71820.156461] 	kfree_skbmem+0x6f/0xa0
[71820.156461] 	__kfree_skb+0x1e/0x30
[71820.156461] 	tcp_ack+0x66e/0x11f0
[71820.156461] 	tcp_rcv_established+0x107/0x6e0
[71820.156461] 	tcp_v4_do_rcv+0xb4/0x330
[71820.156461] 	release_sock+0xfd/0x1f0
[71820.156461] 	tcp_sendmsg+0xa65/0xc60
[71820.156461] 	inet_sendmsg+0xd7/0xf0
[71820.156461] 	sock_sendmsg+0x90/0xb0
[71820.156461] 	SYSC_sendto+0x10e/0x150
[71820.156461] 	SyS_sendto+0xe/0x10
[71820.156461] 	system_call_fastpath+0x12/0x17
[71820.156461] INFO: Slab 0xffffea0007ad0a00 objects=42 used=42 fp=0x          (null) flags=0x5ffe0000004080
[71820.156461] INFO: Object 0xffff8801eb42e600 @offset=26112 fp=0xffff8801eb42b600

[71820.156461] Bytes b4 ffff8801eb42e5f0: 48 1d 41 04 01 00 00 00 5a 5a 5a 5a 5a 5a 5a 5a  H.A.....ZZZZZZZZ
[71820.156461] Object ffff8801eb42e600: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.156461] Object ffff8801eb42e610: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.156461] Object ffff8801eb42e620: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.156461] Object ffff8801eb42e630: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.156461] Object ffff8801eb42e640: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.156461] Object ffff8801eb42e650: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.156461] Object ffff8801eb42e660: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.156461] Object ffff8801eb42e670: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.156461] Object ffff8801eb42e680: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e690: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e6a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e6b0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e6c0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e6d0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e6e0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e6f0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e700: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e710: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e720: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e730: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e740: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e750: 6b 6b 6b 6b 6b 6b 6f 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkokkkkkkkkk
[71820.290941] Object ffff8801eb42e760: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e770: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e780: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e790: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e7a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
[71820.290941] Object ffff8801eb42e7b0: 6b 6b 6b 6b 6b 6b 6b a5                          kkkkkkk.
[71820.290941] Redzone ffff8801eb42e7b8: bb bb bb bb bb bb bb bb                          ........
[71820.290941] Padding ffff8801eb42e8f8: 5a 5a 5a 5a 5a 5a 5a 5a                          ZZZZZZZZ
[71820.290941] CPU: 3 PID: 4219 Comm: phantomjs Tainted: G    B          3.18.0-rc3.x86_64-00116-g6ac94d3 #160
[71820.290941] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS 090006  05/23/2012
[71820.290941]  ffff8801eb42e600 ffff8800d1743c08 ffffffff816db38f 0000000000000000
[71820.290941]  ffff8801fbd34e00 ffff8800d1743c48 ffffffff811a6187 0000000000000008
[71820.290941]  ffff880100000001 ffff8801eb42e757 ffff8801fbd34e00 000000000000006b
[71820.290941] Call Trace:
[71820.290941]  [<ffffffff816db38f>] dump_stack+0x4e/0x68
[71820.290941]  [<ffffffff811a6187>] print_trailer+0x1c7/0x1e0
[71820.290941]  [<ffffffff811a6b7b>] check_bytes_and_report+0xbb/0x110
[71820.290941]  [<ffffffff811a76ee>] check_object+0x10e/0x240
[71820.290941]  [<ffffffff815fb11e>] ? __alloc_skb+0x4e/0x240
[71820.290941]  [<ffffffff816d8b65>] alloc_debug_processing+0x76/0x118
[71820.290941]  [<ffffffff816d981b>] __slab_alloc+0x50a/0x563
[71820.290941]  [<ffffffff810b7f4d>] ? trace_hardirqs_on+0xd/0x10
[71820.290941]  [<ffffffff815fb11e>] ? __alloc_skb+0x4e/0x240
[71820.290941]  [<ffffffff811a9dbe>] kmem_cache_alloc_node+0xfe/0x2a0
[71820.290941]  [<ffffffff815fb11e>] __alloc_skb+0x4e/0x240
[71820.290941]  [<ffffffff81666caa>] tcp_send_fin+0x7a/0x1a0
[71820.290941]  [<ffffffff81657fe6>] tcp_shutdown+0x46/0x60
[71820.290941]  [<ffffffff81682125>] inet_shutdown+0xb5/0x110
[71820.290941]  [<ffffffff815f3d17>] SyS_shutdown+0x47/0x70
[71820.290941]  [<ffffffff816e4e29>] system_call_fastpath+0x12/0x17
[71820.290941] FIX skbuff_fclone_cache: Restoring 0xffff8801eb42e756-0xffff8801eb42e756=0x6b

[71820.290941] FIX skbuff_fclone_cache: Marking all objects used

As I don't know where to file this I'm sending it to networking and
Hyper-V people initially... If anyone can give tips on narrowing down
the true cause that would be helpful. The workload is new and older
kernels on Hyper-V hit other issues so bisection isn't an easy start...

-- 
Sitsofe | http://sucs.org/~sits/

^ permalink raw reply

* [PATCH 1/4] pci: move pci_assivned_vfs() check while disabling VFs to pci sub-system
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
  To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>

From: Vasundhara Volam <vasundhara.volam@emulex.com>

A user must not be allowed to disable VFs while they are already assigned to
a guest. This check is being made in each individual driver that implements
the sriov_configure PCI method.
This patch fixes this code duplication by moving this check to the
sriov_nuvfs_store() routine just before invoking sriov_configure() when
num_vfs is equal to 0.

Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/pci/pci-sysfs.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 2c6643f..6e65b47 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -477,6 +477,11 @@ static ssize_t sriov_numvfs_store(struct device *dev,
 	}
 
 	if (num_vfs == 0) {
+		if (pci_vfs_assigned(pdev)) {
+			dev_warn(&pdev->dev, "Cannot disable VFs while they are assigned\n");
+			return -EBUSY;
+		}
+
 		/* disable VFs */
 		ret = pdev->driver->sriov_configure(pdev, 0);
 		if (ret < 0)
-- 
1.7.1

^ permalink raw reply related

* [PATCH 0/4] move pci_assivned_vfs() check (while disabling VFs) to pci sub-system
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
  To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile

A user must not be allowed to disable VFs while they are already assigned to
a guest. This check is being made in each individual driver that implements
the sriov_configure PCI method.
This patch-set fixes this code duplication by moving this check from
drivers to the sriov_nuvfs_store() routine just before invoking
sriov_configure() when num_vfs is equal to 0.

Vasundhara Volam (4):
  pci: move pci_assivned_vfs() check while disabling VFs to pci
    sub-system
  bnx2x: remove pci_assigned_vfs() check while disabling VFs
  i40e: remove pci_assigned_vfs() check while disabling VFs
  qlcnic: remove pci_assigned_vfs() check while disabling VFs

 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c  |    2 +-
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |    7 +------
 .../net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c   |   10 ----------
 drivers/pci/pci-sysfs.c                            |    5 +++++
 4 files changed, 7 insertions(+), 17 deletions(-)

^ permalink raw reply

* [PATCH 2/4] bnx2x: remove pci_assigned_vfs() check while disabling VFs
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
  To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>

From: Vasundhara Volam <vasundhara.volam@emulex.com>

The pci_assigned_vfs() check (while disabling VFs) is being moved to the
pci-sysfs.c file and will be done before invoking sriov_configure().

Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
index c88b20a..e90a10b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -2481,7 +2481,7 @@ int bnx2x_sriov_configure(struct pci_dev *dev, int num_vfs_param)
 	bp->requested_nr_virtfn = num_vfs_param;
 	if (num_vfs_param == 0) {
 		bnx2x_set_pf_tx_switching(bp, false);
-		bnx2x_disable_sriov(bp);
+		pci_disable_sriov(bp->pdev);
 		return 0;
 	} else {
 		return bnx2x_enable_sriov(bp);
-- 
1.7.1

^ permalink raw reply related

* [PATCH 3/4] i40e: remove pci_assigned_vfs() check while disabling VFs
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
  To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>

From: Vasundhara Volam <vasundhara.volam@emulex.com>

The pci_assigned_vfs() check (while disabling VFs) is being moved to the
pci-sysfs.c file and will be done before invoking sriov_configure().

Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index fff3c27..0028a9a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -951,12 +951,7 @@ int i40e_pci_sriov_configure(struct pci_dev *pdev, int num_vfs)
 	if (num_vfs)
 		return i40e_pci_sriov_enable(pdev, num_vfs);
 
-	if (!pci_vfs_assigned(pf->pdev)) {
-		i40e_free_vfs(pf);
-	} else {
-		dev_warn(&pdev->dev, "Unable to free VFs because some are assigned to VMs.\n");
-		return -EINVAL;
-	}
+	i40e_free_vfs(pf);
 	return 0;
 }
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH 4/4] qlcnic: remove pci_assigned_vfs() check while disabling VFs
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
  To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>

From: Vasundhara Volam <vasundhara.volam@emulex.com>

The pci_assigned_vfs() check (while disabling VFs) is being moved to the
pci-sysfs.c file and will be done before invoking sriov_configure().

Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 .../net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c   |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
index a29538b..9802914 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
@@ -465,16 +465,6 @@ static int qlcnic_pci_sriov_disable(struct qlcnic_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
 
-	if (pci_vfs_assigned(adapter->pdev)) {
-		netdev_err(adapter->netdev,
-			   "SR-IOV VFs belonging to port %d are assigned to VMs. SR-IOV can not be disabled on this port\n",
-			   adapter->portnum);
-		netdev_info(adapter->netdev,
-			    "Please detach SR-IOV VFs belonging to port %d from VMs, and then try to disable SR-IOV on this port\n",
-			    adapter->portnum);
-		return -EPERM;
-	}
-
 	qlcnic_sriov_pf_disable(adapter);
 
 	rtnl_lock();
-- 
1.7.1

^ permalink raw reply related

* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jamal Hadi Salim @ 2014-11-10 12:06 UTC (permalink / raw)
  To: David Miller
  Cc: jiri, netdev, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141110.002336.70350092543746386.davem@davemloft.net>

On 11/10/14 00:23, David Miller wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
> Date: Sun, 09 Nov 2014 22:35:12 -0500
>
>> wouldnt this just break an existing ABI? You may need to introduce a
>> new attribute.
>
> He isn't breaking anything Jamal, he's just changing the internal
> macro name we use for the attribute's maximum length.


It is a _user space visible rename_, how about:

#define MAX_PHYS_ITEM_ID_LEN 32
#define MAX_PHYS_PORT_ID_LEN   MAX_PHYS_ITEM_ID_LEN

I did miss the fact that the size didnt change.

cheers,
jamal

^ permalink raw reply

* Re: [patch net-next v2 00/10] introduce rocker switch driver with hardware accelerated datapath api - phase 1: bridge fdb offload
From: Jamal Hadi Salim @ 2014-11-10 12:16 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141110072301.GA1850@nanopsycho.orion>

On 11/10/14 02:23, Jiri Pirko wrote:

>
> Yes I looked over their patches. Roopas patche's are about new class of
> device which, as I commented in the cover letter, I left out for now and
> can be safely added later on.
>
> I went over the Ben's work very carefully as well. The patches are very
> rough, mostly rtl-chip specific. But again, my patchset is a base on
> which this patches can be build on. I see no issues in that.
>
>> At least please get their sign on - this  is such an important piece of
>> new work that you should make sure you get consensus.
>
> Since I did not use their code now, I only put sign off of Scott.
>

Your last comment was "i am going to merge the patches" ;->
At least send an email explaining your plan to people who have worked
hard to cooperate with you or say it in the cover letter.

>> Otherwise we are back to square one and everyone is going their way with
>> their patches;
>
> I do think that we are in sync. I do not see any counter ways. As I
> said, their work can be added on to the base made of this patchset.
>

Ok, I hope so. I spoke for myself - it is important for this patches
you get their sign-on in my opinion.

cheers,
jamal

^ permalink raw reply

* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jamal Hadi Salim @ 2014-11-10 12:17 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141110074344.GB1850@nanopsycho.orion>

On 11/10/14 02:43, Jiri Pirko wrote:
> Mon, Nov 10, 2014 at 04:35:12AM CET, jhs@mojatatu.com wrote:

> I don't see a reason why this would break kabi:
>
> -#define MAX_PHYS_PORT_ID_LEN 32
> +#define MAX_PHYS_ITEM_ID_LEN 32
>

refer to my response to Dave. Just define MAX_PHYS_PORT_ID_LEN to
MAX_PHYS_ITEM_ID_LEN so people dont have to change their code
because a name change.

cheers,
jamal

^ permalink raw reply

* Re: [patch net-next v2 10/10] rocker: implement L2 bridge offloading
From: Jamal Hadi Salim @ 2014-11-10 12:27 UTC (permalink / raw)
  To: Scott Feldman
  Cc: Jiri Pirko, Netdev, David S. Miller, nhorman, Andy Gospodarek,
	Thomas Graf, dborkman, ogerlitz, jesse, pshelar, azhou, ben,
	stephen, Kirsher, Jeffrey T, vyasevic, xiyou.wangcong,
	Fastabend, John R, edumazet, Florian Fainelli, Roopa Prabhu,
	John Linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
	buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
	simon.ho
In-Reply-To: <CAE4R7bCdfn6rzvY5J5d_aj=eNKoT00iEphGJMnQOWy=WeLePTw@mail.gmail.com>

On 11/10/14 03:46, Scott Feldman wrote:

>
> IFLA_BRPORT_LEARNING is u8 attr and we're only using lower bit to turn
> learning on/off.  Maybe we can use another bit to indicate learning to
> be done in sw or hw.  I don't think adding another bit would break
> existing iproute2.
>
> LEARNING_ENABLED    (1 << 0)
> LEARNING_HW              (1 << 1)
>
> Would this work?
>

Yes to making it a bit. But:
This is not *learning*. You are doing a *sync*.
Those are two different things.

Learning on/off exists today. It signals to the L2 whether you
should learn or not.
I like the way fdb_add/del work with a flag which says
it is the software and/or offloaded version. Please keep that
semantic.
What you are doing above is letting the hardware learn then
syncing to software. You need a different flag there. something
like:

SYNC_HW_FDB (1<<1)


cheers,
jamal

^ permalink raw reply

* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Daniel Borkmann @ 2014-11-10 12:33 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: David Miller, jiri, netdev, nhorman, andy, tgraf, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5460AA45.6000007@mojatatu.com>

On 11/10/2014 01:06 PM, Jamal Hadi Salim wrote:
> On 11/10/14 00:23, David Miller wrote:
>> From: Jamal Hadi Salim <jhs@mojatatu.com>
>> Date: Sun, 09 Nov 2014 22:35:12 -0500
>>
>>> wouldnt this just break an existing ABI? You may need to introduce a
>>> new attribute.
>>
>> He isn't breaking anything Jamal, he's just changing the internal
>> macro name we use for the attribute's maximum length.
>
> It is a _user space visible rename_, how about:
>
> #define MAX_PHYS_ITEM_ID_LEN 32
> #define MAX_PHYS_PORT_ID_LEN   MAX_PHYS_ITEM_ID_LEN
>
> I did miss the fact that the size didnt change.

Actually, it's currently not exposed via any uapi header ...

$ git grep -n MAX_PHYS_PORT_ID_LEN
include/linux/netdevice.h:756:#define MAX_PHYS_PORT_ID_LEN 32
include/linux/netdevice.h:762:  unsigned char id[MAX_PHYS_PORT_ID_LEN];
net/core/rtnetlink.c:871:              + nla_total_size(MAX_PHYS_PORT_ID_LEN); /* IFLA_PHYS_PORT_ID */
net/core/rtnetlink.c:1199:      [IFLA_PHYS_PORT_ID]     = { .type = NLA_BINARY, .len = MAX_PHYS_PORT_ID_LEN },

... and based on commit 66cae9ed6bc4 ("rtnl: export physical port id
via RT netlink") only exported as read-only.

Best,
Daniel

^ permalink raw reply

* Re: [patch net-next v2 06/10] bridge: introduce fdb offloading via switchdev
From: Jamal Hadi Salim @ 2014-11-10 12:47 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141110081552.GD1850@nanopsycho.orion>

On 11/10/14 03:15, Jiri Pirko wrote:
> Mon, Nov 10, 2014 at 04:47:48AM CET, jhs@mojatatu.com wrote:
>> On 11/09/14 05:51, Jiri Pirko wrote:
>>> From: Scott Feldman <sfeldma@gmail.com>
>>>

> Jamal, I believe we discussed this already.

I cant remember how that ended.

> The thing is that current
> fdb_add/del does not need vlanid and master/self flags, because it
> already has that (struct nlattr *tb[]). Here is the whole list of
> parameters to these functions:
>          NDA_DST,
>          NDA_LLADDR,
>          NDA_CACHEINFO,
>          NDA_PROBES,
>          NDA_VLAN,
>          NDA_PORT,
>          NDA_VNI,
>          NDA_IFINDEX,
>          NDA_MASTER,
>
> There are few problems in re-using this. It is netlink based so for calling
> it from bridge code, we would have to construct netlink message. But
> that could be probably changed.

Trying to understand.

A netlink message for a bridge to add an fdb is targeted at the
*bridge port*.
That message has semantic which says "please add this entry
to the software bridge and/or offloaded hardware".
If something is targetted at the bridge port, ->ndo_fdb_add()
is invoked with an internally chewed structure.
Why would you have to construct a new netlink message to the driver?


> As you can see from the list of parameters, this is no longer about fdb (addr,
> vlanid) but this has been extended to something else.

I am still missing understanding that part.
Or maybe are you saying that you dont want to pass netlink
constructs to the driver?

> See vxlan code for
> what this is used for. I believe that fdb_add/del should be renamed to
> something else, perhaps l2neigh_add/del or something like that.
> The other problem is that fdb_add/del is currently used by various
> drivers for different purpose (adding macs to unicast list).
>

Ok, now a small spark ignited in my brain. You did talk about renaming
things to neighXXX in one of the exchanges. I think this is a separate
issue from the question of why you cant refactor ndo_fdb_add/del

The abuse of using this interface for unicast addresses is probably
driven by the fact some of the hardware probably offloads vlanid 0 or
something speacial like 4095 to point to the underlying hardware that
"this belongs to host cpu".
I am not a fan of it (and have posted in exchanges with Vlad in the
past).

cheers,
jamal

^ permalink raw reply

* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jamal Hadi Salim @ 2014-11-10 12:56 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: David Miller, jiri, netdev, nhorman, andy, tgraf, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5460B090.7080106@redhat.com>

On 11/10/14 07:33, Daniel Borkmann wrote:

> $ git grep -n MAX_PHYS_PORT_ID_LEN
> include/linux/netdevice.h:756:#define MAX_PHYS_PORT_ID_LEN 32
> include/linux/netdevice.h:762:  unsigned char id[MAX_PHYS_PORT_ID_LEN];
> net/core/rtnetlink.c:871:              +
> nla_total_size(MAX_PHYS_PORT_ID_LEN); /* IFLA_PHYS_PORT_ID */
> net/core/rtnetlink.c:1199:      [IFLA_PHYS_PORT_ID]     = { .type =
> NLA_BINARY, .len = MAX_PHYS_PORT_ID_LEN },
>
> ... and based on commit 66cae9ed6bc4 ("rtnl: export physical port id
> via RT netlink") only exported as read-only.
>

I guess it is *not exported* if no user space code sees it.
If that is the case, I agree that my suggestion is unneeded.

cheers,
jamal

^ permalink raw reply

* Re: [patch net-next v2 07/10] bridge: call netdev_sw_port_stp_update when bridge port STP status changes
From: Jamal Hadi Salim @ 2014-11-10 13:11 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse, pshelar,
	azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
	john.r.fastabend, edumazet, sfeldma, f.fainelli, roopa, linville,
	jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a, buytenh,
	aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye, simon.horman,
	alexander.h.duyck, john.ronciak, mleitner, shrijeet, gospo, bcrl
In-Reply-To: <1415530280-9190-8-git-send-email-jiri@resnulli.us>

On 11/09/14 05:51, Jiri Pirko wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> To notify switch driver of change in STP state of bridge port, add new
> .ndo op and provide swdev wrapper func to call ndo op. Use it in bridge
> code then.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>   include/linux/netdevice.h |  6 ++++++
>   include/net/switchdev.h   |  6 ++++++
>   net/bridge/br_netlink.c   |  2 ++
>   net/bridge/br_stp.c       |  4 ++++
>   net/bridge/br_stp_if.c    |  3 +++
>   net/bridge/br_stp_timer.c |  2 ++
>   net/switchdev/switchdev.c | 19 +++++++++++++++++++
>   7 files changed, 42 insertions(+)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 116a19d..35f21a95 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1033,6 +1033,10 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>    *			      const unsigned char *addr,
>    *			      u16 vid);
>    *	Called to delete a fdb from switch device port.
> + *
> + * int (*ndo_sw_port_stp_update)(struct net_device *dev, u8 state);
> + *	Called to notify switch device port of bridge port STP
> + *	state change.

You are unconditionally calling
netdev_sw_port_stp_update(p->dev, p->state);
Again issue is policy. Could you make this work the same
way the fdb_add e.g user intent of whether i want to turn
a port in hardware and/or software to disabled/learning/etc
is reflected?

btw: does _sw_ stand for switch? why not _hw_ ?
Could we have one ndo for all flags instead of individual ones.

I know the current user space code uses u8 as a bitflag; but
maybe we can introduce a new u32 flag bitmask that has all the
flags set for backward compat? I can count about a total of 10.

cheers,
jamal

^ permalink raw reply

* Re: [patch net-next v2 00/10] introduce rocker switch driver with hardware accelerated datapath api - phase 1: bridge fdb offload
From: Jiri Pirko @ 2014-11-10 13:12 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5460AC90.9060607@mojatatu.com>

Mon, Nov 10, 2014 at 01:16:16PM CET, jhs@mojatatu.com wrote:
>On 11/10/14 02:23, Jiri Pirko wrote:
>
>>
>>Yes I looked over their patches. Roopas patche's are about new class of
>>device which, as I commented in the cover letter, I left out for now and
>>can be safely added later on.
>>
>>I went over the Ben's work very carefully as well. The patches are very
>>rough, mostly rtl-chip specific. But again, my patchset is a base on
>>which this patches can be build on. I see no issues in that.
>>
>>>At least please get their sign on - this  is such an important piece of
>>>new work that you should make sure you get consensus.
>>
>>Since I did not use their code now, I only put sign off of Scott.
>>
>
>Your last comment was "i am going to merge the patches" ;->
>At least send an email explaining your plan to people who have worked
>hard to cooperate with you or say it in the cover letter.

Well I had feedback only from Roopa and we discussed it in following
email thread. I never had any feedback from Ben. I only saw you pasting
link to his git.

There's really nothing else to merge at the moment. I would love to went
over patches and merge them into my tree if anyone sends them.

>
>>>Otherwise we are back to square one and everyone is going their way with
>>>their patches;
>>
>>I do think that we are in sync. I do not see any counter ways. As I
>>said, their work can be added on to the base made of this patchset.
>>
>
>Ok, I hope so. I spoke for myself - it is important for this patches
>you get their sign-on in my opinion.
>
>cheers,
>jamal
>

^ permalink raw reply

* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jiri Pirko @ 2014-11-10 13:16 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5460ACC0.3020805@mojatatu.com>

Mon, Nov 10, 2014 at 01:17:04PM CET, jhs@mojatatu.com wrote:
>On 11/10/14 02:43, Jiri Pirko wrote:
>>Mon, Nov 10, 2014 at 04:35:12AM CET, jhs@mojatatu.com wrote:
>
>>I don't see a reason why this would break kabi:
>>
>>-#define MAX_PHYS_PORT_ID_LEN 32
>>+#define MAX_PHYS_ITEM_ID_LEN 32
>>
>
>refer to my response to Dave. Just define MAX_PHYS_PORT_ID_LEN to
>MAX_PHYS_ITEM_ID_LEN so people dont have to change their code
>because a name change.

Jamal, please look at the patch & code. MAX_PHYS_PORT_ID_LEN is in
include/linux/netdevice.h which is not part of user exported api.

>
>cheers,
>jamal

^ permalink raw reply

* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jamal Hadi Salim @ 2014-11-10 13:20 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141110131622.GC4256@nanopsycho.orion>

On 11/10/14 08:16, Jiri Pirko wrote:

> Jamal, please look at the patch & code. MAX_PHYS_PORT_ID_LEN is in
> include/linux/netdevice.h which is not part of user exported api.
>

I got it - the confusing part was rtnetlink.c was looking for it
as if it was expecting user space to send it.

cheers,
jamal

^ permalink raw reply

* Re: [patch net-next v2 06/10] bridge: introduce fdb offloading via switchdev
From: Jiri Pirko @ 2014-11-10 13:47 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5460B3E5.7020502@mojatatu.com>

Mon, Nov 10, 2014 at 01:47:33PM CET, jhs@mojatatu.com wrote:
>On 11/10/14 03:15, Jiri Pirko wrote:
>>Mon, Nov 10, 2014 at 04:47:48AM CET, jhs@mojatatu.com wrote:
>>>On 11/09/14 05:51, Jiri Pirko wrote:
>>>>From: Scott Feldman <sfeldma@gmail.com>
>>>>
>
>>Jamal, I believe we discussed this already.
>
>I cant remember how that ended.
>
>>The thing is that current
>>fdb_add/del does not need vlanid and master/self flags, because it
>>already has that (struct nlattr *tb[]). Here is the whole list of
>>parameters to these functions:
>>         NDA_DST,
>>         NDA_LLADDR,
>>         NDA_CACHEINFO,
>>         NDA_PROBES,
>>         NDA_VLAN,
>>         NDA_PORT,
>>         NDA_VNI,
>>         NDA_IFINDEX,
>>         NDA_MASTER,
>>
>>There are few problems in re-using this. It is netlink based so for calling
>>it from bridge code, we would have to construct netlink message. But
>>that could be probably changed.
>
>Trying to understand.
>
>A netlink message for a bridge to add an fdb is targeted at the
>*bridge port*.
>That message has semantic which says "please add this entry
>to the software bridge and/or offloaded hardware".
>If something is targetted at the bridge port, ->ndo_fdb_add()
>is invoked with an internally chewed structure.
>Why would you have to construct a new netlink message to the driver?

Because now, If you would like to pass one of NDA_DST, NDA_LLADDR,
NDA_CACHEINFO, NDA_PROBES, NDA_VLAN, NDA_PORT, NDA_VNI, NDA_IFINDEX,
NDA_MASTER values via ndo_fdb_add/del to the driver, you have to
construct "struct nlattr *tb[]". Preprocessing this tb into struct might
be suitable for some use-case, for some it may not.


>
>
>>As you can see from the list of parameters, this is no longer about fdb (addr,
>>vlanid) but this has been extended to something else.
>
>I am still missing understanding that part.
>Or maybe are you saying that you dont want to pass netlink
>constructs to the driver?

What I try to say is that the naming ndo_fdb_add/del is not accurate
because it is now used for far more than fdb (addr, vlan). See vxlan
code for example.


>
>>See vxlan code for
>>what this is used for. I believe that fdb_add/del should be renamed to
>>something else, perhaps l2neigh_add/del or something like that.
>>The other problem is that fdb_add/del is currently used by various
>>drivers for different purpose (adding macs to unicast list).
>>
>
>Ok, now a small spark ignited in my brain. You did talk about renaming
>things to neighXXX in one of the exchanges. I think this is a separate
>issue from the question of why you cant refactor ndo_fdb_add/del

It can be probably refactored in a way so it fits our fdb offloading
needs. I'm not really sure we would want it. ndo_fdb_* use-case
is dirrerent from what we introduce with ndo_sw_port_fdb_*. The only
similarity is the "fdb" name which in case of ndo_fdb_* is no longer
correct I believe.


>
>The abuse of using this interface for unicast addresses is probably
>driven by the fact some of the hardware probably offloads vlanid 0 or
>something speacial like 4095 to point to the underlying hardware that
>"this belongs to host cpu".
>I am not a fan of it (and have posted in exchanges with Vlad in the
>past).
>
>cheers,
>jamal

^ permalink raw reply

* Re: [patch net-next v2 06/10] bridge: introduce fdb offloading via switchdev
From: Thomas Graf @ 2014-11-10 13:51 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Jamal Hadi Salim, netdev, davem, nhorman, andy, dborkman,
	ogerlitz, jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher,
	vyasevic, xiyou.wangcong, john.r.fastabend, edumazet, sfeldma,
	f.fainelli, roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl@
In-Reply-To: <20141110081552.GD1850@nanopsycho.orion>

On 11/10/14 at 09:15am, Jiri Pirko wrote:
> There are few problems in re-using this. It is netlink based so for calling
> it from bridge code, we would have to construct netlink message. But
> that could be probably changed.
> As you can see from the list of parameters, this is no longer about fdb (addr,
> vlanid) but this has been extended to something else. See vxlan code for
> what this is used for. I believe that fdb_add/del should be renamed to
> something else, perhaps l2neigh_add/del or something like that.
> The other problem is that fdb_add/del is currently used by various
> drivers for different purpose (adding macs to unicast list).

Can you elaborate a bit on the intended semantic differences between
the existing ndo_fdb_add() and ndo_sw_port_fdb_add()? I'm not sure we
need the sw_ prefix for this specific ndo.

I completely agree that relying on Netlink is wrong because we'll have
in-kernel users of the API but I believe that existing ndo_fdb_add()
implementations in i40e, ixgbe, qlcnic and macvlan could use the new
API you propose.

How about we rename the existing ndo_fdb_add() to ndo_neigh_add() as
you propose and convert vxlan over to it and have all others which don't
even depend on the Netlink attributes being passed in (i40e, ixgbe,
qlcnic, macvlan) use ndo_fdb_add() which would have the behaviour of your
proposed ndo_sw_port_fdb_add()?

^ permalink raw reply

* Re: [patch net-next v2 07/10] bridge: call netdev_sw_port_stp_update when bridge port STP status changes
From: Thomas Graf @ 2014-11-10 14:04 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: Jiri Pirko, netdev, davem, nhorman, andy, dborkman, ogerlitz,
	jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
	roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5460B989.8030404@mojatatu.com>

On 11/10/14 at 08:11am, Jamal Hadi Salim wrote:
> You are unconditionally calling
> netdev_sw_port_stp_update(p->dev, p->state);
> Again issue is policy. Could you make this work the same
> way the fdb_add e.g user intent of whether i want to turn
> a port in hardware and/or software to disabled/learning/etc
> is reflected?

Agreed. Can be added in a next series perhaps?

> btw: does _sw_ stand for switch? why not _hw_ ?
> Could we have one ndo for all flags instead of individual ones.
> 
> I know the current user space code uses u8 as a bitflag; but
> maybe we can introduce a new u32 flag bitmask that has all the
> flags set for backward compat? I can count about a total of 10.

I think we can just extend the size of IFLA_BRPORT_STATE, accept
both a u8 and u32, and return a u32 that that is compatible to
existing u8 readers.

^ permalink raw reply

* Re: [RFC PATCH net-next] net: Convert LIMIT_NETDEBUG to net_dbg_ratelimited
From: Nicolas Dichtel @ 2014-11-10 14:27 UTC (permalink / raw)
  To: Joe Perches, David Miller; +Cc: netdev, linux-kernel, Remi Denis-Courmont
In-Reply-To: <1415560642.23530.43.camel@perches.com>

Le 09/11/2014 20:17, Joe Perches a écrit :
> Use the more common dynamic_debug capable net_dbg_ratelimited
> and remove the LIMIT_NETDEBUG macro.
>
> This may have some negative impact on messages that were
> emitted at KERN_INFO that are not not enabled at all unless
> DEBUG is defined or dynamic_debug is enabled.  Even so,
> these messages are now _not_ emitted by default.
>
> This eliminates the use of the net_msg_warn sysctl
> "/proc/sys/net/core/warnings".
>
> All messages are still ratelimited.
>
> Some KERN_LEVEL uses are changed to KERN_DEBUG.
>
> Miscellanea:
>
> o Update the sysctl documentation
> o Remove the embedded uses of pr_fmt
> o Coalesce format fragments
> o Realign arguments
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>
> Let me know if you want this consolidate patch broken up
> into multiple patches or any of the messages and the
> macro kept.
>
>   Documentation/sysctl/net.txt | 12 ++++++++----
>   include/net/sock.h           |  8 +-------
>   include/net/udplite.h        |  6 +++---
>   net/ipv4/icmp.c              |  8 ++++----
>   net/ipv4/inet_fragment.c     |  2 +-
>   net/ipv4/ip_fragment.c       |  3 +--
>   net/ipv4/tcp_input.c         |  8 ++++----
>   net/ipv4/tcp_timer.c         | 18 ++++++++++--------
>   net/ipv4/udp.c               | 30 +++++++++++++++---------------
>   net/ipv6/addrconf.c          |  6 ++----
>   net/ipv6/ah6.c               |  7 +++----
>   net/ipv6/datagram.c          |  4 ++--
>   net/ipv6/esp6.c              |  4 ++--
>   net/ipv6/exthdrs.c           | 18 +++++++++---------
>   net/ipv6/icmp.c              | 15 +++++++--------
>   net/ipv6/mip6.c              | 11 ++++++-----
>   net/ipv6/netfilter.c         |  2 +-
>   net/ipv6/udp.c               | 31 +++++++++++++------------------
>   net/phonet/af_phonet.c       |  9 +++++----
>   net/phonet/pep-gprs.c        |  3 +--
>   net/phonet/pep.c             | 12 ++++++------
>   21 files changed, 104 insertions(+), 113 deletions(-)
>
> diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
> index 04892b8..46cd03d 100644
> --- a/Documentation/sysctl/net.txt
> +++ b/Documentation/sysctl/net.txt
> @@ -120,10 +120,14 @@ seconds.
>   warnings
>   --------
>
> -This controls console messages from the networking stack that can occur because
> -of problems on the network like duplicate address or bad checksums. Normally,
> -this should be enabled, but if the problem persists the messages can be
> -disabled.
> +This sysctl is now unused.
> +
> +This was used to control console messages from the networking stack that
> +occur because of problems on the network like duplicate address or bad
> +checksums.
> +
> +These messages are now emitted at KERN_DEBUG and can generally be enabled
> +and controlled by the dynamic_debug facility.
>
>   netdev_budget
>   -------------
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 6767d75..db363ad 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2276,13 +2276,7 @@ bool sk_ns_capable(const struct sock *sk,
>   bool sk_capable(const struct sock *sk, int cap);
>   bool sk_net_capable(const struct sock *sk, int cap);
>
> -/*
> - *	Enable debug/info messages
> - */
> -extern int net_msg_warn;
> -#define LIMIT_NETDEBUG(fmt, args...) \
> -	do { if (net_msg_warn && net_ratelimit()) printk(fmt,##args); } while(0)
> -
> +extern int net_msg_warn;	/* Unused, but still a sysctl */
Why not removing this variable from this header and from net/core/utils.c?
Just declaring a static variable in net/core/sysctl_net_core.c should be enough.
Am I missing something?

Nicolas

^ permalink raw reply

* Re: [PATCH] stmmac: split to core library and probe drivers
From: Giuseppe CAVALLARO @ 2014-11-10 14:28 UTC (permalink / raw)
  To: Andy Shevchenko, netdev, Kweh Hock Leong, David S . Miller,
	Vince Bridgers
In-Reply-To: <1415615939-6671-1-git-send-email-andriy.shevchenko@linux.intel.com>

Hi Andy

On 11/10/2014 11:38 AM, Andy Shevchenko wrote:
> Instead of registering the platform and PCI drivers in one module let's move
> necessary bits to where it belongs. During this procedure we convert the module
> registration part to use module_*_driver() macros which makes code simplier.
>
>>From now on the driver consists three parts: core library, PCI, and platform
> drivers.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

patch looks fine and net-next.

Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

> ---
>   drivers/net/ethernet/stmicro/stmmac/Kconfig        |  4 +-
>   drivers/net/ethernet/stmicro/stmmac/Makefile       | 15 +++---
>   drivers/net/ethernet/stmicro/stmmac/stmmac.h       | 61 ----------------------
>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 36 ++-----------
>   drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   |  4 +-
>   .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 13 ++---
>   6 files changed, 25 insertions(+), 108 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 33b85ba..7d3af19 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -14,7 +14,7 @@ config STMMAC_ETH
>   if STMMAC_ETH
>
>   config STMMAC_PLATFORM
> -	bool "STMMAC Platform bus support"
> +	tristate "STMMAC Platform bus support"
>   	depends on STMMAC_ETH
>   	default y
>   	---help---
> @@ -27,7 +27,7 @@ config STMMAC_PLATFORM
>   	  If unsure, say N.
>
>   config STMMAC_PCI
> -	bool "STMMAC PCI bus support"
> +	tristate "STMMAC PCI bus support"
>   	depends on STMMAC_ETH && PCI
>   	---help---
>   	  This is to select the Synopsys DWMAC available on PCI devices,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index 034da70..ac4d562 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -1,9 +1,12 @@
>   obj-$(CONFIG_STMMAC_ETH) += stmmac.o
> -stmmac-$(CONFIG_STMMAC_PCI) += stmmac_pci.o
> -stmmac-$(CONFIG_STMMAC_PLATFORM) +=	stmmac_platform.o dwmac-meson.o \
> -					dwmac-sunxi.o dwmac-sti.o \
> -					dwmac-socfpga.o
>   stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o	\
> -	      chain_mode.o dwmac_lib.o dwmac1000_core.o  dwmac1000_dma.o \
> -	      dwmac100_core.o dwmac100_dma.o enh_desc.o  norm_desc.o \
> +	      chain_mode.o dwmac_lib.o dwmac1000_core.o dwmac1000_dma.o	\
> +	      dwmac100_core.o dwmac100_dma.o enh_desc.o norm_desc.o	\
>   	      mmc_core.o stmmac_hwtstamp.o stmmac_ptp.o $(stmmac-y)
> +
> +obj-$(CONFIG_STMMAC_PLATFORM) += stmmac-platform.o
> +stmmac-platform-objs:= stmmac_platform.o dwmac-meson.o dwmac-sunxi.o	\
> +		       dwmac-sti.o dwmac-socfpga.o
> +
> +obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
> +stmmac-pci-objs:= stmmac_pci.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index bd75ee8..c0a3919 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -134,65 +134,4 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device,
>   void stmmac_disable_eee_mode(struct stmmac_priv *priv);
>   bool stmmac_eee_init(struct stmmac_priv *priv);
>
> -#ifdef CONFIG_STMMAC_PLATFORM
> -extern struct platform_driver stmmac_pltfr_driver;
> -
> -static inline int stmmac_register_platform(void)
> -{
> -	int err;
> -
> -	err = platform_driver_register(&stmmac_pltfr_driver);
> -	if (err)
> -		pr_err("stmmac: failed to register the platform driver\n");
> -
> -	return err;
> -}
> -
> -static inline void stmmac_unregister_platform(void)
> -{
> -	platform_driver_unregister(&stmmac_pltfr_driver);
> -}
> -#else
> -static inline int stmmac_register_platform(void)
> -{
> -	pr_debug("stmmac: do not register the platf driver\n");
> -
> -	return 0;
> -}
> -
> -static inline void stmmac_unregister_platform(void)
> -{
> -}
> -#endif /* CONFIG_STMMAC_PLATFORM */
> -
> -#ifdef CONFIG_STMMAC_PCI
> -extern struct pci_driver stmmac_pci_driver;
> -static inline int stmmac_register_pci(void)
> -{
> -	int err;
> -
> -	err = pci_register_driver(&stmmac_pci_driver);
> -	if (err)
> -		pr_err("stmmac: failed to register the PCI driver\n");
> -
> -	return err;
> -}
> -
> -static inline void stmmac_unregister_pci(void)
> -{
> -	pci_unregister_driver(&stmmac_pci_driver);
> -}
> -#else
> -static inline int stmmac_register_pci(void)
> -{
> -	pr_debug("stmmac: do not register the PCI driver\n");
> -
> -	return 0;
> -}
> -
> -static inline void stmmac_unregister_pci(void)
> -{
> -}
> -#endif /* CONFIG_STMMAC_PCI */
> -
>   #endif /* __STMMAC_H__ */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 53db11b..0f1c146 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2885,6 +2885,7 @@ error_clk_get:
>
>   	return ERR_PTR(ret);
>   }
> +EXPORT_SYMBOL_GPL(stmmac_dvr_probe);
>
>   /**
>    * stmmac_dvr_remove
> @@ -2914,8 +2915,8 @@ int stmmac_dvr_remove(struct net_device *ndev)
>
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(stmmac_dvr_remove);
>
> -#ifdef CONFIG_PM
>   int stmmac_suspend(struct net_device *ndev)
>   {
>   	struct stmmac_priv *priv = netdev_priv(ndev);
> @@ -2957,6 +2958,7 @@ int stmmac_suspend(struct net_device *ndev)
>   	priv->oldduplex = -1;
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(stmmac_suspend);
>
>   int stmmac_resume(struct net_device *ndev)
>   {
> @@ -3003,37 +3005,7 @@ int stmmac_resume(struct net_device *ndev)
>
>   	return 0;
>   }
> -#endif /* CONFIG_PM */
> -
> -/* Driver can be configured w/ and w/ both PCI and Platf drivers
> - * depending on the configuration selected.
> - */
> -static int __init stmmac_init(void)
> -{
> -	int ret;
> -
> -	ret = stmmac_register_platform();
> -	if (ret)
> -		goto err;
> -	ret = stmmac_register_pci();
> -	if (ret)
> -		goto err_pci;
> -	return 0;
> -err_pci:
> -	stmmac_unregister_platform();
> -err:
> -	pr_err("stmmac: driver registration failed\n");
> -	return ret;
> -}
> -
> -static void __exit stmmac_exit(void)
> -{
> -	stmmac_unregister_platform();
> -	stmmac_unregister_pci();
> -}
> -
> -module_init(stmmac_init);
> -module_exit(stmmac_exit);
> +EXPORT_SYMBOL_GPL(stmmac_resume);
>
>   #ifndef MODULE
>   static int __init stmmac_cmdline_opt(char *str)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 5084699..77a6d68 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -158,7 +158,7 @@ static const struct pci_device_id stmmac_id_table[] = {
>
>   MODULE_DEVICE_TABLE(pci, stmmac_id_table);
>
> -struct pci_driver stmmac_pci_driver = {
> +static struct pci_driver stmmac_pci_driver = {
>   	.name = STMMAC_RESOURCE_NAME,
>   	.id_table = stmmac_id_table,
>   	.probe = stmmac_pci_probe,
> @@ -168,6 +168,8 @@ struct pci_driver stmmac_pci_driver = {
>   	},
>   };
>
> +module_pci_driver(stmmac_pci_driver);
> +
>   MODULE_DESCRIPTION("STMMAC 10/100/1000 Ethernet PCI driver");
>   MODULE_AUTHOR("Rayagond Kokatanur <rayagond.kokatanur@vayavyalabs.com>");
>   MODULE_AUTHOR("Giuseppe Cavallaro <peppe.cavallaro@st.com>");
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 9f18401..e22a960 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -362,7 +362,7 @@ static int stmmac_pltfr_remove(struct platform_device *pdev)
>   	return ret;
>   }
>
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
>   static int stmmac_pltfr_suspend(struct device *dev)
>   {
>   	int ret;
> @@ -388,13 +388,12 @@ static int stmmac_pltfr_resume(struct device *dev)
>
>   	return stmmac_resume(ndev);
>   }
> -
> -#endif /* CONFIG_PM */
> +#endif /* CONFIG_PM_SLEEP */
>
>   static SIMPLE_DEV_PM_OPS(stmmac_pltfr_pm_ops,
> -			stmmac_pltfr_suspend, stmmac_pltfr_resume);
> +			 stmmac_pltfr_suspend, stmmac_pltfr_resume);
>
> -struct platform_driver stmmac_pltfr_driver = {
> +static struct platform_driver stmmac_pltfr_driver = {
>   	.probe = stmmac_pltfr_probe,
>   	.remove = stmmac_pltfr_remove,
>   	.driver = {
> @@ -402,9 +401,11 @@ struct platform_driver stmmac_pltfr_driver = {
>   		   .owner = THIS_MODULE,
>   		   .pm = &stmmac_pltfr_pm_ops,
>   		   .of_match_table = of_match_ptr(stmmac_dt_ids),
> -		   },
> +	},
>   };
>
> +module_platform_driver(stmmac_pltfr_driver);
> +
>   MODULE_DESCRIPTION("STMMAC 10/100/1000 Ethernet PLATFORM driver");
>   MODULE_AUTHOR("Giuseppe Cavallaro <peppe.cavallaro@st.com>");
>   MODULE_LICENSE("GPL");
>

^ permalink raw reply

* Re: BUG in xennet_make_frags with paged skb data
From: Seth Forshee @ 2014-11-10 14:35 UTC (permalink / raw)
  To: David Vrabel
  Cc: netdev, David S. Miller, Eric Dumazet, Konrad Rzeszutek Wilk,
	Boris Ostrovsky, Stefan Bader, Jay Vosburgh, linux-kernel,
	xen-devel, seth.forshee
In-Reply-To: <545CA27F.4070400@citrix.com>

On Fri, Nov 07, 2014 at 10:44:15AM +0000, David Vrabel wrote:
> On 06/11/14 21:49, Seth Forshee wrote:
> > We've had several reports of hitting the following BUG_ON in
> > xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
> > results of testing with 3.17):
> > 
> >         /* Grant backend access to each skb fragment page. */
> >         for (i = 0; i < frags; i++) {
> >                 skb_frag_t *frag = skb_shinfo(skb)->frags + i;
> >                 struct page *page = skb_frag_page(frag);
> > 
> >                 len = skb_frag_size(frag);
> >                 offset = frag->page_offset;
> > 
> >                 /* Data must not cross a page boundary. */
> >                 BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
> > 
> > When this happens the page in question is a "middle" page in a compound
> > page (i.e. it's a tail page but not the last tail page), and the data is
> > fully contained within the compound page. The data does however cross
> > the hardware page boundary, and since compound_order evaluates to 0 for
> > tail pages the check fails.
> > 
> > In going over this I've been unable to determine whether the BUG_ON in
> > xennet_make_frags is incorrect or the paged skb data is wrong. I can't
> > find that it's documented anywhere, and the networking code itself is a
> > bit ambiguous when it comes to compound pages. On the one hand
> > __skb_fill_page_desc specifically handles adding tail pages as paged
> > data, but on the other hand skb_copy_bits kmaps frag->page.p which could
> > fail with data that extends into another page.
> 
> netfront will safely handle this case so you can remove this BUG_ON()
> (and the one later on).  But it would be better to find out were these
> funny-looking skbs are coming from and (if necessary) fixing the bug there.

There still seems to be disagreement about whether the "funny" skb is
valid though - you imply it isn't, but Eric says it is. I've been trying
to track down where these skbs originate, and so far I've determined
that they come from a socket spliced to a pipe spliced to a socket. It
looks like the particular page/offset/len tuple originates at least as
far back as the first socket, as the tuple is simply copied from an skb
into the pipe and from the pipe into the final skb.

Anyway, it looks like at minimum it should be safe to change the first
BUG_ON to assert that the data is fully within the compound page as
Stefan suggested, then remove the second BUG_ON entirely. This is the
path I plan to pursue unless someone objects.

Thanks,
Seth

^ permalink raw reply

* Re: BUG in xennet_make_frags with paged skb data
From: David Vrabel @ 2014-11-10 14:41 UTC (permalink / raw)
  To: netdev, David S. Miller, Eric Dumazet, Konrad Rzeszutek Wilk,
	Boris Ostrovsky, Stefan Bader, Jay Vosburgh, linux-kernel,
	xen-devel
In-Reply-To: <20141110143517.GA74005@ubuntu-hedt>

On 10/11/14 14:35, Seth Forshee wrote:
> On Fri, Nov 07, 2014 at 10:44:15AM +0000, David Vrabel wrote:
>> On 06/11/14 21:49, Seth Forshee wrote:
>>> We've had several reports of hitting the following BUG_ON in
>>> xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
>>> results of testing with 3.17):
>>>
>>>         /* Grant backend access to each skb fragment page. */
>>>         for (i = 0; i < frags; i++) {
>>>                 skb_frag_t *frag = skb_shinfo(skb)->frags + i;
>>>                 struct page *page = skb_frag_page(frag);
>>>
>>>                 len = skb_frag_size(frag);
>>>                 offset = frag->page_offset;
>>>
>>>                 /* Data must not cross a page boundary. */
>>>                 BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
>>>
>>> When this happens the page in question is a "middle" page in a compound
>>> page (i.e. it's a tail page but not the last tail page), and the data is
>>> fully contained within the compound page. The data does however cross
>>> the hardware page boundary, and since compound_order evaluates to 0 for
>>> tail pages the check fails.
>>>
>>> In going over this I've been unable to determine whether the BUG_ON in
>>> xennet_make_frags is incorrect or the paged skb data is wrong. I can't
>>> find that it's documented anywhere, and the networking code itself is a
>>> bit ambiguous when it comes to compound pages. On the one hand
>>> __skb_fill_page_desc specifically handles adding tail pages as paged
>>> data, but on the other hand skb_copy_bits kmaps frag->page.p which could
>>> fail with data that extends into another page.
>>
>> netfront will safely handle this case so you can remove this BUG_ON()
>> (and the one later on).  But it would be better to find out were these
>> funny-looking skbs are coming from and (if necessary) fixing the bug there.
> 
> There still seems to be disagreement about whether the "funny" skb is
> valid though - you imply it isn't, but Eric says it is. I've been trying
> to track down where these skbs originate, and so far I've determined
> that they come from a socket spliced to a pipe spliced to a socket. It
> looks like the particular page/offset/len tuple originates at least as
> far back as the first socket, as the tuple is simply copied from an skb
> into the pipe and from the pipe into the final skb.

Apologies for the lack of clarity.  I meant either: a) fix the producer
if these skbs are invalid; or b) remove the BUG_ON()s.  Since Eric says
these are actually valid skbs, please do option (b).

i.e., remove both BUG_ON()s.

David

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox