* [PATCH bpf-next V3] net/xdp: Fix suspicious RCU usage warning
From: Tariq Toukan @ 2018-08-13 9:21 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, Eran Ben Elisha, Tariq Toukan, Jesper Dangaard Brouer
Fix the warning below by calling rhashtable_lookup_fast.
Also, make some code movements for better quality and human
readability.
[ 342.450870] WARNING: suspicious RCU usage
[ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
[ 342.462210] -----------------------------
[ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
[ 342.476568]
[ 342.476568] other info that might help us debug this:
[ 342.476568]
[ 342.486978]
[ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
[ 342.495211] 4 locks held by modprobe/3934:
[ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
mlx5_unregister_interface+0x18/0x90 [mlx5_core]
[ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
[ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
[mlx5_core]
[ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
[ 342.541206]
[ 342.541206] stack backtrace:
[ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
[ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
[ 342.565606] Call Trace:
[ 342.568861] dump_stack+0x78/0xb3
[ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
[ 342.578285] ? __call_rcu+0x220/0x300
[ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
[ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
[ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
[ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
[ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
[ 342.613005] __dev_close_many+0xb1/0x120
[ 342.617911] dev_close_many+0xa2/0x170
[ 342.622622] rollback_registered_many+0x148/0x460
[ 342.628401] ? __lock_acquire+0x48d/0x11b0
[ 342.633498] ? unregister_netdev+0xe/0x20
[ 342.638495] rollback_registered+0x56/0x90
[ 342.643588] unregister_netdevice_queue+0x7e/0x100
[ 342.649461] unregister_netdev+0x18/0x20
[ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
[ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
[ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
[ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
[ 342.678094] __x64_sys_delete_module+0x16b/0x240
[ 342.683725] ? do_syscall_64+0x1c/0x210
[ 342.688476] do_syscall_64+0x5a/0x210
[ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
---
net/core/xdp.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
V2 -> V3:
* Fix return value test for rhashtable_remove_fast, per Jesper's comment.
V1 -> V2:
* Use rhashtable_lookup_fast and make some code movements, per Daniel's
and Alexei's comments.
Please queue to -stable v4.18.
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 3dd99e1c04f5..8b1c7b699982 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
mutex_lock(&mem_id_lock);
- xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
- if (!xa) {
- mutex_unlock(&mem_id_lock);
- return;
- }
-
- err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
- WARN_ON(err);
-
- call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
+ xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
+ if (xa && !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
+ call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
mutex_unlock(&mem_id_lock);
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH bpf-next V2] net/xdp: Fix suspicious RCU usage warning
From: Tariq Toukan @ 2018-08-13 9:24 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Tariq Toukan
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eran Ben Elisha
In-Reply-To: <20180813111354.1a651bd8@redhat.com>
On 13/08/2018 12:13 PM, Jesper Dangaard Brouer wrote:
> On Mon, 13 Aug 2018 10:04:39 +0300
> Tariq Toukan <tariqt@mellanox.com> wrote:
>
>> Fix the warning below by calling rhashtable_lookup_fast.
>> Also, make some code movements for better quality and human
>> readability.
>>
>> [ 342.450870] WARNING: suspicious RCU usage
>> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
>> [ 342.462210] -----------------------------
>> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
>> [ 342.476568]
>> [ 342.476568] other info that might help us debug this:
>> [ 342.476568]
>> [ 342.486978]
>> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
>> [ 342.495211] 4 locks held by modprobe/3934:
>> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
>> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
>> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
>> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
>> [mlx5_core]
>> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
>> [ 342.541206]
>> [ 342.541206] stack backtrace:
>> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
>> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
>> [ 342.565606] Call Trace:
>> [ 342.568861] dump_stack+0x78/0xb3
>> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
>> [ 342.578285] ? __call_rcu+0x220/0x300
>> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
>> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
>> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
>> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
>> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
>> [ 342.613005] __dev_close_many+0xb1/0x120
>> [ 342.617911] dev_close_many+0xa2/0x170
>> [ 342.622622] rollback_registered_many+0x148/0x460
>> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
>> [ 342.633498] ? unregister_netdev+0xe/0x20
>> [ 342.638495] rollback_registered+0x56/0x90
>> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
>> [ 342.649461] unregister_netdev+0x18/0x20
>> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
>> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
>> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
>> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
>> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
>> [ 342.683725] ? do_syscall_64+0x1c/0x210
>> [ 342.688476] do_syscall_64+0x5a/0x210
>> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>>
>> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
>> ---
>> net/core/xdp.c | 13 +++----------
>> 1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> V1 -> V2:
>> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
>> and Alexei's comments.
>>
>> Please queue to -stable v4.18.
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 3dd99e1c04f5..8b1c7b699982 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
>>
>> mutex_lock(&mem_id_lock);
>>
>> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
>> - if (!xa) {
>> - mutex_unlock(&mem_id_lock);
>> - return;
>> - }
>> -
>> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
>> - WARN_ON(err);
>> -
>> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
>> + if (xa && rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
>> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>>
>> mutex_unlock(&mem_id_lock);
>> }
>
> This is wrong.
>
> The function rhashtable_remove_fast() returns zero on success.
> Please, fix and send a V3.
>
> Look at example in [1] section "Object removal"
> [1] https://lwn.net/Articles/751374/
>
Right, thanks Jesper!
V3 Sent.
^ permalink raw reply
* Re: [PATCH bpf-next V2] net/xdp: Fix suspicious RCU usage warning
From: Daniel Borkmann @ 2018-08-13 9:26 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Tariq Toukan
Cc: Alexei Starovoitov, netdev, Eran Ben Elisha
In-Reply-To: <20180813111354.1a651bd8@redhat.com>
On 08/13/2018 11:13 AM, Jesper Dangaard Brouer wrote:
> On Mon, 13 Aug 2018 10:04:39 +0300
> Tariq Toukan <tariqt@mellanox.com> wrote:
>
>> Fix the warning below by calling rhashtable_lookup_fast.
>> Also, make some code movements for better quality and human
>> readability.
>>
>> [ 342.450870] WARNING: suspicious RCU usage
>> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
>> [ 342.462210] -----------------------------
>> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
>> [ 342.476568]
>> [ 342.476568] other info that might help us debug this:
>> [ 342.476568]
>> [ 342.486978]
>> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
>> [ 342.495211] 4 locks held by modprobe/3934:
>> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
>> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
>> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
>> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
>> [mlx5_core]
>> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
>> [ 342.541206]
>> [ 342.541206] stack backtrace:
>> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
>> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
>> [ 342.565606] Call Trace:
>> [ 342.568861] dump_stack+0x78/0xb3
>> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
>> [ 342.578285] ? __call_rcu+0x220/0x300
>> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
>> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
>> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
>> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
>> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
>> [ 342.613005] __dev_close_many+0xb1/0x120
>> [ 342.617911] dev_close_many+0xa2/0x170
>> [ 342.622622] rollback_registered_many+0x148/0x460
>> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
>> [ 342.633498] ? unregister_netdev+0xe/0x20
>> [ 342.638495] rollback_registered+0x56/0x90
>> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
>> [ 342.649461] unregister_netdev+0x18/0x20
>> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
>> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
>> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
>> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
>> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
>> [ 342.683725] ? do_syscall_64+0x1c/0x210
>> [ 342.688476] do_syscall_64+0x5a/0x210
>> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>>
>> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
>> ---
>> net/core/xdp.c | 13 +++----------
>> 1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> V1 -> V2:
>> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
>> and Alexei's comments.
>>
>> Please queue to -stable v4.18.
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 3dd99e1c04f5..8b1c7b699982 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
>>
>> mutex_lock(&mem_id_lock);
>>
>> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
>> - if (!xa) {
>> - mutex_unlock(&mem_id_lock);
>> - return;
>> - }
>> -
>> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
>> - WARN_ON(err);
>> -
>> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
>> + if (xa && rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
>> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>>
>> mutex_unlock(&mem_id_lock);
>> }
>
> This is wrong.
>
> The function rhashtable_remove_fast() returns zero on success.
> Please, fix and send a V3.
Good catch, suggestion in https://patchwork.ozlabs.org/patch/945121/ did have the
check for success, so seems this slipped through while adapting to it.
> Look at example in [1] section "Object removal"
> [1] https://lwn.net/Articles/751374/
^ permalink raw reply
* we do mobile apps
From: Jack Dike @ 2018-08-13 5:28 UTC (permalink / raw)
To: netdev
Do you need mobile apps development? We can do it for you.
We are an India base company. Here are the details about us:
Years in business: 8
Staffs: 125
App developed: 350
We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients
across different kind of industries.
Such like retail, media and entertainment, BFSI, hospitality, social media,
eCommerce, food and beverages, etc.
We do below:
Mobile Apps
Mobile App UI/UX designing
App Maintenance and Support
Website or ecommerce portal development
Please reply back if you are interested in what we do.
We will share our portfolios to you.
Regards,
Jack
^ permalink raw reply
* possible deadlock in flush_work (3)
From: syzbot @ 2018-08-13 12:35 UTC (permalink / raw)
To: christian.brauner, davem, dsahern, fw, jbenc, ktkhai,
linux-kernel, lucien.xin, netdev, nicolas.dichtel, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: d6dd6431591b Merge branch 'fixes' of git://git.kernel.org/..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14e800aa400000
kernel config: https://syzkaller.appspot.com/x/.config?x=152cb8ccd35b1f70
dashboard link: https://syzkaller.appspot.com/bug?extid=a8371264572a6872b8a3
compiler: gcc (GCC) 8.0.1 20180413 (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+a8371264572a6872b8a3@syzkaller.appspotmail.com
8021q: adding VLAN 0 to HW filter on device bond0
======================================================
WARNING: possible circular locking dependency detected
4.18.0-rc8+ #185 Not tainted
------------------------------------------------------
syz-executor2/6421 is trying to acquire lock:
00000000209b4e4b ((wq_completion)bond_dev->name){+.+.}, at:
start_flush_work kernel/workqueue.c:2888 [inline]
00000000209b4e4b ((wq_completion)bond_dev->name){+.+.}, at:
flush_work+0x4b8/0x900 kernel/workqueue.c:2917
but task is already holding lock:
00000000f0f3d47a (rtnl_mutex){+.+.}, at: rtnl_lock+0x17/0x20
net/core/rtnetlink.c:77
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (rtnl_mutex){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:757 [inline]
__mutex_lock+0x176/0x1820 kernel/locking/mutex.c:894
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:909
rtnl_lock+0x17/0x20 net/core/rtnetlink.c:77
bond_netdev_notify drivers/net/bonding/bond_main.c:1310 [inline]
bond_netdev_notify_work+0x44/0xd0
drivers/net/bonding/bond_main.c:1320
process_one_work+0xc73/0x1ba0 kernel/workqueue.c:2153
worker_thread+0x189/0x13c0 kernel/workqueue.c:2296
kthread+0x345/0x410 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
-> #1 ((work_completion)(&(&nnw->work)->work)){+.+.}:
process_one_work+0xc0b/0x1ba0 kernel/workqueue.c:2129
worker_thread+0x189/0x13c0 kernel/workqueue.c:2296
kthread+0x345/0x410 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
-> #0 ((wq_completion)bond_dev->name){+.+.}:
lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
start_flush_work kernel/workqueue.c:2889 [inline]
flush_work+0x4dd/0x900 kernel/workqueue.c:2917
__cancel_work_timer+0x4bd/0x830 kernel/workqueue.c:2989
cancel_delayed_work_sync+0x1a/0x20 kernel/workqueue.c:3121
bond_work_cancel_all drivers/net/bonding/bond_main.c:3318 [inline]
bond_close+0x1b/0x130 drivers/net/bonding/bond_main.c:3381
__dev_close_many+0x21e/0x380 net/core/dev.c:1476
__dev_close net/core/dev.c:1488 [inline]
__dev_change_flags+0x38d/0x9c0 net/core/dev.c:6989
dev_change_flags+0x89/0x150 net/core/dev.c:7060
dev_ifsioc+0x84f/0xb30 net/core/dev_ioctl.c:237
dev_ioctl+0x1b5/0xcc0 net/core/dev_ioctl.c:493
sock_do_ioctl+0x1d3/0x3e0 net/socket.c:993
sock_ioctl+0x30d/0x680 net/socket.c:1094
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:500 [inline]
do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
__do_sys_ioctl fs/ioctl.c:708 [inline]
__se_sys_ioctl fs/ioctl.c:706 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
other info that might help us debug this:
Chain exists of:
(wq_completion)bond_dev->name --> (work_completion)(&(&nnw->work)->work)
--> rtnl_mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(rtnl_mutex);
lock((work_completion)(&(&nnw->work)->work));
lock(rtnl_mutex);
lock((wq_completion)bond_dev->name);
*** DEADLOCK ***
1 lock held by syz-executor2/6421:
#0: 00000000f0f3d47a (rtnl_mutex){+.+.}, at: rtnl_lock+0x17/0x20
net/core/rtnetlink.c:77
stack backtrace:
CPU: 0 PID: 6421 Comm: syz-executor2 Not tainted 4.18.0-rc8+ #185
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
print_circular_bug.isra.36.cold.57+0x1bd/0x27d
kernel/locking/lockdep.c:1227
check_prev_add kernel/locking/lockdep.c:1867 [inline]
check_prevs_add kernel/locking/lockdep.c:1980 [inline]
validate_chain kernel/locking/lockdep.c:2421 [inline]
__lock_acquire+0x3449/0x5020 kernel/locking/lockdep.c:3435
lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
start_flush_work kernel/workqueue.c:2889 [inline]
flush_work+0x4dd/0x900 kernel/workqueue.c:2917
__cancel_work_timer+0x4bd/0x830 kernel/workqueue.c:2989
cancel_delayed_work_sync+0x1a/0x20 kernel/workqueue.c:3121
bond_work_cancel_all drivers/net/bonding/bond_main.c:3318 [inline]
bond_close+0x1b/0x130 drivers/net/bonding/bond_main.c:3381
__dev_close_many+0x21e/0x380 net/core/dev.c:1476
__dev_close net/core/dev.c:1488 [inline]
__dev_change_flags+0x38d/0x9c0 net/core/dev.c:6989
dev_change_flags+0x89/0x150 net/core/dev.c:7060
dev_ifsioc+0x84f/0xb30 net/core/dev_ioctl.c:237
dev_ioctl+0x1b5/0xcc0 net/core/dev_ioctl.c:493
sock_do_ioctl+0x1d3/0x3e0 net/socket.c:993
kernel msg: ebtables bug: please report to author: Wrong nr of counters
sock_ioctl+0x30d/0x680 net/socket.c:1094
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:500 [inline]
do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
__do_sys_ioctl fs/ioctl.c:708 [inline]
__se_sys_ioctl fs/ioctl.c:706 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457089
Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fee7b2ccc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007fee7b2cd6d4 RCX: 0000000000457089
RDX: 0000000020000140 RSI: 0000000000008914 RDI: 0000000000000004
RBP: 00000000009300a0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000004d19e0 R14: 00000000004c7454 R15: 0000000000000000
8021q: adding VLAN 0 to HW filter on device bond0
(unnamed net_device) (uninitialized): option miimon: invalid value
(18446744073709551615)
(unnamed net_device) (uninitialized): option miimon: allowed values 0 -
2147483647
(unnamed net_device) (uninitialized): option miimon: invalid value
(18446744073709551615)
(unnamed net_device) (uninitialized): option miimon: allowed values 0 -
2147483647
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
device bond0 left promiscuous mode
IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
8021q: adding VLAN 0 to HW filter on device bond0
---
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: [RFC net-next 00/15] net: A socket API for LoRa
From: Alan Cox @ 2018-08-13 12:36 UTC (permalink / raw)
To: Andreas Färber
Cc: Michal Kubeček, Konstantin Böhm, shess, Pieter ROBYNS,
contact@snootlab.com, Xue Liu, Ken Yu, Michael Röder,
Stefan Schmidt, Rob Herring, lora, Alexander Graf, Jan Jongboom,
Janus Piwek, Jon Ortego, Jian-Hong Pan, devicetree, Jiri Pirko,
Hasnain Virk, Daniele Comel, Marcel Holtmann, Mark Brown,
Dollar Chen
In-Reply-To: <11276f7d-0175-05e9-a9bd-c72868067081@suse.de>
> The simple answer is that, inspired by CAN, it uses an ifindex to select
> the interface the user asked to use. That then also answers Alan's next
> question: This ifindex determines which interface it goes out to.
>
> sockaddr_lora was in patch 02/15, latest code here:
> https://git.kernel.org/pub/scm/linux/kernel/git/afaerber/linux-lora.git/tree/include/uapi/linux/lora.h?h=lora-next
And any loopback just becomes an ifindex, likewise any virtual lorawan
device (eg when testing in a cloud or simulating radio properties).
> I still think the user will need to explicitly say which interface they
> want to bind their socket to. AFAIU the device EUI is more comparable to
It does make it very hard for any vaguely complex environment because if
for example you have two interfaces enumerated via USB they will appear
in random order each boot.
CANbus is a bit of a mess in this sense but it's so statically configured
and embedded into industrial devices it's less of a problem.
I just wonder if the name would be a better binding (so you can sort the
order out), or a local physical identifier of some kind so that your
enumeration is consistent.
> Loopback mode would require a separate virtual device driver such as
> fakelr or vlora.
And a tunnel device, which is easy enough if you've got tap support or
similar.
Alan
^ permalink raw reply
* Re: [PATCH] mt76: Enable NL80211_EXT_FEATURE_CQM_RSSI_LIST
From: Arend van Spriel @ 2018-08-13 12:39 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Kalle Valo, kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w,
linux-wireless, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAJ0CqmVZNAnHwnU4YqRRv7R8DyHi2MiFJr1bvQDPVszUU5Ez3g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 8/13/2018 12:55 PM, Lorenzo Bianconi wrote:
>> On 8/12/2018 8:14 PM, Kalle Valo wrote:
>>> Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>>>
>>>> Enable the use of CQM with mt76-devices.
>>>>
>>>> Signed-off-by: Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>> drivers/net/wireless/mediatek/mt76/mac80211.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
>>>> index 029d54bc..3eb328ff 100644
>>>> --- a/drivers/net/wireless/mediatek/mt76/mac80211.c
>>>> +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
>>>> @@ -305,6 +305,8 @@ int mt76_register_device(struct mt76_dev *dev, bool vht,
>>>>
>>>> wiphy->features |= NL80211_FEATURE_ACTIVE_MONITOR;
>>>>
>>>> + wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
>>>
>>> So have you tested this and with what devices? For example, does it work
>>> with recently added USB devices?
>>
>> I was looking into this as it looks suspicious to me. From reading the
>> description of this ext_feature flag it seems this is an extention of CQM:
>>
>> """
>> * @NL80211_EXT_FEATURE_CQM_RSSI_LIST: With this driver the
>> * %NL80211_ATTR_CQM_RSSI_THOLD attribute accepts a list of zero or more
>> * RSSI threshold values to monitor rather than exactly one threshold.
>> """
>>
>> Also looking at mt76x2_bss_info_changed() it does not handle
>> BSS_CHANGED_CQM so I doubt it has support for it (yet). The driver does
>> not use IEEE80211_VIF_SUPPORTS_CQM_RSSI which is a requirement for it.
>>
>> Regards,
>> Arend
>>
>
> According to my understanding (please correct me if I am wrong)
> BSS_CHANGED_CQM is only needed if CQM_RSSI is handled
> by the driver/fw, while if it is not set mac80211 will take care of that
> in ieee80211_handle_beacon_sig routine.
Yeah. That explains it. Seems like mac80211 could actually set the
NL80211_EXT_FEATURE_CQM_RSSI_LIST flag is the driver does not set
IEEE80211_VIF_SUPPORTS_CQM_RSSI. That way all mac80211 drivers support
the list. Just a problem as the ext_feature is not a per-vif flag.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH bpf-next V3] net/xdp: Fix suspicious RCU usage warning
From: Jesper Dangaard Brouer @ 2018-08-13 10:31 UTC (permalink / raw)
To: Tariq Toukan
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eran Ben Elisha,
brouer, Neil Brown, Paul E. McKenney
In-Reply-To: <1534152118-15968-1-git-send-email-tariqt@mellanox.com>
On Mon, 13 Aug 2018 12:21:58 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:
> Fix the warning below by calling rhashtable_lookup_fast.
> Also, make some code movements for better quality and human
> readability.
>
> [ 342.450870] WARNING: suspicious RCU usage
> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
> [ 342.462210] -----------------------------
> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
> [ 342.476568]
> [ 342.476568] other info that might help us debug this:
> [ 342.476568]
> [ 342.486978]
> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
> [ 342.495211] 4 locks held by modprobe/3934:
> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
> [mlx5_core]
> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
> [ 342.541206]
> [ 342.541206] stack backtrace:
> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
> [ 342.565606] Call Trace:
> [ 342.568861] dump_stack+0x78/0xb3
> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
> [ 342.578285] ? __call_rcu+0x220/0x300
> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
> [ 342.613005] __dev_close_many+0xb1/0x120
> [ 342.617911] dev_close_many+0xa2/0x170
> [ 342.622622] rollback_registered_many+0x148/0x460
> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
> [ 342.633498] ? unregister_netdev+0xe/0x20
> [ 342.638495] rollback_registered+0x56/0x90
> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
> [ 342.649461] unregister_netdev+0x18/0x20
> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
> [ 342.683725] ? do_syscall_64+0x1c/0x210
> [ 342.688476] do_syscall_64+0x5a/0x210
> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> net/core/xdp.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> V2 -> V3:
> * Fix return value test for rhashtable_remove_fast, per Jesper's comment.
>
> V1 -> V2:
> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
> and Alexei's comments.
>
> Please queue to -stable v4.18.
>
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 3dd99e1c04f5..8b1c7b699982 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
>
> mutex_lock(&mem_id_lock);
>
> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
> - if (!xa) {
> - mutex_unlock(&mem_id_lock);
> - return;
> - }
> -
> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
> - WARN_ON(err);
> -
> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
> + if (xa && !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>
> mutex_unlock(&mem_id_lock);
> }
I would personally prefer to write it as in the example as "== 0", look
at example in [1] section "Object removal", but is it semantically the
same to write !rhashtable_remove_fast(). So, I'm fine with this.
In the example[1], the sequence is wrapped in rcu_read_lock/unlock,
while you have not done so. The rhashtable_lookup_fast and
rhashtable_remove_fast calls have their own rcu_read_lock/unlock, but
the outer rcu_read_lock/unlock, makes sure that a RCU period cannot
slip in between the two calls.
I still think your fix is correct, due to the mutex_lock. Given the
mutex sync removal and insert in this rhashtable.
I do wonder if it would be better to add the outer rcu_read_lock/unlock,
calls if someone else reads and copy-paste this code (and don't have an
mutex sync scheme) ?
If you think this is all fine, and want to proceed as is then you have
my ack:
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
[1] https://lwn.net/Articles/751374/
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net 0/2] net_sched: Fix two tc_index filter init issues
From: Hangbin Liu @ 2018-08-13 10:44 UTC (permalink / raw)
To: netdev; +Cc: David Miller, WANG Cong, Hangbin Liu
These two patches fix two tc_index filter init issues. The first one fixes
missing exts info in new filter, which will cause NULL pointer dereference
when delete tcindex filter. The second one fixes missing res info when create
new filter, which will make filter unbind failed.
Hangbin Liu (2):
net_sched: fix NULL pointer dereference when delete tcindex filter
net_sched: Fix missing res info when create new filter
net/sched/cls_tcindex.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
--
2.5.5
^ permalink raw reply
* [PATCH net 1/2] net_sched: fix NULL pointer dereference when delete tcindex filter
From: Hangbin Liu @ 2018-08-13 10:44 UTC (permalink / raw)
To: netdev; +Cc: David Miller, WANG Cong, Hangbin Liu
In-Reply-To: <1534157044-19753-1-git-send-email-liuhangbin@gmail.com>
Li Shuang reported the following crash:
[ 71.267724] BUG: unable to handle kernel NULL pointer dereference at 0000000000000004
[ 71.276456] PGD 800000085d9bd067 P4D 800000085d9bd067 PUD 859a0b067 PMD 0
[ 71.284127] Oops: 0000 [#1] SMP PTI
[ 71.288015] CPU: 12 PID: 2386 Comm: tc Not tainted 4.18.0-rc8.latest+ #131
[ 71.295686] Hardware name: Dell Inc. PowerEdge R730/0WCJNT, BIOS 2.1.5 04/11/2016
[ 71.304037] RIP: 0010:tcindex_delete+0x72/0x280 [cls_tcindex]
[ 71.310446] Code: 00 31 f6 48 87 75 20 48 85 f6 74 11 48 8b 47 18 48 8b 40 08 48 8b 40 50 e8 fb a6 f8 fc 48 85 db 0f 84 dc 00 00 00 48 8b 73 18 <8b> 56 04 48 8d 7e 04 85 d2 0f 84 7b 01 00
[ 71.331517] RSP: 0018:ffffb45207b3f898 EFLAGS: 00010282
[ 71.337345] RAX: ffff8ad3d72d6360 RBX: ffff8acc84393680 RCX: 000000000000002e
[ 71.345306] RDX: ffff8ad3d72c8570 RSI: 0000000000000000 RDI: ffff8ad847a45800
[ 71.353277] RBP: ffff8acc84393688 R08: ffff8ad3d72c8400 R09: 0000000000000000
[ 71.361238] R10: ffff8ad3de786e00 R11: 0000000000000000 R12: ffffb45207b3f8c7
[ 71.369199] R13: ffff8ad3d93bd2a0 R14: 000000000000002e R15: ffff8ad3d72c9600
[ 71.377161] FS: 00007f9d3ec3e740(0000) GS:ffff8ad3df980000(0000) knlGS:0000000000000000
[ 71.386188] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 71.392597] CR2: 0000000000000004 CR3: 0000000852f06003 CR4: 00000000001606e0
[ 71.400558] Call Trace:
[ 71.403299] tcindex_destroy_element+0x25/0x40 [cls_tcindex]
[ 71.409611] tcindex_walk+0xbb/0x110 [cls_tcindex]
[ 71.414953] tcindex_destroy+0x44/0x90 [cls_tcindex]
[ 71.420492] ? tcindex_delete+0x280/0x280 [cls_tcindex]
[ 71.426323] tcf_proto_destroy+0x16/0x40
[ 71.430696] tcf_chain_flush+0x51/0x70
[ 71.434876] tcf_block_put_ext.part.30+0x8f/0x1b0
[ 71.440122] tcf_block_put+0x4d/0x70
[ 71.444108] cbq_destroy+0x4d/0xd0 [sch_cbq]
[ 71.448869] qdisc_destroy+0x62/0x130
[ 71.452951] dsmark_destroy+0x2a/0x70 [sch_dsmark]
[ 71.458300] qdisc_destroy+0x62/0x130
[ 71.462373] qdisc_graft+0x3ba/0x470
[ 71.466359] tc_get_qdisc+0x2a6/0x2c0
[ 71.470443] ? cred_has_capability+0x7d/0x130
[ 71.475307] rtnetlink_rcv_msg+0x263/0x2d0
[ 71.479875] ? rtnl_calcit.isra.30+0x110/0x110
[ 71.484832] netlink_rcv_skb+0x4d/0x130
[ 71.489109] netlink_unicast+0x1a3/0x250
[ 71.493482] netlink_sendmsg+0x2ae/0x3a0
[ 71.497859] sock_sendmsg+0x36/0x40
[ 71.501748] ___sys_sendmsg+0x26f/0x2d0
[ 71.506029] ? handle_pte_fault+0x586/0xdf0
[ 71.510694] ? __handle_mm_fault+0x389/0x500
[ 71.515457] ? __sys_sendmsg+0x5e/0xa0
[ 71.519636] __sys_sendmsg+0x5e/0xa0
[ 71.523626] do_syscall_64+0x5b/0x180
[ 71.527711] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 71.533345] RIP: 0033:0x7f9d3e257f10
[ 71.537331] Code: c3 48 8b 05 82 6f 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d 8d d0 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8
[ 71.558401] RSP: 002b:00007fff6f893398 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[ 71.566848] RAX: ffffffffffffffda RBX: 000000005b71274d RCX: 00007f9d3e257f10
[ 71.574810] RDX: 0000000000000000 RSI: 00007fff6f8933e0 RDI: 0000000000000003
[ 71.582770] RBP: 00007fff6f8933e0 R08: 000000000000ffff R09: 0000000000000003
[ 71.590729] R10: 00007fff6f892e20 R11: 0000000000000246 R12: 0000000000000000
[ 71.598689] R13: 0000000000662ee0 R14: 0000000000000000 R15: 0000000000000000
[ 71.606651] Modules linked in: sch_cbq cls_tcindex sch_dsmark xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_coni
[ 71.685425] libahci i2c_algo_bit i2c_core i40e libata dca mdio megaraid_sas dm_mirror dm_region_hash dm_log dm_mod
[ 71.697075] CR2: 0000000000000004
[ 71.700792] ---[ end trace f604eb1acacd978b ]---
Reproducer:
tc qdisc add dev lo handle 1:0 root dsmark indices 64 set_tc_index
tc filter add dev lo parent 1:0 protocol ip prio 1 tcindex mask 0xfc shift 2
tc qdisc add dev lo parent 1:0 handle 2:0 cbq bandwidth 10Mbit cell 8 avpkt 1000 mpu 64
tc class add dev lo parent 2:0 classid 2:1 cbq bandwidth 10Mbit rate 1500Kbit avpkt 1000 prio 1 bounded isolated allot 1514 weight 1 maxburst 10
tc filter add dev lo parent 2:0 protocol ip prio 1 handle 0x2e tcindex classid 2:1 pass_on
tc qdisc add dev lo parent 2:1 pfifo limit 5
tc qdisc del dev lo root
This is because in tcindex_set_parms, when there is no old_r, we set new
exts to cr.exts. And we didn't set it to filter when r == &new_filter_result.
Then in tcindex_delete() -> tcf_exts_get_net(), we will get NULL pointer
dereference as we didn't init exts.
Fix it by moving tcf_exts_change() after "if (old_r && old_r != r)" check.
Then we don't need "cr" as there is no errout after that.
Fixes: bf63ac73b3e13 ("net_sched: fix an oops in tcindex filter")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/sched/cls_tcindex.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 32f4bbd..ddaa4e6 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -447,11 +447,6 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
tcf_bind_filter(tp, &cr.res, base);
}
- if (old_r)
- tcf_exts_change(&r->exts, &e);
- else
- tcf_exts_change(&cr.exts, &e);
-
if (old_r && old_r != r) {
err = tcindex_filter_result_init(old_r);
if (err < 0) {
@@ -462,6 +457,8 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
oldp = p;
r->res = cr.res;
+ tcf_exts_change(&r->exts, &e);
+
rcu_assign_pointer(tp->root, cp);
if (r == &new_filter_result) {
--
2.5.5
^ permalink raw reply related
* [PATCH net 2/2] net_sched: Fix missing res info when create new tc_index filter
From: Hangbin Liu @ 2018-08-13 10:44 UTC (permalink / raw)
To: netdev; +Cc: David Miller, WANG Cong, Hangbin Liu
In-Reply-To: <1534157044-19753-1-git-send-email-liuhangbin@gmail.com>
Li Shuang reported the following warn:
[ 733.484610] WARNING: CPU: 6 PID: 21123 at net/sched/sch_cbq.c:1418 cbq_destroy_class+0x5d/0x70 [sch_cbq]
[ 733.495190] Modules linked in: sch_cbq cls_tcindex sch_dsmark rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat l
[ 733.574155] syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm igb ixgbe ahci libahci i2c_algo_bit libata i40e i2c_core dca mdio megaraid_sas dm_mirror dm_region_hash dm_log dm_mod
[ 733.592500] CPU: 6 PID: 21123 Comm: tc Not tainted 4.18.0-rc8.latest+ #131
[ 733.600169] Hardware name: Dell Inc. PowerEdge R730/0WCJNT, BIOS 2.1.5 04/11/2016
[ 733.608518] RIP: 0010:cbq_destroy_class+0x5d/0x70 [sch_cbq]
[ 733.614734] Code: e7 d9 d2 48 8b 7b 48 e8 61 05 da d2 48 8d bb f8 00 00 00 e8 75 ae d5 d2 48 39 eb 74 0a 48 89 df 5b 5d e9 16 6c 94 d2 5b 5d c3 <0f> 0b eb b6 0f 1f 44 00 00 66 2e 0f 1f 84
[ 733.635798] RSP: 0018:ffffbfbb066bb9d8 EFLAGS: 00010202
[ 733.641627] RAX: 0000000000000001 RBX: ffff9cdd17392800 RCX: 000000008010000f
[ 733.649588] RDX: ffff9cdd1df547e0 RSI: ffff9cdd17392800 RDI: ffff9cdd0f84c800
[ 733.657547] RBP: ffff9cdd0f84c800 R08: 0000000000000001 R09: 0000000000000000
[ 733.665508] R10: ffff9cdd0f84d000 R11: 0000000000000001 R12: 0000000000000001
[ 733.673469] R13: 0000000000000000 R14: 0000000000000001 R15: ffff9cdd17392200
[ 733.681430] FS: 00007f911890a740(0000) GS:ffff9cdd1f8c0000(0000) knlGS:0000000000000000
[ 733.690456] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 733.696864] CR2: 0000000000b5544c CR3: 0000000859374002 CR4: 00000000001606e0
[ 733.704826] Call Trace:
[ 733.707554] cbq_destroy+0xa1/0xd0 [sch_cbq]
[ 733.712318] qdisc_destroy+0x62/0x130
[ 733.716401] dsmark_destroy+0x2a/0x70 [sch_dsmark]
[ 733.721745] qdisc_destroy+0x62/0x130
[ 733.725829] qdisc_graft+0x3ba/0x470
[ 733.729817] tc_get_qdisc+0x2a6/0x2c0
[ 733.733901] ? cred_has_capability+0x7d/0x130
[ 733.738761] rtnetlink_rcv_msg+0x263/0x2d0
[ 733.743330] ? rtnl_calcit.isra.30+0x110/0x110
[ 733.748287] netlink_rcv_skb+0x4d/0x130
[ 733.752576] netlink_unicast+0x1a3/0x250
[ 733.756949] netlink_sendmsg+0x2ae/0x3a0
[ 733.761324] sock_sendmsg+0x36/0x40
[ 733.765213] ___sys_sendmsg+0x26f/0x2d0
[ 733.769493] ? handle_pte_fault+0x586/0xdf0
[ 733.774158] ? __handle_mm_fault+0x389/0x500
[ 733.778919] ? __sys_sendmsg+0x5e/0xa0
[ 733.783099] __sys_sendmsg+0x5e/0xa0
[ 733.787087] do_syscall_64+0x5b/0x180
[ 733.791171] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 733.796805] RIP: 0033:0x7f9117f23f10
[ 733.800791] Code: c3 48 8b 05 82 6f 2c 00 f7 db 64 89 18 48 83 cb ff eb dd 0f 1f 80 00 00 00 00 83 3d 8d d0 2c 00 00 75 10 b8 2e 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8
[ 733.821873] RSP: 002b:00007ffe96818398 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[ 733.830319] RAX: ffffffffffffffda RBX: 000000005b71244c RCX: 00007f9117f23f10
[ 733.838280] RDX: 0000000000000000 RSI: 00007ffe968183e0 RDI: 0000000000000003
[ 733.846241] RBP: 00007ffe968183e0 R08: 000000000000ffff R09: 0000000000000003
[ 733.854202] R10: 00007ffe96817e20 R11: 0000000000000246 R12: 0000000000000000
[ 733.862161] R13: 0000000000662ee0 R14: 0000000000000000 R15: 0000000000000000
[ 733.870121] ---[ end trace 28edd4aad712ddca ]---
This is because we didn't update f->result.res when create new filter. Then in
tcindex_delete() -> tcf_unbind_filter(), we will failed to find out the res
and unbind filter, which will trigger the WARN_ON() in cbq_destroy_class().
Fix it by updating f->result.res when create new filter.
Fixes: 6e0565697a106 ("net_sched: fix another crash in cls_tcindex")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/sched/cls_tcindex.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index ddaa4e6..9ccc93f 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -465,6 +465,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
struct tcindex_filter *nfp;
struct tcindex_filter __rcu **fp;
+ f->result.res = r->res;
tcf_exts_change(&f->result.exts, &r->exts);
fp = cp->h + (handle % cp->hash);
--
2.5.5
^ permalink raw reply related
* Re: [PATCH bpf-next V3] net/xdp: Fix suspicious RCU usage warning
From: Tariq Toukan @ 2018-08-13 10:57 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Tariq Toukan
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eran Ben Elisha,
Neil Brown, Paul E. McKenney
In-Reply-To: <20180813123159.1f447108@redhat.com>
On 13/08/2018 1:31 PM, Jesper Dangaard Brouer wrote:
> On Mon, 13 Aug 2018 12:21:58 +0300
> Tariq Toukan <tariqt@mellanox.com> wrote:
>
>> Fix the warning below by calling rhashtable_lookup_fast.
>> Also, make some code movements for better quality and human
>> readability.
>>
>> [ 342.450870] WARNING: suspicious RCU usage
>> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
>> [ 342.462210] -----------------------------
>> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
>> [ 342.476568]
>> [ 342.476568] other info that might help us debug this:
>> [ 342.476568]
>> [ 342.486978]
>> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
>> [ 342.495211] 4 locks held by modprobe/3934:
>> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
>> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
>> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
>> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
>> [mlx5_core]
>> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
>> [ 342.541206]
>> [ 342.541206] stack backtrace:
>> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
>> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
>> [ 342.565606] Call Trace:
>> [ 342.568861] dump_stack+0x78/0xb3
>> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
>> [ 342.578285] ? __call_rcu+0x220/0x300
>> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
>> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
>> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
>> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
>> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
>> [ 342.613005] __dev_close_many+0xb1/0x120
>> [ 342.617911] dev_close_many+0xa2/0x170
>> [ 342.622622] rollback_registered_many+0x148/0x460
>> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
>> [ 342.633498] ? unregister_netdev+0xe/0x20
>> [ 342.638495] rollback_registered+0x56/0x90
>> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
>> [ 342.649461] unregister_netdev+0x18/0x20
>> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
>> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
>> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
>> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
>> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
>> [ 342.683725] ? do_syscall_64+0x1c/0x210
>> [ 342.688476] do_syscall_64+0x5a/0x210
>> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>>
>> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
>> ---
>> net/core/xdp.c | 13 +++----------
>> 1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> V2 -> V3:
>> * Fix return value test for rhashtable_remove_fast, per Jesper's comment.
>>
>> V1 -> V2:
>> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
>> and Alexei's comments.
>>
>> Please queue to -stable v4.18.
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 3dd99e1c04f5..8b1c7b699982 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
>>
>> mutex_lock(&mem_id_lock);
>>
>> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
>> - if (!xa) {
>> - mutex_unlock(&mem_id_lock);
>> - return;
>> - }
>> -
>> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
>> - WARN_ON(err);
>> -
>> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
>> + if (xa && !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
>> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>>
>> mutex_unlock(&mem_id_lock);
>> }
>
> I would personally prefer to write it as in the example as "== 0", look
> at example in [1] section "Object removal", but is it semantically the
> same to write !rhashtable_remove_fast(). So, I'm fine with this.
>
I thought the coding convention is to not explicitly compare to zero,
just like we do not compare to NULL on page allocation, but do:
if (!page)
But I don't mind changing this one.
> In the example[1], the sequence is wrapped in rcu_read_lock/unlock,
> while you have not done so. The rhashtable_lookup_fast and
> rhashtable_remove_fast calls have their own rcu_read_lock/unlock, but
> the outer rcu_read_lock/unlock, makes sure that a RCU period cannot
> slip in between the two calls.
>
> I still think your fix is correct, due to the mutex_lock. Given the
> mutex sync removal and insert in this rhashtable.
>
Right, we rely here on the mutex to avoid the scenario you described.
So the outer rcu lock calls are not necessary.
> I do wonder if it would be better to add the outer rcu_read_lock/unlock,
> calls if someone else reads and copy-paste this code (and don't have an
> mutex sync scheme) ?
>
Yeah it'll be safer for future unaware developers, but I think reviewers
should always comment and make it clear that the best generic reference
is [1], not any specific/optimized use case.
If you guys still want to me to fix this then please let me know and
I'll re-spin.
> If you think this is all fine, and want to proceed as is then you have
> my ack:
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
>
> [1] https://lwn.net/Articles/751374/
>
^ permalink raw reply
* Re: [Query]: DSA Understanding
From: Lad, Prabhakar @ 2018-08-13 11:06 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, netdev
In-Reply-To: <d54e8489-6ba0-2ad9-4eb5-c8d3f847f34a@gmail.com>
Hi Florian,
On Fri, Aug 10, 2018 at 6:36 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 08/10/2018 04:26 AM, Lad, Prabhakar wrote:
> > Hi Andrew,
> >
> > On Thu, Aug 9, 2018 at 6:23 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >>
> >>> Its coming from the switch lan4 I have attached the png, where
> >>> C4:F3:12:08:FE:7F is
> >>> the mac of lan4, which is broadcast to ff:ff:ff:ff:ff:ff, which is
> >>> causing rx counter on
> >>> PC to go up.
> >>
> >> So, big packets are making it from the switch to the PC. But the small
> >> ARP packets are not.
> >>
> >> This is what Florian was suggesting.
> >>
> >> ARP packets are smaller than 64 bytes, which is the minimum packet
> >> size for Ethernet. Any packets smaller than 64 bytes are called runt
> >> packets. They have to be padded upto 64 bytes in order to make them
> >> valid. Otherwise the destination, or any switch along the path, might
> >> throw them away.
> >>
> >> What could be happening is that the CSPW driver or hardware is padding
> >> the packet to 64 bytes. But that packet has a DSA header in it. The
> >> switch removes the header, recalculate the checksum and sends the
> >> packet. It is now either 4 or 8 bytes smaller, depending on what DSA
> >> header was used. It then becomes a runt packet.
> >>
> > Thank you for the clarification, this really helped me out.
> >
> >> Florian had to fix this problem recently.
> >>
> >> http://patchwork.ozlabs.org/patch/836534/
> >>
> > But seems like this patch was never accepted, instead
> > brcm_tag_xmit_ll() does it if I am understanding it correctly.
> > similarly to this ksz_xmit() is taking care of padding.
>
> net/dsa/tag_brcm.c ended up doing the padding because that was a more
> generic and central location:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/net/dsa/tag_brcm.c#n73
>
> >
> >> You probably need something similar for the cpsw.
> >>
> > looking at the tag_ksz.c in xmit function this is taken care of
>
> I agree, this should be padding packets correctly, can you still
> instrument cpsw to make sure that what comes to its ndo_start_xmit() is
> ETH_ZLEN + tag_len or more?
>
Yes I can confirm the skb->len is always >= 62 (ETH_ZLEN + 2)
Cheers,
--Prabhakar
^ permalink raw reply
* [PATCH][bpf-next] bpf: test: fix spelling mistake "REUSEEPORT" -> "REUSEPORT"
From: Colin King @ 2018-08-13 14:00 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Shuah Khan, netdev,
linux-kselftest
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Trivial fix to spelling mistake in error message
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
tools/testing/selftests/bpf/test_maps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 4b7c74f5faa7..6f54f84144a0 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -1191,7 +1191,7 @@ static void prepare_reuseport_grp(int type, int map_fd,
err = setsockopt(fd64, SOL_SOCKET, SO_REUSEPORT,
&optval, sizeof(optval));
- CHECK(err == -1, "setsockopt(SO_REUSEEPORT)",
+ CHECK(err == -1, "setsockopt(SO_REUSEPORT)",
"err:%d errno:%d\n", err, errno);
/* reuseport_array does not allow unbound sk */
--
2.17.1
^ permalink raw reply related
* [PATCH v2 iproute2-next] sch_cake: Make gso-splitting configurable
From: Toke Høiland-Jørgensen @ 2018-08-13 11:36 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Toke Høiland-Jørgensen, Dave Taht
This patch makes sch_cake's gso/gro splitting configurable
from userspace.
To disable breaking apart superpackets in sch_cake:
tc qdisc replace dev whatever root cake no-split-gso
to enable:
tc qdisc replace dev whatever root cake split-gso
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
Signed-off-by: Dave Taht <dave.taht@gmail.com>
---
v2:
- Fix x-mas tree variable order
- Also update man page
man/man8/tc-cake.8 | 23 +++++++++++++++++++++++
tc/q_cake.c | 9 +++++++++
2 files changed, 32 insertions(+)
diff --git a/man/man8/tc-cake.8 b/man/man8/tc-cake.8
index 0e84bc6e..b8e3089b 100644
--- a/man/man8/tc-cake.8
+++ b/man/man8/tc-cake.8
@@ -73,6 +73,12 @@ TIME |
]
.br
[
+.BR split-gso*
+|
+.BR no-split-gso
+]
+.br
+[
.BR ack-filter
|
.BR ack-filter-aggressive
@@ -546,6 +552,23 @@ If you are shaping inbound, and cannot trust the diffserv markings (as is the
case for Comcast Cable, among others), it is best to use a single queue
"besteffort" mode with wash.
+.PP
+.B split-gso
+
+.br
+ This option controls whether CAKE will split General Segmentation
+Offload (GSO) super-packets into their on-the-wire components and
+dequeue them individually.
+
+.br
+Super-packets are created by the networking stack to improve efficiency.
+However, because they are larger they take longer to dequeue, which
+translates to higher latency for competing flows, especially at lower
+bandwidths. CAKE defaults to splitting GSO packets to achieve the lowest
+possible latency. At link speeds higher than 10 Gbps, setting the
+no-split-gso parameter can increase the maximum achievable throughput by
+retaining the full GSO packets.
+
.SH EXAMPLES
# tc qdisc delete root dev eth0
.br
diff --git a/tc/q_cake.c b/tc/q_cake.c
index f1e232a6..50de46a7 100644
--- a/tc/q_cake.c
+++ b/tc/q_cake.c
@@ -79,6 +79,7 @@ static void explain(void)
" dual-srchost | dual-dsthost | triple-isolate* ]\n"
" [ nat | nonat* ]\n"
" [ wash | nowash* ]\n"
+" [ split-gso* | no-split-gso ]\n"
" [ ack-filter | ack-filter-aggressive | no-ack-filter* ]\n"
" [ memlimit LIMIT ]\n"
" [ ptm | atm | noatm* ] [ overhead N | conservative | raw* ]\n"
@@ -99,6 +100,7 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, char **argv,
__u64 bandwidth = 0;
int ack_filter = -1;
struct rtattr *tail;
+ int split_gso = -1;
int unlimited = 0;
int flowmode = -1;
int autorate = -1;
@@ -155,6 +157,10 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, char **argv,
wash = 0;
} else if (strcmp(*argv, "wash") == 0) {
wash = 1;
+ } else if (strcmp(*argv, "split-gso") == 0) {
+ split_gso = 1;
+ } else if (strcmp(*argv, "no-split-gso") == 0) {
+ split_gso = 0;
} else if (strcmp(*argv, "flowblind") == 0) {
flowmode = CAKE_FLOW_NONE;
} else if (strcmp(*argv, "srchost") == 0) {
@@ -374,6 +380,9 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, char **argv,
addattr_l(n, 1024, TCA_CAKE_NAT, &nat, sizeof(nat));
if (wash != -1)
addattr_l(n, 1024, TCA_CAKE_WASH, &wash, sizeof(wash));
+ if (split_gso != -1)
+ addattr_l(n, 1024, TCA_CAKE_SPLIT_GSO, &split_gso,
+ sizeof(split_gso));
if (ingress != -1)
addattr_l(n, 1024, TCA_CAKE_INGRESS, &ingress, sizeof(ingress));
if (ack_filter != -1)
--
2.18.0
^ permalink raw reply related
* Re: [PATCH] net: Change the layout of structure trace_event_raw_fib_table_lookup
From: David Ahern @ 2018-08-13 14:40 UTC (permalink / raw)
To: Zong Li, rostedt, mingo, netdev, linux-kernel; +Cc: zongbox, greentime
In-Reply-To: <1534127212-13186-1-git-send-email-zong@andestech.com>
On 8/12/18 8:26 PM, Zong Li wrote:
> There is an unalignment access about the structure
> 'trace_event_raw_fib_table_lookup'.
>
> In include/trace/events/fib.h, there is a memory operation which casting
> the 'src' data member to a pointer, and then store a value to this
> pointer point to.
>
> p32 = (__be32 *) __entry->src;
> *p32 = flp->saddr;
>
> The offset of 'src' in structure trace_event_raw_fib_table_lookup is not
> four bytes alignment. On some architectures, they don't permit the
> unalignment access, it need to pay the price to handle this situation in
> exception handler.
>
> Adjust the layout of structure to avoid this case.
>
> Signed-off-by: Zong Li <zong@andestech.com>
> ---
> include/trace/events/fib.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Fixes: 9f323973c915 ("net/ipv4: Udate fib_table_lookup tracepoint")
Acked-by: David Ahern <dsahern@gmail.com>
Do you also need a similar change to fib6_table_lookup in
include/trace/events/fib6.h?
^ permalink raw reply
* Re: [PATCH bpf-next V3] net/xdp: Fix suspicious RCU usage warning
From: Jesper Dangaard Brouer @ 2018-08-13 12:02 UTC (permalink / raw)
To: Tariq Toukan
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eran Ben Elisha,
Neil Brown, Paul E. McKenney, brouer
In-Reply-To: <85462631-9e86-5681-3ee7-558079d4776c@mellanox.com>
On Mon, 13 Aug 2018 13:57:04 +0300
Tariq Toukan <tariqt@mellanox.com> wrote:
> On 13/08/2018 1:31 PM, Jesper Dangaard Brouer wrote:
> > On Mon, 13 Aug 2018 12:21:58 +0300
> > Tariq Toukan <tariqt@mellanox.com> wrote:
> >
> >> Fix the warning below by calling rhashtable_lookup_fast.
> >> Also, make some code movements for better quality and human
> >> readability.
> >>
> >> [ 342.450870] WARNING: suspicious RCU usage
> >> [ 342.455856] 4.18.0-rc2+ #17 Tainted: G O
> >> [ 342.462210] -----------------------------
> >> [ 342.467202] ./include/linux/rhashtable.h:481 suspicious rcu_dereference_check() usage!
> >> [ 342.476568]
> >> [ 342.476568] other info that might help us debug this:
> >> [ 342.476568]
> >> [ 342.486978]
> >> [ 342.486978] rcu_scheduler_active = 2, debug_locks = 1
> >> [ 342.495211] 4 locks held by modprobe/3934:
> >> [ 342.500265] #0: 00000000e23116b2 (mlx5_intf_mutex){+.+.}, at:
> >> mlx5_unregister_interface+0x18/0x90 [mlx5_core]
> >> [ 342.511953] #1: 00000000ca16db96 (rtnl_mutex){+.+.}, at: unregister_netdev+0xe/0x20
> >> [ 342.521109] #2: 00000000a46e2c4b (&priv->state_lock){+.+.}, at: mlx5e_close+0x29/0x60
> >> [mlx5_core]
> >> [ 342.531642] #3: 0000000060c5bde3 (mem_id_lock){+.+.}, at: xdp_rxq_info_unreg+0x93/0x6b0
> >> [ 342.541206]
> >> [ 342.541206] stack backtrace:
> >> [ 342.547075] CPU: 12 PID: 3934 Comm: modprobe Tainted: G O 4.18.0-rc2+ #17
> >> [ 342.556621] Hardware name: Dell Inc. PowerEdge R730/0H21J3, BIOS 1.5.4 10/002/2015
> >> [ 342.565606] Call Trace:
> >> [ 342.568861] dump_stack+0x78/0xb3
> >> [ 342.573086] xdp_rxq_info_unreg+0x3f5/0x6b0
> >> [ 342.578285] ? __call_rcu+0x220/0x300
> >> [ 342.582911] mlx5e_free_rq+0x38/0xc0 [mlx5_core]
> >> [ 342.588602] mlx5e_close_channel+0x20/0x120 [mlx5_core]
> >> [ 342.594976] mlx5e_close_channels+0x26/0x40 [mlx5_core]
> >> [ 342.601345] mlx5e_close_locked+0x44/0x50 [mlx5_core]
> >> [ 342.607519] mlx5e_close+0x42/0x60 [mlx5_core]
> >> [ 342.613005] __dev_close_many+0xb1/0x120
> >> [ 342.617911] dev_close_many+0xa2/0x170
> >> [ 342.622622] rollback_registered_many+0x148/0x460
> >> [ 342.628401] ? __lock_acquire+0x48d/0x11b0
> >> [ 342.633498] ? unregister_netdev+0xe/0x20
> >> [ 342.638495] rollback_registered+0x56/0x90
> >> [ 342.643588] unregister_netdevice_queue+0x7e/0x100
> >> [ 342.649461] unregister_netdev+0x18/0x20
> >> [ 342.654362] mlx5e_remove+0x2a/0x50 [mlx5_core]
> >> [ 342.659944] mlx5_remove_device+0xe5/0x110 [mlx5_core]
> >> [ 342.666208] mlx5_unregister_interface+0x39/0x90 [mlx5_core]
> >> [ 342.673038] cleanup+0x5/0xbfc [mlx5_core]
> >> [ 342.678094] __x64_sys_delete_module+0x16b/0x240
> >> [ 342.683725] ? do_syscall_64+0x1c/0x210
> >> [ 342.688476] do_syscall_64+0x5a/0x210
> >> [ 342.693025] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> >>
> >> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
> >> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> >> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> >> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> >> ---
> >> net/core/xdp.c | 13 +++----------
> >> 1 file changed, 3 insertions(+), 10 deletions(-)
> >>
> >> V2 -> V3:
> >> * Fix return value test for rhashtable_remove_fast, per Jesper's comment.
> >>
> >> V1 -> V2:
> >> * Use rhashtable_lookup_fast and make some code movements, per Daniel's
> >> and Alexei's comments.
> >>
> >> Please queue to -stable v4.18.
> >>
> >> diff --git a/net/core/xdp.c b/net/core/xdp.c
> >> index 3dd99e1c04f5..8b1c7b699982 100644
> >> --- a/net/core/xdp.c
> >> +++ b/net/core/xdp.c
> >> @@ -105,16 +105,9 @@ static void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
> >>
> >> mutex_lock(&mem_id_lock);
> >>
> >> - xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
> >> - if (!xa) {
> >> - mutex_unlock(&mem_id_lock);
> >> - return;
> >> - }
> >> -
> >> - err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
> >> - WARN_ON(err);
> >> -
> >> - call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
> >> + xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
> >> + if (xa && !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
> >> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
> >>
> >> mutex_unlock(&mem_id_lock);
> >> }
> >
> > I would personally prefer to write it as in the example as "== 0", look
> > at example in [1] section "Object removal", but is it semantically the
> > same to write !rhashtable_remove_fast(). So, I'm fine with this.
> >
>
> I thought the coding convention is to not explicitly compare to zero,
> just like we do not compare to NULL on page allocation, but do:
> if (!page)
If the return value is a pointer, then I use the (!ptr) check, and also
if the return value is a bool. In this case where the success is 0, I
find it slightly confusing to read if(!remove) then success-case.
> But I don't mind changing this one.
I also don't care much... if you do respin, it would be nice to do.
> > In the example[1], the sequence is wrapped in rcu_read_lock/unlock,
> > while you have not done so. The rhashtable_lookup_fast and
> > rhashtable_remove_fast calls have their own rcu_read_lock/unlock,
> > but the outer rcu_read_lock/unlock, makes sure that a RCU period
> > cannot slip in between the two calls.
> >
> > I still think your fix is correct, due to the mutex_lock. Given the
> > mutex sync removal and insert in this rhashtable.
> >
>
> Right, we rely here on the mutex to avoid the scenario you described.
> So the outer rcu lock calls are not necessary.
>
> > I do wonder if it would be better to add the outer
> > rcu_read_lock/unlock, calls if someone else reads and copy-paste
> > this code (and don't have an mutex sync scheme) ?
> >
>
> Yeah it'll be safer for future unaware developers, but I think
> reviewers should always comment and make it clear that the best
> generic reference is [1], not any specific/optimized use case.
>
> If you guys still want to me to fix this then please let me know and
> I'll re-spin.
I'll let Daniel make the choice.
> > If you think this is all fine, and want to proceed as is then you
> > have my ack:
> >
> > Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
> >
> >
> > [1] https://lwn.net/Articles/751374/
> >
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH 2/3] IB/ipoib: Stop using dev_id to expose port numbers
From: Arseny Maslennikov @ 2018-08-13 11:42 UTC (permalink / raw)
To: linux-rdma; +Cc: Arseny Maslennikov, Doug Ledford, Jason Gunthorpe, netdev
In-Reply-To: <20180813114224.7065-1-ar@cs.msu.ru>
Some InfiniBand network devices have multiple ports on the same PCI
function. Prior to this the kernel erroneously used the `dev_id' sysfs
field of those network interfaces to convey the port number to userspace.
`dev_id' is currently reserved for distinguishing stacked ifaces
(e.g: VLANs) with the same hardware address as their parent device.
Similar fixes to net/mlx4_en and many other drivers, which started
exporting this information through `dev_id' before 3.15, were accepted
into the kernel 4 years ago.
See 76a066f2a2a0268b565459c417b59724b5a3197b, commit message:
`net/mlx4_en: Expose port number through sysfs'.
I would be OK with this commit not being backported to stable, since
it might break admin-supplied udev rules and the likes.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 6eb0594fffec..f64535038147 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -2252,7 +2252,6 @@ static struct net_device *ipoib_add_port(const char *format,
}
SET_NETDEV_DEV(priv->dev, hca->dev.parent);
- priv->dev->dev_id = port - 1;
priv->dev->dev_port = port - 1;
result = ib_query_port(hca, port, &attr);
--
2.18.0
^ permalink raw reply related
* [PATCH 3/3] Documentation/ABI: document /sys/class/net/*/dev_port
From: Arseny Maslennikov @ 2018-08-13 11:42 UTC (permalink / raw)
To: linux-rdma; +Cc: Arseny Maslennikov, Doug Ledford, Jason Gunthorpe, netdev
In-Reply-To: <20180813114224.7065-1-ar@cs.msu.ru>
The sysfs field was introduced 4 years ago along with fixes to various
drivers that erroneously used `dev_id' for that purpose, but it was not
properly documented anywhere.
See commit v3.14-rc3-739-g3f85944fe207.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
Documentation/ABI/testing/sysfs-class-net | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-class-net b/Documentation/ABI/testing/sysfs-class-net
index 2f1788111cd9..1593d8997ade 100644
--- a/Documentation/ABI/testing/sysfs-class-net
+++ b/Documentation/ABI/testing/sysfs-class-net
@@ -91,6 +91,16 @@ Description:
stacked (e.g: VLAN interfaces) but still have the same MAC
address as their parent device.
+What: /sys/class/net/<iface>/dev_port
+Date: February 2014
+KernelVersion: 3.15
+Contact: netdev@vger.kernel.org
+Description:
+ Indicates the port number of this network device, formatted
+ as a decimal value. Some NICs have multiple independent ports
+ on the same PCI bus, device and function. This field allows
+ userspace to distinguish the respective interfaces.
+
What: /sys/class/net/<iface>/dormant
Date: March 2006
KernelVersion: 2.6.17
--
2.18.0
^ permalink raw reply related
* [PATCH 1/3] IB/ipoib: Use dev_port to expose network interface port numbers
From: Arseny Maslennikov @ 2018-08-13 11:42 UTC (permalink / raw)
To: linux-rdma; +Cc: Arseny Maslennikov, Doug Ledford, Jason Gunthorpe, netdev
In-Reply-To: <20180813114224.7065-1-ar@cs.msu.ru>
Some InfiniBand network devices have multiple ports on the same PCI
function. This initializes the `dev_port' sysfs field of those
network interfaces with their port number.
The use of `dev_id' was considered correct until Linux 3.15,
when another field, `dev_port', was defined for this particular
purpose and `dev_id' was reserved for distinguishing stacked ifaces
(e.g: VLANs) with the same hardware address as their parent device.
Similar fixes to net/mlx4_en and many other drivers, which started
exporting this information through `dev_id' before 3.15, were accepted
into the kernel 4 years ago.
See 76a066f2a2a0268b565459c417b59724b5a3197b, commit message:
`net/mlx4_en: Expose port number through sysfs'.
Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 26cde95bc0f3..6eb0594fffec 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -2253,6 +2253,7 @@ static struct net_device *ipoib_add_port(const char *format,
SET_NETDEV_DEV(priv->dev, hca->dev.parent);
priv->dev->dev_id = port - 1;
+ priv->dev->dev_port = port - 1;
result = ib_query_port(hca, port, &attr);
if (result) {
--
2.18.0
^ permalink raw reply related
* [PATCH 0/3] net, IB/ipoib: Use dev_port to disambiguate
From: Arseny Maslennikov @ 2018-08-13 11:42 UTC (permalink / raw)
To: linux-rdma; +Cc: Arseny Maslennikov, Doug Ledford, Jason Gunthorpe, netdev
Pre-3.15 userspace had trouble distinguishing different ports of a NIC
on a single PCI bus/device/function. To solve this, a sysfs field `dev_port'
was introduced quite a while ago (commit v3.14-rc3-739-g3f85944fe207), and
some relevant device drivers were fixed to use it, but not in case of IPoIB.
The convention for some reason never got documented in the kernel, but
was immediately adopted by userspace (notably udev[1][2], biosdevname[3])
3/3 documents the sysfs field — that's why I'm CC-ing netdev.
This series was tested on current LTS and 4.18.
[1] https://lists.freedesktop.org/archives/systemd-devel/2014-June/020788.html
[2] https://lists.freedesktop.org/archives/systemd-devel/2014-July/020804.html
[3] https://github.com/CloudAutomationNTools/biosdevname/blob/c795d51dd93a5309652f0d635f12a3ecfabfaa72/src/eths.c#L38
Arseny Maslennikov (3):
IB/ipoib: Use dev_port to expose network interface port numbers
IB/ipoib: Stop using dev_id to expose port numbers
Documentation/ABI: document /sys/class/net/*/dev_port
Documentation/ABI/testing/sysfs-class-net | 10 ++++++++++
drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 +-
2 files changed, 11 insertions(+), 1 deletion(-)
--
2.18.0
^ permalink raw reply
* Re: [PATCH bpf-next V3] net/xdp: Fix suspicious RCU usage warning
From: Daniel Borkmann @ 2018-08-13 12:22 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Tariq Toukan
Cc: Alexei Starovoitov, netdev, Eran Ben Elisha, Neil Brown,
Paul E. McKenney
In-Reply-To: <20180813140243.4484815e@redhat.com>
On 08/13/2018 02:02 PM, Jesper Dangaard Brouer wrote:
> On Mon, 13 Aug 2018 13:57:04 +0300
> Tariq Toukan <tariqt@mellanox.com> wrote:
>> On 13/08/2018 1:31 PM, Jesper Dangaard Brouer wrote:
>>> On Mon, 13 Aug 2018 12:21:58 +0300
>>> Tariq Toukan <tariqt@mellanox.com> wrote:
[...]
>>> In the example[1], the sequence is wrapped in rcu_read_lock/unlock,
>>> while you have not done so. The rhashtable_lookup_fast and
>>> rhashtable_remove_fast calls have their own rcu_read_lock/unlock,
>>> but the outer rcu_read_lock/unlock, makes sure that a RCU period
>>> cannot slip in between the two calls.
>>>
>>> I still think your fix is correct, due to the mutex_lock. Given the
>>> mutex sync removal and insert in this rhashtable.
>>
>> Right, we rely here on the mutex to avoid the scenario you described.
>> So the outer rcu lock calls are not necessary.
Agree.
>>> I do wonder if it would be better to add the outer
>>> rcu_read_lock/unlock, calls if someone else reads and copy-paste
>>> this code (and don't have an mutex sync scheme) ?
>>
>> Yeah it'll be safer for future unaware developers, but I think
>> reviewers should always comment and make it clear that the best
>> generic reference is [1], not any specific/optimized use case.
>>
>> If you guys still want to me to fix this then please let me know and
>> I'll re-spin.
>
> I'll let Daniel make the choice.
Patch is fine as is. If we would be adding the RCU read lock/unlock pair
even though it's not necessary but for other developers to copy paste
from, I think this might be double-confusing: in the one case for people
reading the current code as they will wonder why the additional RCU read
side is needed here (so it will leave them puzzling), and in the other
case for people trying to copy-paste from it wondering whether they would
need similar scheme with mutex in addition. So I strongly prefer to 'do
the right thing' based on the situation. Given BPF PR is still pending,
I'll get the patch in once it has been pulled.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net 2/2] net/mlx5e: Cleanup of dcbnl related fields
From: Yuval Shaia @ 2018-08-13 12:32 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: David S. Miller, netdev, Huy Nguyen
In-Reply-To: <20180808224808.12600-3-saeedm@mellanox.com>
On Wed, Aug 08, 2018 at 03:48:08PM -0700, Saeed Mahameed wrote:
> From: Huy Nguyen <huyn@mellanox.com>
>
> Remove unused netdev_registered_init/remove in en.h
> Return ENOSUPPORT if the check MLX5_DSCP_SUPPORTED fails.
> Remove extra white space
>
> Fixes: 2a5e7a1344f4 ("net/mlx5e: Add dcbnl dscp to priority support")
> Signed-off-by: Huy Nguyen <huyn@mellanox.com>
> Cc: Yuval Shaia <yuval.shaia@oracle.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
FWIW:
Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
Tested-by: Yuval Shaia <yuval.shaia@oracle.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 --
> .../ethernet/mellanox/mlx5/core/en_dcbnl.c | 30 +++++++------------
> 2 files changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index eb9eb7aa953a..405236cf0b04 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -858,8 +858,6 @@ struct mlx5e_profile {
> mlx5e_fp_handle_rx_cqe handle_rx_cqe;
> mlx5e_fp_handle_rx_cqe handle_rx_cqe_mpwqe;
> } rx_handlers;
> - void (*netdev_registered_init)(struct mlx5e_priv *priv);
> - void (*netdev_registered_remove)(struct mlx5e_priv *priv);
> int max_tc;
> };
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> index e33afa8d2417..722998d68564 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
> @@ -443,16 +443,12 @@ static int mlx5e_dcbnl_ieee_setapp(struct net_device *dev, struct dcb_app *app)
> bool is_new;
> int err;
>
> - if (app->selector != IEEE_8021QAZ_APP_SEL_DSCP)
> - return -EINVAL;
> -
> - if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager))
> - return -EINVAL;
> -
> - if (!MLX5_DSCP_SUPPORTED(priv->mdev))
> - return -EINVAL;
> + if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager) ||
> + !MLX5_DSCP_SUPPORTED(priv->mdev))
> + return -EOPNOTSUPP;
>
> - if (app->protocol >= MLX5E_MAX_DSCP)
> + if ((app->selector != IEEE_8021QAZ_APP_SEL_DSCP) ||
> + (app->protocol >= MLX5E_MAX_DSCP))
> return -EINVAL;
>
> /* Save the old entry info */
> @@ -500,16 +496,12 @@ static int mlx5e_dcbnl_ieee_delapp(struct net_device *dev, struct dcb_app *app)
> struct mlx5e_priv *priv = netdev_priv(dev);
> int err;
>
> - if (app->selector != IEEE_8021QAZ_APP_SEL_DSCP)
> - return -EINVAL;
> -
> - if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager))
> - return -EINVAL;
> -
> - if (!MLX5_DSCP_SUPPORTED(priv->mdev))
> - return -EINVAL;
> + if (!MLX5_CAP_GEN(priv->mdev, vport_group_manager) ||
> + !MLX5_DSCP_SUPPORTED(priv->mdev))
> + return -EOPNOTSUPP;
>
> - if (app->protocol >= MLX5E_MAX_DSCP)
> + if ((app->selector != IEEE_8021QAZ_APP_SEL_DSCP) ||
> + (app->protocol >= MLX5E_MAX_DSCP))
> return -EINVAL;
>
> /* Skip if no dscp app entry */
> @@ -1146,7 +1138,7 @@ static int mlx5e_set_trust_state(struct mlx5e_priv *priv, u8 trust_state)
> {
> int err;
>
> - err = mlx5_set_trust_state(priv->mdev, trust_state);
> + err = mlx5_set_trust_state(priv->mdev, trust_state);
> if (err)
> return err;
> priv->dcbx_dp.trust_state = trust_state;
> --
> 2.17.0
>
^ permalink raw reply
* Re: [PATCH 2/3] IB/ipoib: Stop using dev_id to expose port numbers
From: Yuval Shaia @ 2018-08-13 12:40 UTC (permalink / raw)
To: Arseny Maslennikov; +Cc: linux-rdma, Doug Ledford, Jason Gunthorpe, netdev
In-Reply-To: <20180813114224.7065-3-ar@cs.msu.ru>
On Mon, Aug 13, 2018 at 02:42:23PM +0300, Arseny Maslennikov wrote:
> Some InfiniBand network devices have multiple ports on the same PCI
> function. Prior to this the kernel erroneously used the `dev_id' sysfs
> field of those network interfaces to convey the port number to userspace.
>
> `dev_id' is currently reserved for distinguishing stacked ifaces
> (e.g: VLANs) with the same hardware address as their parent device.
>
> Similar fixes to net/mlx4_en and many other drivers, which started
> exporting this information through `dev_id' before 3.15, were accepted
> into the kernel 4 years ago.
> See 76a066f2a2a0268b565459c417b59724b5a3197b, commit message:
> `net/mlx4_en: Expose port number through sysfs'.
>
> I would be OK with this commit not being backported to stable, since
> it might break admin-supplied udev rules and the likes.
>
> Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> ---
> drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 6eb0594fffec..f64535038147 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -2252,7 +2252,6 @@ static struct net_device *ipoib_add_port(const char *format,
> }
>
> SET_NETDEV_DEV(priv->dev, hca->dev.parent);
> - priv->dev->dev_id = port - 1;
Correct me if i'm wrong here but besides some changes in commit message
looks like patch 1/3 is the same as 2/3, isn't it?
Yuval
> priv->dev->dev_port = port - 1;
>
> result = ib_query_port(hca, port, &attr);
> --
> 2.18.0
>
^ permalink raw reply
* Re: possible deadlock in flush_work (3)
From: Xin Long @ 2018-08-13 15:39 UTC (permalink / raw)
To: syzbot
Cc: Christian Brauner, davem, David Ahern, Florian Westphal,
Jiri Benc, Kirill Tkhai, LKML, network dev, Nicolas Dichtel,
syzkaller-bugs, Moni Shoua
In-Reply-To: <0000000000009637a6057350529b@google.com>
On Mon, Aug 13, 2018 at 8:35 PM, syzbot
<syzbot+a8371264572a6872b8a3@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: d6dd6431591b Merge branch 'fixes' of git://git.kernel.org/..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=14e800aa400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=152cb8ccd35b1f70
> dashboard link: https://syzkaller.appspot.com/bug?extid=a8371264572a6872b8a3
> compiler: gcc (GCC) 8.0.1 20180413 (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+a8371264572a6872b8a3@syzkaller.appspotmail.com
>
> 8021q: adding VLAN 0 to HW filter on device bond0
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 4.18.0-rc8+ #185 Not tainted
> ------------------------------------------------------
> syz-executor2/6421 is trying to acquire lock:
> 00000000209b4e4b ((wq_completion)bond_dev->name){+.+.}, at: start_flush_work
> kernel/workqueue.c:2888 [inline]
> 00000000209b4e4b ((wq_completion)bond_dev->name){+.+.}, at:
> flush_work+0x4b8/0x900 kernel/workqueue.c:2917
>
> but task is already holding lock:
> 00000000f0f3d47a (rtnl_mutex){+.+.}, at: rtnl_lock+0x17/0x20
> net/core/rtnetlink.c:77
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #2 (rtnl_mutex){+.+.}:
> __mutex_lock_common kernel/locking/mutex.c:757 [inline]
> __mutex_lock+0x176/0x1820 kernel/locking/mutex.c:894
> mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:909
> rtnl_lock+0x17/0x20 net/core/rtnetlink.c:77
> bond_netdev_notify drivers/net/bonding/bond_main.c:1310 [inline]
> bond_netdev_notify_work+0x44/0xd0
> drivers/net/bonding/bond_main.c:1320
> process_one_work+0xc73/0x1ba0 kernel/workqueue.c:2153
> worker_thread+0x189/0x13c0 kernel/workqueue.c:2296
> kthread+0x345/0x410 kernel/kthread.c:246
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
>
> -> #1 ((work_completion)(&(&nnw->work)->work)){+.+.}:
> process_one_work+0xc0b/0x1ba0 kernel/workqueue.c:2129
> worker_thread+0x189/0x13c0 kernel/workqueue.c:2296
> kthread+0x345/0x410 kernel/kthread.c:246
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
>
> -> #0 ((wq_completion)bond_dev->name){+.+.}:
> lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
> start_flush_work kernel/workqueue.c:2889 [inline]
> flush_work+0x4dd/0x900 kernel/workqueue.c:2917
> __cancel_work_timer+0x4bd/0x830 kernel/workqueue.c:2989
> cancel_delayed_work_sync+0x1a/0x20 kernel/workqueue.c:3121
> bond_work_cancel_all drivers/net/bonding/bond_main.c:3318 [inline]
> bond_close+0x1b/0x130 drivers/net/bonding/bond_main.c:3381
> __dev_close_many+0x21e/0x380 net/core/dev.c:1476
> __dev_close net/core/dev.c:1488 [inline]
> __dev_change_flags+0x38d/0x9c0 net/core/dev.c:6989
> dev_change_flags+0x89/0x150 net/core/dev.c:7060
> dev_ifsioc+0x84f/0xb30 net/core/dev_ioctl.c:237
> dev_ioctl+0x1b5/0xcc0 net/core/dev_ioctl.c:493
> sock_do_ioctl+0x1d3/0x3e0 net/socket.c:993
> sock_ioctl+0x30d/0x680 net/socket.c:1094
> vfs_ioctl fs/ioctl.c:46 [inline]
> file_ioctl fs/ioctl.c:500 [inline]
> do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
> ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
> __do_sys_ioctl fs/ioctl.c:708 [inline]
> __se_sys_ioctl fs/ioctl.c:706 [inline]
> __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> other info that might help us debug this:
>
> Chain exists of:
> (wq_completion)bond_dev->name --> (work_completion)(&(&nnw->work)->work)
> --> rtnl_mutex
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(rtnl_mutex);
> lock((work_completion)(&(&nnw->work)->work));
> lock(rtnl_mutex);
> lock((wq_completion)bond_dev->name);
nnw->work is queuing up into bond->wq, so it seems bond_netdev_notify()
should have used rtnl_trylock() instead of rtnl_lock(), as do other
delayed_work handlers in bond->wq.
>
> *** DEADLOCK ***
>
> 1 lock held by syz-executor2/6421:
> #0: 00000000f0f3d47a (rtnl_mutex){+.+.}, at: rtnl_lock+0x17/0x20
> net/core/rtnetlink.c:77
>
> stack backtrace:
> CPU: 0 PID: 6421 Comm: syz-executor2 Not tainted 4.18.0-rc8+ #185
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
> print_circular_bug.isra.36.cold.57+0x1bd/0x27d
> kernel/locking/lockdep.c:1227
> check_prev_add kernel/locking/lockdep.c:1867 [inline]
> check_prevs_add kernel/locking/lockdep.c:1980 [inline]
> validate_chain kernel/locking/lockdep.c:2421 [inline]
> __lock_acquire+0x3449/0x5020 kernel/locking/lockdep.c:3435
> lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
> start_flush_work kernel/workqueue.c:2889 [inline]
> flush_work+0x4dd/0x900 kernel/workqueue.c:2917
> __cancel_work_timer+0x4bd/0x830 kernel/workqueue.c:2989
> cancel_delayed_work_sync+0x1a/0x20 kernel/workqueue.c:3121
> bond_work_cancel_all drivers/net/bonding/bond_main.c:3318 [inline]
> bond_close+0x1b/0x130 drivers/net/bonding/bond_main.c:3381
> __dev_close_many+0x21e/0x380 net/core/dev.c:1476
> __dev_close net/core/dev.c:1488 [inline]
> __dev_change_flags+0x38d/0x9c0 net/core/dev.c:6989
> dev_change_flags+0x89/0x150 net/core/dev.c:7060
> dev_ifsioc+0x84f/0xb30 net/core/dev_ioctl.c:237
> dev_ioctl+0x1b5/0xcc0 net/core/dev_ioctl.c:493
> sock_do_ioctl+0x1d3/0x3e0 net/socket.c:993
> kernel msg: ebtables bug: please report to author: Wrong nr of counters
> sock_ioctl+0x30d/0x680 net/socket.c:1094
> vfs_ioctl fs/ioctl.c:46 [inline]
> file_ioctl fs/ioctl.c:500 [inline]
> do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
> ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
> __do_sys_ioctl fs/ioctl.c:708 [inline]
> __se_sys_ioctl fs/ioctl.c:706 [inline]
> __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x457089
> Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff
> 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007fee7b2ccc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 00007fee7b2cd6d4 RCX: 0000000000457089
> RDX: 0000000020000140 RSI: 0000000000008914 RDI: 0000000000000004
> RBP: 00000000009300a0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
> R13: 00000000004d19e0 R14: 00000000004c7454 R15: 0000000000000000
> 8021q: adding VLAN 0 to HW filter on device bond0
> (unnamed net_device) (uninitialized): option miimon: invalid value
> (18446744073709551615)
> (unnamed net_device) (uninitialized): option miimon: allowed values 0 -
> 2147483647
> (unnamed net_device) (uninitialized): option miimon: invalid value
> (18446744073709551615)
> (unnamed net_device) (uninitialized): option miimon: allowed values 0 -
> 2147483647
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> device bond0 left promiscuous mode
> IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> IPv6: ADDRCONF(NETDEV_UP): bond0: link is not ready
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
> 8021q: adding VLAN 0 to HW filter on device bond0
>
>
> ---
> 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox