* Re: [PATCH net-next] net: phy: let genphy_c45_read_abilities also check aneg capability
From: Maxime Chevallier @ 2019-02-22 16:51 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Andrew Lunn, Florian Fainelli, David Miller,
netdev@vger.kernel.org
In-Reply-To: <a798f789-94a9-f1ac-1cba-276a2c517089@gmail.com>
Hello Heiner,
On Fri, 22 Feb 2019 08:23:04 +0100
Heiner Kallweit <hkallweit1@gmail.com> wrote:
>When using genphy_c45_read_abilities() as get_features callback we
>also have to set the autoneg capability in phydev->supported.
>@@ -336,6 +336,17 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev)
This makes the helper not reading PMA-only registers, so the naming
isn't really relevant anymore, but that's a small detail.
Other than that,
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Thanks,
Maxime
^ permalink raw reply
* Re: [PATCH net-next 3/5] devlink: create a special NDO for getting the devlink instance
From: Jakub Kicinski @ 2019-02-22 16:56 UTC (permalink / raw)
To: Michal Kubecek; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222095131.GQ23151@unicorn.suse.cz>
On Fri, 22 Feb 2019 10:51:31 +0100, Michal Kubecek wrote:
> On Thu, Feb 21, 2019 at 09:46:18AM -0800, Jakub Kicinski wrote:
> > Instead of iterating over all devlink ports add a NDO which
> > will return the devlink instance from the driver.
> >
> > Suggested-by: Jiri Pirko <jiri@resnulli.us>
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> > include/linux/netdevice.h | 6 ++++
> > net/core/devlink.c | 62 +++++++++++++--------------------------
> > 2 files changed, 26 insertions(+), 42 deletions(-)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index aab4d9f6613d..eebcef6b8191 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -940,6 +940,8 @@ struct dev_ifalias {
> > char ifalias[];
> > };
> >
> > +struct devlink;
> > +
> > /*
> > * This structure defines the management hooks for network devices.
> > * The following hooks can be defined; unless noted otherwise, they are
> > @@ -1248,6 +1250,9 @@ struct dev_ifalias {
> > * that got dropped are freed/returned via xdp_return_frame().
> > * Returns negative number, means general error invoking ndo, meaning
> > * no frames were xmit'ed and core-caller will free all frames.
> > + * struct devlink *(*ndo_get_devlink)(struct net_device *dev);
> > + * Get devlink instance associated with a given netdev.
> > + * Called with a reference on the device only, rtnl_lock is not held.
>
> The formulation is a bit ambiguous. Do I understand correctly that what
> it says is that device reference is sufficient and rtnl_lock is not
> necessary (but there is no harm if caller holds rtnl_lock)?
Yes, only reference is held, the expectation is that drivers free
netdevs before they unregister devlink. The devlink vs rtnl ordering
gets a bit tricky, drivers should definitely not expect rtnl to be held
in this NDO. If the caller comes from rtnl path (like ethtool) it will
likely have to release rtnl before taking devlink->lock.
^ permalink raw reply
* INFO: task hung in rtnetlink_rcv_msg
From: syzbot @ 2019-02-22 17:00 UTC (permalink / raw)
To: ast, christian, daniel, davem, dsahern, hawk, idosch,
jakub.kicinski, john.fastabend, kafai, ktkhai, linux-kernel,
netdev, petrm, roopa, songliubraving, syzkaller-bugs, xdp-newbies,
yhs
Hello,
syzbot found the following crash on:
HEAD commit: 64c0133eb88a Merge tag 'armsoc-fixes' of git://git.kernel...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14976800c00000
kernel config: https://syzkaller.appspot.com/x/.config?x=ee434566c893c7b1
dashboard link: https://syzkaller.appspot.com/bug?extid=8218a8a0ff60c19b8eae
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11c8a14cc00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1189db74c00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+8218a8a0ff60c19b8eae@syzkaller.appspotmail.com
INFO: task syz-executor725:8005 blocked for more than 140 seconds.
Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D27984 8005 7591 0x00000004
Call Trace:
context_switch kernel/sched/core.c:2844 [inline]
__schedule+0x817/0x1cc0 kernel/sched/core.c:3485
schedule+0x92/0x180 kernel/sched/core.c:3529
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
__mutex_lock_common kernel/locking/mutex.c:1002 [inline]
__mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
rtnl_lock net/core/rtnetlink.c:77 [inline]
rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xdd/0x130 net/socket.c:631
___sys_sendmsg+0x806/0x930 net/socket.c:2114
__sys_sendmsg+0x105/0x1d0 net/socket.c:2152
__do_sys_sendmsg net/socket.c:2161 [inline]
__se_sys_sendmsg net/socket.c:2159 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac037 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
INFO: task syz-executor725:8008 blocked for more than 140 seconds.
Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D27552 8008 7590 0x00000004
Call Trace:
context_switch kernel/sched/core.c:2844 [inline]
__schedule+0x817/0x1cc0 kernel/sched/core.c:3485
schedule+0x92/0x180 kernel/sched/core.c:3529
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
__mutex_lock_common kernel/locking/mutex.c:1002 [inline]
__mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
rtnl_lock net/core/rtnetlink.c:77 [inline]
rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xdd/0x130 net/socket.c:631
___sys_sendmsg+0x806/0x930 net/socket.c:2114
__sys_sendmsg+0x105/0x1d0 net/socket.c:2152
__do_sys_sendmsg net/socket.c:2161 [inline]
__se_sys_sendmsg net/socket.c:2159 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac03d R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
INFO: task syz-executor725:8009 blocked for more than 140 seconds.
Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D28008 8009 7595 0x00000004
Call Trace:
context_switch kernel/sched/core.c:2844 [inline]
__schedule+0x817/0x1cc0 kernel/sched/core.c:3485
schedule+0x92/0x180 kernel/sched/core.c:3529
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
__mutex_lock_common kernel/locking/mutex.c:1002 [inline]
__mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
rtnl_lock net/core/rtnetlink.c:77 [inline]
rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xdd/0x130 net/socket.c:631
___sys_sendmsg+0x806/0x930 net/socket.c:2114
__sys_sendmsg+0x105/0x1d0 net/socket.c:2152
__do_sys_sendmsg net/socket.c:2161 [inline]
__se_sys_sendmsg net/socket.c:2159 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac03d R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
INFO: task syz-executor725:8010 blocked for more than 140 seconds.
Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D28000 8010 7596 0x00000004
Call Trace:
context_switch kernel/sched/core.c:2844 [inline]
__schedule+0x817/0x1cc0 kernel/sched/core.c:3485
schedule+0x92/0x180 kernel/sched/core.c:3529
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
__mutex_lock_common kernel/locking/mutex.c:1002 [inline]
__mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
rtnl_lock net/core/rtnetlink.c:77 [inline]
rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xdd/0x130 net/socket.c:631
___sys_sendmsg+0x806/0x930 net/socket.c:2114
__sys_sendmsg+0x105/0x1d0 net/socket.c:2152
__do_sys_sendmsg net/socket.c:2161 [inline]
__se_sys_sendmsg net/socket.c:2159 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac03e R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
INFO: task syz-executor725:8011 blocked for more than 140 seconds.
Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D28008 8011 7594 0x00000004
Call Trace:
context_switch kernel/sched/core.c:2844 [inline]
__schedule+0x817/0x1cc0 kernel/sched/core.c:3485
schedule+0x92/0x180 kernel/sched/core.c:3529
schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
__mutex_lock_common kernel/locking/mutex.c:1002 [inline]
__mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
rtnl_lock net/core/rtnetlink.c:77 [inline]
rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xdd/0x130 net/socket.c:631
___sys_sendmsg+0x806/0x930 net/socket.c:2114
__sys_sendmsg+0x105/0x1d0 net/socket.c:2152
__do_sys_sendmsg net/socket.c:2161 [inline]
__se_sys_sendmsg net/socket.c:2159 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac03b R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
Showing all locks held in the system:
3 locks held by kworker/1:1/22:
#0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:
__write_once_size include/linux/compiler.h:220 [inline]
#0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:
arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:
atomic64_set include/asm-generic/atomic-instrumented.h:40 [inline]
#0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:
atomic_long_set include/asm-generic/atomic-long.h:59 [inline]
#0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:
set_work_data kernel/workqueue.c:617 [inline]
#0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:
set_work_pool_and_clear_pending kernel/workqueue.c:644 [inline]
#0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:
process_one_work+0x87e/0x1790 kernel/workqueue.c:2144
#1: 00000000e8391065 ((addr_chk_work).work){+.+.}, at:
process_one_work+0x8b4/0x1790 kernel/workqueue.c:2148
#2: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock+0x17/0x20
net/core/rtnetlink.c:77
1 lock held by khungtaskd/1040:
#0: 00000000083344d4 (rcu_read_lock){....}, at:
debug_show_all_locks+0x5f/0x27e kernel/locking/lockdep.c:4389
1 lock held by rsyslogd/7472:
#0: 000000007562dcc1 (&f->f_pos_lock){+.+.}, at: __fdget_pos+0xee/0x110
fs/file.c:795
2 locks held by getty/7562:
#0: 000000000837ceb2 (&tty->ldisc_sem){++++}, at:
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
#1: 00000000d91e6e17 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7563:
#0: 00000000243db696 (&tty->ldisc_sem){++++}, at:
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
#1: 000000000002034d (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7564:
#0: 00000000a59e483b (&tty->ldisc_sem){++++}, at:
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
#1: 000000004032d53a (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7565:
#0: 0000000025343c97 (&tty->ldisc_sem){++++}, at:
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
#1: 00000000727bf19a (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7566:
#0: 000000003dac20af (&tty->ldisc_sem){++++}, at:
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
#1: 00000000fef38c44 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7567:
#0: 00000000da1bfc9c (&tty->ldisc_sem){++++}, at:
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
#1: 00000000f82cf636 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7568:
#0: 00000000a5d9fcdf (&tty->ldisc_sem){++++}, at:
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
#1: 000000005ae165cb (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
1 lock held by syz-executor725/8002:
1 lock held by syz-executor725/8005:
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock
net/core/rtnetlink.c:77 [inline]
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00
net/core/rtnetlink.c:5127
1 lock held by syz-executor725/8008:
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock
net/core/rtnetlink.c:77 [inline]
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00
net/core/rtnetlink.c:5127
1 lock held by syz-executor725/8009:
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock
net/core/rtnetlink.c:77 [inline]
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00
net/core/rtnetlink.c:5127
1 lock held by syz-executor725/8010:
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock
net/core/rtnetlink.c:77 [inline]
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00
net/core/rtnetlink.c:5127
1 lock held by syz-executor725/8011:
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock
net/core/rtnetlink.c:77 [inline]
#0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00
net/core/rtnetlink.c:5127
=============================================
NMI backtrace for cpu 0
CPU: 0 PID: 1040 Comm: khungtaskd Not tainted 5.0.0-rc6+ #75
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
nmi_cpu_backtrace.cold+0x63/0xa4 lib/nmi_backtrace.c:101
nmi_trigger_cpumask_backtrace+0x1be/0x236 lib/nmi_backtrace.c:62
arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
trigger_all_cpu_backtrace include/linux/nmi.h:146 [inline]
check_hung_uninterruptible_tasks kernel/hung_task.c:203 [inline]
watchdog+0x9df/0xee0 kernel/hung_task.c:287
kthread+0x357/0x430 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
Sending NMI from CPU 0 to CPUs 1:
INFO: NMI handler (nmi_cpu_backtrace_handler) took too long to run: 1.415
msecs
NMI backtrace for cpu 1
CPU: 1 PID: 8002 Comm: syz-executor725 Not tainted 5.0.0-rc6+ #75
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:unwind_next_frame.part.0+0x1a8/0x9a0
arch/x86/kernel/unwind_frame.c:329
Code: 8b 6d 20 4c 89 ee 48 89 df 4c 89 95 60 ff ff ff 4c 89 9d 68 ff ff ff
4c 89 8d 70 ff ff ff e8 6f f7 ff ff 4c 8b 8d 70 ff ff ff <41> 89 c0 4c 8b
9d 68 ff ff ff b8 01 00 00 00 45 84 c0 4c 8b 95 60
RSP: 0018:ffff888096136e28 EFLAGS: 00000286
RAX: 0000000000000001 RBX: ffff888096136f00 RCX: 1ffff11012c26de8
RDX: dffffc0000000000 RSI: 1ffff11012c26d00 RDI: ffff888096136f48
RBP: ffff888096136ed8 R08: 0000000000000001 R09: ffff888096136f50
R10: ffff888096136f28 R11: ffff888096136f38 R12: 1ffff11012c26dca
R13: ffff888096137b70 R14: ffff888096137b40 R15: ffff888096137f58
FS: 00000000024b2880(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffff600400 CR3: 000000009810b000 CR4: 00000000001406e0
Call Trace:
unwind_next_frame+0x3b/0x50 arch/x86/kernel/unwind_frame.c:287
__save_stack_trace+0x7a/0xf0 arch/x86/kernel/stacktrace.c:44
save_stack_trace+0x1a/0x20 arch/x86/kernel/stacktrace.c:60
save_stack+0x45/0xd0 mm/kasan/common.c:73
set_track mm/kasan/common.c:85 [inline]
__kasan_kmalloc mm/kasan/common.c:496 [inline]
__kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:469
kasan_kmalloc+0x9/0x10 mm/kasan/common.c:504
__do_kmalloc_node mm/slab.c:3673 [inline]
__kmalloc_node_track_caller+0x4e/0x70 mm/slab.c:3687
__kmalloc_reserve.isra.0+0x40/0xf0 net/core/skbuff.c:140
pskb_expand_head+0x14e/0xdd0 net/core/skbuff.c:1465
netlink_trim+0x215/0x270 net/netlink/af_netlink.c:1292
netlink_unicast+0xbf/0x720 net/netlink/af_netlink.c:1326
rtnetlink_send+0xf0/0x110 net/core/rtnetlink.c:721
tcf_add_notify net/sched/act_api.c:1325 [inline]
tcf_action_add+0x243/0x370 net/sched/act_api.c:1344
tc_ctl_action+0x3b6/0x4bd net/sched/act_api.c:1392
rtnetlink_rcv_msg+0x465/0xb00 net/core/rtnetlink.c:5130
netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xdd/0x130 net/socket.c:631
___sys_sendmsg+0x806/0x930 net/socket.c:2114
__sys_sendmsg+0x105/0x1d0 net/socket.c:2152
__do_sys_sendmsg net/socket.c:2161 [inline]
__se_sys_sendmsg net/socket.c:2159 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: e8 ac e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 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 0b 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac031 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
---
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.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH net-next 4/5] nfp: add .ndo_get_devlink
From: Jakub Kicinski @ 2019-02-22 17:02 UTC (permalink / raw)
To: Michal Kubecek; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222100450.GR23151@unicorn.suse.cz>
On Fri, 22 Feb 2019 11:04:50 +0100, Michal Kubecek wrote:
> On Thu, Feb 21, 2019 at 09:46:19AM -0800, Jakub Kicinski wrote:
> > Support getting devlink instance from a new NDO.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> > drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 ++
> > drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 11 +++++++++++
> > drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 1 +
> > drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
> > 4 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > index d578d856a009..f8d422713705 100644
> > --- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > +++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > @@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
> > int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
> > struct nfp_net *nn, unsigned int id);
> >
> > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
> > +
> > #endif
> > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > index db2da99f6aa7..e9eca99cf493 100644
> > --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > @@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
> > {
> > devlink_port_unregister(&port->dl_port);
> > }
> > +
> > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
> > +{
> > + struct nfp_app *app;
> > +
> > + app = nfp_app_from_netdev(netdev);
> > + if (!app)
> > + return NULL;
> > +
> > + return priv_to_devlink(app->pf);
> > +}
>
> AFAICS this would return a pointer to zero initialized struct devlink
> when built with CONFIG_DEVLINK=n. Then devlink_compat_running_version()
> would execute
>
> if (!dev->netdev_ops->ndo_get_devlink)
> return;
>
> devlink = dev->netdev_ops->ndo_get_devlink(dev);
> if (!devlink || !devlink->ops->info_get)
> return;
>
> with non-null devlink but null devlink->ops so that it dereferences null
> pointer (and so does devlink_compat_flash_update()).
devlink_compat_flash_update() is this if CONFIG_DEVLINK=n:
static inline int
devlink_compat_flash_update(struct net_device *dev, const char *file_name)
{
return -EOPNOTSUPP;
}
No?
> Maybe it would be safer not to call ndo_get_devlink directly and have
> an inline wrapper like
>
> #if IS_ENABLED(CONFIG_NET_DEVLINK)
> static inline struct devlink *dev_get_devlink(struct net_device *dev)
> {
> if (dev->netdev_ops->ndo_get_devlink)
> return dev->netdev_ops->ndo_get_devlink();
> else
> retrurn NULL;
> }
> #else
> static inline struct devlink *dev_get_devlink(struct net_device *dev)
> {
> return NULL;
> }
> #endif
>
> so that one can simply call the wrapper and check return value for NULL.
Only devlink code can call this ndo, and it doesn't exist with
DEVLINK=n. I don't dislike wrappers for NDOs, but I'll defer to Jiri
to decide if we want a wrapper here (without the #if/#else, just the
first part for code clarity) :)
^ permalink raw reply
* Re: [PATCH v5 0/5] M_CAN Framework re-write
From: Dan Murphy @ 2019-02-22 17:05 UTC (permalink / raw)
To: Wolfgang Grandegger, mkl, davem; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <b60b8781-d979-24e7-805a-95f9237c816e@ti.com>
Wolfgang
On 2/22/19 6:50 AM, Dan Murphy wrote:
> Wolfgang
>
> On 2/22/19 3:38 AM, Wolfgang Grandegger wrote:
>> Hello Dan,
>>
>> what kernel version is that patch series for. I have problems to apply it!
>>
>
> It is based off of Master
>
> commit 2137397c92aec3713fa10be3c9b830f9a1674e60 (linux_master/master)
>
> And I successfully rebased on top of
>
> commit 8a61716ff2ab23eddd1f7a05a075a374e4d0c3d4 (linux_master/master)
> Merge tag 'ceph-for-5.0-rc8' of git://github.com/ceph/ceph-client
>
I just pulled these patches and they applied fine to the top commit of linux master
Do I need to rebase on top of a for-next branch for you?
Dan
> Dan
>
>> Wolfgang.
>>
>> Am 21.02.19 um 17:41 schrieb Wolfgang Grandegger:
>>> Hello Dan,
>>>
>>> I will have a closer look end of this week!
>>>
>>> Wolfgang.
>>>
>>> Am 21.02.19 um 17:24 schrieb Dan Murphy:
>>>> Bump
>>>>
>>>> On 2/14/19 12:27 PM, Dan Murphy wrote:
>>>>> Hello
>>>>>
>>>>> OK I did not give up on this patch series just got a little preoccupied with
>>>>> some other kernel work. But here is the update per the comments.
>>>>>
>>>>> It should be understood I broke these out for reviewability.
>>>>> For instance the first patch does not compile on its own as including this
>>>>> patch should not change the current functionality and it pulls all the io-mapped
>>>>> code from the m_can base file to a platfrom file.
>>>>>
>>>>> The next patch "Migrate the m_can code to use the framework"
>>>>> is the change to the kernel for the io-mapped conversion from a flat file to use
>>>>> the framework. Finally the rename patch just renames the m_can_priv to
>>>>> m_can_classdev. I broke this change out specifically for readability of the
>>>>> migration patch per comments on the code.
>>>>>
>>>>> AFAIC the first 3 patches can all be squashed into a single patch. Or the
>>>>> first 2 patches in the series can be re-arranged but then m_can functionality is
>>>>> affected in the migration patch.
>>>>>
>>>>> Again the first 3 patches here are all just for readability and review purposes.
>>>>>
>>>>> Dan
>>>>>
>>>>> Dan Murphy (5):
>>>>> can: m_can: Create a m_can platform framework
>>>>> can: m_can: Migrate the m_can code to use the framework
>>>>> can: m_can: Rename m_can_priv to m_can_classdev
>>>>> dt-bindings: can: tcan4x5x: Add DT bindings for TCAN4x5X driver
>>>>> can: tcan4x5x: Add tcan4x5x driver to the kernel
>>>>>
>>>>> .../devicetree/bindings/net/can/tcan4x5x.txt | 37 +
>>>>> drivers/net/can/m_can/Kconfig | 14 +-
>>>>> drivers/net/can/m_can/Makefile | 2 +
>>>>> drivers/net/can/m_can/m_can.c | 788 +++++++++---------
>>>>> drivers/net/can/m_can/m_can.h | 159 ++++
>>>>> drivers/net/can/m_can/m_can_platform.c | 198 +++++
>>>>> drivers/net/can/m_can/tcan4x5x.c | 531 ++++++++++++
>>>>> 7 files changed, 1320 insertions(+), 409 deletions(-)
>>>>> create mode 100644 Documentation/devicetree/bindings/net/can/tcan4x5x.txt
>>>>> create mode 100644 drivers/net/can/m_can/m_can.h
>>>>> create mode 100644 drivers/net/can/m_can/m_can_platform.c
>>>>> create mode 100644 drivers/net/can/m_can/tcan4x5x.c
>>>>>
>>>>
>>>>
>
>
--
------------------
Dan Murphy
^ permalink raw reply
* WARNING in xt_compat_add_offset
From: syzbot @ 2019-02-22 17:10 UTC (permalink / raw)
To: coreteam, davem, fw, kadlec, linux-kernel, netdev,
netfilter-devel, pablo, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 8a61716ff2ab Merge tag 'ceph-for-5.0-rc8' of git://github...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1456fa6cc00000
kernel config: https://syzkaller.appspot.com/x/.config?x=7132344728e7ec3f
dashboard link: https://syzkaller.appspot.com/bug?extid=276ddebab3382bbf72db
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
userspace arch: i386
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=140c0914c00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+276ddebab3382bbf72db@syzkaller.appspotmail.com
IPv6: ADDRCONF(NETDEV_CHANGE): hsr_slave_0: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): hsr_slave_1: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): hsr0: link becomes ready
8021q: adding VLAN 0 to HW filter on device batadv0
cannot load conntrack support for proto=7
WARNING: CPU: 1 PID: 7458 at net/netfilter/x_tables.c:654
xt_compat_add_offset+0x22a/0x290 net/netfilter/x_tables.c:654
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 7458 Comm: syz-executor.0 Not tainted 5.0.0-rc7+ #83
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
panic+0x2cb/0x65c kernel/panic.c:214
__warn.cold+0x20/0x45 kernel/panic.c:571
report_bug+0x263/0x2b0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
fixup_bug arch/x86/kernel/traps.c:173 [inline]
do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
RIP: 0010:xt_compat_add_offset+0x22a/0x290 net/netfilter/x_tables.c:654
Code: 00 01 e8 59 67 bb fb 44 89 e0 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f
5d c3 e8 42 67 bb fb 0f 0b e9 56 fe ff ff e8 36 67 bb fb <0f> 0b 41 bc f4
ff ff ff eb ce 4c 89 f7 e8 14 6a f2 fb e9 75 ff ff
RSP: 0018:ffff8880a8197808 EFLAGS: 00010293
RAX: ffff88809055e040 RBX: ffff8882166548d0 RCX: ffffffff85b47892
RDX: 0000000000000000 RSI: ffffffff85b47a4a RDI: ffff8882166549f0
RBP: ffff8880a8197838 R08: ffff88809055e040 R09: ffffed1042cca92f
R10: ffffed1042cca92e R11: ffff888216654977 R12: 0000000000000018
R13: 0000000000000030 R14: ffff88809055e040 R15: 0000000000000000
size_entry_mwt net/bridge/netfilter/ebtables.c:2183 [inline]
compat_copy_entries+0x51b/0x1360 net/bridge/netfilter/ebtables.c:2208
compat_do_replace+0x3b3/0x680 net/bridge/netfilter/ebtables.c:2302
compat_do_ebt_set_ctl+0x229/0x278 net/bridge/netfilter/ebtables.c:2384
compat_nf_sockopt net/netfilter/nf_sockopt.c:144 [inline]
compat_nf_setsockopt+0x9b/0x140 net/netfilter/nf_sockopt.c:156
compat_ip_setsockopt net/ipv4/ip_sockglue.c:1284 [inline]
compat_ip_setsockopt+0x106/0x140 net/ipv4/ip_sockglue.c:1265
compat_udp_setsockopt+0x68/0xb0 net/ipv4/udp.c:2629
compat_ipv6_setsockopt+0xca/0x210 net/ipv6/ipv6_sockglue.c:959
inet_csk_compat_setsockopt+0x99/0x120 net/ipv4/inet_connection_sock.c:1054
compat_tcp_setsockopt+0x4d/0x80 net/ipv4/tcp.c:3079
compat_sock_common_setsockopt+0xb4/0x150 net/core/sock.c:3002
__compat_sys_setsockopt+0x176/0x610 net/compat.c:404
__do_compat_sys_setsockopt net/compat.c:417 [inline]
__se_compat_sys_setsockopt net/compat.c:414 [inline]
__ia32_compat_sys_setsockopt+0xbd/0x150 net/compat.c:414
do_syscall_32_irqs_on arch/x86/entry/common.c:326 [inline]
do_fast_syscall_32+0x281/0xc98 arch/x86/entry/common.c:397
entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7fd3869
Code: 85 d2 74 02 89 0a 5b 5d c3 8b 04 24 c3 8b 14 24 c3 8b 3c 24 c3 90 90
90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90
90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:00000000f7fcf0cc EFLAGS: 00000296 ORIG_RAX: 000000000000016e
RAX: ffffffffffffffda RBX: 0000000000000006 RCX: 0000000000000000
RDX: 0000000000000080 RSI: 00000000200000c0 RDI: 0000000000000270
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
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.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH 1/3] ipv4: icmp: use icmp_sk_exit()
From: Eric Dumazet @ 2019-02-22 17:11 UTC (permalink / raw)
To: Kefeng Wang, davem, netdev
In-Reply-To: <20190222015800.192135-1-wangkefeng.wang@huawei.com>
On 02/21/2019 05:57 PM, Kefeng Wang wrote:
> Simply use icmp_sk_exit() when inet_ctl_sock_create() fail in icmp_sk_init().
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> net/ipv4/icmp.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 065997f414e6..364cfe5e414b 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -1245,9 +1245,7 @@ static int __net_init icmp_sk_init(struct net *net)
> return 0;
>
> fail:
> - for_each_possible_cpu(i)
> - inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv4.icmp_sk, i));
> - free_percpu(net->ipv4.icmp_sk);
> + icmp_sk_exit(net);
> return err;
> }
>
>
I do not like this. Future changes in icmp_sk_exit() might trigger a bug in this seldom tested path.
^ permalink raw reply
* Re: [PATCH 1/3] ipv4: icmp: use icmp_sk_exit()
From: Eric Dumazet @ 2019-02-22 17:16 UTC (permalink / raw)
To: Kefeng Wang, davem, netdev
In-Reply-To: <bb408779-5ddb-f9ef-e16b-1d0d7a5c3719@gmail.com>
On 02/22/2019 09:11 AM, Eric Dumazet wrote:
>
>
> On 02/21/2019 05:57 PM, Kefeng Wang wrote:
>> Simply use icmp_sk_exit() when inet_ctl_sock_create() fail in icmp_sk_init().
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>> net/ipv4/icmp.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>> index 065997f414e6..364cfe5e414b 100644
>> --- a/net/ipv4/icmp.c
>> +++ b/net/ipv4/icmp.c
>> @@ -1245,9 +1245,7 @@ static int __net_init icmp_sk_init(struct net *net)
>> return 0;
>>
>> fail:
>> - for_each_possible_cpu(i)
>> - inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv4.icmp_sk, i));
>> - free_percpu(net->ipv4.icmp_sk);
>> + icmp_sk_exit(net);
>> return err;
>> }
>>
>>
>
>
> I do not like this. Future changes in icmp_sk_exit() might trigger a bug in this seldom tested path.
>
Oh I see the next patches, now it makes sense.
Please send a proper patch series, with a cover letter, so that it is obvious.
Also you need to include "net-next" instead of plain [PATCH] to express this targets net-next tree.
Carefully read Documentation/networking/netdev-FAQ.rst
Thank you.
^ permalink raw reply
* Re: stmmac / meson8b-dwmac
From: Anand Moon @ 2019-02-22 17:21 UTC (permalink / raw)
To: Simon Huelck
Cc: Jerome Brunet, Jose Abreu, Martin Blumenstingl, linux-amlogic,
netdev, Alexandre TORGUE, Emiliano Ingrassia, Gpeppe.cavallaro
In-Reply-To: <e009cbbb-e31d-2438-e454-2fa10ebec542@gmx.de>
Hi All,
On Fri, 22 Feb 2019 at 01:05, Simon Huelck <simonmail@gmx.de> wrote:
>
> Am 21.02.2019 um 18:46 schrieb Jerome Brunet:
> > On Thu, 2019-02-21 at 18:27 +0100, Simon Huelck wrote:
> >> Hi,
> >>
> >>
> >>
> >> this was changed recently, with a patch for the EEE stuff , see here:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.0-rc7&id=e35e26b26e955c53e61c154ba26b9bb15da6b858
> > Hu, I was not aware this finally went through. Good !
> > As explained in the patch and by Jose, the GMAC should be using IRQ_LEVEL.
> >
> > The realtek PHY has EEE enabled by default. Having this enabled generates a
> > lot of (Low Power) Interrupts.
> >
> > Previously, when the GMAC used IRQ_EDGE. Because it is wrong, we would
> > eventually miss an IRQ and the interface would just die. Unfortunately, it was
> > not that easy find out.
> >
> > 2 years ago, we just noticed that disabling EEE would make the failure go
> > away. Forcing this EEE feature off through DT was merely a work around.
> >
> > Now that the real cause of the problem is known, there is no reason to keep
> > this hack around.
> >
> > Whether EEE adds a performance penality and why, is another topic.
> > As Jose pointed out, you can disable EEE at runtime, using ethtool.
> >
> > Jerome
> >
> Hi,
>
>
>
> i disabled EEE via ethtool and via the .dtb , but the performance
> penalty stays. Kernel 4.14 still gives me the former "good" performance.
>
>
>
> regards,
>
> Simon
>
>
Sorry this is off the topic.
I am using Archlinux on Odroid C1+ and the latest kernel loads with no issue.
only issue is I have is the each time their is random MAC address so I
get new IP from dhcp server.
How can I avoid this. I have tried to enable eFuse driver but with no success.
On Odroid C2 I dont have any issue, it always retained the unique MAC address.
Best Regards
-Anand
^ permalink raw reply
* Re: [PATCH net-next 4/5] nfp: add .ndo_get_devlink
From: Michal Kubecek @ 2019-02-22 17:27 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222090204.20c410e3@cakuba.netronome.com>
On Fri, Feb 22, 2019 at 09:02:04AM -0800, Jakub Kicinski wrote:
> On Fri, 22 Feb 2019 11:04:50 +0100, Michal Kubecek wrote:
> > On Thu, Feb 21, 2019 at 09:46:19AM -0800, Jakub Kicinski wrote:
> > > Support getting devlink instance from a new NDO.
> > >
> > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > ---
> > > drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 ++
> > > drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 11 +++++++++++
> > > drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 1 +
> > > drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
> > > 4 files changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > > index d578d856a009..f8d422713705 100644
> > > --- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > > +++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > > @@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
> > > int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
> > > struct nfp_net *nn, unsigned int id);
> > >
> > > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
> > > +
> > > #endif
> > > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > > index db2da99f6aa7..e9eca99cf493 100644
> > > --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > > +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > > @@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
> > > {
> > > devlink_port_unregister(&port->dl_port);
> > > }
> > > +
> > > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
> > > +{
> > > + struct nfp_app *app;
> > > +
> > > + app = nfp_app_from_netdev(netdev);
> > > + if (!app)
> > > + return NULL;
> > > +
> > > + return priv_to_devlink(app->pf);
> > > +}
> >
> > AFAICS this would return a pointer to zero initialized struct devlink
> > when built with CONFIG_DEVLINK=n. Then devlink_compat_running_version()
> > would execute
> >
> > if (!dev->netdev_ops->ndo_get_devlink)
> > return;
> >
> > devlink = dev->netdev_ops->ndo_get_devlink(dev);
> > if (!devlink || !devlink->ops->info_get)
> > return;
> >
> > with non-null devlink but null devlink->ops so that it dereferences null
> > pointer (and so does devlink_compat_flash_update()).
>
> devlink_compat_flash_update() is this if CONFIG_DEVLINK=n:
>
> static inline int
> devlink_compat_flash_update(struct net_device *dev, const char *file_name)
> {
> return -EOPNOTSUPP;
> }
>
> No?
You are right, I missed that devlink_compat_running_version() and
devlink_compat_flash_update() also have alternative versions when
devlink is disabled. So there is no problem.
> > Maybe it would be safer not to call ndo_get_devlink directly and have
> > an inline wrapper like
> >
> > #if IS_ENABLED(CONFIG_NET_DEVLINK)
> > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > {
> > if (dev->netdev_ops->ndo_get_devlink)
> > return dev->netdev_ops->ndo_get_devlink();
> > else
> > retrurn NULL;
> > }
> > #else
> > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > {
> > return NULL;
> > }
> > #endif
> >
> > so that one can simply call the wrapper and check return value for NULL.
>
> Only devlink code can call this ndo, and it doesn't exist with
> DEVLINK=n. I don't dislike wrappers for NDOs, but I'll defer to Jiri
> to decide if we want a wrapper here (without the #if/#else, just the
> first part for code clarity) :)
If the NDO is only supposed to be called from devlink code (or, more
precisely, code built only with CONFIG_DEVLINK=y), it should be IMHO
mentioned in its description. Another option would be enforcing it by
adding #ifdef around the ndo_get_devlink entry in struct net_device_ops
but that would require using ifdefs also in each driver providing the
NDO which seems inconvenient.
Michal
^ permalink raw reply
* Re: [PATCH net-next 0/8] Add tests for unlocked flower classifier implementation
From: Cong Wang @ 2019-02-22 17:35 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, David Miller,
shuah, batuhanosmantaskaya, Lucas Bates, Davide Caratti,
Marcelo Ricardo Leitner, Chris Mi
In-Reply-To: <20190222140047.13215-1-vladbu@mellanox.com>
On Fri, Feb 22, 2019 at 6:00 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Implement tests for tdc testsuite to verify concurrent rules update with
> rtnl-unlocked flower classifier implementation. The goal of these tests
> is to verify general flower classifier correctness by updating filters
> on same classifier instance in parallel and to verify its atomicity by
> concurrently updating filters in same handle range. All three filter
> update operations (add, replace, delete) are tested.
>
> Existing script tdc_batch.py is re-used for batch file generation. It is
> extended with several optional CLI arguments that are needed for
> concurrency tests. Thin wrapper tdc_multibatch.py is implemented on top
> of tdc_batch.py to simplify its usage when generating multiple batch
> files for several test configurations.
>
> Parallelism in tests is implemented by running multiple instances of tc
> in batch mode with xargs tool. Xargs is chosen for its ease of use and
> because it is available by default on most modern Linux distributions.
I don't look into the details, but I love to see we have tests for parallel
filter updates! So for the whole patchset:
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks!
^ permalink raw reply
* [PATCH v2 0/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 17:39 UTC (permalink / raw)
To: David Miller
Cc: paul@paul-moore.com, netdev@vger.kernel.org,
linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org
In-Reply-To: <8873761550853329@myt6-67cd1de25d8a.qloud-c.yandex.net>
The original discussion is here:
https://lore.kernel.org/linux-security-module/16659801547571984@sas1-890ba5c2334a.qloud-c.yandex.net/
Changes in v2:
- original patch divided on 2 parts: add __icmp_send function, fix cipso_v4_error
- add __ip_option_compile not using icmp_send in case nof error
---
include/net/icmp.h | 9 ++++++++-
include/net/ip.h | 2 ++
net/ipv4/cipso_ipv4.c | 17 +++++++++++++++--
net/ipv4/icmp.c | 7 ++++---
net/ipv4/ip_options.c | 22 +++++++++++++++++-----
5 files changed, 46 insertions(+), 11 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 1/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 17:43 UTC (permalink / raw)
To: David Miller
Cc: paul@paul-moore.com, netdev@vger.kernel.org,
linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org
In-Reply-To: <8873761550853329@myt6-67cd1de25d8a.qloud-c.yandex.net>
Add __icmp_send function having ip_options struct parameter
---
include/net/icmp.h | 9 ++++++++-
net/ipv4/icmp.c | 7 ++++---
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/net/icmp.h b/include/net/icmp.h
index 6ac3a5b..e0f709d 100644
--- a/include/net/icmp.h
+++ b/include/net/icmp.h
@@ -22,6 +22,7 @@
#include <net/inet_sock.h>
#include <net/snmp.h>
+#include <net/ip.h>
struct icmp_err {
int errno;
@@ -39,7 +40,13 @@ struct icmp_err {
struct sk_buff;
struct net;
-void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info);
+void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
+ const struct ip_options *opt);
+static inline void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
+{
+ __icmp_send(skb_in, type, code, info, &IPCB(skb_in)->opt);
+}
+
int icmp_rcv(struct sk_buff *skb);
int icmp_err(struct sk_buff *skb, u32 info);
int icmp_init(void);
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 065997f..3f24414 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -570,7 +570,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
* MUST reply to only the first fragment.
*/
-void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
+void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
+ const struct ip_options *opt)
{
struct iphdr *iph;
int room;
@@ -691,7 +692,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
iph->tos;
mark = IP4_REPLY_MARK(net, skb_in->mark);
- if (ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in))
+ if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in, opt))
goto out_unlock;
@@ -742,7 +743,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
local_bh_enable();
out:;
}
-EXPORT_SYMBOL(icmp_send);
+EXPORT_SYMBOL(__icmp_send);
static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
---
^ permalink raw reply related
* Re: [PATCH net-next v3 3/4] enetc: Add ENETC PF level external MDIO support
From: Andrew Lunn @ 2019-02-22 17:44 UTC (permalink / raw)
To: Claudiu Manoil
Cc: Shawn Guo, Li Yang, David S . Miller, devicetree,
alexandru.marginean, linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1550847859-17346-4-git-send-email-claudiu.manoil@nxp.com>
On Fri, Feb 22, 2019 at 05:04:18PM +0200, Claudiu Manoil wrote:
> Each ENETC PF has its own MDIO interface, the corresponding
> MDIO registers are mapped in the ENETC's Port register block.
> The current patch adds a driver for these PF level MDIO buses,
> so that each PF can manage directly its own external link.
>
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Eric Dumazet @ 2019-02-22 17:45 UTC (permalink / raw)
To: Eric Biggers, netdev, David S . Miller
Cc: linux-kernel, Mao Wenan, Cong Wang, Lorenzo Colitti, Tetsuo Handa,
Al Viro
In-Reply-To: <20190221221356.173485-1-ebiggers@kernel.org>
On 02/21/2019 02:13 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> closed concurrently with fchownat(). However, it ignored that many
> other proto_ops::release() methods don't set sock->sk to NULL and
> therefore allow the same use-after-free:
>
I fail to see how setting a pointer to NULL can avoid races.
We lack some kind of protection, rcu or something, if another thread can change sock->sk at anytime
while sockfs_setattr() is used.
sockfs_setattr()
...
if (sock->sk)
// even if sock->sk was not NULL for the if (...).
// it can be NULL right now, compiler could read sock->sk a second time and catch a NULL.
sock->sk->sk_uid = iattr->ia_uid;
> - base_sock_release
> - bnep_sock_release
> - cmtp_sock_release
> - data_sock_release
> - dn_release
> - hci_sock_release
> - hidp_sock_release
> - iucv_sock_release
> - l2cap_sock_release
> - llcp_sock_release
> - llc_ui_release
> - rawsock_release
> - rfcomm_sock_release
> - sco_sock_release
> - svc_release
> - vcc_release
> - x25_release
>
> Rather than fixing all these and relying on every socket type to get
> this right forever, just make __sock_release() set sock->sk to NULL
> itself after calling proto_ops::release().
>
> Reproducer that produces the KASAN splat when any of these socket types
> are configured into the kernel:
>
> #include <pthread.h>
> #include <stdlib.h>
> #include <sys/socket.h>
> #include <unistd.h>
>
> pthread_t t;
> volatile int fd;
>
> void *close_thread(void *arg)
> {
> for (;;) {
> usleep(rand() % 100);
> close(fd);
> }
> }
>
> int main()
> {
> pthread_create(&t, NULL, close_thread, NULL);
> for (;;) {
> fd = socket(rand() % 50, rand() % 11, 0);
> fchownat(fd, "", 1000, 1000, 0x1000);
> close(fd);
> }
> }
>
> Fixes: 86741ec25462 ("net: core: Add a UID field to struct sock.")
> Cc: <stable@vger.kernel.org> # v4.10+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>
> NOTE: I am not an expert in the networking code, so please carefully
> check that I haven't missed some reason why this simple fix won't do.
>
> net/socket.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/socket.c b/net/socket.c
> index d80d87a395ea..320f51b22b19 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -577,6 +577,7 @@ static void __sock_release(struct socket *sock, struct inode *inode)
> if (inode)
> inode_lock(inode);
> sock->ops->release(sock);
> + sock->sk = NULL;
> if (inode)
> inode_unlock(inode);
> sock->ops = NULL;
>
^ permalink raw reply
* Re: [PATCH] net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
From: Russell King - ARM Linux admin @ 2019-02-22 17:48 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Vivien Didelot
In-Reply-To: <20190217195524.rhc4rsvzsalptyua@shell.armlinux.org.uk>
On Sun, Feb 17, 2019 at 07:55:24PM +0000, Russell King - ARM Linux admin wrote:
> On Sun, Feb 17, 2019 at 07:11:43PM +0100, Andrew Lunn wrote:
> > The following false positive lockdep splat has been observed.
> >
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 4.20.0+ #302 Not tainted
> > ------------------------------------------------------
> > systemd-udevd/160 is trying to acquire lock:
> > edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704
> >
> > but task is already holding lock:
> > edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704
> >
> > which lock already depends on the new lock.
> >
> > the existing dependency chain (in reverse order) is:
> >
> > -> #1 (&desc->request_mutex){+.+.}:
> > mutex_lock_nested+0x1c/0x24
> > __setup_irq+0xa0/0x704
> > request_threaded_irq+0xd0/0x150
> > mv88e6xxx_probe+0x41c/0x694 [mv88e6xxx]
> > mdio_probe+0x2c/0x54
> > really_probe+0x200/0x2c4
> > driver_probe_device+0x5c/0x174
> > __driver_attach+0xd8/0xdc
> > bus_for_each_dev+0x58/0x7c
> > bus_add_driver+0xe4/0x1f0
> > driver_register+0x7c/0x110
> > mdio_driver_register+0x24/0x58
> > do_one_initcall+0x74/0x2e8
> > do_init_module+0x60/0x1d0
> > load_module+0x1968/0x1ff4
> > sys_finit_module+0x8c/0x98
> > ret_fast_syscall+0x0/0x28
> > 0xbedf2ae8
> >
> > -> #0 (&chip->reg_lock){+.+.}:
> > __mutex_lock+0x50/0x8b8
> > mutex_lock_nested+0x1c/0x24
> > __setup_irq+0x640/0x704
> > request_threaded_irq+0xd0/0x150
> > mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx]
> > mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx]
> > mdio_probe+0x2c/0x54
> > really_probe+0x200/0x2c4
> > driver_probe_device+0x5c/0x174
> > __driver_attach+0xd8/0xdc
> > bus_for_each_dev+0x58/0x7c
> > bus_add_driver+0xe4/0x1f0
> > driver_register+0x7c/0x110
> > mdio_driver_register+0x24/0x58
> > do_one_initcall+0x74/0x2e8
> > do_init_module+0x60/0x1d0
> > load_module+0x1968/0x1ff4
> > sys_finit_module+0x8c/0x98
> > ret_fast_syscall+0x0/0x28
> > 0xbedf2ae8
> >
> > other info that might help us debug this:
> >
> > Possible unsafe locking scenario:
> >
> > CPU0 CPU1
> > ---- ----
> > lock(&desc->request_mutex);
> > lock(&chip->reg_lock);
> > lock(&desc->request_mutex);
> > lock(&chip->reg_lock);
> >
> > &desc->request_mutex refer to two different mutex. #1 is the GPIO for
> > the chip interrupt. #2 is the chained interrupt between global 1 and
> > global 2.
> >
> > Add lockdep classes to the GPIO interrupt to avoid this.
> >
> > Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >
> > Hi Russell
> >
> > Does this fix it for you on Clearfog?
>
> Yes, that also fixes the problem, but I do think this is just papering
> over mv88e6xxx needlessly holding locks when it doesn't need to do so.
Hi Andrew,
Do we have a way forward for this issue?
Thanks.
>
> >
> > drivers/net/dsa/mv88e6xxx/chip.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> > index 32e7af5caa69..936d53a92144 100644
> > --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> > @@ -442,12 +442,20 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
> >
> > static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
> > {
> > + static struct lock_class_key lock_key;
> > + static struct lock_class_key request_key;
> > int err;
> >
> > err = mv88e6xxx_g1_irq_setup_common(chip);
> > if (err)
> > return err;
> >
> > + /* These lock classes tells lockdep that global 1 irqs are in
> > + * a different category than their parent GPIO, so it won't
> > + * report false recursion.
> > + */
> > + irq_set_lockdep_class(chip->irq, &lock_key, &request_key);
> > +
> > err = request_threaded_irq(chip->irq, NULL,
> > mv88e6xxx_g1_irq_thread_fn,
> > IRQF_ONESHOT | IRQF_SHARED,
> > --
> > 2.20.1
> >
> >
>
> --
> 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
--
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
* Re: [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces
From: Stefano Brivio @ 2019-02-22 17:49 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <1d298b6839828c96cd8a0f50bdfac893cc645895.1550851038.git.pabeni@redhat.com>
On Fri, 22 Feb 2019 17:06:32 +0100
Paolo Abeni <pabeni@redhat.com> wrote:
> Otherwise, the configured IPv6 address could be still "tentative"
> at test time, possibly causing tests failures.
> We can also drop some sleep along the code and decrease the
> timeout for most commands so that the test runtime decreases.
Thanks for fixing this!
I tried this back then and it didn't work, perhaps some DAD changes
intervened meanwhile (or I simply tried it wrong).
Just one comment:
> Fixes: d1f1b9cbf34c ("selftests: net: Introduce first PMTU test")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> tools/testing/selftests/net/pmtu.sh | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index e2c94e47707c..634e91e8fe25 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -263,8 +263,6 @@ setup_fou_or_gue() {
>
> ${ns_a} ip link set ${encap}_a up
> ${ns_b} ip link set ${encap}_b up
> -
> - sleep 1
> }
>
> setup_fou44() {
> @@ -302,6 +300,10 @@ setup_gue66() {
> setup_namespaces() {
> for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do
> ip netns add ${n} || return 1
> +
> + # disable dad, so that we don't have to wait to use the
> + # configured IPv6 addresses
For consistency: "Disable DAD ..."
--
Stefano
^ permalink raw reply
* Re: [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup
From: Stefano Brivio @ 2019-02-22 17:50 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <06a88ed3b2efab312f6301e9fe8eabd4383a9d3a.1550851038.git.pabeni@redhat.com>
On Fri, 22 Feb 2019 17:06:33 +0100
Paolo Abeni <pabeni@redhat.com> wrote:
> Add a couple of new tests, explicitly checking that the kernel
> timely releases pmtu exceptions on related device removal.
> This is mostly a regression test vs the issue fixed by:
> https://patchwork.ozlabs.org/patch/1045488/
>
> Only 2 new test cases have been added, instead of extending all
> the existing ones, because the reproducer requires executing
> several commands and would slow down too much the tests otherwise.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> tools/testing/selftests/net/pmtu.sh | 52 ++++++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index 634e91e8fe25..4bc72bc26899 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -135,7 +135,9 @@ tests="
> pmtu_vti6_default_mtu vti6: default MTU assignment
> pmtu_vti4_link_add_mtu vti4: MTU setting on link creation
> pmtu_vti6_link_add_mtu vti6: MTU setting on link creation
> - pmtu_vti6_link_change_mtu vti6: MTU changes on link changes"
> + pmtu_vti6_link_change_mtu vti6: MTU changes on link changes
> + pmtu_ipv4_exception_cleanup ipv4: cleanup of cached exceptions
> + pmtu_ipv6_exception_cleanup ipv6: cleanup of cached exceptions"
>
> NS_A="ns-$(mktemp -u XXXXXX)"
> NS_B="ns-$(mktemp -u XXXXXX)"
> @@ -1006,6 +1008,54 @@ test_pmtu_vti6_link_change_mtu() {
> return ${fail}
> }
>
> +test_cleanup_vxlanY_or_geneveY_exception() {
> + outer="${1}"
> + encap="${2}"
As you don't implement GENEVE tests, this function can be kept simpler.
> + ll_mtu=4000
> +
> + which taskset 2>/dev/null | return 2
> + which timeout 2>/dev/null | return 2
This test will not be skipped if taskset or timeout are not available,
I guess you meant: || return 2.
Besides, 'which' can output to both stdout and stderr. See how it's
used anywhere else in this script, e.g.:
which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
You might also want to skip this test on non-SMP.
You should also indicate why the test is skipped, see how it's done in
other places, e.g.:
${ns_a} ip link add dummy0 mtu 1500 type dummy
[ $? -ne 0 ] && err " dummy not supported" && return 2
> + cpu_list=`grep processor /proc/cpuinfo | cut -d ' ' -f 2`
For consistency, please use $( ... )
If you grep -m2, then you can skip the ncpus thing below.
> +
> + setup namespaces routing ${encap}${outer} || return 2
> + trace "${ns_a}" ${encap}_a "${ns_b}" ${encap}_b \
> + "${ns_a}" veth_A-R1 "${ns_r1}" veth_R1-A \
> + "${ns_b}" veth_B-R1 "${ns_r1}" veth_R1-B
> +
> + # Create route exception by exceeding link layer MTU
> + mtu "${ns_a}" veth_A-R1 $((${ll_mtu} + 1000))
> + mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000))
> + mtu "${ns_b}" veth_B-R1 ${ll_mtu}
> + mtu "${ns_r1}" veth_R1-B ${ll_mtu}
> +
> + mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000))
> + mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000))
> +
> + # fill exception cache for multiple cpus [2]
> + # we can always use inner ipv4 for that
Please keep comments consistent with the rest, "Fill ... CPUs (2) ... IPv4".
> + ncpus=0
> + for cpu in $cpu_list; do
Nit: ${cpu_list}
> + taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null
> + ncpus=$((ncpus + 1))
> + [ ${ncpus} -gt 1 ] && break
> + done
> +
> + fail=0
No need for fail=0...
> + if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then
> + err " can't delete veth device in a timely manner, pmtu dst likely leaked"
(For consistency: "PMTU")
> + fail=1
...just return 1 here and
> + fi
> + return ${fail}
...skip the explicit return. The return code comes from the last
command executed.
> +}
> +
> +test_pmtu_ipv6_exception_cleanup() {
> + test_cleanup_vxlanY_or_geneveY_exception 6 vxlan
> +}
> +
> +test_pmtu_ipv4_exception_cleanup() {
> + test_cleanup_vxlanY_or_geneveY_exception 4 vxlan
> +}
> +
> usage() {
> echo
> echo "$0 [OPTIONS] [TEST]..."
--
Stefano
^ permalink raw reply
* Re: [PATCH v2 2/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 17:50 UTC (permalink / raw)
To: David Miller
Cc: paul@paul-moore.com, netdev@vger.kernel.org,
linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org
In-Reply-To: <8873761550853329@myt6-67cd1de25d8a.qloud-c.yandex.net>
Extract IP options in cipso_v4_error and use __icmp_send.
---
include/net/ip.h | 2 ++
net/ipv4/cipso_ipv4.c | 17 +++++++++++++++--
net/ipv4/ip_options.c | 22 +++++++++++++++++-----
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 8866bfc..f0e8d06 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -667,6 +667,8 @@ static inline int ip_options_echo(struct net *net, struct ip_options *dopt,
}
void ip_options_fragment(struct sk_buff *skb);
+int __ip_options_compile(struct net *net, struct ip_options *opt,
+ struct sk_buff *skb, __be32 *info);
int ip_options_compile(struct net *net, struct ip_options *opt,
struct sk_buff *skb);
int ip_options_get(struct net *net, struct ip_options_rcu **optp,
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 777fa3b..eff86a7 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1735,13 +1735,26 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
*/
void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
{
+ unsigned char optbuf[sizeof(struct ip_options) + 40];
+ struct ip_options *opt = (struct ip_options *)optbuf;
+
if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
return;
+ /*
+ * We might be called above the IP layer,
+ * so we can not use icmp_send and IPCB here.
+ */
+
+ memset(opt, 0, sizeof(struct ip_options));
+ opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr);
+ if (__ip_options_compile(dev_net(skb->dev), opt, skb, NULL))
+ return;
+
if (gateway)
- icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, opt);
else
- icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, opt);
}
/**
diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index ed194d4..32a3504 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -251,8 +251,9 @@ static void spec_dst_fill(__be32 *spec_dst, struct sk_buff *skb)
* If opt == NULL, then skb->data should point to IP header.
*/
-int ip_options_compile(struct net *net,
- struct ip_options *opt, struct sk_buff *skb)
+int __ip_options_compile(struct net *net,
+ struct ip_options *opt, struct sk_buff *skb,
+ __be32 *info)
{
__be32 spec_dst = htonl(INADDR_ANY);
unsigned char *pp_ptr = NULL;
@@ -468,11 +469,22 @@ int ip_options_compile(struct net *net,
return 0;
error:
- if (skb) {
- icmp_send(skb, ICMP_PARAMETERPROB, 0, htonl((pp_ptr-iph)<<24));
- }
+ if (info)
+ *info = htonl((pp_ptr-iph)<<24);
return -EINVAL;
}
+
+int ip_options_compile(struct net *net,
+ struct ip_options *opt, struct sk_buff *skb)
+{
+ int ret;
+ __be32 info;
+
+ ret = __ip_options_compile(net, opt, skb, &info);
+ if (ret != 0 && skb)
+ icmp_send(skb, ICMP_PARAMETERPROB, 0, info);
+ return ret;
+}
EXPORT_SYMBOL(ip_options_compile);
/*
---
^ permalink raw reply related
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Eric Biggers @ 2019-02-22 17:57 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S . Miller, linux-kernel, Mao Wenan, Cong Wang,
Lorenzo Colitti, Tetsuo Handa, Al Viro
In-Reply-To: <e4d0f1e8-68ff-512e-ad69-480523ff300f@gmail.com>
Hi Eric,
On Fri, Feb 22, 2019 at 09:45:35AM -0800, Eric Dumazet wrote:
>
>
> On 02/21/2019 02:13 PM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> > fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> > closed concurrently with fchownat(). However, it ignored that many
> > other proto_ops::release() methods don't set sock->sk to NULL and
> > therefore allow the same use-after-free:
> >
>
> I fail to see how setting a pointer to NULL can avoid races.
>
>
> We lack some kind of protection, rcu or something, if another thread can change sock->sk at anytime
> while sockfs_setattr() is used.
>
> sockfs_setattr()
> ...
> if (sock->sk)
>
> // even if sock->sk was not NULL for the if (...).
>
> // it can be NULL right now, compiler could read sock->sk a second time and catch a NULL.
>
> sock->sk->sk_uid = iattr->ia_uid;
>
>
->setattr() is called under inode_lock(), which __sock_release() also takes. So
the uses of sock->sk are serialized. See commit 6d8c50dcb029 ("socket: close
race condition between sock_close() and sockfs_setattr()").
The issue now is that if ->setattr() happens *after* __sock_release() (which is
possible if fchownat() gets the reference to the file's 'struct path', then the
file is close()d by another thread, then fchownat() continues), it will see
stale sock->sk because for many socket types it wasn't set to NULL earlier.
- Eric
^ permalink raw reply
* Re: [PATCH] net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
From: Andrew Lunn @ 2019-02-22 17:59 UTC (permalink / raw)
To: Russell King - ARM Linux admin; +Cc: netdev, Vivien Didelot
In-Reply-To: <20190222174856.tmisqn7ih3ea3xvz@shell.armlinux.org.uk>
> Hi Andrew,
>
> Do we have a way forward for this issue?
Hi Russell
Yes. I tested releasing the mutex around the request for the
interrupt. That works. So i will submit a patchset adding both the
lockdep class, and the unlock/lock pair.
Andrew
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Stephen Hemminger @ 2019-02-22 18:01 UTC (permalink / raw)
To: Jiri Pirko
Cc: Aya Levin, David Ahern, netdev, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha
In-Reply-To: <20190222085832.GA2254@nanopsycho>
On Fri, 22 Feb 2019 09:58:32 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote:
> >On Thu, 21 Feb 2019 15:42:40 +0200
> >Aya Levin <ayal@mellanox.com> wrote:
> >
> >> Add a new helper functions which outputs only values (without name
> >> label) for different types: boolean, uint, uint64, string and binary.
> >> In addition add a helper function which prints only the name label.
> >>
> >> Signed-off-by: Aya Levin <ayal@mellanox.com>
> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
> >> ---
> >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 65 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/devlink/devlink.c b/devlink/devlink.c
> >> index b073ae020d52..5d69c4f24f29 100644
> >> --- a/devlink/devlink.c
> >> +++ b/devlink/devlink.c
> >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl)
> >> pr_out("\n");
> >> }
> >>
> >> -
> >> static void pr_out_str(struct dl *dl, const char *name, const char *val)
> >> {
> >> if (dl->json_output) {
> >> @@ -1636,6 +1635,71 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
> >> }
> >> }
> >>
> >> +static void pr_out_bool_value(struct dl *dl, bool value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_bool(dl->jw, value);
> >> + else
> >> + pr_out(" %s", value ? "true" : "false");
> >> +}
> >> +
> >> +static void pr_out_uint_value(struct dl *dl, unsigned int value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_uint(dl->jw, value);
> >> + else
> >> + pr_out(" %u", value);
> >> +}
> >> +
> >> +static void pr_out_uint64_value(struct dl *dl, uint64_t value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_u64(dl->jw, value);
> >> + else
> >> + pr_out(" %lu", value);
> >> +}
> >> +
> >> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
> >> +{
> >> + int i = 1;
> >> +
> >> + if (dl->json_output)
> >> + jsonw_start_array(dl->jw);
> >> + else
> >> + pr_out("\n");
> >> +
> >> + while (i < len) {
> >> + if (dl->json_output) {
> >> + jsonw_printf(dl->jw, "%d", data[i]);
> >> + } else {
> >> + pr_out(" %02x", data[i]);
> >> + if (!(i % 16))
> >> + pr_out("\n");
> >> + }
> >> + i++;
> >> + }
> >> + if (dl->json_output)
> >> + jsonw_end_array(dl->jw);
> >> + else if ((i - 1) % 16)
> >> + pr_out("\n");
> >> +}
> >> +
> >> +static void pr_out_str_value(struct dl *dl, const char *value)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_string(dl->jw, value);
> >> + else
> >> + pr_out(" %s", value);
> >> +}
> >> +
> >> +static void pr_out_name(struct dl *dl, const char *name)
> >> +{
> >> + if (dl->json_output)
> >> + jsonw_name(dl->jw, name);
> >> + else
> >> + pr_out(" %s:", name);
> >> +}
> >> +
> >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
> >> {
> >> if (dl->json_output) {
> >
> >NAK
> >
> >Please convert devlink to use existing iproute2 json_print infrasructure
> >rather than reinventing it.
>
> That requires whole devlink/devlink.c code change. Could this be done as
> a follow-up?
There is a tradeoff, if more code creeps in it is harder to do the code change.
Can you commit to a timeframe to convert the JSON support code?
^ permalink raw reply
* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Cong Wang @ 2019-02-22 18:05 UTC (permalink / raw)
To: Eric Biggers
Cc: Linux Kernel Network Developers, David S . Miller, LKML,
Mao Wenan, Lorenzo Colitti, Tetsuo Handa, Al Viro
In-Reply-To: <20190221221356.173485-1-ebiggers@kernel.org>
On Thu, Feb 21, 2019 at 2:14 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> closed concurrently with fchownat(). However, it ignored that many
> other proto_ops::release() methods don't set sock->sk to NULL and
> therefore allow the same use-after-free:
>
> - base_sock_release
> - bnep_sock_release
> - cmtp_sock_release
> - data_sock_release
> - dn_release
> - hci_sock_release
> - hidp_sock_release
> - iucv_sock_release
> - l2cap_sock_release
> - llcp_sock_release
> - llc_ui_release
> - rawsock_release
> - rfcomm_sock_release
> - sco_sock_release
> - svc_release
> - vcc_release
> - x25_release
>
> Rather than fixing all these and relying on every socket type to get
> this right forever, just make __sock_release() set sock->sk to NULL
> itself after calling proto_ops::release().
>
> Reproducer that produces the KASAN splat when any of these socket types
> are configured into the kernel:
>
> #include <pthread.h>
> #include <stdlib.h>
> #include <sys/socket.h>
> #include <unistd.h>
>
> pthread_t t;
> volatile int fd;
>
> void *close_thread(void *arg)
> {
> for (;;) {
> usleep(rand() % 100);
> close(fd);
> }
> }
>
> int main()
> {
> pthread_create(&t, NULL, close_thread, NULL);
> for (;;) {
> fd = socket(rand() % 50, rand() % 11, 0);
> fchownat(fd, "", 1000, 1000, 0x1000);
> close(fd);
> }
> }
>
> Fixes: 86741ec25462 ("net: core: Add a UID field to struct sock.")
> Cc: <stable@vger.kernel.org> # v4.10+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Jiri Pirko @ 2019-02-22 17:59 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Aya Levin, David Ahern, netdev, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha
In-Reply-To: <20190222100125.20d8dd54@shemminger-XPS-13-9360>
Fri, Feb 22, 2019 at 07:01:25PM CET, stephen@networkplumber.org wrote:
>On Fri, 22 Feb 2019 09:58:32 +0100
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote:
>> >On Thu, 21 Feb 2019 15:42:40 +0200
>> >Aya Levin <ayal@mellanox.com> wrote:
>> >
>> >> Add a new helper functions which outputs only values (without name
>> >> label) for different types: boolean, uint, uint64, string and binary.
>> >> In addition add a helper function which prints only the name label.
>> >>
>> >> Signed-off-by: Aya Levin <ayal@mellanox.com>
>> >> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>> >> ---
>> >> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> >> 1 file changed, 65 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/devlink/devlink.c b/devlink/devlink.c
>> >> index b073ae020d52..5d69c4f24f29 100644
>> >> --- a/devlink/devlink.c
>> >> +++ b/devlink/devlink.c
>> >> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl)
>> >> pr_out("\n");
>> >> }
>> >>
>> >> -
>> >> static void pr_out_str(struct dl *dl, const char *name, const char *val)
>> >> {
>> >> if (dl->json_output) {
>> >> @@ -1636,6 +1635,71 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
>> >> }
>> >> }
>> >>
>> >> +static void pr_out_bool_value(struct dl *dl, bool value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_bool(dl->jw, value);
>> >> + else
>> >> + pr_out(" %s", value ? "true" : "false");
>> >> +}
>> >> +
>> >> +static void pr_out_uint_value(struct dl *dl, unsigned int value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_uint(dl->jw, value);
>> >> + else
>> >> + pr_out(" %u", value);
>> >> +}
>> >> +
>> >> +static void pr_out_uint64_value(struct dl *dl, uint64_t value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_u64(dl->jw, value);
>> >> + else
>> >> + pr_out(" %lu", value);
>> >> +}
>> >> +
>> >> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
>> >> +{
>> >> + int i = 1;
>> >> +
>> >> + if (dl->json_output)
>> >> + jsonw_start_array(dl->jw);
>> >> + else
>> >> + pr_out("\n");
>> >> +
>> >> + while (i < len) {
>> >> + if (dl->json_output) {
>> >> + jsonw_printf(dl->jw, "%d", data[i]);
>> >> + } else {
>> >> + pr_out(" %02x", data[i]);
>> >> + if (!(i % 16))
>> >> + pr_out("\n");
>> >> + }
>> >> + i++;
>> >> + }
>> >> + if (dl->json_output)
>> >> + jsonw_end_array(dl->jw);
>> >> + else if ((i - 1) % 16)
>> >> + pr_out("\n");
>> >> +}
>> >> +
>> >> +static void pr_out_str_value(struct dl *dl, const char *value)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_string(dl->jw, value);
>> >> + else
>> >> + pr_out(" %s", value);
>> >> +}
>> >> +
>> >> +static void pr_out_name(struct dl *dl, const char *name)
>> >> +{
>> >> + if (dl->json_output)
>> >> + jsonw_name(dl->jw, name);
>> >> + else
>> >> + pr_out(" %s:", name);
>> >> +}
>> >> +
>> >> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
>> >> {
>> >> if (dl->json_output) {
>> >
>> >NAK
>> >
>> >Please convert devlink to use existing iproute2 json_print infrasructure
>> >rather than reinventing it.
>>
>> That requires whole devlink/devlink.c code change. Could this be done as
>> a follow-up?
>
>There is a tradeoff, if more code creeps in it is harder to do the code change.
>Can you commit to a timeframe to convert the JSON support code?
Okay, I'll take care of it in March. Promise.
^ permalink raw reply
* Re: [RFC] rtnetlink: handle multiple vlan tags in set_vf_vlan
From: Stephen Hemminger @ 2019-02-22 18:09 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: netdev@vger.kernel.org, Stephen Hemminger
In-Reply-To: <24d8a96d-4bac-fe8d-1308-c2caee2a9d2c@mellanox.com>
On Fri, 22 Feb 2019 04:58:06 +0000
Moshe Shemesh <moshe@mellanox.com> wrote:
> On 2/21/2019 7:54 PM, Stephen Hemminger wrote:
> > The netlink API for IFLA_VF_VLAN_LIST allows multiple VLAN tags to
> > be passed (and the message was validated) but only the first VLAN
> > tag was being passed to the device. Change to iterate over each tag received.
> >
> > Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > net/core/rtnetlink.c | 19 ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index a51cab95ba64..3a9ec988ae21 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -2207,11 +2207,10 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> > if (tb[IFLA_VF_VLAN_LIST]) {
> > struct ifla_vf_vlan_info *ivvl[MAX_VLAN_LIST_LEN];
> > struct nlattr *attr;
> > - int rem, len = 0;
> > + int i, rem, len = 0;
> >
> > - err = -EOPNOTSUPP;
> > if (!ops->ndo_set_vf_vlan)
> > - return err;
> > + return -EOPNOTSUPP;
> >
> > nla_for_each_nested(attr, tb[IFLA_VF_VLAN_LIST], rem) {
> > if (nla_type(attr) != IFLA_VF_VLAN_INFO ||
> > @@ -2224,13 +2223,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
> >
> > len++;
> > }
> > - if (len == 0)
> > - return -EINVAL;
> >
> > - err = ops->ndo_set_vf_vlan(dev, ivvl[0]->vf, ivvl[0]->vlan,
> > - ivvl[0]->qos, ivvl[0]->vlan_proto);
> > - if (err < 0)
> > - return err;
> > + err = -EINVAL; /* empty list error */
> > + for (i = 0; i < len; i++) {
> > + err = ops->ndo_set_vf_vlan(dev, ivvl[i]->vf,
> > + ivvl[i]->vlan, ivvl[i]->qos,
> > + ivvl[i]->vlan_proto);
>
> Doing that each vlan will just overwrite the vf vlan configuration set
> by its preceding one.
> Note #define MAX_VLAN_LIST_LEN 1
> The point here was that I had to add the rtnl interface to set vf vlan
> with option to set vf vlan protocol. While doing that I was asked to add
> option to get a list of vlans from user to support QinQ, so once it will
> be needed there won't be a need to add another rtnl interface.
>
> The driver which will support setting double vlan or more per vf will
> change MAX_VLAN_LIST_LEN and change the ndo function.
>
OK, then the original code kind of makes sense now. Maybe the whole loop
could be eliminated.
I expected that the vlan list was for allowing multiple vlan's to go to
a single VF but it is really a future (unlikely to ever be implemented)
support for QinQ.
^ 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