| Message ID | 20221110085150.520800-1-idosch@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1fb22ed671950bbc0c402034d206fcb334d1fd3a |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] devlink: Fix warning when unregistering a port | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 2 this patch: 2 |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/build_clang | success | Errors and warnings before: 5 this patch: 5 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 2 this patch: 2 |
| netdev/checkpatch | warning | WARNING: Possible repeated word: 'Google' |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
Hello: This patch was applied to netdev/net-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Thu, 10 Nov 2022 10:51:50 +0200 you wrote: > When a devlink port is unregistered, its type is expected to be unset or > otherwise a WARNING is generated [1]. This was supposed to be handled by > cited commit by clearing the type upon 'NETDEV_PRE_UNINIT'. > > The assumption was that no other events can be generated for the netdev > after this event, but this proved to be wrong. After the event is > generated, netdev_wait_allrefs_any() will rebroadcast a > 'NETDEV_UNREGISTER' until the netdev's reference count drops to 1. This > causes devlink to set the port type back to Ethernet. > > [...] Here is the summary with links: - [net-next] devlink: Fix warning when unregistering a port https://git.kernel.org/netdev/net-next/c/1fb22ed67195 You are awesome, thank you!
diff --git a/net/core/devlink.c b/net/core/devlink.c index 6bbe230c4ec5..7f789bbcbbd7 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -10177,7 +10177,7 @@ static int devlink_netdevice_event(struct notifier_block *nb, * we take into account netdev pointer appearing in this * namespace. */ - __devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH, + __devlink_port_type_set(devlink_port, devlink_port->type, netdev); break; case NETDEV_UNREGISTER: @@ -10185,7 +10185,7 @@ static int devlink_netdevice_event(struct notifier_block *nb, * also during net namespace change so we need to clear * pointer to netdev that is going to another net namespace. */ - __devlink_port_type_set(devlink_port, DEVLINK_PORT_TYPE_ETH, + __devlink_port_type_set(devlink_port, devlink_port->type, NULL); break; case NETDEV_PRE_UNINIT:
When a devlink port is unregistered, its type is expected to be unset or otherwise a WARNING is generated [1]. This was supposed to be handled by cited commit by clearing the type upon 'NETDEV_PRE_UNINIT'. The assumption was that no other events can be generated for the netdev after this event, but this proved to be wrong. After the event is generated, netdev_wait_allrefs_any() will rebroadcast a 'NETDEV_UNREGISTER' until the netdev's reference count drops to 1. This causes devlink to set the port type back to Ethernet. Fix by only setting and clearing the port type upon 'NETDEV_POST_INIT' and 'NETDEV_PRE_UNINIT', respectively. For all other events, preserve the port type. [1] WARNING: CPU: 0 PID: 11 at net/core/devlink.c:9998 devl_port_unregister+0x2f6/0x390 net/core/devlink.c:9998 Modules linked in: CPU: 1 PID: 11 Comm: kworker/u4:1 Not tainted 6.1.0-rc3-next-20221107-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/26/2022 Workqueue: netns cleanup_net RIP: 0010:devl_port_unregister+0x2f6/0x390 net/core/devlink.c:9998 [...] Call Trace: <TASK> __nsim_dev_port_del+0x1bb/0x240 drivers/net/netdevsim/dev.c:1433 nsim_dev_port_del_all drivers/net/netdevsim/dev.c:1443 [inline] nsim_dev_reload_destroy+0x171/0x510 drivers/net/netdevsim/dev.c:1660 nsim_dev_reload_down+0x6b/0xd0 drivers/net/netdevsim/dev.c:968 devlink_reload+0x1c2/0x6b0 net/core/devlink.c:4501 devlink_pernet_pre_exit+0x104/0x1c0 net/core/devlink.c:12609 ops_pre_exit_list net/core/net_namespace.c:159 [inline] cleanup_net+0x451/0xb10 net/core/net_namespace.c:594 process_one_work+0x9bf/0x1710 kernel/workqueue.c:2289 worker_thread+0x665/0x1080 kernel/workqueue.c:2436 kthread+0x2e4/0x3a0 kernel/kthread.c:376 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308 </TASK> Fixes: 02a68a47eade ("net: devlink: track netdev with devlink_port assigned") Reported-by: syzbot+85e47e1a08b3e159b159@syzkaller.appspotmail.com Reported-by: syzbot+c2ca18f0fccdd1f09c66@syzkaller.appspotmail.com Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: Ido Schimmel <idosch@nvidia.com> --- net/core/devlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)