* [PATCH][next] rtlwifi: rtl8192ce: fix typo, "PairwiseENcAlgorithm" -> "PairwiseEncAlgorithm"
From: Colin King @ 2019-02-20 9:27 UTC (permalink / raw)
To: Ping-Ke Shih, Kalle Valo, David S . Miller, Larry Finger,
linux-wireless, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is an uppercase 'N' that should be a lowercase 'n', fix this.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
index 5cf5a85d1376..d8f496e2d68b 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
@@ -876,7 +876,7 @@ void rtl92ce_enable_hw_security_config(struct ieee80211_hw *hw)
u8 sec_reg_value;
RT_TRACE(rtlpriv, COMP_INIT, DBG_LOUD,
- "PairwiseENcAlgorithm = %d GroupEncAlgorithm = %d\n",
+ "PairwiseEncAlgorithm = %d GroupEncAlgorithm = %d\n",
rtlpriv->sec.pairwise_enc_algorithm,
rtlpriv->sec.group_enc_algorithm);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net] team: use operstate consistently for linkup
From: George Wilkie @ 2019-02-20 8:21 UTC (permalink / raw)
To: David Miller; +Cc: jiri, netdev
In-Reply-To: <20190219.141830.1399219758870863227.davem@davemloft.net>
On Tue, Feb 19, 2019 at 02:18:30PM -0800, David Miller wrote:
> From: George Wilkie <gwilkie@vyatta.att-mail.com>
> Date: Tue, 19 Feb 2019 15:57:15 +0000
>
> > Fixes: f1d22a1e0595 ("team: account for oper state")
> >
> > Signed-off-by: George Wilkie <gwilkie@vyatta.att-mail.com>
>
> Please do not put an empty line between Fixes: and other tags, all tags
> are equal and should be grouped together.
>
Fixed in v2. Thanks.
^ permalink raw reply
* BUG: assuming atomic context at kernel/seccomp.c:LINE
From: syzbot @ 2019-02-20 9:32 UTC (permalink / raw)
To: ast, daniel, kafai, keescook, linux-kernel, luto, netdev,
songliubraving, syzkaller-bugs, wad, yhs
Hello,
syzbot found the following crash on:
HEAD commit: abf446c90405 Add linux-next specific files for 20190220
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=17f250d8c00000
kernel config: https://syzkaller.appspot.com/x/.config?x=463cb576ac40e350
dashboard link: https://syzkaller.appspot.com/bug?extid=8bf19ee2aa580de7a2a7
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+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
BUG: assuming atomic context at kernel/seccomp.c:271
in_atomic(): 0, irqs_disabled(): 0, pid: 12803, name: syz-executor.5
no locks held by syz-executor.5/12803.
CPU: 1 PID: 12803 Comm: syz-executor.5 Not tainted 5.0.0-rc7-next-20190220
#39
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+0x172/0x1f0 lib/dump_stack.c:113
__cant_sleep kernel/sched/core.c:6218 [inline]
__cant_sleep.cold+0xa3/0xbb kernel/sched/core.c:6195
seccomp_run_filters kernel/seccomp.c:271 [inline]
__seccomp_filter+0x12b/0x12b0 kernel/seccomp.c:801
__secure_computing+0x101/0x360 kernel/seccomp.c:932
syscall_trace_enter+0x5bf/0xe10 arch/x86/entry/common.c:120
do_syscall_64+0x479/0x610 arch/x86/entry/common.c:280
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x45ac8a
Code: 25 18 00 00 00 00 74 01 f0 48 0f b1 3d df ba 5f 00 48 39 c2 75 da f3
c3 0f 1f 84 00 00 00 00 00 48 63 ff b8 e4 00 00 00 0f 05 <48> 3d 00 f0 ff
ff 77 06 f3 c3 0f 1f 40 00 48 c7 c2 d4 ff ff ff f7
RSP: 002b:00007f92ed7b2c58 EFLAGS: 00000246 ORIG_RAX: 00000000000000e4
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000045ac8a
RDX: 0000000000017230 RSI: 00007f92ed7b2c60 RDI: 0000000000000001
RBP: 000000000073bf00 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00000000004c4cd5 R14: 00000000004d8890 R15: 00000000ffffffff
---
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] iwlwifi: mvm: Use div64_s64 instead of do_div in iwl_mvm_debug_range_resp
From: Luca Coelho @ 2019-02-20 9:39 UTC (permalink / raw)
To: Nick Desaulniers, Nathan Chancellor
Cc: Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless,
Kalle Valo, linux-wireless, netdev, LKML
In-Reply-To: <CAKwvOdmFE0kQoxvRDcarT+kYLXxqKYJfVHo4uTiYm7-wDefrrw@mail.gmail.com>
On Tue, 2019-02-19 at 11:05 -0800, Nick Desaulniers wrote:
> On Tue, Feb 19, 2019 at 10:21 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> > Clang warns:
> >
> > drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c:465:2:
> > warning:
> > comparison of distinct pointer types ('typeof ((rtt_avg)) *' (aka
> > 'long
> > long *') and 'uint64_t *' (aka 'unsigned long long *'))
> > [-Wcompare-distinct-pointer-types]
> > do_div(rtt_avg, 6666);
> > ^~~~~~~~~~~~~~~~~~~~~
> > include/asm-generic/div64.h:222:28: note: expanded from macro
> > 'do_div'
> > (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
> > ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~
> > 1 warning generated.
> >
> > do_div expects an unsigned dividend. Use div64_s64, which expects a
> > signed dividend.
>
> Eh, IIRC, signed vs unsigned division has implications for rounding
> towards zero or not, but I doubt that the round trip time average
> (RTT
> avg) should ever be negative. General rule of thumb for C is to keep
> arithmetic signed (even when working with non zero values), so rather
> than make the literal (6666) a unsigned long, I agree with your
> change
> to keep the division signed as well. Thanks for the fix.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Thanks for the patch and for the review. I've applied this to our
internal tree and it will be sent upstreaming following our normal
upstreaming process.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH net] sk_msg: Keep reference on socket file while psock lives
From: Jakub Sitnicki @ 2019-02-20 9:47 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, John Fastabend, Marek Majkowski
In-Reply-To: <5439765e-1288-c379-0ead-75597092a404@iogearbox.net>
Hi Daniel,
On Tue, Feb 19, 2019 at 05:00 PM CET, Daniel Borkmann wrote:
> On 02/11/2019 10:09 AM, Jakub Sitnicki wrote:
>> Backlog work for psock (sk_psock_backlog) might sleep while waiting for
>> memory to free up when sending packets. While sleeping, socket can
>> disappear from under our feet together with its wait queue because the
>> userspace has closed it.
>>
>> This breaks an assumption in sk_stream_wait_memory, which expects the
>> wait queue to be still there when it wakes up resulting in a
>> use-after-free:
>>
>> ==================================================================
>> BUG: KASAN: use-after-free in remove_wait_queue+0x31/0x70
>> Write of size 8 at addr ffff888069a0c4e8 by task kworker/0:2/110
>>
>> CPU: 0 PID: 110 Comm: kworker/0:2 Not tainted 5.0.0-rc2-00335-g28f9d1a3d4fe-dirty #14
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-2.fc27 04/01/2014
>> Workqueue: events sk_psock_backlog
>> Call Trace:
>> print_address_description+0x6e/0x2b0
>> ? remove_wait_queue+0x31/0x70
>> kasan_report+0xfd/0x177
>> ? remove_wait_queue+0x31/0x70
>> ? remove_wait_queue+0x31/0x70
>> remove_wait_queue+0x31/0x70
>> sk_stream_wait_memory+0x4dd/0x5f0
>> ? sk_stream_wait_close+0x1b0/0x1b0
>> ? wait_woken+0xc0/0xc0
>> ? tcp_current_mss+0xc5/0x110
>> tcp_sendmsg_locked+0x634/0x15d0
>> ? tcp_set_state+0x2e0/0x2e0
>> ? __kasan_slab_free+0x1d1/0x230
>> ? kmem_cache_free+0x70/0x140
>> ? sk_psock_backlog+0x40c/0x4b0
>> ? process_one_work+0x40b/0x660
>> ? worker_thread+0x82/0x680
>> ? kthread+0x1b9/0x1e0
>> ? ret_from_fork+0x1f/0x30
>> ? check_preempt_curr+0xaf/0x130
>> ? iov_iter_kvec+0x5f/0x70
>> ? kernel_sendmsg_locked+0xa0/0xe0
>> skb_send_sock_locked+0x273/0x3c0
>> ? skb_splice_bits+0x180/0x180
>> ? start_thread+0xe0/0xe0
>> ? update_min_vruntime.constprop.27+0x88/0xc0
>> sk_psock_backlog+0xb3/0x4b0
>> ? strscpy+0xbf/0x1e0
>> process_one_work+0x40b/0x660
>> worker_thread+0x82/0x680
>> ? process_one_work+0x660/0x660
>> kthread+0x1b9/0x1e0
>> ? __kthread_create_on_node+0x250/0x250
>> ret_from_fork+0x1f/0x30
>>
>> Allocated by task 109:
>> sock_alloc_inode+0x54/0x120
>> alloc_inode+0x28/0xb0
>> new_inode_pseudo+0x7/0x60
>> sock_alloc+0x21/0xe0
>> __sys_accept4+0xc2/0x330
>> __x64_sys_accept+0x3b/0x50
>> do_syscall_64+0xb2/0x3e0
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> Freed by task 7:
>> kfree+0x7f/0x140
>> rcu_process_callbacks+0xe0/0x100
>> __do_softirq+0xe5/0x29a
>>
>> The buggy address belongs to the object at ffff888069a0c4e0
>> which belongs to the cache kmalloc-64 of size 64
>> The buggy address is located 8 bytes inside of
>> 64-byte region [ffff888069a0c4e0, ffff888069a0c520)
>> The buggy address belongs to the page:
>> page:ffffea0001a68300 count:1 mapcount:0 mapping:ffff88806d4018c0 index:0x0
>> flags: 0x4000000000000200(slab)
>> raw: 4000000000000200 dead000000000100 dead000000000200 ffff88806d4018c0
>> raw: 0000000000000000 00000000002a002a 00000001ffffffff 0000000000000000
>> page dumped because: kasan: bad access detected
>>
>> Memory state around the buggy address:
>> ffff888069a0c380: fb fb fb fb fc fc fc fc fb fb fb fb fb fb fb fb
>> ffff888069a0c400: fc fc fc fc fb fb fb fb fb fb fb fb fc fc fc fc
>>> ffff888069a0c480: 00 00 00 00 00 00 00 00 fc fc fc fc fb fb fb fb
>> ^
>> ffff888069a0c500: fb fb fb fb fc fc fc fc fb fb fb fb fb fb fb fb
>> ffff888069a0c580: fc fc fc fc fb fb fb fb fb fb fb fb fc fc fc fc
>> ==================================================================
>>
>> Avoid it by keeping a reference to the socket file until the psock gets
>> destroyed.
>>
>> While at it, rearrange the order of reference grabbing and
>> initialization to match the destructor in reverse.
>>
>> Reported-by: Marek Majkowski <marek@cloudflare.com>
>> Link: https://lore.kernel.org/netdev/CAJPywTLwgXNEZ2dZVoa=udiZmtrWJ0q5SuBW64aYs0Y1khXX3A@mail.gmail.com
>> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
>> ---
>> net/core/skmsg.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
>> index 8c826603bf36..a38442b8580b 100644
>> --- a/net/core/skmsg.c
>> +++ b/net/core/skmsg.c
>> @@ -493,8 +493,13 @@ struct sk_psock *sk_psock_init(struct sock *sk, int node)
>> sk_psock_set_state(psock, SK_PSOCK_TX_ENABLED);
>> refcount_set(&psock->refcnt, 1);
>>
>> - rcu_assign_sk_user_data(sk, psock);
>> + /* Hold on to socket wait queue. Backlog work waits on it for
>> + * memory when sending. We must cancel work before socket wait
>> + * queue can go away.
>> + */
>> + get_file(sk->sk_socket->file);
>> sock_hold(sk);
>> + rcu_assign_sk_user_data(sk, psock);
>>
>> return psock;
>> }
>> @@ -558,6 +563,7 @@ static void sk_psock_destroy_deferred(struct work_struct *gc)
>> if (psock->sk_redir)
>> sock_put(psock->sk_redir);
>> sock_put(psock->sk);
>> + fput(psock->sk->sk_socket->file);
>
> Thanks for the report (and sorry for the late reply). I think holding ref on
> the struct file just so we keep it alive till deferred destruction might be
> papering over the actual underlying bug. Nothing obvious pops out from staring
> at the code right now; as a reproducer to run, did you use the prog in the link
> above and hit it after your strparser fix?
I get you, I actually sat on this fix for a moment because I had a
similar concern, that holding a file ref is a heavy-handed fix and I'm
not seeing the real problem.
For me it came down to this:
1. tcp_sendmsg_locked that we call from psock backlog work can end up
waiting for memory. We somehow need to ensure that the socket wait
queue does not disappear until tcp_sendmsg_locked returns.
2. KCM, which I assume must have the same problem, holds a reference on
the socket file.
I'm curious if there is another angle to it.
To answer your actual questions - your guesses are correct on both
accounts.
For the reproducer, I've used the TCP echo program from Marek [1]. On
the client side, I had something like:
while :; do
nc 10.0.0.1 12345 > /dev/null < /dev/zero &
pid=$!
sleep 0.1
kill $pid
done
I can dig out the test scripts or help testing any patches.
I was testing with the strparser fix applied, 1d79895aef18 ("sk_msg:
Always cancel strp work before freeing the psock"), which unfortunately
was not enough.
The explanation there was that the socket descriptor can get closed, and
in consequence the socket file can get destroyed, before the deferred
destructor for psock runs. So psock backlog work can be still very much
alive and running while the socket file is gone.
Thanks for looking into it,
-Jakub
[1] https://gist.github.com/majek/a09bcbeb8ab548cde6c18c930895c3f2
^ permalink raw reply
* Re: BUG: assuming atomic context at kernel/seccomp.c:LINE
From: Daniel Borkmann @ 2019-02-20 10:00 UTC (permalink / raw)
To: syzbot, ast, kafai, keescook, linux-kernel, luto, netdev,
songliubraving, syzkaller-bugs, wad, yhs
In-Reply-To: <000000000000cedfe1058250076c@google.com>
On 02/20/2019 10:32 AM, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: abf446c90405 Add linux-next specific files for 20190220
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=17f250d8c00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=463cb576ac40e350
> dashboard link: https://syzkaller.appspot.com/bug?extid=8bf19ee2aa580de7a2a7
> 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+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
>
> BUG: assuming atomic context at kernel/seccomp.c:271
> in_atomic(): 0, irqs_disabled(): 0, pid: 12803, name: syz-executor.5
> no locks held by syz-executor.5/12803.
> CPU: 1 PID: 12803 Comm: syz-executor.5 Not tainted 5.0.0-rc7-next-20190220 #39
> 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+0x172/0x1f0 lib/dump_stack.c:113
> __cant_sleep kernel/sched/core.c:6218 [inline]
> __cant_sleep.cold+0xa3/0xbb kernel/sched/core.c:6195
> seccomp_run_filters kernel/seccomp.c:271 [inline]
> __seccomp_filter+0x12b/0x12b0 kernel/seccomp.c:801
> __secure_computing+0x101/0x360 kernel/seccomp.c:932
> syscall_trace_enter+0x5bf/0xe10 arch/x86/entry/common.c:120
> do_syscall_64+0x479/0x610 arch/x86/entry/common.c:280
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
False positive; bpf-next only. Pushing this out in a bit:
From d56547070162a105ff666f3324e558fa6492aedd Mon Sep 17 00:00:00 2001
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 20 Feb 2019 10:51:17 +0100
Subject: [PATCH bpf-next] bpf, seccomp: fix false positive preemption splat for
cbpf->ebpf progs
In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
a check was added for BPF_PROG_RUN() that for every invocation preemption is
disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this does
not count for seccomp because only cBPF -> eBPF is loaded here and it does not
make use of any functionality that would require this assertion. Fix this false
positive by adding and using __BPF_PROG_RUN() variant that does not have the
cant_sleep(); check.
Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/filter.h | 9 ++++++++-
kernel/seccomp.c | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index f32b3ec..2f3e29a 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -533,7 +533,14 @@ struct sk_filter {
struct bpf_prog *prog;
};
-#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
+#define bpf_prog_run__non_preempt(prog, ctx) \
+ ({ cant_sleep(); __BPF_PROG_RUN(prog, ctx); })
+/* Native eBPF or cBPF -> eBPF transitions. Preemption must be disabled. */
+#define BPF_PROG_RUN(prog, ctx) \
+ bpf_prog_run__non_preempt(prog, ctx)
+/* cBPF -> eBPF only, but not for native eBPF. */
+#define __BPF_PROG_RUN(prog, ctx) \
+ (*(prog)->bpf_func)(ctx, (prog)->insnsi)
#define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index e815781..826d4e4 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -268,7 +268,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
* value always takes priority (ignoring the DATA).
*/
for (; f; f = f->prev) {
- u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
+ u32 cur_ret = __BPF_PROG_RUN(f->prog, sd);
if (ACTION_ONLY(cur_ret) < ACTION_ONLY(ret)) {
ret = cur_ret;
--
2.9.5
^ permalink raw reply related
* Re: KASAN: use-after-free Read in br_mdb_ip_get
From: Herbert Xu @ 2019-02-20 10:23 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Nikolay Aleksandrov, Thomas Graf, syzbot, bridge, David Miller,
LKML, netdev, Roopa Prabhu, syzkaller-bugs
In-Reply-To: <CACT4Y+ZA8gBURbeZaDtrt5NoqFy8a8W3jyaWbs34Qjic4Bu+DA@mail.gmail.com>
On Mon, Jan 28, 2019 at 09:28:36AM +0100, Dmitry Vyukov wrote:
>
> > 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..
>
> Humm.... +rhashtable.c maintianers
>
> The code in br_multicast_new_group is effectively:
>
> mp = kzalloc(sizeof(*mp), GFP_ATOMIC);
> err = rhashtable_lookup_insert_fast(&br->mdb_hash_tbl, &mp->rhnode,
> br_mdb_rht_params);
> if (err)
> kfree(mp);
>
> So it looks like rhashtable_lookup_insert_fast both returned an error
> and left the new object linked in the table. Since it happened only
> once, it may have something to do with concurrent resizing/shrinking.
I've looked through the rhashtable code in question and everything
looks OK. So I suspect some earlier corruption has occured to cause
this anomalous result. Is it possible to collect earlier alloc/free
stack traces on the object in question?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH net] net: dsa: fix unintended change of bridge interface STP state
From: Russell King @ 2019-02-20 10:32 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Vivien Didelot; +Cc: David S. Miller, netdev
When a DSA port is added to a bridge and brought up, the resulting STP
state programmed into the hardware depends on the order that these
operations are performed. However, the Linux bridge code believes that
the port is in disabled mode.
If the DSA port is first added to a bridge and then brought up, it will
be in blocking mode. If it is brought up and then added to the bridge,
it will be in disabled mode.
This difference is caused by DSA always setting the STP mode in
dsa_port_enable() whether or not this port is part of a bridge. Since
bridge always sets the STP state when the port is added, brought up or
taken down, it is unnecessary for us to manipulate the STP state.
Apparently, this code was copied from Rocker, and the very next day a
similar fix for Rocker was merged but was not propagated to DSA. See
e47172ab7e41 ("rocker: put port in FORWADING state after leaving bridge")
Fixes: b73adef67765 ("net: dsa: integrate with SWITCHDEV for HW bridging")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
net/dsa/port.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/dsa/port.c b/net/dsa/port.c
index ed0595459df1..ea7efc86b9d7 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -69,7 +69,6 @@ static void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy)
{
- u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
struct dsa_switch *ds = dp->ds;
int port = dp->index;
int err;
@@ -80,7 +79,8 @@ int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy)
return err;
}
- dsa_port_set_state_now(dp, stp_state);
+ if (!dp->bridge_dev)
+ dsa_port_set_state_now(dp, BR_STATE_FORWARDING);
return 0;
}
@@ -90,7 +90,8 @@ void dsa_port_disable(struct dsa_port *dp, struct phy_device *phy)
struct dsa_switch *ds = dp->ds;
int port = dp->index;
- dsa_port_set_state_now(dp, BR_STATE_DISABLED);
+ if (!dp->bridge_dev)
+ dsa_port_set_state_now(dp, BR_STATE_DISABLED);
if (ds->ops->port_disable)
ds->ops->port_disable(ds, port, phy);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] iwlwifi: mvm: Use div64_s64 instead of do_div in iwl_mvm_debug_range_resp
From: Arnd Bergmann @ 2019-02-20 10:51 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Johannes Berg, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, Kalle Valo, linux-wireless, Networking,
Linux Kernel Mailing List, Nick Desaulniers
In-Reply-To: <20190219182105.19933-1-natechancellor@gmail.com>
On Tue, Feb 19, 2019 at 7:22 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> index e9822a3ec373..92b22250eb7d 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> @@ -462,7 +462,7 @@ static void iwl_mvm_debug_range_resp(struct iwl_mvm *mvm, u8 index,
> {
> s64 rtt_avg = res->ftm.rtt_avg * 100;
>
> - do_div(rtt_avg, 6666);
> + div64_s64(rtt_avg, 6666);
This is wrong: div64_s64 does not modify its argument like do_div(), but
it returns the result instead. You also don't want to divide by a 64-bit
value when the second argument is a small constant.
I think the correct way should be
s64 rtt_avg = div_s64(res->ftm.rtt_avg * 100, 6666);
If you know that the value is positive, using unsigned types
and div_u64() would be slightly faster.
Arnd
^ permalink raw reply
* Re: [PATCH net-next v2 07/10] net: phy: marvell10g: Add support for 2.5GBASET
From: Maxime Chevallier @ 2019-02-20 10:54 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: davem, netdev, linux-kernel, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, linux-arm-kernel, Antoine Tenart,
thomas.petazzoni, gregory.clement, miquel.raynal, nadavh, stefanc,
mw
In-Reply-To: <20190207234824.dhynfaglfxllxhei@shell.armlinux.org.uk>
Hello Russell,
On Thu, 7 Feb 2019 23:48:24 +0000
Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
>On Thu, Feb 07, 2019 at 10:49:36AM +0100, Maxime Chevallier wrote:
>> The Marvell Alaska family of PHYs supports 2.5GBaseT and 5GBaseT modes,
>> as defined in the 802.3bz specification.
>>
>> When the link partner requests a 2.5GBASET link, the PHY will
>> reconfigure it's MII interface to 2500BASEX.
>>
>> At 5G, the PHY will reconfigure it's interface to 5GBASE-R, but this
>> mode isn't supported by any MAC for now.
>>
>> This was tested with :
>> - The 88X3310, which is on the MacchiatoBin
>
>Hi Maxime,
>
>Looking deeper at this, I think we actually need an additional patch at
>the beginning of your series.
>
>The default AN advertisement in 7.32 is 0x1181 - which includes the
>2.5G and 5G modes. We need to clear these bits, so that when the 10G
>mode disabled via ethtool, we do not switch to 2.5G or 5G speed (both
>of which are not currently reported as supported.) Such a patch needs
>backporting to stable kernels.
Good catch. The issue seems fixed by Andrew's patch :
3de97f3c6308 ("net: phy: marvell10g: use genphy_c45_an_config_aneg")
However, the fix should indeed be backported to the -stable trees, I've
been able to repdocude this on 4.20. I'll take care of sending a patch
to -net for that.
Thanks for reporting this,
Maxime
^ permalink raw reply
* [PATCH v3 4/8] net: thunderx: add nicvf_send_msg_to_pf result check for set_rx_mode_task
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
The rx_set_mode invokes number of messages to be send to PF for receive
mode configuration. In case if there any issues we need to stop sending
messages and release allocated memory.
This commit is to implement check of nicvf_msg_send_to_pf() result.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 19b58fc3ca41..45f06504a61b 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1953,7 +1953,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
/* flush DMAC filters and reset RX mode */
mbx.xcast.msg = NIC_MBOX_MSG_RESET_XCAST;
- nicvf_send_msg_to_pf(nic, &mbx);
+ if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
+ goto free_mc;
if (mode & BGX_XCAST_MCAST_FILTER) {
/* once enabling filtering, we need to signal to PF to add
@@ -1961,7 +1962,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
*/
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
mbx.xcast.data.mac = 0;
- nicvf_send_msg_to_pf(nic, &mbx);
+ if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
+ goto free_mc;
}
/* check if we have any specific MACs to be added to PF DMAC filter */
@@ -1970,9 +1972,9 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
for (idx = 0; idx < mc_addrs->count; idx++) {
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
mbx.xcast.data.mac = mc_addrs->mc[idx];
- nicvf_send_msg_to_pf(nic, &mbx);
+ if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
+ goto free_mc;
}
- kfree(mc_addrs);
}
/* and finally set rx mode for PF accordingly */
@@ -1980,6 +1982,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
mbx.xcast.data.mode = mode;
nicvf_send_msg_to_pf(nic, &mbx);
+free_mc:
+ kfree(mc_addrs);
}
static void nicvf_set_rx_mode_task(struct work_struct *work_arg)
--
2.17.2
^ permalink raw reply related
* [PATCH v3 6/8] net: thunderx: add mutex to protect mailbox from concurrent calls for same VF
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
In some cases it could happen that nicvf_send_msg_to_pf() could be called
concurrently for the same NIC VF, and thus re-writing mailbox contents and
breaking messaging sequence with PF by re-writing NICVF data.
This commit is to implement mutex for NICVF to protect mailbox registers
and NICVF messaging control data from concurrent access.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 2 ++
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 13 ++++++++++---
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 227343625e83..86cda3f4b37b 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -329,6 +329,8 @@ struct nicvf {
spinlock_t rx_mode_wq_lock;
/* workqueue for handling kernel ndo_set_rx_mode() calls */
struct workqueue_struct *nicvf_rx_mode_wq;
+ /* mutex to protect VF's mailbox contents from concurrent access */
+ struct mutex rx_mode_mtx;
/* PTP timestamp */
struct cavium_ptp *ptp_clock;
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index da5986ca7bee..2332e3e95e0e 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -124,6 +124,9 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx)
{
int timeout = NIC_MBOX_MSG_TIMEOUT;
int sleep = 10;
+ int ret = 0;
+
+ mutex_lock(&nic->rx_mode_mtx);
nic->pf_acked = false;
nic->pf_nacked = false;
@@ -136,7 +139,8 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx)
netdev_err(nic->netdev,
"PF NACK to mbox msg 0x%02x from VF%d\n",
(mbx->msg.msg & 0xFF), nic->vf_id);
- return -EINVAL;
+ ret = -EINVAL;
+ break;
}
msleep(sleep);
if (nic->pf_acked)
@@ -146,10 +150,12 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx)
netdev_err(nic->netdev,
"PF didn't ACK to mbox msg 0x%02x from VF%d\n",
(mbx->msg.msg & 0xFF), nic->vf_id);
- return -EBUSY;
+ ret = -EBUSY;
+ break;
}
}
- return 0;
+ mutex_unlock(&nic->rx_mode_mtx);
+ return ret;
}
/* Checks if VF is able to comminicate with PF
@@ -2208,6 +2214,7 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
nic->vf_id);
INIT_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task);
spin_lock_init(&nic->rx_mode_wq_lock);
+ mutex_init(&nic->rx_mode_mtx);
err = register_netdev(netdev);
if (err) {
--
2.17.2
^ permalink raw reply related
* [PATCH v3 1/8] net: thunderx: correct typo in macro name
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
Correct STREERING to STEERING at macro name for BGX steering register.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 2 +-
drivers/net/ethernet/cavium/thunder/thunder_bgx.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index e337da6ba2a4..673c57b8023f 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -1217,7 +1217,7 @@ static void bgx_init_hw(struct bgx *bgx)
/* Disable MAC steering (NCSI traffic) */
for (i = 0; i < RX_TRAFFIC_STEER_RULE_COUNT; i++)
- bgx_reg_write(bgx, 0, BGX_CMR_RX_STREERING + (i * 8), 0x00);
+ bgx_reg_write(bgx, 0, BGX_CMR_RX_STEERING + (i * 8), 0x00);
}
static u8 bgx_get_lane2sds_cfg(struct bgx *bgx, struct lmac *lmac)
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
index cbdd20b9ee6f..5cbc54e9eb19 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.h
@@ -60,7 +60,7 @@
#define RX_DMACX_CAM_EN BIT_ULL(48)
#define RX_DMACX_CAM_LMACID(x) (((u64)x) << 49)
#define RX_DMAC_COUNT 32
-#define BGX_CMR_RX_STREERING 0x300
+#define BGX_CMR_RX_STEERING 0x300
#define RX_TRAFFIC_STEER_RULE_COUNT 8
#define BGX_CMR_CHAN_MSK_AND 0x450
#define BGX_CMR_BIST_STATUS 0x460
--
2.17.2
^ permalink raw reply related
* [PATCH v3 7/8] net: thunderx: move link state polling function to VF
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
Move the link change polling task to VF side in order to
prevent races between VF and PF while sending link change
message(s). This commit is to implement link change request
to be initiated by VF.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 2 +-
.../net/ethernet/cavium/thunder/nic_main.c | 39 ++++++++++++--
.../net/ethernet/cavium/thunder/nicvf_main.c | 52 +++++++++++++------
3 files changed, 74 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 86cda3f4b37b..62636c1ed141 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -331,7 +331,7 @@ struct nicvf {
struct workqueue_struct *nicvf_rx_mode_wq;
/* mutex to protect VF's mailbox contents from concurrent access */
struct mutex rx_mode_mtx;
-
+ struct delayed_work link_change_work;
/* PTP timestamp */
struct cavium_ptp *ptp_clock;
/* Inbound timestamping is on */
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 620dbe082ca0..8ab71dae3988 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -929,6 +929,35 @@ static void nic_config_timestamp(struct nicpf *nic, int vf, struct set_ptp *ptp)
nic_reg_write(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3), pkind_val);
}
+static void nic_link_status_get(struct nicpf *nic, u8 vf)
+{
+ union nic_mbx mbx = {};
+ struct bgx_link_status link;
+ u8 bgx, lmac;
+
+ mbx.link_status.msg = NIC_MBOX_MSG_BGX_LINK_CHANGE;
+
+ /* Get BGX, LMAC indices for the VF */
+ bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+ lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+
+ /* Get interface link status */
+ bgx_get_lmac_link_state(nic->node, bgx, lmac, &link);
+
+ nic->link[vf] = link.link_up;
+ nic->duplex[vf] = link.duplex;
+ nic->speed[vf] = link.speed;
+
+ /* Send a mbox message to VF with current link status */
+ mbx.link_status.link_up = link.link_up;
+ mbx.link_status.duplex = link.duplex;
+ mbx.link_status.speed = link.speed;
+ mbx.link_status.mac_type = link.mac_type;
+
+ /* reply with link status */
+ nic_send_msg_to_vf(nic, vf, &mbx);
+}
+
/* Interrupt handler to handle mailbox messages from VFs */
static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
{
@@ -1108,6 +1137,13 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
bgx_set_xcast_mode(nic->node, bgx, lmac, mbx.xcast.mode);
break;
+ case NIC_MBOX_MSG_BGX_LINK_CHANGE:
+ if (vf >= nic->num_vf_en) {
+ ret = -1; /* NACK */
+ break;
+ }
+ nic_link_status_get(nic, vf);
+ goto unlock;
default:
dev_err(&nic->pdev->dev,
"Invalid msg from VF%d, msg 0x%x\n", vf, mbx.msg.msg);
@@ -1419,9 +1455,6 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_disable_sriov;
}
- INIT_DELAYED_WORK(&nic->dwork, nic_poll_for_link);
- queue_delayed_work(nic->check_link, &nic->dwork, 0);
-
return 0;
err_disable_sriov:
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 2332e3e95e0e..503cfadff4ac 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -242,21 +242,24 @@ static void nicvf_handle_mbx_intr(struct nicvf *nic)
break;
case NIC_MBOX_MSG_BGX_LINK_CHANGE:
nic->pf_acked = true;
- nic->link_up = mbx.link_status.link_up;
- nic->duplex = mbx.link_status.duplex;
- nic->speed = mbx.link_status.speed;
- nic->mac_type = mbx.link_status.mac_type;
- if (nic->link_up) {
- netdev_info(nic->netdev, "Link is Up %d Mbps %s duplex\n",
- nic->speed,
- nic->duplex == DUPLEX_FULL ?
- "Full" : "Half");
- netif_carrier_on(nic->netdev);
- netif_tx_start_all_queues(nic->netdev);
- } else {
- netdev_info(nic->netdev, "Link is Down\n");
- netif_carrier_off(nic->netdev);
- netif_tx_stop_all_queues(nic->netdev);
+ if (nic->link_up != mbx.link_status.link_up) {
+ nic->link_up = mbx.link_status.link_up;
+ nic->duplex = mbx.link_status.duplex;
+ nic->speed = mbx.link_status.speed;
+ nic->mac_type = mbx.link_status.mac_type;
+ if (nic->link_up) {
+ netdev_info(nic->netdev,
+ "Link is Up %d Mbps %s duplex\n",
+ nic->speed,
+ nic->duplex == DUPLEX_FULL ?
+ "Full" : "Half");
+ netif_carrier_on(nic->netdev);
+ netif_tx_start_all_queues(nic->netdev);
+ } else {
+ netdev_info(nic->netdev, "Link is Down\n");
+ netif_carrier_off(nic->netdev);
+ netif_tx_stop_all_queues(nic->netdev);
+ }
}
break;
case NIC_MBOX_MSG_ALLOC_SQS:
@@ -1325,6 +1328,8 @@ int nicvf_stop(struct net_device *netdev)
struct nicvf_cq_poll *cq_poll = NULL;
union nic_mbx mbx = {};
+ cancel_delayed_work_sync(&nic->link_change_work);
+
/* wait till all queued set_rx_mode tasks completes */
drain_workqueue(nic->nicvf_rx_mode_wq);
@@ -1427,6 +1432,18 @@ static int nicvf_update_hw_max_frs(struct nicvf *nic, int mtu)
return nicvf_send_msg_to_pf(nic, &mbx);
}
+static void nicvf_link_status_check_task(struct work_struct *work_arg)
+{
+ struct nicvf *nic = container_of(work_arg,
+ struct nicvf,
+ link_change_work.work);
+ union nic_mbx mbx = {};
+ mbx.msg.msg = NIC_MBOX_MSG_BGX_LINK_CHANGE;
+ nicvf_send_msg_to_pf(nic, &mbx);
+ queue_delayed_work(nic->nicvf_rx_mode_wq,
+ &nic->link_change_work, 2 * HZ);
+}
+
int nicvf_open(struct net_device *netdev)
{
int cpu, err, qidx;
@@ -1533,6 +1550,11 @@ int nicvf_open(struct net_device *netdev)
/* Send VF config done msg to PF */
nicvf_send_cfg_done(nic);
+ INIT_DELAYED_WORK(&nic->link_change_work,
+ nicvf_link_status_check_task);
+ queue_delayed_work(nic->nicvf_rx_mode_wq,
+ &nic->link_change_work, 0);
+
return 0;
cleanup:
nicvf_disable_intr(nic, NICVF_INTR_MBOX, 0);
--
2.17.2
^ permalink raw reply related
* [PATCH v3 0/8] nic: thunderx: fix communication races between VF & PF
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20180327150736.10718-1-Vadim.Lomovtsev@caviumnetworks.com>
The ThunderX CN88XX NIC Virtual Function driver uses mailbox interface
to communicate to physical function driver. Each of VF has it's own pair
of mailbox registers to read from and write to. The mailbox registers
has no protection from possible races, so it has to be implemented
at software side.
After long term testing by loop of 'ip link set <ifname> up/down'
command it was found that there are two possible scenarios when
race condition appears:
1. VF receives link change message from PF and VF send RX mode
configuration message to PF in the same time from separate thread.
2. PF receives RX mode configuration from VF and in the same time,
in separate thread PF detects link status change and sends appropriate
message to particular VF.
Both cases leads to mailbox data to be rewritten, NIC VF messaging control
data to be updated incorrectly and communication sequence gets broken.
This patch series is to address race condition with VF & PF communication.
Changes:
v1 -> v2
- 0000: correct typo in cover letter subject: 'betwen' -> 'between';
- move link state polling request task from pf to vf
instead of cheking status of mailbox irq;
v2 -> v3
- 0003: change return type of nicvf_send_cfg_done() function
from int to void;
- 0007: update subject and remove unused variable 'netdev'
from nicvf_link_status_check_task() function;
Vadim Lomovtsev (8):
net: thunderx: correct typo in macro name
net: thunderx: replace global nicvf_rx_mode_wq work queue for all VFs
to private for each of them.
net: thunderx: make CFG_DONE message to run through generic send-ack
sequence
net: thunderx: add nicvf_send_msg_to_pf result check for
set_rx_mode_task
net: thunderx: rework xcast message structure to make it fit into 64
bit
net: thunderx: add mutex to protect mailbox from concurrent calls for
same VF
net: thunderx: add LINK_CHANGE message handler at nicpf
net: thunderx: remove link change polling code and info from nicpf
drivers/net/ethernet/cavium/thunder/nic.h | 14 +-
.../net/ethernet/cavium/thunder/nic_main.c | 149 ++++++------------
.../net/ethernet/cavium/thunder/nicvf_main.c | 130 ++++++++++-----
.../net/ethernet/cavium/thunder/thunder_bgx.c | 2 +-
.../net/ethernet/cavium/thunder/thunder_bgx.h | 2 +-
5 files changed, 144 insertions(+), 153 deletions(-)
--
2.17.2
^ permalink raw reply
* [PATCH v3 8/8] net: thunderx: remove link change polling code and info from nicpf
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
Since link change polling routine was moved to nicvf side,
we don't need anymore polling function at nicpf side along
with link status info for all enabled Vfs as at VF side
this info is already tracked.
This commit is to remove unnecessary code & fields from
nicpf structure.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
.../net/ethernet/cavium/thunder/nic_main.c | 114 ++----------------
1 file changed, 12 insertions(+), 102 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 8ab71dae3988..c90252829ed3 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -57,14 +57,8 @@ struct nicpf {
#define NIC_GET_BGX_FROM_VF_LMAC_MAP(map) ((map >> 4) & 0xF)
#define NIC_GET_LMAC_FROM_VF_LMAC_MAP(map) (map & 0xF)
u8 *vf_lmac_map;
- struct delayed_work dwork;
- struct workqueue_struct *check_link;
- u8 *link;
- u8 *duplex;
- u32 *speed;
u16 cpi_base[MAX_NUM_VFS_SUPPORTED];
u16 rssi_base[MAX_NUM_VFS_SUPPORTED];
- bool mbx_lock[MAX_NUM_VFS_SUPPORTED];
/* MSI-X */
u8 num_vec;
@@ -929,6 +923,10 @@ static void nic_config_timestamp(struct nicpf *nic, int vf, struct set_ptp *ptp)
nic_reg_write(nic, NIC_PF_PKIND_0_15_CFG | (pkind_idx << 3), pkind_val);
}
+/* Get BGX LMAC link status and update corresponding VF
+ * if there is a change, valid only if internal L2 switch
+ * is not present otherwise VF link is always treated as up
+ */
static void nic_link_status_get(struct nicpf *nic, u8 vf)
{
union nic_mbx mbx = {};
@@ -944,10 +942,6 @@ static void nic_link_status_get(struct nicpf *nic, u8 vf)
/* Get interface link status */
bgx_get_lmac_link_state(nic->node, bgx, lmac, &link);
- nic->link[vf] = link.link_up;
- nic->duplex[vf] = link.duplex;
- nic->speed[vf] = link.speed;
-
/* Send a mbox message to VF with current link status */
mbx.link_status.link_up = link.link_up;
mbx.link_status.duplex = link.duplex;
@@ -970,8 +964,6 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
int i;
int ret = 0;
- nic->mbx_lock[vf] = true;
-
mbx_addr = nic_get_mbx_addr(vf);
mbx_data = (u64 *)&mbx;
@@ -986,12 +978,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
switch (mbx.msg.msg) {
case NIC_MBOX_MSG_READY:
nic_mbx_send_ready(nic, vf);
- if (vf < nic->num_vf_en) {
- nic->link[vf] = 0;
- nic->duplex[vf] = 0;
- nic->speed[vf] = 0;
- }
- goto unlock;
+ return;
case NIC_MBOX_MSG_QS_CFG:
reg_addr = NIC_PF_QSET_0_127_CFG |
(mbx.qs.num << NIC_QS_ID_SHIFT);
@@ -1060,7 +1047,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
case NIC_MBOX_MSG_RSS_SIZE:
nic_send_rss_size(nic, vf);
- goto unlock;
+ return;
case NIC_MBOX_MSG_RSS_CFG:
case NIC_MBOX_MSG_RSS_CFG_CONT:
nic_config_rss(nic, &mbx.rss_cfg);
@@ -1078,19 +1065,19 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
case NIC_MBOX_MSG_ALLOC_SQS:
nic_alloc_sqs(nic, &mbx.sqs_alloc);
- goto unlock;
+ return;
case NIC_MBOX_MSG_NICVF_PTR:
nic->nicvf[vf] = mbx.nicvf.nicvf;
break;
case NIC_MBOX_MSG_PNICVF_PTR:
nic_send_pnicvf(nic, vf);
- goto unlock;
+ return;
case NIC_MBOX_MSG_SNICVF_PTR:
nic_send_snicvf(nic, &mbx.nicvf);
- goto unlock;
+ return;
case NIC_MBOX_MSG_BGX_STATS:
nic_get_bgx_stats(nic, &mbx.bgx_stats);
- goto unlock;
+ return;
case NIC_MBOX_MSG_LOOPBACK:
ret = nic_config_loopback(nic, &mbx.lbk);
break;
@@ -1099,7 +1086,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
case NIC_MBOX_MSG_PFC:
nic_pause_frame(nic, vf, &mbx.pfc);
- goto unlock;
+ return;
case NIC_MBOX_MSG_PTP_CFG:
nic_config_timestamp(nic, vf, &mbx.ptp);
break;
@@ -1143,7 +1130,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
}
nic_link_status_get(nic, vf);
- goto unlock;
+ return;
default:
dev_err(&nic->pdev->dev,
"Invalid msg from VF%d, msg 0x%x\n", vf, mbx.msg.msg);
@@ -1157,8 +1144,6 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
mbx.msg.msg, vf);
nic_mbx_send_nack(nic, vf);
}
-unlock:
- nic->mbx_lock[vf] = false;
}
static irqreturn_t nic_mbx_intr_handler(int irq, void *nic_irq)
@@ -1306,52 +1291,6 @@ static int nic_sriov_init(struct pci_dev *pdev, struct nicpf *nic)
return 0;
}
-/* Poll for BGX LMAC link status and update corresponding VF
- * if there is a change, valid only if internal L2 switch
- * is not present otherwise VF link is always treated as up
- */
-static void nic_poll_for_link(struct work_struct *work)
-{
- union nic_mbx mbx = {};
- struct nicpf *nic;
- struct bgx_link_status link;
- u8 vf, bgx, lmac;
-
- nic = container_of(work, struct nicpf, dwork.work);
-
- mbx.link_status.msg = NIC_MBOX_MSG_BGX_LINK_CHANGE;
-
- for (vf = 0; vf < nic->num_vf_en; vf++) {
- /* Poll only if VF is UP */
- if (!nic->vf_enabled[vf])
- continue;
-
- /* Get BGX, LMAC indices for the VF */
- bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
- lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
- /* Get interface link status */
- bgx_get_lmac_link_state(nic->node, bgx, lmac, &link);
-
- /* Inform VF only if link status changed */
- if (nic->link[vf] == link.link_up)
- continue;
-
- if (!nic->mbx_lock[vf]) {
- nic->link[vf] = link.link_up;
- nic->duplex[vf] = link.duplex;
- nic->speed[vf] = link.speed;
-
- /* Send a mbox message to VF with current link status */
- mbx.link_status.link_up = link.link_up;
- mbx.link_status.duplex = link.duplex;
- mbx.link_status.speed = link.speed;
- mbx.link_status.mac_type = link.mac_type;
- nic_send_msg_to_vf(nic, vf, &mbx);
- }
- }
- queue_delayed_work(nic->check_link, &nic->dwork, HZ * 2);
-}
-
static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
struct device *dev = &pdev->dev;
@@ -1420,18 +1359,6 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (!nic->vf_lmac_map)
goto err_release_regions;
- nic->link = devm_kmalloc_array(dev, max_lmac, sizeof(u8), GFP_KERNEL);
- if (!nic->link)
- goto err_release_regions;
-
- nic->duplex = devm_kmalloc_array(dev, max_lmac, sizeof(u8), GFP_KERNEL);
- if (!nic->duplex)
- goto err_release_regions;
-
- nic->speed = devm_kmalloc_array(dev, max_lmac, sizeof(u32), GFP_KERNEL);
- if (!nic->speed)
- goto err_release_regions;
-
/* Initialize hardware */
nic_init_hw(nic);
@@ -1447,19 +1374,8 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (err)
goto err_unregister_interrupts;
- /* Register a physical link status poll fn() */
- nic->check_link = alloc_workqueue("check_link_status",
- WQ_UNBOUND | WQ_MEM_RECLAIM, 1);
- if (!nic->check_link) {
- err = -ENOMEM;
- goto err_disable_sriov;
- }
-
return 0;
-err_disable_sriov:
- if (nic->flags & NIC_SRIOV_ENABLED)
- pci_disable_sriov(pdev);
err_unregister_interrupts:
nic_unregister_interrupts(nic);
err_release_regions:
@@ -1480,12 +1396,6 @@ static void nic_remove(struct pci_dev *pdev)
if (nic->flags & NIC_SRIOV_ENABLED)
pci_disable_sriov(pdev);
- if (nic->check_link) {
- /* Destroy work Queue */
- cancel_delayed_work_sync(&nic->dwork);
- destroy_workqueue(nic->check_link);
- }
-
nic_unregister_interrupts(nic);
pci_release_regions(pdev);
--
2.17.2
^ permalink raw reply related
* [PATCH v3 2/8] net: thunderx: replace global nicvf_rx_mode_wq work queue for all VFs to private for each of them.
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
Having one work queue for receive mode configuration ndo_set_rx_mode()
call for all VFs results in making each of them wait till the
set_rx_mode() call completes for another VF if any of close, set
receive mode and change flags calls being already invoked. Potentially
this could cause device state change before appropriate call of receive
mode configuration completes, so the call itself became meaningless,
corrupt data or break configuration sequence.
We don't need any delays in NIC VF configuration sequence so having delayed
work call with 0 delay has no sense.
This commit is to implement one work queue for each NIC VF for set_rx_mode
task and to let them work independently and replacing delayed_work
with work_struct.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 4 ++-
.../net/ethernet/cavium/thunder/nicvf_main.c | 30 ++++++++++---------
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index f4d81765221e..376a96bce33f 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -271,7 +271,7 @@ struct xcast_addr_list {
};
struct nicvf_work {
- struct delayed_work work;
+ struct work_struct work;
u8 mode;
struct xcast_addr_list *mc;
};
@@ -327,6 +327,8 @@ struct nicvf {
struct nicvf_work rx_mode_work;
/* spinlock to protect workqueue arguments from concurrent access */
spinlock_t rx_mode_wq_lock;
+ /* workqueue for handling kernel ndo_set_rx_mode() calls */
+ struct workqueue_struct *nicvf_rx_mode_wq;
/* PTP timestamp */
struct cavium_ptp *ptp_clock;
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 88f8a8fa93cd..abf24e7dff2d 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -68,9 +68,6 @@ module_param(cpi_alg, int, 0444);
MODULE_PARM_DESC(cpi_alg,
"PFC algorithm (0=none, 1=VLAN, 2=VLAN16, 3=IP Diffserv)");
-/* workqueue for handling kernel ndo_set_rx_mode() calls */
-static struct workqueue_struct *nicvf_rx_mode_wq;
-
static inline u8 nicvf_netdev_qidx(struct nicvf *nic, u8 qidx)
{
if (nic->sqs_mode)
@@ -1311,6 +1308,9 @@ int nicvf_stop(struct net_device *netdev)
struct nicvf_cq_poll *cq_poll = NULL;
union nic_mbx mbx = {};
+ /* wait till all queued set_rx_mode tasks completes */
+ drain_workqueue(nic->nicvf_rx_mode_wq);
+
mbx.msg.msg = NIC_MBOX_MSG_SHUTDOWN;
nicvf_send_msg_to_pf(nic, &mbx);
@@ -1418,6 +1418,9 @@ int nicvf_open(struct net_device *netdev)
struct nicvf_cq_poll *cq_poll = NULL;
union nic_mbx mbx = {};
+ /* wait till all queued set_rx_mode tasks completes if any */
+ drain_workqueue(nic->nicvf_rx_mode_wq);
+
netif_carrier_off(netdev);
err = nicvf_register_misc_interrupt(nic);
@@ -1973,7 +1976,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
static void nicvf_set_rx_mode_task(struct work_struct *work_arg)
{
struct nicvf_work *vf_work = container_of(work_arg, struct nicvf_work,
- work.work);
+ work);
struct nicvf *nic = container_of(vf_work, struct nicvf, rx_mode_work);
u8 mode;
struct xcast_addr_list *mc;
@@ -2030,7 +2033,7 @@ static void nicvf_set_rx_mode(struct net_device *netdev)
kfree(nic->rx_mode_work.mc);
nic->rx_mode_work.mc = mc_list;
nic->rx_mode_work.mode = mode;
- queue_delayed_work(nicvf_rx_mode_wq, &nic->rx_mode_work.work, 0);
+ queue_work(nic->nicvf_rx_mode_wq, &nic->rx_mode_work.work);
spin_unlock(&nic->rx_mode_wq_lock);
}
@@ -2187,7 +2190,10 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
INIT_WORK(&nic->reset_task, nicvf_reset_task);
- INIT_DELAYED_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task);
+ nic->nicvf_rx_mode_wq = alloc_ordered_workqueue("nicvf_rx_mode_wq_VF%d",
+ WQ_MEM_RECLAIM,
+ nic->vf_id);
+ INIT_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task);
spin_lock_init(&nic->rx_mode_wq_lock);
err = register_netdev(netdev);
@@ -2228,13 +2234,15 @@ static void nicvf_remove(struct pci_dev *pdev)
nic = netdev_priv(netdev);
pnetdev = nic->pnicvf->netdev;
- cancel_delayed_work_sync(&nic->rx_mode_work.work);
-
/* Check if this Qset is assigned to different VF.
* If yes, clean primary and all secondary Qsets.
*/
if (pnetdev && (pnetdev->reg_state == NETREG_REGISTERED))
unregister_netdev(pnetdev);
+ if (nic->nicvf_rx_mode_wq) {
+ destroy_workqueue(nic->nicvf_rx_mode_wq);
+ nic->nicvf_rx_mode_wq = NULL;
+ }
nicvf_unregister_interrupts(nic);
pci_set_drvdata(pdev, NULL);
if (nic->drv_stats)
@@ -2261,17 +2269,11 @@ static struct pci_driver nicvf_driver = {
static int __init nicvf_init_module(void)
{
pr_info("%s, ver %s\n", DRV_NAME, DRV_VERSION);
- nicvf_rx_mode_wq = alloc_ordered_workqueue("nicvf_generic",
- WQ_MEM_RECLAIM);
return pci_register_driver(&nicvf_driver);
}
static void __exit nicvf_cleanup_module(void)
{
- if (nicvf_rx_mode_wq) {
- destroy_workqueue(nicvf_rx_mode_wq);
- nicvf_rx_mode_wq = NULL;
- }
pci_unregister_driver(&nicvf_driver);
}
--
2.17.2
^ permalink raw reply related
* [PATCH v3 5/8] net: thunderx: rework xcast message structure to make it fit into 64 bit
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
To communicate to PF each of ThunderX NIC VF uses mailbox which is
pair of 64 bit registers available to both VFn and PF.
This commit is to change the xcast message structure in order to
fit it into 64 bit.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 6 ++----
drivers/net/ethernet/cavium/thunder/nic_main.c | 4 ++--
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 6 +++---
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 376a96bce33f..227343625e83 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -577,10 +577,8 @@ struct set_ptp {
struct xcast {
u8 msg;
- union {
- u8 mode;
- u64 mac;
- } data;
+ u8 mode;
+ u64 mac:48;
};
/* 128 bit shared memory between PF and each VF */
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 90497a27df18..620dbe082ca0 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -1094,7 +1094,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
bgx_set_dmac_cam_filter(nic->node, bgx, lmac,
- mbx.xcast.data.mac,
+ mbx.xcast.mac,
vf < NIC_VF_PER_MBX_REG ? vf :
vf - NIC_VF_PER_MBX_REG);
break;
@@ -1106,7 +1106,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
}
bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
- bgx_set_xcast_mode(nic->node, bgx, lmac, mbx.xcast.data.mode);
+ bgx_set_xcast_mode(nic->node, bgx, lmac, mbx.xcast.mode);
break;
default:
dev_err(&nic->pdev->dev,
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 45f06504a61b..da5986ca7bee 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1961,7 +1961,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
* its' own LMAC to the filter to accept packets for it.
*/
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
- mbx.xcast.data.mac = 0;
+ mbx.xcast.mac = 0;
if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
goto free_mc;
}
@@ -1971,7 +1971,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
/* now go through kernel list of MACs and add them one by one */
for (idx = 0; idx < mc_addrs->count; idx++) {
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
- mbx.xcast.data.mac = mc_addrs->mc[idx];
+ mbx.xcast.mac = mc_addrs->mc[idx];
if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
goto free_mc;
}
@@ -1979,7 +1979,7 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
/* and finally set rx mode for PF accordingly */
mbx.xcast.msg = NIC_MBOX_MSG_SET_XCAST;
- mbx.xcast.data.mode = mode;
+ mbx.xcast.mode = mode;
nicvf_send_msg_to_pf(nic, &mbx);
free_mc:
--
2.17.2
^ permalink raw reply related
* [PATCH v3 3/8] net: thunderx: make CFG_DONE message to run through generic send-ack sequence
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
At the end of NIC VF initialization VF sends CFG_DONE message to PF without
using nicvf_msg_send_to_pf routine. This potentially could re-write data in
mailbox. This commit is to implement common way of sending CFG_DONE message
by the same way with other configuration messages by using
nicvf_send_msg_to_pf() routine.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic_main.c | 2 +-
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 15 ++++++++++++---
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 6c8dcb65ff03..90497a27df18 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -1039,7 +1039,7 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
case NIC_MBOX_MSG_CFG_DONE:
/* Last message of VF config msg sequence */
nic_enable_vf(nic, vf, true);
- goto unlock;
+ break;
case NIC_MBOX_MSG_SHUTDOWN:
/* First msg in VF teardown sequence */
if (vf >= nic->num_vf_en)
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index abf24e7dff2d..19b58fc3ca41 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -169,6 +169,17 @@ static int nicvf_check_pf_ready(struct nicvf *nic)
return 1;
}
+static void nicvf_send_cfg_done(struct nicvf *nic)
+{
+ union nic_mbx mbx = {};
+
+ mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
+ if (nicvf_send_msg_to_pf(nic, &mbx)) {
+ netdev_err(nic->netdev,
+ "PF didn't respond to CFG DONE msg\n");
+ }
+}
+
static void nicvf_read_bgx_stats(struct nicvf *nic, struct bgx_stats_msg *bgx)
{
if (bgx->rx)
@@ -1416,7 +1427,6 @@ int nicvf_open(struct net_device *netdev)
struct nicvf *nic = netdev_priv(netdev);
struct queue_set *qs = nic->qs;
struct nicvf_cq_poll *cq_poll = NULL;
- union nic_mbx mbx = {};
/* wait till all queued set_rx_mode tasks completes if any */
drain_workqueue(nic->nicvf_rx_mode_wq);
@@ -1515,8 +1525,7 @@ int nicvf_open(struct net_device *netdev)
nicvf_enable_intr(nic, NICVF_INTR_RBDR, qidx);
/* Send VF config done msg to PF */
- mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
- nicvf_write_to_mbx(nic, &mbx);
+ nicvf_send_cfg_done(nic);
return 0;
cleanup:
--
2.17.2
^ permalink raw reply related
* [PATCH bpf-next] bpf, seccomp: fix false positive preemption splat for cbpf->ebpf progs
From: Daniel Borkmann @ 2019-02-20 11:06 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
a check was added for BPF_PROG_RUN() that for every invocation preemption has
to be disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this
does not count for seccomp because only cBPF -> eBPF is loaded here and it does
not make use of any functionality that would require this assertion. Fix this
false positive by adding and using __BPF_PROG_RUN() variant that does not have
the cant_sleep(); check.
Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/filter.h | 9 ++++++++-
kernel/seccomp.c | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index f32b3ec..2648fd7 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -533,7 +533,14 @@ struct sk_filter {
struct bpf_prog *prog;
};
-#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
+#define bpf_prog_run__non_preempt(prog, ctx) \
+ ({ cant_sleep(); __BPF_PROG_RUN(prog, ctx); })
+/* Native eBPF or cBPF -> eBPF transitions. Preemption must be disabled. */
+#define BPF_PROG_RUN(prog, ctx) \
+ bpf_prog_run__non_preempt(prog, ctx)
+/* Direct use for cBPF -> eBPF only, but not for native eBPF. */
+#define __BPF_PROG_RUN(prog, ctx) \
+ (*(prog)->bpf_func)(ctx, (prog)->insnsi)
#define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index e815781..826d4e4 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -268,7 +268,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
* value always takes priority (ignoring the DATA).
*/
for (; f; f = f->prev) {
- u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
+ u32 cur_ret = __BPF_PROG_RUN(f->prog, sd);
if (ACTION_ONLY(cur_ret) < ACTION_ONLY(ret)) {
ret = cur_ret;
--
2.9.5
^ permalink raw reply related
* Re: [PATCH v3 0/8] nic: thunderx: fix communication races between VF & PF
From: Vadim Lomovtsev @ 2019-02-20 11:19 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
sorry for occasionally reply to old thread.
On Wed, Feb 20, 2019 at 11:02:42AM +0000, Vadim Lomovtsev wrote:
> The ThunderX CN88XX NIC Virtual Function driver uses mailbox interface
> to communicate to physical function driver. Each of VF has it's own pair
> of mailbox registers to read from and write to. The mailbox registers
> has no protection from possible races, so it has to be implemented
> at software side.
>
> After long term testing by loop of 'ip link set <ifname> up/down'
> command it was found that there are two possible scenarios when
> race condition appears:
> 1. VF receives link change message from PF and VF send RX mode
> configuration message to PF in the same time from separate thread.
> 2. PF receives RX mode configuration from VF and in the same time,
> in separate thread PF detects link status change and sends appropriate
> message to particular VF.
>
> Both cases leads to mailbox data to be rewritten, NIC VF messaging control
> data to be updated incorrectly and communication sequence gets broken.
>
> This patch series is to address race condition with VF & PF communication.
>
> Changes:
> v1 -> v2
> - 0000: correct typo in cover letter subject: 'betwen' -> 'between';
> - move link state polling request task from pf to vf
> instead of cheking status of mailbox irq;
> v2 -> v3
> - 0003: change return type of nicvf_send_cfg_done() function
> from int to void;
> - 0007: update subject and remove unused variable 'netdev'
> from nicvf_link_status_check_task() function;
>
> Vadim Lomovtsev (8):
> net: thunderx: correct typo in macro name
> net: thunderx: replace global nicvf_rx_mode_wq work queue for all VFs
> to private for each of them.
> net: thunderx: make CFG_DONE message to run through generic send-ack
> sequence
> net: thunderx: add nicvf_send_msg_to_pf result check for
> set_rx_mode_task
> net: thunderx: rework xcast message structure to make it fit into 64
> bit
> net: thunderx: add mutex to protect mailbox from concurrent calls for
> same VF
> net: thunderx: add LINK_CHANGE message handler at nicpf
> net: thunderx: remove link change polling code and info from nicpf
>
> drivers/net/ethernet/cavium/thunder/nic.h | 14 +-
> .../net/ethernet/cavium/thunder/nic_main.c | 149 ++++++------------
> .../net/ethernet/cavium/thunder/nicvf_main.c | 130 ++++++++++-----
> .../net/ethernet/cavium/thunder/thunder_bgx.c | 2 +-
> .../net/ethernet/cavium/thunder/thunder_bgx.h | 2 +-
> 5 files changed, 144 insertions(+), 153 deletions(-)
>
> --
> 2.17.2
^ permalink raw reply
* Re: BUG: assuming atomic context at kernel/seccomp.c:LINE
From: syzbot @ 2019-02-20 12:33 UTC (permalink / raw)
To: ast, daniel, kafai, keescook, linux-kernel, luto, netdev,
songliubraving, syzkaller-bugs, wad, yhs
In-Reply-To: <000000000000cedfe1058250076c@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: abf446c90405 Add linux-next specific files for 20190220
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=101e7fb0c00000
kernel config: https://syzkaller.appspot.com/x/.config?x=463cb576ac40e350
dashboard link: https://syzkaller.appspot.com/bug?extid=8bf19ee2aa580de7a2a7
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11a52778c00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12a1007cc00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
BUG: assuming atomic context at kernel/seccomp.c:271
in_atomic(): 0, irqs_disabled(): 0, pid: 7853, name: syz-executor140
no locks held by syz-executor140/7853.
CPU: 1 PID: 7853 Comm: syz-executor140 Not tainted 5.0.0-rc7-next-20190220
#39
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+0x172/0x1f0 lib/dump_stack.c:113
__cant_sleep kernel/sched/core.c:6218 [inline]
__cant_sleep.cold+0xa3/0xbb kernel/sched/core.c:6195
seccomp_run_filters kernel/seccomp.c:271 [inline]
__seccomp_filter+0x12b/0x12b0 kernel/seccomp.c:801
__secure_computing+0x101/0x360 kernel/seccomp.c:932
syscall_trace_enter+0x5bf/0xe10 arch/x86/entry/common.c:120
do_syscall_64+0x479/0x610 arch/x86/entry/common.c:280
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x43ec58
Code: 00 00 be 3c 00 00 00 eb 19 66 0f 1f 84 00 00 00 00 00 48 89 d7 89 f0
0f 05 48 3d 00 f0 ff ff 77 21 f4 48 89 d7 44 89 c0 0f 05 <48> 3d 00 f0 ff
ff 76 e0 f7 d8 64 41 89 01 eb d8 0f 1f 84 00 00 00
RSP: 002b:00007ffc2d0b2f48 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000043ec58
RDX: 0000000000000000 RSI: 0000000
^ permalink raw reply
* Re: [RFC PATCH net-next v3 10/21] ethtool: provide string sets with GET_STRSET request
From: Michal Kubecek @ 2019-02-20 12:34 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, David Miller, Andrew Lunn, Jiri Pirko, linux-kernel
In-Reply-To: <20190219185610.4c6aa82a@cakuba.netronome.com>
On Tue, Feb 19, 2019 at 06:56:10PM -0800, Jakub Kicinski wrote:
> On Mon, 18 Feb 2019 19:22:14 +0100 (CET), Michal Kubecek wrote:
> > Requests a contents of one or more string sets, i.e. indexed arrays of
> > strings; this information is provided by ETHTOOL_GSSET_INFO and
> > ETHTOOL_GSTRINGS commands of ioctl interface. There are three types of
> > requests:
> >
> > - no NLM_F_DUMP, no device: get "global" stringsets
> > - no NLM_F_DUMP, with device: get string sets related to the device
> > - NLM_F_DUMP, no device: get device related string sets for all devices
> >
> > It's possible to request all string sets of given type or only specific
> > sets. With ETHA_STRSET_COUNTS flag, only set sizes (number of strings) are
> > returned.
>
> > +GET_STRSET
> > +----------
> > +
> > +Requests contents of a string set as provided by ioctl commands
> > +ETHTOOL_GSSET_INFO and ETHTOOL_GSTRINGS. String sets are not user writeable so
> > +that the corresponding SET_STRSET message is only used in kernel replies.
> > +There are two types of string sets: global (independent of a device, e.g.
> > +device feature names) and device specific (e.g. device private flags).
> > +
> > +Request contents:
> > +
> > + ETHA_STRSET_DEV (nested) device identification
> > + ETHA_STRSET_COUNTS (flag) request only string counts
> > + ETHA_STRSET_STRINGSET (nested) string set to request
> > + ETHA_STRINGSET_ID (u32) set id
> > +
> > +Kernel response contents:
> > +
> > + ETHA_STRSET_DEV (nested) device identification
> > + ETHA_STRSET_STRINGSET (nested) string set to request
>
> Is it common to put the device information outside of the main
> attribute nest?
There may be multiple string sets (ETHA_STRSET_STIRNGSET nests) in one
message but they would be either all global or all related to the same
device. If string sets for multiple devices are sent, it would be in
response to a dump request and there would be one message per device.
>
> > + ETHA_STRINGSET_ID (u32) set id
> > + ETHA_STRINGSET_COUNT (u32) number of strings
> > + ETHA_STRINGSET_STRINGS (nested) array of strings
> > + ETHA_STRING_INDEX (u32) string index
> > + ETHA_STRING_VALUE (string) string value
> > +
> > +ETHA_STRSET_DEV, if present, identifies the device to request device specific
> > +string sets for. Depending on its presence a and NLM_F_DUMP flag, there are
> > +three type of GET_STRSET requests:
> > +
> > + - no NLM_F_DUMP, no device: get "global" stringsets
> > + - no NLM_F_DUMP, with device: get string sets related to the device
> > + - NLM_F_DUMP, no device: get device related string sets for all devices
> > +
> > +If there is no ETHA_STRSET_STRINGSET attribute, all string sets of requested
> > +type are returned, otherwise only those specified in the request. Flag
> > +ETHA_STRSET_COUNTS tells kernel to only return string counts of the sets, not
> > +the actual strings.
> > +
> > +
>
> > +static int get_strset_id(const struct nlattr *nest, u32 *val,
> > + struct genl_info *info)
> > +{
> > + struct nlattr *tb[ETHA_STRINGSET_MAX + 1];
> > + int ret;
> > +
> > + ret = nla_parse_nested(tb, ETHA_STRINGSET_MAX, nest, stringset_policy,
> > + info ? info->extack : NULL);
>
> Would it make sense to use strict parsing everywhere from the start?
> You seem to add REJECTS, but won't attributes > max get ignored?
Good point, I forgot about this when the strict checking helpers were
added. I'll change the uses of nlmsg_parse() and nla_parse_nested() to
their strict variants (and add nla_parse_nested_strict()).
Michal
^ permalink raw reply
* [PATCH net-next v3 0/3] net: dsa: mv88e6xxx: fix IPv6
From: Russell King - ARM Linux admin @ 2019-02-20 12:36 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Vivien Didelot
Cc: David S. Miller, Heiner Kallweit, netdev
We have had some emails in private over this issue, this is my current
patch set rebased on top of net-next which provides working IPv6 (and
probably other protocols as well) over mv88e6xxx DSA switches.
The problem comes down to mv88e6xxx defaulting to not flood unknown
unicast and multicast datagrams, as they would be by dumb switches,
and as the Linux bridge code does by default.
There is also the issue of IPv6 over a vlan that is transparent to the
bridge; the multicast querier will not reach inside the vlan, and so
the switch can not learn about multicast routing within the vlan.
These flood settings can be disabled via the Linux bridge code if it's
desired to make the switch behave more like a managed switch, eg, by
enabling the multicast querier. However, the multicast querier
defaults to being disabled which effectively means that by default,
mv88e6xxx switches block all multicast traffic. This is at odds with
the Linux bridge documentation, and the defaults that the Linux bridge
code adopts.
So, this patch set adds DSA support for Linux bridge flags, adds
mv88e6xxx support for the unicast and multicast flooding flags, and
lastly enables flooding of these frames by default to match the
Linux bridge defaults.
drivers/net/dsa/mv88e6xxx/chip.c | 19 +++++++++++++++++++
include/net/dsa.h | 2 ++
net/dsa/dsa_priv.h | 2 ++
net/dsa/port.c | 28 +++++++++++++++++++++++++++-
net/dsa/slave.c | 6 ++++++
5 files changed, 56 insertions(+), 1 deletion(-)
v2: fix a couple of compile errors in patch 2 and patch 3 (oops).
v3: change interface between core DSA and drivers
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* [PATCH net-next v3 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Russell King @ 2019-02-20 12:36 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Vivien Didelot
Cc: Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190220123615.fcyrlhz5jpx5ecgv@shell.armlinux.org.uk>
Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
the multicast and unicast flood properties to be controlled. These
can be controlled on a per-port basis via commands such as:
bridge link set dev lan1 flood on|off
bridge link set dev lan1 mcast_flood on|off
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/mv88e6xxx/chip.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 32e7af5caa69..937370639fe4 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4692,6 +4692,24 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
return err;
}
+static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
+ bool unicast, bool multicast)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+ int ret = -EOPNOTSUPP;
+
+ WARN_ON_ONCE(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port));
+
+ mutex_lock(&chip->reg_lock);
+ if (chip->info->ops->port_set_egress_floods)
+ ret = chip->info->ops->port_set_egress_floods(chip, port,
+ unicast,
+ multicast);
+ mutex_unlock(&chip->reg_lock);
+
+ return ret;
+}
+
static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
#if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
.probe = mv88e6xxx_drv_probe,
@@ -4719,6 +4737,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
.set_ageing_time = mv88e6xxx_set_ageing_time,
.port_bridge_join = mv88e6xxx_port_bridge_join,
.port_bridge_leave = mv88e6xxx_port_bridge_leave,
+ .port_egress_floods = mv88e6xxx_port_egress_floods,
.port_stp_state_set = mv88e6xxx_port_stp_state_set,
.port_fast_age = mv88e6xxx_port_fast_age,
.port_vlan_filtering = mv88e6xxx_port_vlan_filtering,
--
2.7.4
^ permalink raw reply related
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