Netdev List
 help / color / mirror / Atom feed
* Re: [pull request][net 0/6] Mellanox, mlx5 fixes 2019-01-25
From: David Miller @ 2019-01-27 19:39 UTC (permalink / raw)
  To: saeedm; +Cc: netdev
In-Reply-To: <20190125200140.6222-1-saeedm@mellanox.com>

From: Saeed Mahameed <saeedm@mellanox.com>
Date: Fri, 25 Jan 2019 12:01:34 -0800

> This series introduces some fixes to mlx5 driver.
> For more information please see tag log below.
> 
> Please pull and let me know if there is any problem.

Pulled.

> For -stable v4.13
> ('net/mlx5e: Allow MAC invalidation while spoofchk is ON')
> 
> For -stable v4.18
> ('Revert "net/mlx5e: E-Switch, Initialize eswitch only if eswitch manager"')

Queued up.

^ permalink raw reply

* Re: Error in the hso driver
From: Yavuz, Tuba @ 2019-01-27 19:49 UTC (permalink / raw)
  To: Greg KH; +Cc: netdev@vger.kernel.org, davem@davemloft.net
In-Reply-To: <20190127193503.GC25051@kroah.com>

Here is a proposed patch:

--- drivers/net/usb/hso.c.orig	2019-01-27 14:45:58.232683119 -0500
+++ drivers/net/usb/hso.c	2019-01-27 14:47:43.592683629 -0500
@@ -2377,7 +2377,7 @@ static void hso_free_net_device(struct h
 
 	remove_net_device(hso_net->parent);
 
-	if (hso_net->net)
+	if (hso_net->net && hso_net->net->reg_state == NETREG_REGISTERED)
 		unregister_netdev(hso_net->net);
 
 	/* start freeing */



Tuba Yavuz, Ph.D.
Assistant Professor
Electrical and Computer Engineering Department
University of Florida
Gainesville, FL 32611
Webpage: http://www.tuba.ece.ufl.edu/
Email: tuba@ece.ufl.edu
Phone: (352) 846 0202

________________________________________
From: Greg KH <greg@kroah.com>
Sent: Sunday, January 27, 2019 2:35 PM
To: Yavuz, Tuba
Cc: netdev@vger.kernel.org; davem@davemloft.net
Subject: Re: Error in the hso driver

On Sun, Jan 27, 2019 at 06:38:41PM +0000, Yavuz, Tuba wrote:
> Hello,
>
>
> On an error path inside the hso_create_net_device function of the hso driver (drivers/net/usb/hso), hso_free_net_device gets called. This causes a negative reference count in the net device if register_netdev has not been called yet as hso_free_net_device calls unregister_netdev regardless. I think the driver should distinguish these cases and call unregister_netdev only if register_netdev has been called.
>
>
> Detected on v4.14-rc2 but I believe the problem still exists in the newer versions.

Can you send a patch to solve this issue?  That's the best way to handle
it as you get the proper credit and we can understand exactly what you
are trying to show here.

thanks,

greg k-h

^ permalink raw reply

* Re: Error in the hso driver
From: Greg KH @ 2019-01-27 19:53 UTC (permalink / raw)
  To: Yavuz, Tuba; +Cc: netdev@vger.kernel.org, davem@davemloft.net
In-Reply-To: <1548618592071.43193@ece.ufl.edu>

On Sun, Jan 27, 2019 at 07:49:52PM +0000, Yavuz, Tuba wrote:
> Here is a proposed patch:
> 
> --- drivers/net/usb/hso.c.orig	2019-01-27 14:45:58.232683119 -0500
> +++ drivers/net/usb/hso.c	2019-01-27 14:47:43.592683629 -0500
> @@ -2377,7 +2377,7 @@ static void hso_free_net_device(struct h
>  
>  	remove_net_device(hso_net->parent);
>  
> -	if (hso_net->net)
> +	if (hso_net->net && hso_net->net->reg_state == NETREG_REGISTERED)
>  		unregister_netdev(hso_net->net);
>  
>  	/* start freeing */
> 
> 

Can you resend it in a format that it can be applied?
Documentation/SubmittingPatches should have all of the needed details.

thanks,

greg k-h

^ permalink raw reply

* [PATCH] : net : hso : unregister_netdev only if it has been registered
From: Yavuz, Tuba @ 2019-01-27 20:14 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Greg KH, Yavuz, Tuba

     
On an error path inside the hso_create_net_device function of the hso 
driver, hso_free_net_device gets called. This causes potentially a 
negative reference count in the net device if register_netdev has not 
been called yet as hso_free_net_device calls unregister_netdev 
regardless. I think the driver should distinguish these cases and call 
unregister_netdev only if register_netdev has been called.


Reported-by: Tuba Yavuz <tuba@ece.ufl.edu>
Signed-off-by: Tuba Yavuz <tuba@ece.ufl.edu>
---


--- drivers/net/usb/hso.c.orig	2019-01-27 14:45:58.232683119 -0500
+++ drivers/net/usb/hso.c	2019-01-27 14:47:43.592683629 -0500
@@ -2377,7 +2377,7 @@ static void hso_free_net_device(struct h
 
 	remove_net_device(hso_net->parent);
 
-	if (hso_net->net)
+	if (hso_net->net && hso_net->net->reg_state == NETREG_REGISTERED)
 		unregister_netdev(hso_net->net);
 
 	/* start freeing */​

^ permalink raw reply

* Re: Error in the hso driver
From: Andrew Lunn @ 2019-01-27 20:19 UTC (permalink / raw)
  To: Yavuz, Tuba; +Cc: Greg KH, netdev@vger.kernel.org, davem@davemloft.net
In-Reply-To: <1548618592071.43193@ece.ufl.edu>

On Sun, Jan 27, 2019 at 07:49:52PM +0000, Yavuz, Tuba wrote:
> Here is a proposed patch:
> 
> --- drivers/net/usb/hso.c.orig	2019-01-27 14:45:58.232683119 -0500
> +++ drivers/net/usb/hso.c	2019-01-27 14:47:43.592683629 -0500
> @@ -2377,7 +2377,7 @@ static void hso_free_net_device(struct h
>  
>  	remove_net_device(hso_net->parent);
>  
> -	if (hso_net->net)
> +	if (hso_net->net && hso_net->net->reg_state == NETREG_REGISTERED)
>  		unregister_netdev(hso_net->net);
>  
>  	/* start freeing */

Hi Tuba

Using the free function from the probe often has problems. It is
better to put the cleanup at the end of the probe, and use a
collection of goto's and labels.

	   Andrew

^ permalink raw reply

* KASAN: use-after-free Read in br_mdb_ip_get
From: syzbot @ 2019-01-27 20:26 UTC (permalink / raw)
  To: bridge, davem, linux-kernel, netdev, nikolay, roopa,
	syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    ba6069759381 Merge tag 'mmc-v5.0-rc2' of git://git.kernel...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17b342c4c00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=505743eba4e4f68
dashboard link: https://syzkaller.appspot.com/bug?extid=bc5ab0af2dbf3b0ae897
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+bc5ab0af2dbf3b0ae897@syzkaller.appspotmail.com

bridge0: port 2(bridge_slave_1) entered blocking state
bridge0: port 2(bridge_slave_1) entered forwarding state
bridge0: port 1(bridge_slave_0) entered blocking state
bridge0: port 1(bridge_slave_0) entered forwarding state
==================================================================
BUG: KASAN: use-after-free in memcmp+0xb3/0xc0 lib/string.c:862
Read of size 1 at addr ffff88809f1efe70 by task syz-executor1/8111

CPU: 0 PID: 8111 Comm: syz-executor1 Not tainted 5.0.0-rc3+ #43
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  <IRQ>
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1db/0x2d0 lib/dump_stack.c:113
  print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187
IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
  kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
8021q: adding VLAN 0 to HW filter on device bond0
  __asan_report_load1_noabort+0x14/0x20 mm/kasan/generic_report.c:132
  memcmp+0xb3/0xc0 lib/string.c:862
  memcmp include/linux/string.h:393 [inline]
  rhashtable_compare include/linux/rhashtable.h:473 [inline]
  __rhashtable_lookup include/linux/rhashtable.h:498 [inline]
  rhashtable_lookup include/linux/rhashtable.h:534 [inline]
  br_mdb_ip_get+0x694/0xe30 net/bridge/br_multicast.c:97
IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
  br_multicast_new_group+0x77/0x200 net/bridge/br_multicast.c:467
IPv6: ADDRCONF(NETDEV_UP): team0: link is not ready
8021q: adding VLAN 0 to HW filter on device team0
  br_multicast_add_group+0x4ce/0x7d0 net/bridge/br_multicast.c:552
  br_ip6_multicast_add_group net/bridge/br_multicast.c:626 [inline]
  br_ip6_multicast_mld2_report net/bridge/br_multicast.c:1043 [inline]
  br_multicast_ipv6_rcv net/bridge/br_multicast.c:1667 [inline]
  br_multicast_rcv+0x24aa/0x4270 net/bridge/br_multicast.c:1705
IPv6: ADDRCONF(NETDEV_UP): hsr0: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): hsr0: link becomes ready
IPv6: ADDRCONF(NETDEV_UP): vxcan1: link is not ready
  br_dev_xmit+0x7f4/0x1780 net/bridge/br_device.c:93
8021q: adding VLAN 0 to HW filter on device batadv0
  __netdev_start_xmit include/linux/netdevice.h:4382 [inline]
  netdev_start_xmit include/linux/netdevice.h:4391 [inline]
  xmit_one net/core/dev.c:3278 [inline]
  dev_hard_start_xmit+0x261/0xc70 net/core/dev.c:3294
  __dev_queue_xmit+0x2f8a/0x3a60 net/core/dev.c:3864
  dev_queue_xmit+0x18/0x20 net/core/dev.c:3897
  neigh_hh_output include/net/neighbour.h:498 [inline]
  neigh_output include/net/neighbour.h:506 [inline]
  ip6_finish_output2+0x141a/0x28e0 net/ipv6/ip6_output.c:120
  ip6_finish_output+0x577/0xc30 net/ipv6/ip6_output.c:154
  NF_HOOK_COND include/linux/netfilter.h:278 [inline]
  ip6_output+0x23c/0xa00 net/ipv6/ip6_output.c:171
  dst_output include/net/dst.h:444 [inline]
  NF_HOOK include/linux/netfilter.h:289 [inline]
  NF_HOOK include/linux/netfilter.h:283 [inline]
  mld_sendpack+0xa44/0xfd0 net/ipv6/mcast.c:1683
  mld_send_cr net/ipv6/mcast.c:1979 [inline]
  mld_ifc_timer_expire+0x449/0x8a0 net/ipv6/mcast.c:2478
  call_timer_fn+0x254/0x900 kernel/time/timer.c:1325
  expire_timers kernel/time/timer.c:1362 [inline]
  __run_timers+0x6fc/0xd50 kernel/time/timer.c:1681
  run_timer_softirq+0x52/0xb0 kernel/time/timer.c:1694
  __do_softirq+0x30b/0xb11 kernel/softirq.c:292
  invoke_softirq kernel/softirq.c:373 [inline]
  irq_exit+0x180/0x1d0 kernel/softirq.c:413
  exiting_irq arch/x86/include/asm/apic.h:536 [inline]
  smp_apic_timer_interrupt+0x1b7/0x760 arch/x86/kernel/apic/apic.c:1062
  apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
  </IRQ>
RIP: 0010:kasan_check_read+0x0/0x20 mm/kasan/common.c:99
Code: ef e9 14 eb ff ff 48 8b 73 58 89 c2 48 c7 c7 f0 c2 3c 89 f7 da e8 54  
69 a2 ff e9 c4 f5 ff ff 90 90 90 90 90 90 90 90 90 90 90 <55> 89 f6 31 d2  
48 89 e5 48 8b 4d 08 e8 ff 23 00 00 5d c3 0f 1f 00
IPVS: ftp: loaded support on port[0] = 21
RSP: 0018:ffff88809e937428 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000000 RBX: ffffffff8a318400 RCX: ffffffff8162f642
RDX: 0000000000000001 RSI: 0000000000000008 RDI: ffffffff8a318400
RBP: ffff88809e937530 R08: 1ffffffff1463080 R09: fffffbfff1463081
R10: fffffbfff1463080 R11: ffffffff8a318407 R12: ffff88809ac4c600
R13: ffff88809e937508 R14: dffffc0000000000 R15: ffffed1013d26e91
  mutex_optimistic_spin kernel/locking/mutex.c:646 [inline]
  __mutex_lock_common kernel/locking/mutex.c:928 [inline]
  __mutex_lock+0x377/0x1670 kernel/locking/mutex.c:1072
IPVS: ftp: loaded support on port[0] = 21
  mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
  rtnl_lock net/core/rtnetlink.c:77 [inline]
  rtnetlink_rcv_msg+0x425/0xc30 net/core/rtnetlink.c:5127
  netlink_rcv_skb+0x17d/0x410 net/netlink/af_netlink.c:2477
  rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
  netlink_unicast+0x574/0x770 net/netlink/af_netlink.c:1336
  netlink_sendmsg+0xa05/0xf90 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg+0xdd/0x130 net/socket.c:631
  __sys_sendto+0x387/0x5f0 net/socket.c:1788
  __do_sys_sendto net/socket.c:1800 [inline]
  __se_sys_sendto net/socket.c:1796 [inline]
  __x64_sys_sendto+0xe1/0x1a0 net/socket.c:1796
  do_syscall_64+0x1a3/0x800 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x411fc3
Code: ff 0f 83 40 18 00 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00  
83 3d cd 42 64 00 00 75 17 49 89 ca b8 2c 00 00 00 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 11 18 00 00 c3 48 83 ec 08 e8 87 fa ff ff
RSP: 002b:0000000000a4fb28 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000411fc3
RDX: 000000000000006c RSI: 0000000000a50070 RDI: 0000000000000003
RBP: 0000000000000003 R08: 0000000000a4fb30 R09: 000000000000000c
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000a4fef0
R13: 0000000000a4fbb8 R14: 0000000000a4fc80 R15: 00000000004bcf8a

Allocated by task 8111:
  save_stack+0x45/0xd0 mm/kasan/common.c:73
  set_track mm/kasan/common.c:85 [inline]
  __kasan_kmalloc mm/kasan/common.c:496 [inline]
  __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:469
  kasan_kmalloc+0x9/0x10 mm/kasan/common.c:504
  kmem_cache_alloc_trace+0x151/0x760 mm/slab.c:3609
  kmalloc include/linux/slab.h:545 [inline]
  kzalloc include/linux/slab.h:740 [inline]
  br_multicast_new_group.part.0+0xdc/0x1a40 net/bridge/br_multicast.c:476
  br_multicast_new_group+0x19d/0x200 net/bridge/br_multicast.c:471
  br_multicast_add_group+0x4ce/0x7d0 net/bridge/br_multicast.c:552
  br_ip6_multicast_add_group net/bridge/br_multicast.c:626 [inline]
  br_ip6_multicast_mld2_report net/bridge/br_multicast.c:1043 [inline]
  br_multicast_ipv6_rcv net/bridge/br_multicast.c:1667 [inline]
  br_multicast_rcv+0x24aa/0x4270 net/bridge/br_multicast.c:1705
  br_dev_xmit+0x7f4/0x1780 net/bridge/br_device.c:93
  __netdev_start_xmit include/linux/netdevice.h:4382 [inline]
  netdev_start_xmit include/linux/netdevice.h:4391 [inline]
  xmit_one net/core/dev.c:3278 [inline]
  dev_hard_start_xmit+0x261/0xc70 net/core/dev.c:3294
  __dev_queue_xmit+0x2f8a/0x3a60 net/core/dev.c:3864
  dev_queue_xmit+0x18/0x20 net/core/dev.c:3897
  neigh_resolve_output net/core/neighbour.c:1476 [inline]
  neigh_resolve_output+0x6a0/0xb30 net/core/neighbour.c:1456
  neigh_output include/net/neighbour.h:508 [inline]
  ip6_finish_output2+0xc56/0x28e0 net/ipv6/ip6_output.c:120
  ip6_finish_output+0x577/0xc30 net/ipv6/ip6_output.c:154
  NF_HOOK_COND include/linux/netfilter.h:278 [inline]
  ip6_output+0x23c/0xa00 net/ipv6/ip6_output.c:171
  dst_output include/net/dst.h:444 [inline]
  NF_HOOK include/linux/netfilter.h:289 [inline]
  NF_HOOK include/linux/netfilter.h:283 [inline]
  mld_sendpack+0xa44/0xfd0 net/ipv6/mcast.c:1683
  mld_send_cr net/ipv6/mcast.c:1979 [inline]
  mld_ifc_timer_expire+0x449/0x8a0 net/ipv6/mcast.c:2478
  call_timer_fn+0x254/0x900 kernel/time/timer.c:1325
  expire_timers kernel/time/timer.c:1362 [inline]
  __run_timers+0x6fc/0xd50 kernel/time/timer.c:1681
  run_timer_softirq+0x52/0xb0 kernel/time/timer.c:1694
  __do_softirq+0x30b/0xb11 kernel/softirq.c:292

Freed by task 8111:
  save_stack+0x45/0xd0 mm/kasan/common.c:73
  set_track mm/kasan/common.c:85 [inline]
  __kasan_slab_free+0x102/0x150 mm/kasan/common.c:458
  kasan_slab_free+0xe/0x10 mm/kasan/common.c:466
  __cache_free mm/slab.c:3487 [inline]
  kfree+0xcf/0x230 mm/slab.c:3806
  br_multicast_new_group.part.0+0x1489/0x1a40 net/bridge/br_multicast.c:486
  br_multicast_new_group+0x19d/0x200 net/bridge/br_multicast.c:471
  br_multicast_add_group+0x4ce/0x7d0 net/bridge/br_multicast.c:552
  br_ip6_multicast_add_group net/bridge/br_multicast.c:626 [inline]
  br_ip6_multicast_mld2_report net/bridge/br_multicast.c:1043 [inline]
  br_multicast_ipv6_rcv net/bridge/br_multicast.c:1667 [inline]
  br_multicast_rcv+0x24aa/0x4270 net/bridge/br_multicast.c:1705
  br_dev_xmit+0x7f4/0x1780 net/bridge/br_device.c:93
  __netdev_start_xmit include/linux/netdevice.h:4382 [inline]
  netdev_start_xmit include/linux/netdevice.h:4391 [inline]
  xmit_one net/core/dev.c:3278 [inline]
  dev_hard_start_xmit+0x261/0xc70 net/core/dev.c:3294
  __dev_queue_xmit+0x2f8a/0x3a60 net/core/dev.c:3864
  dev_queue_xmit+0x18/0x20 net/core/dev.c:3897
  neigh_resolve_output net/core/neighbour.c:1476 [inline]
  neigh_resolve_output+0x6a0/0xb30 net/core/neighbour.c:1456
  neigh_output include/net/neighbour.h:508 [inline]
  ip6_finish_output2+0xc56/0x28e0 net/ipv6/ip6_output.c:120
  ip6_finish_output+0x577/0xc30 net/ipv6/ip6_output.c:154
  NF_HOOK_COND include/linux/netfilter.h:278 [inline]
  ip6_output+0x23c/0xa00 net/ipv6/ip6_output.c:171
  dst_output include/net/dst.h:444 [inline]
  NF_HOOK include/linux/netfilter.h:289 [inline]
  NF_HOOK include/linux/netfilter.h:283 [inline]
  mld_sendpack+0xa44/0xfd0 net/ipv6/mcast.c:1683
  mld_send_cr net/ipv6/mcast.c:1979 [inline]
  mld_ifc_timer_expire+0x449/0x8a0 net/ipv6/mcast.c:2478
  call_timer_fn+0x254/0x900 kernel/time/timer.c:1325
  expire_timers kernel/time/timer.c:1362 [inline]
  __run_timers+0x6fc/0xd50 kernel/time/timer.c:1681
  run_timer_softirq+0x52/0xb0 kernel/time/timer.c:1694
  __do_softirq+0x30b/0xb11 kernel/softirq.c:292

The buggy address belongs to the object at ffff88809f1efe00
  which belongs to the cache kmalloc-192 of size 192
The buggy address is located 112 bytes inside of
  192-byte region [ffff88809f1efe00, ffff88809f1efec0)
The buggy address belongs to the page:
page:ffffea00027c7bc0 count:1 mapcount:0 mapping:ffff88812c3f0040 index:0x0
flags: 0x1fffc0000000200(slab)
raw: 01fffc0000000200 ffffea00027df108 ffffea00027c7c48 ffff88812c3f0040
raw: 0000000000000000 ffff88809f1ef000 0000000100000010 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff88809f1efd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff88809f1efd80: 00 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff88809f1efe00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                              ^
  ffff88809f1efe80: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
  ffff88809f1eff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.

^ permalink raw reply

* Re: Error in the hso driver
From: Yavuz, Tuba @ 2019-01-27 20:30 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Greg KH, netdev@vger.kernel.org, davem@davemloft.net
In-Reply-To: <20190127201924.GA32046@lunn.ch>

I agree and it would be much easier to track cleanup operations. I think this is happening because probe function calls hso_create_net_device, which performs the cleaning. The driver may need some revision to achieve cleanup inside the probe,

Best,

Tuba Yavuz, Ph.D.
Assistant Professor
Electrical and Computer Engineering Department
University of Florida
Gainesville, FL 32611
Webpage: http://www.tuba.ece.ufl.edu/
Email: tuba@ece.ufl.edu
Phone: (352) 846 0202

________________________________________
From: Andrew Lunn <andrew@lunn.ch>
Sent: Sunday, January 27, 2019 3:19 PM
To: Yavuz, Tuba
Cc: Greg KH; netdev@vger.kernel.org; davem@davemloft.net
Subject: Re: Error in the hso driver

On Sun, Jan 27, 2019 at 07:49:52PM +0000, Yavuz, Tuba wrote:
> Here is a proposed patch:
>
> --- drivers/net/usb/hso.c.orig        2019-01-27 14:45:58.232683119 -0500
> +++ drivers/net/usb/hso.c     2019-01-27 14:47:43.592683629 -0500
> @@ -2377,7 +2377,7 @@ static void hso_free_net_device(struct h
>
>       remove_net_device(hso_net->parent);
>
> -     if (hso_net->net)
> +     if (hso_net->net && hso_net->net->reg_state == NETREG_REGISTERED)
>               unregister_netdev(hso_net->net);
>
>       /* start freeing */

Hi Tuba

Using the free function from the probe often has problems. It is
better to put the cleanup at the end of the probe, and use a
collection of goto's and labels.

           Andrew

^ permalink raw reply

* Re: WoL broken in r8169.c since kernel 4.19
From: Marc Haber @ 2019-01-27 20:55 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: netdev@vger.kernel.org
In-Reply-To: <6ecafe07-3278-a9c4-d76d-971c0df26ce8@gmail.com>

On Sat, Jan 26, 2019 at 08:22:33PM +0100, Heiner Kallweit wrote:
> You could go with this range:
> from 4ff364662814 ("r8169: replace get_protocol with vlan_get_protocol")
> to 649f0837a8cc ("r8169: fix broken Wake-on-LAN from S5 (poweroff)")

I am quite inexperienced with bisecting and am therefore giving a better
account of my actions here.

[2/4994]mh@fan:~/linux/git/linux ((v4.18.16) %) $ git pull
remote: Counting objects: 786, done.
remote: Compressing objects: 100% (197/197), done.
remote: Total 786 (delta 659), reused 697 (delta 589)
Receiving objects: 100% (786/786), 146.00 KiB | 1.20 MiB/s, done.
Resolving deltas: 100% (659/659), completed with 237 local objects.
From git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux
   ba6069759381..1fc7f56db7a7  master     -> origin/master
You are not currently on a branch.
1 [3/4995]mh@fan:~/linux/git/linux ((v4.18.16) %) $ git checkout 4ff364662814
Checking out files: 100% (13994/13994), done.
Previous HEAD position was 6b3252287aa2 Linux 4.18.16
HEAD is now at 4ff364662814 r8169: replace get_protocol with vlan_get_protocol
[4/4996]mh@fan:~/linux/git/linux ((4ff364662814...) %) $ git bisect start drivers/net/ethernet/realtek/r8169.c
[5/4997]mh@fan:~/linux/git/linux ((4ff364662814...) %|BISECTING) $ 
(copy 4.20 config, make oldconfig, say no to everything, make deb-pkg, install and try)
this boots and wakes up on magic packet from suspend to ram
[2/4994]mh@fan:~/linux/git/linux ((4ff364662814...) *%|BISECTING) $ git bisect good
[3/4995]mh@fan:~/linux/git/linux ((4ff364662814...) *%|BISECTING) $ git checkout 649f0837a8cc
Checking out files: 100% (21794/21794), done.
M       scripts/package/Makefile
Previous HEAD position was 4ff364662814 r8169: replace get_protocol with vlan_get_protocol
HEAD is now at 649f0837a8cc r8169: fix broken Wake-on-LAN from S5 (poweroff)
[4/4996]mh@fan:~/linux/git/linux ((649f0837a8cc...) *%|BISECTING) $ 
(copy 4.20 config, make oldconfig, say no to everything, make deb-pkg, install and try)
this boots and does not wake up from StR on magic packet, after 
waking up with any key on the keyboard I had an amdgpu issue on the console
and a dead console, but was able to ssh in and reboot.

[2/4994]mh@fan:~/linux/git/linux ((649f0837a8cc...) *%|BISECTING) $ git
bisect bad
Bisecting: 33 revisions left to test after this (roughly 5 steps)
[098b01ad9837b4d4d0022f407300f069a999e55a] r8169: don't include asm
headers directly
[3/4995]mh@fan:~/linux/git/linux ((098b01ad9837...) *%|BISECTING) $ 

I think I am on the right way here, am I?

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Leimen, Germany    |  lose things."    Winona Ryder | Fon: *49 6224 1600402
Nordisch by Nature |  How to make an American Quilt | Fax: *49 6224 1600421

^ permalink raw reply

* Re: [PATCH bpf-next v2 0/3] selftests: bpf: break up test_verifier
From: Alexei Starovoitov @ 2019-01-27 20:56 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Daniel Borkmann, oss-drivers, Network Development
In-Reply-To: <20190127103328.266e363e@cakuba.hsd1.ca.comcast.net>

On Sun, Jan 27, 2019 at 10:33 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> Looks like v2 just applies cleanly again, should I repost?

v2 set:
https://patchwork.ozlabs.org/series/88320/mbox/
doesn't apply cleanly.
Pls report.

^ permalink raw reply

* Re: WoL broken in r8169.c since kernel 4.19
From: Heiner Kallweit @ 2019-01-27 21:09 UTC (permalink / raw)
  To: Marc Haber; +Cc: netdev@vger.kernel.org
In-Reply-To: <20190127205555.GF27062@torres.zugschlus.de>

On 27.01.2019 21:55, Marc Haber wrote:
> On Sat, Jan 26, 2019 at 08:22:33PM +0100, Heiner Kallweit wrote:
>> You could go with this range:
>> from 4ff364662814 ("r8169: replace get_protocol with vlan_get_protocol")
>> to 649f0837a8cc ("r8169: fix broken Wake-on-LAN from S5 (poweroff)")
> 
> I am quite inexperienced with bisecting and am therefore giving a better
> account of my actions here.
> 
> [2/4994]mh@fan:~/linux/git/linux ((v4.18.16) %) $ git pull
> remote: Counting objects: 786, done.
> remote: Compressing objects: 100% (197/197), done.
> remote: Total 786 (delta 659), reused 697 (delta 589)
> Receiving objects: 100% (786/786), 146.00 KiB | 1.20 MiB/s, done.
> Resolving deltas: 100% (659/659), completed with 237 local objects.
>>From git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux
>    ba6069759381..1fc7f56db7a7  master     -> origin/master
> You are not currently on a branch.
> 1 [3/4995]mh@fan:~/linux/git/linux ((v4.18.16) %) $ git checkout 4ff364662814
> Checking out files: 100% (13994/13994), done.
> Previous HEAD position was 6b3252287aa2 Linux 4.18.16
> HEAD is now at 4ff364662814 r8169: replace get_protocol with vlan_get_protocol
> [4/4996]mh@fan:~/linux/git/linux ((4ff364662814...) %) $ git bisect start drivers/net/ethernet/realtek/r8169.c
> [5/4997]mh@fan:~/linux/git/linux ((4ff364662814...) %|BISECTING) $ 
> (copy 4.20 config, make oldconfig, say no to everything, make deb-pkg, install and try)
> this boots and wakes up on magic packet from suspend to ram
> [2/4994]mh@fan:~/linux/git/linux ((4ff364662814...) *%|BISECTING) $ git bisect good
> [3/4995]mh@fan:~/linux/git/linux ((4ff364662814...) *%|BISECTING) $ git checkout 649f0837a8cc
> Checking out files: 100% (21794/21794), done.
> M       scripts/package/Makefile
> Previous HEAD position was 4ff364662814 r8169: replace get_protocol with vlan_get_protocol
> HEAD is now at 649f0837a8cc r8169: fix broken Wake-on-LAN from S5 (poweroff)
> [4/4996]mh@fan:~/linux/git/linux ((649f0837a8cc...) *%|BISECTING) $ 
> (copy 4.20 config, make oldconfig, say no to everything, make deb-pkg, install and try)
> this boots and does not wake up from StR on magic packet, after 
> waking up with any key on the keyboard I had an amdgpu issue on the console
> and a dead console, but was able to ssh in and reboot.
> 
> [2/4994]mh@fan:~/linux/git/linux ((649f0837a8cc...) *%|BISECTING) $ git
> bisect bad
> Bisecting: 33 revisions left to test after this (roughly 5 steps)
> [098b01ad9837b4d4d0022f407300f069a999e55a] r8169: don't include asm
> headers directly
> [3/4995]mh@fan:~/linux/git/linux ((098b01ad9837...) *%|BISECTING) $ 
> 
> I think I am on the right way here, am I?
> 
> Greetings
> Marc
> 
Yes. All you have to do after each "git bisect good/bad" is build again,
test, and make current build as good or bad.

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] mv88e6xxx DSA suspend to RAM support
From: David Miller @ 2019-01-27 21:22 UTC (permalink / raw)
  To: miquel.raynal
  Cc: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel,
	thomas.petazzoni, gregory.clement, antoine.tenart,
	maxime.chevallier, nadavh
In-Reply-To: <20190125095507.29334-1-miquel.raynal@bootlin.com>

From: Miquel Raynal <miquel.raynal@bootlin.com>
Date: Fri, 25 Jan 2019 10:55:05 +0100

> After a first attempt of bringing S2RAM support to the DSA switch, it
> has been reported that a part of the configuration was lost during the
> cycle. This second version adds a first patch that saves the rules in
> a per-chip list when they are applied, so that in the second patch we
> bring S2RAM support by also re-applying these rules.
 ...

It seems like the discussion on patch #1 has hit a brick wall.

The last suggestion was to use:

+       switch (mode) {
+       case PHY_INTERFACE_MODE_2500BASEX:
+               phydev->speed = SPEED_2500;
+               break;
+       case PHY_INTERFACE_MODE_10GKR:
+               phydev->speed = SPEED_10000;
+               break;
+

in dsa_port_fixed_link_register_of(), but that doesn't work
because swphy will fail without speed=1000 in the dts.

Well, whatever is decided, the commit message need to be updated
to explain why this course of action for the most immediate fix
was ultimately taken.

Thanks.

^ permalink raw reply

* Re: stable request: ipv6: Consider sk_bound_dev_if when binding a socket to an address
From: David Miller @ 2019-01-27 21:25 UTC (permalink / raw)
  To: dsahern; +Cc: carnil, netdev
In-Reply-To: <af171b58-ee28-9f7f-cd6e-79c8aa087633@gmail.com>

From: David Ahern <dsahern@gmail.com>
Date: Sun, 27 Jan 2019 09:17:17 -0700

> Hi Dave:
> 
> Request for c5ee066333eb("ipv6: Consider sk_bound_dev_if when binding a
> socket to an address") to be backported.
> 
> The v4-mapped version - ec90ad334986 ("ipv6: Consider sk_bound_dev_if
> when binding a socket to a v4 mapped address") - has been backported to
> stable releases, but the v6 socket version c5ee066333eb has not (and is
> not on the current stable list).

Added, I have no idea how that slipped through.

Thanks for catching!

^ permalink raw reply

* Re: [PATCH] net: phy: at803x: Use helpers to access MMD PHY registers
From: David Miller @ 2019-01-27 21:27 UTC (permalink / raw)
  To: ccaione; +Cc: andrew, f.fainelli, hkallweit1, netdev, linux-kernel
In-Reply-To: <20190125123510.20511-1-ccaione@baylibre.com>

From: Carlo Caione <ccaione@baylibre.com>
Date: Fri, 25 Jan 2019 12:35:10 +0000

> Libphy provides a standard set of helpers to access the MMD PHY
> registers. Use those instead of relying on custom driver-specific
> functions.
> 
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>

Applied to net-next.

^ permalink raw reply

* Re: [PATCH v2] net: ipv4: ip_input: fix blank line coding style issues
From: David Miller @ 2019-01-27 21:28 UTC (permalink / raw)
  To: albin_yang; +Cc: netdev, andrew, yang.wei9
In-Reply-To: <1548427310-3553-1-git-send-email-albin_yang@163.com>

From: Yang Wei <albin_yang@163.com>
Date: Fri, 25 Jan 2019 22:41:50 +0800

> Fix blank line coding style issues, make the code cleaner. 
> Remove a redundant blank line in ip_rcv_core(). 
> Insert a blank line in ip_rcv() between different statement blocks.
> 
> Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>

Applied to net-next.

^ permalink raw reply

* Re: [PATCH net-next 0/2] tcp: change pingpong to 3 in delayed ack logic
From: David Miller @ 2019-01-27 21:29 UTC (permalink / raw)
  To: weiwan; +Cc: netdev, ycheng, ncardwell, edumazet
In-Reply-To: <20190125185320.164580-1-weiwan@google.com>

From: Wei Wang <weiwan@google.com>
Date: Fri, 25 Jan 2019 10:53:18 -0800

> TCP receiver today tries not to delay the ACKs to speed up the initial
> slow start (a.k.a QUICK ACK mechanism). However the previous design
> does not work well with modern TCP applications that starts with an
> application-level handshake. For example, a HTTPs server often
> receives the SSL hello and responds right away which triggers the TCP
> stack to stop the quick ack and start delaying the ACKs based only one
> instance of ping-pong. This patchset changes the threshold from 1 to 3
> ping-pong transactions, so that we only start to delay the acks after
> the receiver responds data quickly three times.

Series applied, thanks.

^ permalink raw reply

* Re: KASAN: use-after-free Read in br_mdb_ip_get
From: Nikolay Aleksandrov @ 2019-01-27 21:34 UTC (permalink / raw)
  To: syzbot, bridge, davem, linux-kernel, netdev, roopa,
	syzkaller-bugs
In-Reply-To: <000000000000862b160580765e94@google.com>

On 27/01/2019 22:26, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    ba6069759381 Merge tag 'mmc-v5.0-rc2' of git://git.kernel...
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=17b342c4c00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=505743eba4e4f68
> dashboard link: https://syzkaller.appspot.com/bug?extid=bc5ab0af2dbf3b0ae897
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> 
> Unfortunately, I don't have any reproducer for this crash yet.
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+bc5ab0af2dbf3b0ae897@syzkaller.appspotmail.com
> 
> bridge0: port 2(bridge_slave_1) entered blocking state
> bridge0: port 2(bridge_slave_1) entered forwarding state
> bridge0: port 1(bridge_slave_0) entered blocking state
> bridge0: port 1(bridge_slave_0) entered forwarding state
> ==================================================================
> BUG: KASAN: use-after-free in memcmp+0xb3/0xc0 lib/string.c:862
> Read of size 1 at addr ffff88809f1efe70 by task syz-executor1/8111
> 
[snip]
> 
> Allocated by task 8111:
>  save_stack+0x45/0xd0 mm/kasan/common.c:73
>  set_track mm/kasan/common.c:85 [inline]
>  __kasan_kmalloc mm/kasan/common.c:496 [inline]
>  __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:469
>  kasan_kmalloc+0x9/0x10 mm/kasan/common.c:504
>  kmem_cache_alloc_trace+0x151/0x760 mm/slab.c:3609
>  kmalloc include/linux/slab.h:545 [inline]
>  kzalloc include/linux/slab.h:740 [inline]
>  br_multicast_new_group.part.0+0xdc/0x1a40 net/bridge/br_multicast.c:476
>  br_multicast_new_group+0x19d/0x200 net/bridge/br_multicast.c:471
>  br_multicast_add_group+0x4ce/0x7d0 net/bridge/br_multicast.c:552
>  br_ip6_multicast_add_group net/bridge/br_multicast.c:626 [inline]
>  br_ip6_multicast_mld2_report net/bridge/br_multicast.c:1043 [inline]
>  br_multicast_ipv6_rcv net/bridge/br_multicast.c:1667 [inline]
>  br_multicast_rcv+0x24aa/0x4270 net/bridge/br_multicast.c:1705
>  br_dev_xmit+0x7f4/0x1780 net/bridge/br_device.c:93
>  __netdev_start_xmit include/linux/netdevice.h:4382 [inline]
>  netdev_start_xmit include/linux/netdevice.h:4391 [inline]
>  xmit_one net/core/dev.c:3278 [inline]
>  dev_hard_start_xmit+0x261/0xc70 net/core/dev.c:3294
>  __dev_queue_xmit+0x2f8a/0x3a60 net/core/dev.c:3864
>  dev_queue_xmit+0x18/0x20 net/core/dev.c:3897
>  neigh_resolve_output net/core/neighbour.c:1476 [inline]
>  neigh_resolve_output+0x6a0/0xb30 net/core/neighbour.c:1456
>  neigh_output include/net/neighbour.h:508 [inline]
>  ip6_finish_output2+0xc56/0x28e0 net/ipv6/ip6_output.c:120
>  ip6_finish_output+0x577/0xc30 net/ipv6/ip6_output.c:154
>  NF_HOOK_COND include/linux/netfilter.h:278 [inline]
>  ip6_output+0x23c/0xa00 net/ipv6/ip6_output.c:171
>  dst_output include/net/dst.h:444 [inline]
>  NF_HOOK include/linux/netfilter.h:289 [inline]
>  NF_HOOK include/linux/netfilter.h:283 [inline]
>  mld_sendpack+0xa44/0xfd0 net/ipv6/mcast.c:1683
>  mld_send_cr net/ipv6/mcast.c:1979 [inline]
>  mld_ifc_timer_expire+0x449/0x8a0 net/ipv6/mcast.c:2478
>  call_timer_fn+0x254/0x900 kernel/time/timer.c:1325
>  expire_timers kernel/time/timer.c:1362 [inline]
>  __run_timers+0x6fc/0xd50 kernel/time/timer.c:1681
>  run_timer_softirq+0x52/0xb0 kernel/time/timer.c:1694
>  __do_softirq+0x30b/0xb11 kernel/softirq.c:292
> 
> Freed by task 8111:
>  save_stack+0x45/0xd0 mm/kasan/common.c:73
>  set_track mm/kasan/common.c:85 [inline]
>  __kasan_slab_free+0x102/0x150 mm/kasan/common.c:458
>  kasan_slab_free+0xe/0x10 mm/kasan/common.c:466
>  __cache_free mm/slab.c:3487 [inline]
>  kfree+0xcf/0x230 mm/slab.c:3806
>  br_multicast_new_group.part.0+0x1489/0x1a40 net/bridge/br_multicast.c:486


Weird, this is the kfree() on the error path of br_multicast_new_group()
when rhashtable_lookup_insert_fast() fails, which means the entry should
not be linked in the rhashtable or the hlist.
All other frees are via kfree_rcu. I'll keep looking.. 

>  br_multicast_new_group+0x19d/0x200 net/bridge/br_multicast.c:471
>  br_multicast_add_group+0x4ce/0x7d0 net/bridge/br_multicast.c:552
>  br_ip6_multicast_add_group net/bridge/br_multicast.c:626 [inline]
>  br_ip6_multicast_mld2_report net/bridge/br_multicast.c:1043 [inline]
>  br_multicast_ipv6_rcv net/bridge/br_multicast.c:1667 [inline]
>  br_multicast_rcv+0x24aa/0x4270 net/bridge/br_multicast.c:1705
>  br_dev_xmit+0x7f4/0x1780 net/bridge/br_device.c:93
>  __netdev_start_xmit include/linux/netdevice.h:4382 [inline]
>  netdev_start_xmit include/linux/netdevice.h:4391 [inline]
>  xmit_one net/core/dev.c:3278 [inline]
>  dev_hard_start_xmit+0x261/0xc70 net/core/dev.c:3294
>  __dev_queue_xmit+0x2f8a/0x3a60 net/core/dev.c:3864
>  dev_queue_xmit+0x18/0x20 net/core/dev.c:3897
>  neigh_resolve_output net/core/neighbour.c:1476 [inline]
>  neigh_resolve_output+0x6a0/0xb30 net/core/neighbour.c:1456
>  neigh_output include/net/neighbour.h:508 [inline]
>  ip6_finish_output2+0xc56/0x28e0 net/ipv6/ip6_output.c:120
>  ip6_finish_output+0x577/0xc30 net/ipv6/ip6_output.c:154
>  NF_HOOK_COND include/linux/netfilter.h:278 [inline]
>  ip6_output+0x23c/0xa00 net/ipv6/ip6_output.c:171
>  dst_output include/net/dst.h:444 [inline]
>  NF_HOOK include/linux/netfilter.h:289 [inline]
>  NF_HOOK include/linux/netfilter.h:283 [inline]
>  mld_sendpack+0xa44/0xfd0 net/ipv6/mcast.c:1683
>  mld_send_cr net/ipv6/mcast.c:1979 [inline]
>  mld_ifc_timer_expire+0x449/0x8a0 net/ipv6/mcast.c:2478
>  call_timer_fn+0x254/0x900 kernel/time/timer.c:1325
>  expire_timers kernel/time/timer.c:1362 [inline]
>  __run_timers+0x6fc/0xd50 kernel/time/timer.c:1681
>  run_timer_softirq+0x52/0xb0 kernel/time/timer.c:1694
>  __do_softirq+0x30b/0xb11 kernel/softirq.c:292
> 
> The buggy address belongs to the object at ffff88809f1efe00
>  which belongs to the cache kmalloc-192 of size 192
> The buggy address is located 112 bytes inside of
>  192-byte region [ffff88809f1efe00, ffff88809f1efec0)
> The buggy address belongs to the page:
> page:ffffea00027c7bc0 count:1 mapcount:0 mapping:ffff88812c3f0040 index:0x0
> flags: 0x1fffc0000000200(slab)
> raw: 01fffc0000000200 ffffea00027df108 ffffea00027c7c48 ffff88812c3f0040
> raw: 0000000000000000 ffff88809f1ef000 0000000100000010 0000000000000000
> page dumped because: kasan: bad access detected
> 
> Memory state around the buggy address:
>  ffff88809f1efd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffff88809f1efd80: 00 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff88809f1efe00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>                                                              ^
>  ffff88809f1efe80: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>  ffff88809f1eff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
> 
> 
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
> 
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.


^ permalink raw reply

* Re: [PATCH net-next v2 0/2] mv88e6xxx DSA suspend to RAM support
From: Andrew Lunn @ 2019-01-27 21:46 UTC (permalink / raw)
  To: David Miller
  Cc: miquel.raynal, vivien.didelot, f.fainelli, netdev, linux-kernel,
	thomas.petazzoni, gregory.clement, antoine.tenart,
	maxime.chevallier, nadavh
In-Reply-To: <20190127.132250.578158295862987283.davem@davemloft.net>

On Sun, Jan 27, 2019 at 01:22:50PM -0800, David Miller wrote:
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Date: Fri, 25 Jan 2019 10:55:05 +0100
> 
> > After a first attempt of bringing S2RAM support to the DSA switch, it
> > has been reported that a part of the configuration was lost during the
> > cycle. This second version adds a first patch that saves the rules in
> > a per-chip list when they are applied, so that in the second patch we
> > bring S2RAM support by also re-applying these rules.
>  ...
> 
> It seems like the discussion on patch #1 has hit a brick wall.
> 
> The last suggestion was to use:
> 
> +       switch (mode) {
> +       case PHY_INTERFACE_MODE_2500BASEX:
> +               phydev->speed = SPEED_2500;
> +               break;
> +       case PHY_INTERFACE_MODE_10GKR:
> +               phydev->speed = SPEED_10000;
> +               break;
> +
> 
> in dsa_port_fixed_link_register_of(), but that doesn't work
> because swphy will fail without speed=1000 in the dts.
> 
> Well, whatever is decided, the commit message need to be updated
> to explain why this course of action for the most immediate fix
> was ultimately taken.

Hi David

Err. 

I think you are replying to the wrong patch? This comment fits to

https://www.spinics.net/lists/netdev/msg546662.html

Although i don't like it, i think for the moment we are going to have
to accept the proposed fix.

	Andrew

^ permalink raw reply

* [PATCH net] net: dsa: mv88e6xxx: Fix serdes irq setup going recursive
From: Andrew Lunn @ 2019-01-27 21:48 UTC (permalink / raw)
  To: David Miller; +Cc: Vivien Didelot, netdev, Andrew Lunn

Duec to a typo, mv88e6390_serdes_irq_setup() calls itself, rather than
mv88e6390x_serdes_irq_setup(). It then blows the stack, and shortly
after the machine blows up.

Fixes: 2defda1f4b91 ("net: dsa: mv88e6xxx: Add support for SERDES on ports 2-8 for 6390X")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 2caa8c8b4b55..1bfc5ff8d81d 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -664,7 +664,7 @@ int mv88e6390_serdes_irq_setup(struct mv88e6xxx_chip *chip, int port)
 	if (port < 9)
 		return 0;
 
-	return mv88e6390_serdes_irq_setup(chip, port);
+	return mv88e6390x_serdes_irq_setup(chip, port);
 }
 
 void mv88e6390x_serdes_irq_free(struct mv88e6xxx_chip *chip, int port)
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port
From: Michal Kubecek @ 2019-01-27 23:07 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, Vasundhara Volam, davem, michael.chan, jiri
In-Reply-To: <20190122141842.26908b27@cakuba.netronome.com>

On Tue, Jan 22, 2019 at 02:18:42PM -0800, Jakub Kicinski wrote:
> 
> I think reusing new netlink ethtool with a special flag would be a nice,
> complete solution.  We could also address link settings this way (which
> are a pre-requisite for WoL).
> 
> I have no strong preference on the mechanism, but for ease of setting
> as well as dumping would it be workable to use a nesting, like this:
> 
> Run time settings:
>   [ETHTOOL_SETTINGS_BLA]
>     [ETHTOOL_BLA_VAL_1]
>     [ETHTOOL_BLA_VAL_2]
>     ...
> 
> Persistent:
>   [ETHTOOL_PERSISTENT]
>     [ETHTOOL_SETTINGS_BLA]
>       [ETHTOOL_BLA_VAL_1]
>       [ETHTOOL_BLA_VAL_2]
> 
> IOW encapsulate settings into a "persistent" attribute?
> 
> How does that look to you, Michal?

It's certainly one of the options worth considering. What I like is that
this approach would not require knowing (or rather guessing) which
attributes are going to allow persistent settings in the future.

Michal

^ permalink raw reply

* Packets being dropped somewhere in the kernel, between iptables and packet capture layers
From: Niklas Hambüchen @ 2019-01-28  0:17 UTC (permalink / raw)
  To: netdev

Hello network developers,

I'm sending this to netdev@vger.kernel.org even though http://vger.kernel.org/lkml/ still suggests linux-net@vger.kernel.org, because the latter seems to be inactive since 2011 and full of spam, and I got "unresolvable address" for it. Perhaps somebody should update the page that recommends it.
Nevertheless, please let me know if here is the wrong place.

## Problem

Some DNS requests on my machine take 5 seconds when a specific network interface is used.
This makes using a web browser a total pain.

I suspect this is a problem in the kernel, somewhere between the iptables layer and the level on which Wireshark/tcpdump operates.

I give the reasoning for this suspicion below.

It happens in around 5% of the invocations of `dig ifconfig.me @1.1.1.1`.
(It doesn't matter which nameserver or domain is used, I'm just using `1.1.1.1` as an example because it can be nicely grepped in logs.)
My usual DNS resolution time is 1 ms.

I can reproduce the problem in less than 1 second by running `while true; do date; time dig ifconfig.me @1.1.1.1; done`.
A couple successful invocations flush by, then `dig` hangs for 5 seconds.

I'm fairly certain 5 seconds is a timeout implemented by `dig`/`glibc`; it's what it does when it doesn't get a response to its UDP requests.

`strace` demonstrates that the userspace side does what it should:

```
sendmsg(20<socket:[92070424]>, {msg_name(16)={sa_family=AF_INET, sin_port=htons(53), sin_addr=inet_addr("8.8.8.8")}, msg_iov(1)=[{"\372\f\1 \0\1\0\0\0\0\0\1\10ifconfig\2me\0\0\1\0\1\0\0)"..., 40}], msg_controllen=0, msg_flags=0}, 0) = 40
epoll_wait(
  -- here it hangs
  -- 5 seconds hang here
<... futex resumed> )       = -1 ETIMEDOUT (Connection timed out)
  -- dig's 5 second timeout triggers, it retries
sendmsg(20<socket:[92070424]>, {msg_name(16)={sa_family=AF_INET, sin_port=htons(53), sin_addr=inet_addr("8.8.8.8")}, msg_iov(1)=[{"\372\f\1 \0\1\0\0\0\0\0\1\10ifconfig\2me\0\0\1\0\1\0\0)"..., 40}], msg_controllen=0, msg_flags=0}, 0) = 40
  -- 1 ms later
recvmsg(20<socket:[92070424]>, {msg_name(16)={sa_family=AF_INET, sin_port=htons(53), sin_addr=inet_addr("8.8.8.8")}, msg_iov(1)=[{"\372\f\201\200\0\1\0\4\0\0\0\1\10ifconfig\2me\0\0\1\0\1\300\f\0"..., 65535}], msg_controllen=56, [{cmsg_len=32, cmsg_level=SOL_SOCKET, cmsg_type=0x1d /* SCM_??? */}, {cmsg_len=17, cmsg_level=SOL_IP, cmsg_type=IP_TOS, {tos=0}}], msg_flags=0}, 0) = 104
```

I have recorded this in Wireshark.
It displays only 1 DNS request being made / UDP packet being sent, not 2, even though the userspace program clearly does tell the kernel to send 2 packets with its 2 `sendmsg()` syscalls:
The request in Wireshark is answered within 1 ms as expected.

So it seems that the kernel never sends the first packet.

(Side info:
Enabling the TCP mode of dig via `time dig +tcp ifconfig.me` changes the timeout to 1 second.
This suggests that the problem is with sending packets _in general_, not only UDP.
EDNS is generally used, but `+noedns` doesn't change the timeouts, no matter if `+tcp` is used or not.
)

I further discovered the problem is specific to the network interface I use.

My machine has 2 Ethernet ports:

* Realtek RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
* Intel Ethernet Connection (2) I218-V

The problem occurs _only_ on the Realtek port.

Details about the hardware from `lshw`:

```
        *-network
             description: Ethernet interface
             product: Ethernet Connection (2) I218-V
             vendor: Intel Corporation
             physical id: 19
             bus info: pci@0000:00:19.0
             logical name: enp0s25
             version: 00
             serial: d0:50:99:5c:cf:1c
             capacity: 1Gbit/s
             width: 32 bits
             clock: 33MHz
             capabilities: pm msi bus_master cap_list ethernet physical tp 10bt 10bt-fd 100bt 100bt-fd 1000bt-fd autonegotiation
             configuration: autonegotiation=on broadcast=yes driver=e1000e driverversion=3.2.6-k firmware=0.1-4 latency=0 link=no multicast=yes port=twisted pair
             resources: irq:42 memory:ef500000-ef51ffff memory:ef538000-ef538fff ioport:f040(size=32)
...
           *-network
                description: Ethernet interface
                product: RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
                vendor: Realtek Semiconductor Co., Ltd.
                physical id: 0
                bus info: pci@0000:03:00.0
                logical name: enp3s0
                version: 11
                serial: d0:50:99:5c:cf:1a
                size: 1Gbit/s
                capacity: 1Gbit/s
                width: 64 bits
                clock: 33MHz
                capabilities: pm msi pciexpress msix vpd bus_master cap_list ethernet physical tp mii 10bt 10bt-fd 100bt 100bt-fd 1000bt 1000bt-fd autonegotiation
                configuration: autonegotiation=on broadcast=yes driver=r8169 driverversion=2.3LK-NAPI duplex=full firmware=rtl8168g-2_0.0.1 02/06/13 ip=192.168.1.100 latency=0 link=yes multicast=yes port=MII speed=1Gbit/s
                resources: irq:37 ioport:d000(size=256) memory:ef400000-ef400fff memory:e2100000-e2103fff
```

User `longxia` from the `##linux` freenode IRC channel brought in the great suggestion to use `iptables` logging to see whether the packet missing in Wireshark is present there.


## Investigation with `iptables` logging facilities

All the below is for `UDP`. `enp3s0` is the Realtek Ethernet port.

I used `iptables -F && iptables -I OUTPUT 1 -j LOG && iptables -I OUTPUT 1 -j LOG`.

I observe during the hang:

```
Jan 27 23:24:16.210162 ares kernel: IN= OUT=enp3s0 SRC=192.168.1.100 DST=1.1.1.1 LEN=68 TOS=0x00 PREC=0x00 TTL=64 ID=57836 PROTO=UDP SPT=33554 DPT=53 LEN=48
# hang here
Jan 27 23:24:21.210106 ares kernel: IN= OUT=enp3s0 SRC=192.168.1.100 DST=1.1.1.1 LEN=68 TOS=0x00 PREC=0x00 TTL=64 ID=57843 PROTO=UDP SPT=33554 DPT=53 LEN=48
Jan 27 23:24:21.210645 ares kernel: IN=enp3s0 OUT= MAC=d0:50:99:5c:cf:1a:44:d9:e7:41:32:61:08:00 SRC=1.1.1.1 DST=192.168.1.100 LEN=132 TOS=0x00 PREC=0x00 TTL=54 ID=24125 DF PROTO=UDP SPT=53 DPT=33554 LEN=112
```

In the corresponding Wireshark capture below, we can see (with filter `dns.qry.name == "ifconfig.me"`) the two packets from after then hang.
But the first outgoing packet is not present!

```
2784  22:24:21.208712960  192.168.1.100 1.1.1.1 DNS 82  Standard query 0x8a93 A ifconfig.me OPT
2785  22:24:21.210606902  1.1.1.1 192.168.1.100 DNS 146 Standard query response 0x8a93 A ifconfig.me A 216.239.36.21 A 216.239.38.21 A 216.239.32.21 A 216.239.34.21 OPT
```

So I suspect that either:

* the packet gets lost in the kernel code path between the iptables layer and the layer that Wireshark attaches to
* the packet get sent, but forever reason doesn't show up in Wireshark (and the problem is the network not giving a response; unlikely given that the other network card works)
* the packet gets dropped by the kernel for some legitimate reason (e.g. hardware returning an error), but the kernel fails to report this error anywhere

Which of it could it be?
How should I investigate deeper?

Thanks a lot,
Niklas


PS:
You can find a version-controlled investigation log of this problem, including this email in nice formatting, at https://github.com/nh2/dig-5-seconds-hang.

^ permalink raw reply

* Re: [PATCH V2] net: i825xx: replace dev_kfree_skb_irq bydev_consume_skb_irq
From: Andrew Lunn @ 2019-01-28  2:03 UTC (permalink / raw)
  To: yang.wei9; +Cc: albin_yang, netdev, davem
In-Reply-To: <201901280950423897221@zte.com.cn>

On Mon, Jan 28, 2019 at 09:50:42AM +0800, yang.wei9@zte.com.cn wrote:
> > It is normal to include here, under the ---, what has changed since
> > the previous version of the patch.
> >
> > Please also read
> >
> > https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html
> >
> > Your patches don't have the correct subject.
> 
> Hi Andrew
> 
> The previous patch (as follow) is wrong, so I submit a new and named V2.
> Please reject the previous patch.
> 
> net: i825xx: replace dev_kfree_skb_irq by dev_consume_skb_irq
> http://patchwork.ozlabs.org/patch/1031590/
> 
> The original suject in patch is as follow, but it be truncated due to return character.
> I have changed it, make the suject in one line. Do I need to resubmit it?

https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#q-how-do-i-indicate-which-tree-net-vs-net-next-my-patch-should-be-in

	Andrew

^ permalink raw reply

* [PATCH v5 bpf-next 3/9] tools/bpf: sync include/uapi/linux/bpf.h
From: Alexei Starovoitov @ 2019-01-28  2:50 UTC (permalink / raw)
  To: davem
  Cc: daniel, peterz, jannh, paulmck, will.deacon, mingo, netdev,
	kernel-team
In-Reply-To: <20190128025010.342241-1-ast@kernel.org>

sync bpf.h

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/include/uapi/linux/bpf.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 60b99b730a41..86f7c438d40f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2422,7 +2422,9 @@ union bpf_attr {
 	FN(map_peek_elem),		\
 	FN(msg_push_data),		\
 	FN(msg_pop_data),		\
-	FN(rc_pointer_rel),
+	FN(rc_pointer_rel),		\
+	FN(spin_lock),			\
+	FN(spin_unlock),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -3056,4 +3058,7 @@ struct bpf_line_info {
 	__u32	line_col;
 };
 
+struct bpf_spin_lock {
+	__u32	val;
+};
 #endif /* _UAPI__LINUX_BPF_H__ */
-- 
2.20.0


^ permalink raw reply related

* [PATCH v5 bpf-next 5/9] selftests/bpf: add bpf_spin_lock C test
From: Alexei Starovoitov @ 2019-01-28  2:50 UTC (permalink / raw)
  To: davem
  Cc: daniel, peterz, jannh, paulmck, will.deacon, mingo, netdev,
	kernel-team
In-Reply-To: <20190128025010.342241-1-ast@kernel.org>

add bpf_spin_lock C based test that requires latest llvm with BTF support

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/testing/selftests/bpf/Makefile         |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h    |   4 +
 tools/testing/selftests/bpf/test_progs.c     |  43 +++++++-
 tools/testing/selftests/bpf/test_spin_lock.c | 108 +++++++++++++++++++
 4 files changed, 155 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/test_spin_lock.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index d61b023c8ddc..48eb9ecd2015 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -35,7 +35,7 @@ BPF_OBJ_FILES = \
 	sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
 	get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
 	test_skb_cgroup_id_kern.o bpf_flow.o netcnt_prog.o test_xdp_vlan.o \
-	xdp_dummy.o test_map_in_map.o
+	xdp_dummy.o test_map_in_map.o test_spin_lock.o
 
 # Objects are built with default compilation flags and with sub-register
 # code-gen enabled.
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 6c77cf7bedce..6a0ce0f055c5 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -172,6 +172,10 @@ static int (*bpf_skb_vlan_pop)(void *ctx) =
 	(void *) BPF_FUNC_skb_vlan_pop;
 static int (*bpf_rc_pointer_rel)(void *ctx, int rel_x, int rel_y) =
 	(void *) BPF_FUNC_rc_pointer_rel;
+static void (*bpf_spin_lock)(struct bpf_spin_lock *lock) =
+	(void *) BPF_FUNC_spin_lock;
+static void (*bpf_spin_unlock)(struct bpf_spin_lock *lock) =
+	(void *) BPF_FUNC_spin_unlock;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 126fc624290d..6425e95c3f16 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -28,7 +28,7 @@ typedef __u16 __sum16;
 #include <sys/wait.h>
 #include <sys/types.h>
 #include <fcntl.h>
-
+#include <pthread.h>
 #include <linux/bpf.h>
 #include <linux/err.h>
 #include <bpf/bpf.h>
@@ -1882,6 +1882,46 @@ static void test_queue_stack_map(int type)
 	bpf_object__close(obj);
 }
 
+static void *parallel_bpf_prog_test_run(void *arg)
+{
+	__u32 duration, retval;
+	int err, prog_fd = *(u32 *) arg;
+
+	err = bpf_prog_test_run(prog_fd, 10000, &pkt_v4, sizeof(pkt_v4),
+				NULL, NULL, &retval, &duration);
+	CHECK(err || retval, "",
+	      "err %d errno %d retval %d duration %d\n",
+	      err, errno, retval, duration);
+	pthread_exit(arg);
+}
+
+static void test_spin_lock(void)
+{
+	const char *file = "./test_spin_lock.o";
+	pthread_t thread_id[4];
+	struct bpf_object *obj;
+	int prog_fd;
+	int err = 0, i;
+	void *ret;
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
+	if (err) {
+		printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
+		goto close_prog;
+	}
+	for (i = 0; i < 4; i++)
+		assert(pthread_create(&thread_id[i], NULL,
+				      &parallel_bpf_prog_test_run, &prog_fd) == 0);
+	for (i = 0; i < 4; i++)
+		assert(pthread_join(thread_id[i], &ret) == 0 &&
+		       ret == (void *)&prog_fd);
+	goto close_prog_noerr;
+close_prog:
+	error_cnt++;
+close_prog_noerr:
+	bpf_object__close(obj);
+}
+
 int main(void)
 {
 	srand(time(NULL));
@@ -1909,6 +1949,7 @@ int main(void)
 	test_reference_tracking();
 	test_queue_stack_map(QUEUE);
 	test_queue_stack_map(STACK);
+	test_spin_lock();
 
 	printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
 	return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
diff --git a/tools/testing/selftests/bpf/test_spin_lock.c b/tools/testing/selftests/bpf/test_spin_lock.c
new file mode 100644
index 000000000000..40f904312090
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_spin_lock.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Facebook
+#include <linux/bpf.h>
+#include <linux/version.h>
+#include "bpf_helpers.h"
+
+struct hmap_elem {
+	volatile int cnt;
+	struct bpf_spin_lock lock;
+	int test_padding;
+};
+
+struct bpf_map_def SEC("maps") hmap = {
+	.type = BPF_MAP_TYPE_HASH,
+	.key_size = sizeof(int),
+	.value_size = sizeof(struct hmap_elem),
+	.max_entries = 1,
+};
+
+BPF_ANNOTATE_KV_PAIR(hmap, int, struct hmap_elem);
+
+
+struct cls_elem {
+	struct bpf_spin_lock lock;
+	volatile int cnt;
+};
+
+struct bpf_map_def SEC("maps") cls_map = {
+	.type = BPF_MAP_TYPE_CGROUP_STORAGE,
+	.key_size = sizeof(struct bpf_cgroup_storage_key),
+	.value_size = sizeof(struct cls_elem),
+};
+
+BPF_ANNOTATE_KV_PAIR(cls_map, struct bpf_cgroup_storage_key,
+		     struct cls_elem);
+
+struct bpf_vqueue {
+	struct bpf_spin_lock lock;
+	/* 4 byte hole */
+	unsigned long long lasttime;
+	int credit;
+	unsigned int rate;
+};
+
+struct bpf_map_def SEC("maps") vqueue = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(int),
+	.value_size = sizeof(struct bpf_vqueue),
+	.max_entries = 1,
+};
+
+BPF_ANNOTATE_KV_PAIR(vqueue, int, struct bpf_vqueue);
+#define CREDIT_PER_NS(delta, rate) (((delta) * rate) >> 20)
+
+SEC("spin_lock_demo")
+int bpf_sping_lock_test(struct __sk_buff *skb)
+{
+	volatile int credit = 0, max_credit = 100, pkt_len = 64;
+	struct hmap_elem zero = {}, *val;
+	unsigned long long curtime;
+	struct bpf_vqueue *q;
+	struct cls_elem *cls;
+	int key = 0;
+	int err = 0;
+
+	val = bpf_map_lookup_elem(&hmap, &key);
+	if (!val) {
+		bpf_map_update_elem(&hmap, &key, &zero, 0);
+		val = bpf_map_lookup_elem(&hmap, &key);
+		if (!val) {
+			err = 1;
+			goto err;
+		}
+	}
+	/* spin_lock in hash map run time test */
+	bpf_spin_lock(&val->lock);
+	if (val->cnt)
+		val->cnt--;
+	else
+		val->cnt++;
+	if (val->cnt != 0 && val->cnt != 1)
+		err = 1;
+	bpf_spin_unlock(&val->lock);
+
+	/* spin_lock in array. virtual queue demo */
+	q = bpf_map_lookup_elem(&vqueue, &key);
+	if (!q)
+		goto err;
+	curtime = bpf_ktime_get_ns();
+	bpf_spin_lock(&q->lock);
+	q->credit += CREDIT_PER_NS(curtime - q->lasttime, q->rate);
+	q->lasttime = curtime;
+	if (q->credit > max_credit)
+		q->credit = max_credit;
+	q->credit -= pkt_len;
+	credit = q->credit;
+	bpf_spin_unlock(&q->lock);
+
+	/* spin_lock in cgroup local storage */
+	cls = bpf_get_local_storage(&cls_map, 0);
+	bpf_spin_lock(&cls->lock);
+	cls->cnt++;
+	bpf_spin_unlock(&cls->lock);
+
+err:
+	return err;
+}
+char _license[] SEC("license") = "GPL";
-- 
2.20.0


^ permalink raw reply related

* [PATCH v5 bpf-next 2/9] bpf: add support for bpf_spin_lock to cgroup local storage
From: Alexei Starovoitov @ 2019-01-28  2:50 UTC (permalink / raw)
  To: davem
  Cc: daniel, peterz, jannh, paulmck, will.deacon, mingo, netdev,
	kernel-team
In-Reply-To: <20190128025010.342241-1-ast@kernel.org>

Allow 'struct bpf_spin_lock' to reside inside cgroup local storage.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/local_storage.c | 2 ++
 kernel/bpf/syscall.c       | 3 ++-
 kernel/bpf/verifier.c      | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 07a34ef562a0..0295427f06e2 100644
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -147,6 +147,7 @@ static int cgroup_storage_update_elem(struct bpf_map *map, void *_key,
 		return -ENOMEM;
 
 	memcpy(&new->data[0], value, map->value_size);
+	check_and_init_map_lock(map, new->data);
 
 	new = xchg(&storage->buf, new);
 	kfree_rcu(new, rcu);
@@ -483,6 +484,7 @@ struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(struct bpf_prog *prog,
 		storage->buf = kmalloc_node(size, flags, map->numa_node);
 		if (!storage->buf)
 			goto enomem;
+		check_and_init_map_lock(map, storage->buf->data);
 	} else {
 		storage->percpu_buf = __alloc_percpu_gfp(size, 8, flags);
 		if (!storage->percpu_buf)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ebf0a673cb83..b29e6dc44650 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -482,7 +482,8 @@ static int map_check_btf(struct bpf_map *map, const struct btf *btf,
 
 	if (map_value_has_spin_lock(map)) {
 		if (map->map_type != BPF_MAP_TYPE_HASH &&
-		    map->map_type != BPF_MAP_TYPE_ARRAY)
+		    map->map_type != BPF_MAP_TYPE_ARRAY &&
+		    map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE)
 			return -ENOTSUPP;
 		if (map->spin_lock_off + sizeof(struct bpf_spin_lock) >
 		    map->value_size) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b6af9d2cb236..dd1bd7485a57 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3089,6 +3089,8 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 		regs[BPF_REG_0].map_ptr = meta.map_ptr;
 		if (fn->ret_type == RET_PTR_TO_MAP_VALUE) {
 			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;
+			if (map_value_has_spin_lock(meta.map_ptr))
+				regs[BPF_REG_0].id = ++env->id_gen;
 		} else {
 			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
 			regs[BPF_REG_0].id = ++env->id_gen;
-- 
2.20.0


^ permalink raw reply related

* [PATCH v5 bpf-next 6/9] bpf: introduce BPF_F_LOCK flag
From: Alexei Starovoitov @ 2019-01-28  2:50 UTC (permalink / raw)
  To: davem
  Cc: daniel, peterz, jannh, paulmck, will.deacon, mingo, netdev,
	kernel-team
In-Reply-To: <20190128025010.342241-1-ast@kernel.org>

Introduce BPF_F_LOCK flag for map_lookup and map_update syscall commands
and for map_update() helper function.
In all these cases take a lock of existing element (which was provided
in BTF description) before copying (in or out) the rest of map value.

Implementation details that are part of uapi:

Array:
The array map takes the element lock for lookup/update.

Hash:
hash map also takes the lock for lookup/update and tries to avoid the bucket lock.
If old element exists it takes the element lock and updates the element in place.
If element doesn't exist it allocates new one and inserts into hash table
while holding the bucket lock.
In rare case the hashmap has to take both the bucket lock and the element lock
to update old value in place.

Cgroup local storage:
It is similar to array. update in place and lookup are done with lock taken.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 include/linux/bpf.h        |  2 ++
 include/uapi/linux/bpf.h   |  1 +
 kernel/bpf/arraymap.c      | 24 ++++++++++++++--------
 kernel/bpf/hashtab.c       | 42 +++++++++++++++++++++++++++++++++++---
 kernel/bpf/helpers.c       | 14 +++++++++++++
 kernel/bpf/local_storage.c | 14 ++++++++++++-
 kernel/bpf/syscall.c       | 25 +++++++++++++++++++++--
 7 files changed, 108 insertions(+), 14 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index ee0352bd728f..34a517137b63 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -119,6 +119,8 @@ static inline void copy_map_value(struct bpf_map *map, void *dst, void *src)
 		memcpy(dst, src, map->value_size);
 	}
 }
+void copy_map_value_locked(struct bpf_map *map, void *dst, void *src,
+			   bool lock_src);
 
 struct bpf_offload_dev;
 struct bpf_offloaded_map;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 86f7c438d40f..1777fa0c61e4 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -267,6 +267,7 @@ enum bpf_attach_type {
 #define BPF_ANY		0 /* create new element or update existing */
 #define BPF_NOEXIST	1 /* create new element if it didn't exist */
 #define BPF_EXIST	2 /* update existing element */
+#define BPF_F_LOCK	4 /* spin_lock-ed map_lookup/map_update */
 
 /* flags for BPF_MAP_CREATE command */
 #define BPF_F_NO_PREALLOC	(1U << 0)
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index d6d979910a2a..c72e0d8e1e65 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -253,8 +253,9 @@ static int array_map_update_elem(struct bpf_map *map, void *key, void *value,
 {
 	struct bpf_array *array = container_of(map, struct bpf_array, map);
 	u32 index = *(u32 *)key;
+	char *val;
 
-	if (unlikely(map_flags > BPF_EXIST))
+	if (unlikely((map_flags & ~BPF_F_LOCK) > BPF_EXIST))
 		/* unknown flags */
 		return -EINVAL;
 
@@ -262,18 +263,25 @@ static int array_map_update_elem(struct bpf_map *map, void *key, void *value,
 		/* all elements were pre-allocated, cannot insert a new one */
 		return -E2BIG;
 
-	if (unlikely(map_flags == BPF_NOEXIST))
+	if (unlikely(map_flags & BPF_NOEXIST))
 		/* all elements already exist */
 		return -EEXIST;
 
-	if (array->map.map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
+	if (unlikely((map_flags & BPF_F_LOCK) &&
+		     !map_value_has_spin_lock(map)))
+		return -EINVAL;
+
+	if (array->map.map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
 		memcpy(this_cpu_ptr(array->pptrs[index & array->index_mask]),
 		       value, map->value_size);
-	else
-		copy_map_value(map,
-			       array->value +
-			       array->elem_size * (index & array->index_mask),
-			       value);
+	} else {
+		val = array->value +
+			array->elem_size * (index & array->index_mask);
+		if (map_flags & BPF_F_LOCK)
+			copy_map_value_locked(map, val, value, false);
+		else
+			copy_map_value(map, val, value);
+	}
 	return 0;
 }
 
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 6d3b22c5ad68..937776531998 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -804,11 +804,11 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
 static int check_flags(struct bpf_htab *htab, struct htab_elem *l_old,
 		       u64 map_flags)
 {
-	if (l_old && map_flags == BPF_NOEXIST)
+	if (l_old && (map_flags & ~BPF_F_LOCK) == BPF_NOEXIST)
 		/* elem already exists */
 		return -EEXIST;
 
-	if (!l_old && map_flags == BPF_EXIST)
+	if (!l_old && (map_flags & ~BPF_F_LOCK) == BPF_EXIST)
 		/* elem doesn't exist, cannot update it */
 		return -ENOENT;
 
@@ -827,7 +827,7 @@ static int htab_map_update_elem(struct bpf_map *map, void *key, void *value,
 	u32 key_size, hash;
 	int ret;
 
-	if (unlikely(map_flags > BPF_EXIST))
+	if (unlikely((map_flags & ~BPF_F_LOCK) > BPF_EXIST))
 		/* unknown flags */
 		return -EINVAL;
 
@@ -840,6 +840,28 @@ static int htab_map_update_elem(struct bpf_map *map, void *key, void *value,
 	b = __select_bucket(htab, hash);
 	head = &b->head;
 
+	if (unlikely(map_flags & BPF_F_LOCK)) {
+		if (unlikely(!map_value_has_spin_lock(map)))
+			return -EINVAL;
+		/* find an element without taking the bucket lock */
+		l_old = lookup_nulls_elem_raw(head, hash, key, key_size,
+					      htab->n_buckets);
+		ret = check_flags(htab, l_old, map_flags);
+		if (ret)
+			return ret;
+		if (l_old) {
+			/* grab the element lock and update value in place */
+			copy_map_value_locked(map,
+					      l_old->key + round_up(key_size, 8),
+					      value, false);
+			return 0;
+		}
+		/* fall through, grab the bucket lock and lookup again.
+		 * 99.9% chance that the element won't be found,
+		 * but second lookup under lock has to be done.
+		 */
+	}
+
 	/* bpf_map_update_elem() can be called in_irq() */
 	raw_spin_lock_irqsave(&b->lock, flags);
 
@@ -849,6 +871,20 @@ static int htab_map_update_elem(struct bpf_map *map, void *key, void *value,
 	if (ret)
 		goto err;
 
+	if (unlikely(l_old && (map_flags & BPF_F_LOCK))) {
+		/* first lookup without the bucket lock didn't find the element,
+		 * but second lookup with the bucket lock found it.
+		 * This case is highly unlikely, but has to be dealt with:
+		 * grab the element lock in addition to the bucket lock
+		 * and update element in place
+		 */
+		copy_map_value_locked(map,
+				      l_old->key + round_up(key_size, 8),
+				      value, false);
+		ret = 0;
+		goto err;
+	}
+
 	l_new = alloc_htab_elem(htab, key, value, key_size, hash, false, false,
 				l_old);
 	if (IS_ERR(l_new)) {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e1d6aefbab50..9186ac509606 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -278,6 +278,20 @@ const struct bpf_func_proto bpf_spin_unlock_proto = {
 	.arg1_type	= ARG_PTR_TO_SPIN_LOCK,
 };
 
+void copy_map_value_locked(struct bpf_map *map, void *dst, void *src,
+			   bool lock_src)
+{
+	struct bpf_spin_lock *lock;
+
+	if (lock_src)
+		lock = src + map->spin_lock_off;
+	else
+		lock = dst + map->spin_lock_off;
+	____bpf_spin_lock(lock);
+	copy_map_value(map, dst, src);
+	____bpf_spin_unlock(lock);
+}
+
 #ifdef CONFIG_CGROUPS
 BPF_CALL_0(bpf_get_current_cgroup_id)
 {
diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 0295427f06e2..6b572e2de7fb 100644
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -131,7 +131,14 @@ static int cgroup_storage_update_elem(struct bpf_map *map, void *_key,
 	struct bpf_cgroup_storage *storage;
 	struct bpf_storage_buffer *new;
 
-	if (flags != BPF_ANY && flags != BPF_EXIST)
+	if (unlikely(flags & ~(BPF_F_LOCK | BPF_EXIST | BPF_NOEXIST)))
+		return -EINVAL;
+
+	if (unlikely(flags & BPF_NOEXIST))
+		return -EINVAL;
+
+	if (unlikely((flags & BPF_F_LOCK) &&
+		     !map_value_has_spin_lock(map)))
 		return -EINVAL;
 
 	storage = cgroup_storage_lookup((struct bpf_cgroup_storage_map *)map,
@@ -139,6 +146,11 @@ static int cgroup_storage_update_elem(struct bpf_map *map, void *_key,
 	if (!storage)
 		return -ENOENT;
 
+	if (flags & BPF_F_LOCK) {
+		copy_map_value_locked(map, storage->buf->data, value, false);
+		return 0;
+	}
+
 	new = kmalloc_node(sizeof(struct bpf_storage_buffer) +
 			   map->value_size,
 			   __GFP_ZERO | GFP_ATOMIC | __GFP_NOWARN,
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b29e6dc44650..0834958f1dc4 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -682,7 +682,7 @@ static void *__bpf_copy_key(void __user *ukey, u64 key_size)
 }
 
 /* last field in 'union bpf_attr' used by this command */
-#define BPF_MAP_LOOKUP_ELEM_LAST_FIELD value
+#define BPF_MAP_LOOKUP_ELEM_LAST_FIELD flags
 
 static int map_lookup_elem(union bpf_attr *attr)
 {
@@ -698,6 +698,9 @@ static int map_lookup_elem(union bpf_attr *attr)
 	if (CHECK_ATTR(BPF_MAP_LOOKUP_ELEM))
 		return -EINVAL;
 
+	if (attr->flags & ~BPF_F_LOCK)
+		return -EINVAL;
+
 	f = fdget(ufd);
 	map = __bpf_map_get(f);
 	if (IS_ERR(map))
@@ -708,6 +711,12 @@ static int map_lookup_elem(union bpf_attr *attr)
 		goto err_put;
 	}
 
+	if ((attr->flags & BPF_F_LOCK) &&
+	    !map_value_has_spin_lock(map)) {
+		err = -EINVAL;
+		goto err_put;
+	}
+
 	key = __bpf_copy_key(ukey, map->key_size);
 	if (IS_ERR(key)) {
 		err = PTR_ERR(key);
@@ -758,7 +767,13 @@ static int map_lookup_elem(union bpf_attr *attr)
 			err = -ENOENT;
 		} else {
 			err = 0;
-			copy_map_value(map, value, ptr);
+			if (attr->flags & BPF_F_LOCK)
+				/* lock 'ptr' and copy everything but lock */
+				copy_map_value_locked(map, value, ptr, true);
+			else
+				copy_map_value(map, value, ptr);
+			/* mask lock, since value wasn't zero inited */
+			check_and_init_map_lock(map, value);
 		}
 		rcu_read_unlock();
 	}
@@ -818,6 +833,12 @@ static int map_update_elem(union bpf_attr *attr)
 		goto err_put;
 	}
 
+	if ((attr->flags & BPF_F_LOCK) &&
+	    !map_value_has_spin_lock(map)) {
+		err = -EINVAL;
+		goto err_put;
+	}
+
 	key = __bpf_copy_key(ukey, map->key_size);
 	if (IS_ERR(key)) {
 		err = PTR_ERR(key);
-- 
2.20.0


^ permalink raw reply related


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