| Message ID | 20210819183015.38988-1-brett.creeley@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [net] ice: Only lock to update netdev dev_addr | expand |
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Brett Creeley > Sent: Friday, August 20, 2021 12:00 AM > To: intel-wired-lan@lists.osuosl.org > Subject: [Intel-wired-lan] [PATCH net] ice: Only lock to update netdev > dev_addr > > commit 3ba7f53f8bf1 ("ice: don't remove netdev->dev_addr from uc sync > list") introduced calls to netif_addr_lock_bh() and > netif_addr_unlock_bh() in the driver's ndo_set_mac() callback. This is fine > since the driver is updated the netdev's dev_addr, but since this is a spinlock, > the driver cannot sleep when the lock is held. > Unfortunately the functions to add/delete MAC filters depend on a mutex. > This was causing a trace with the lock debug kernel config options enabled > when changing the mac address via iproute. > > [ 203.273059] BUG: sleeping function called from invalid context at > kernel/locking/mutex.c:281 [ 203.273065] in_atomic(): 1, irqs_disabled(): 0, > non_block: 0, pid: 6698, name: ip [ 203.273068] Preemption disabled at: > [ 203.273068] [<ffffffffc04aaeab>] ice_set_mac_address+0x8b/0x1c0 [ice] > [ 203.273097] CPU: 31 PID: 6698 Comm: ip Tainted: G S W I 5.14.0- > rc4_next-queue_dev-queue-13aug-01094-gaba1e4adb54e #2 > [ 203.273100] Hardware name: Intel Corporation S2600WFT/S2600WFT, BIOS > SE5C620.86B.02.01.0010.010620200716 01/06/2020 [ 203.273102] Call Trace: > [ 203.273107] dump_stack_lvl+0x33/0x42 [ 203.273113] ? > ice_set_mac_address+0x8b/0x1c0 [ice] [ 203.273124] > ___might_sleep.cold.150+0xda/0xea [ 203.273131] mutex_lock+0x1c/0x40 [ > 203.273136] ice_remove_mac+0xe3/0x180 [ice] [ 203.273155] ? > ice_fltr_add_mac_list+0x20/0x20 [ice] [ 203.273175] > ice_fltr_prepare_mac+0x43/0xa0 [ice] [ 203.273194] > ice_set_mac_address+0xab/0x1c0 [ice] [ 203.273206] > dev_set_mac_address+0xb8/0x120 [ 203.273210] > dev_set_mac_address_user+0x2c/0x50 > [ 203.273212] do_setlink+0x1dd/0x10e0 > [ 203.273217] ? __nla_validate_parse+0x12d/0x1a0 [ 203.273221] > __rtnl_newlink+0x530/0x910 [ 203.273224] ? > __kmalloc_node_track_caller+0x17f/0x380 > [ 203.273230] ? preempt_count_add+0x68/0xa0 [ 203.273236] ? > _raw_spin_lock_irqsave+0x1f/0x30 [ 203.273241] ? > kmem_cache_alloc_trace+0x4d/0x440 [ 203.273244] > rtnl_newlink+0x43/0x60 [ 203.273245] rtnetlink_rcv_msg+0x13a/0x380 [ > 203.273248] ? rtnl_calcit.isra.40+0x130/0x130 [ 203.273250] > netlink_rcv_skb+0x4e/0x100 [ 203.273256] netlink_unicast+0x1a2/0x280 [ > 203.273258] netlink_sendmsg+0x242/0x490 [ 203.273260] > sock_sendmsg+0x58/0x60 [ 203.273263] ____sys_sendmsg+0x1ef/0x260 [ > 203.273265] ? copy_msghdr_from_user+0x5c/0x90 [ 203.273268] ? > ____sys_recvmsg+0xe6/0x170 [ 203.273270] ___sys_sendmsg+0x7c/0xc0 [ > 203.273272] ? copy_msghdr_from_user+0x5c/0x90 [ 203.273274] ? > ___sys_recvmsg+0x89/0xc0 [ 203.273276] ? __netlink_sendskb+0x50/0x50 [ > 203.273278] ? mod_objcg_state+0xee/0x310 [ 203.273282] ? > __dentry_kill+0x114/0x170 [ 203.273286] ? get_max_files+0x10/0x10 [ > 203.273288] __sys_sendmsg+0x57/0xa0 [ 203.273290] > do_syscall_64+0x37/0x80 [ 203.273295] > entry_SYSCALL_64_after_hwframe+0x44/0xae > [ 203.273296] RIP: 0033:0x7f8edf96e278 > [ 203.273298] Code: 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 80 00 00 00 00 f3 0f 1e > fa 48 8d 05 25 63 2c 00 8b 00 85 c0 75 17 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff > 77 58 c3 0f 1f 80 00 00 00 00 41 54 41 89 d4 55 [ 203.273300] RSP: > 002b:00007ffcb8bdac08 EFLAGS: 00000246 ORIG_RAX: 000000000000002e [ > 203.273303] RAX: ffffffffffffffda RBX: 000000006115e0ae RCX: > 00007f8edf96e278 [ 203.273304] RDX: 0000000000000000 RSI: > 00007ffcb8bdac70 RDI: 0000000000000003 [ 203.273305] RBP: > 0000000000000000 R08: 0000000000000001 R09: 00007ffcb8bda5b0 [ > 203.273306] R10: 0000000000000000 R11: 0000000000000246 R12: > 0000000000000001 [ 203.273306] R13: 0000555e10092020 R14: > 0000000000000000 R15: 0000000000000005 > > Fix this by only locking when changing the netdev->dev_addr. Also, make > sure to restore the old netdev->dev_addr on any failures. > > Fixes: 3ba7f53f8bf1 ("ice-linux: don't remove netdev->dev_addr from uc > sync list") > Signed-off-by: Brett Creeley <brett.creeley@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_main.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index fe2ded775f25..1efb25a9be3b 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -5122,6 +5122,7 @@ static int ice_set_mac_address(struct net_device *netdev, void *pi) struct ice_hw *hw = &pf->hw; struct sockaddr *addr = pi; enum ice_status status; + u8 old_mac[ETH_ALEN]; u8 flags = 0; int err = 0; u8 *mac; @@ -5144,6 +5145,11 @@ static int ice_set_mac_address(struct net_device *netdev, void *pi) } netif_addr_lock_bh(netdev); + ether_addr_copy(old_mac, netdev->dev_addr); + /* change the netdev's MAC address */ + memcpy(netdev->dev_addr, mac, netdev->addr_len); + netif_addr_unlock_bh(netdev); + /* Clean up old MAC filter. Not an error if old filter doesn't exist */ status = ice_fltr_remove_mac(vsi, netdev->dev_addr, ICE_FWD_TO_VSI); if (status && status != ICE_ERR_DOES_NOT_EXIST) { @@ -5168,13 +5174,12 @@ static int ice_set_mac_address(struct net_device *netdev, void *pi) if (err) { netdev_err(netdev, "can't set MAC %pM. filter update failed\n", mac); + netif_addr_lock_bh(netdev); + ether_addr_copy(netdev->dev_addr, old_mac); netif_addr_unlock_bh(netdev); return err; } - /* change the netdev's MAC address */ - memcpy(netdev->dev_addr, mac, netdev->addr_len); - netif_addr_unlock_bh(netdev); netdev_dbg(vsi->netdev, "updated MAC address to %pM\n", netdev->dev_addr);
commit 3ba7f53f8bf1 ("ice: don't remove netdev->dev_addr from uc sync list") introduced calls to netif_addr_lock_bh() and netif_addr_unlock_bh() in the driver's ndo_set_mac() callback. This is fine since the driver is updated the netdev's dev_addr, but since this is a spinlock, the driver cannot sleep when the lock is held. Unfortunately the functions to add/delete MAC filters depend on a mutex. This was causing a trace with the lock debug kernel config options enabled when changing the mac address via iproute. [ 203.273059] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:281 [ 203.273065] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 6698, name: ip [ 203.273068] Preemption disabled at: [ 203.273068] [<ffffffffc04aaeab>] ice_set_mac_address+0x8b/0x1c0 [ice] [ 203.273097] CPU: 31 PID: 6698 Comm: ip Tainted: G S W I 5.14.0-rc4_next-queue_dev-queue-13aug-01094-gaba1e4adb54e #2 [ 203.273100] Hardware name: Intel Corporation S2600WFT/S2600WFT, BIOS SE5C620.86B.02.01.0010.010620200716 01/06/2020 [ 203.273102] Call Trace: [ 203.273107] dump_stack_lvl+0x33/0x42 [ 203.273113] ? ice_set_mac_address+0x8b/0x1c0 [ice] [ 203.273124] ___might_sleep.cold.150+0xda/0xea [ 203.273131] mutex_lock+0x1c/0x40 [ 203.273136] ice_remove_mac+0xe3/0x180 [ice] [ 203.273155] ? ice_fltr_add_mac_list+0x20/0x20 [ice] [ 203.273175] ice_fltr_prepare_mac+0x43/0xa0 [ice] [ 203.273194] ice_set_mac_address+0xab/0x1c0 [ice] [ 203.273206] dev_set_mac_address+0xb8/0x120 [ 203.273210] dev_set_mac_address_user+0x2c/0x50 [ 203.273212] do_setlink+0x1dd/0x10e0 [ 203.273217] ? __nla_validate_parse+0x12d/0x1a0 [ 203.273221] __rtnl_newlink+0x530/0x910 [ 203.273224] ? __kmalloc_node_track_caller+0x17f/0x380 [ 203.273230] ? preempt_count_add+0x68/0xa0 [ 203.273236] ? _raw_spin_lock_irqsave+0x1f/0x30 [ 203.273241] ? kmem_cache_alloc_trace+0x4d/0x440 [ 203.273244] rtnl_newlink+0x43/0x60 [ 203.273245] rtnetlink_rcv_msg+0x13a/0x380 [ 203.273248] ? rtnl_calcit.isra.40+0x130/0x130 [ 203.273250] netlink_rcv_skb+0x4e/0x100 [ 203.273256] netlink_unicast+0x1a2/0x280 [ 203.273258] netlink_sendmsg+0x242/0x490 [ 203.273260] sock_sendmsg+0x58/0x60 [ 203.273263] ____sys_sendmsg+0x1ef/0x260 [ 203.273265] ? copy_msghdr_from_user+0x5c/0x90 [ 203.273268] ? ____sys_recvmsg+0xe6/0x170 [ 203.273270] ___sys_sendmsg+0x7c/0xc0 [ 203.273272] ? copy_msghdr_from_user+0x5c/0x90 [ 203.273274] ? ___sys_recvmsg+0x89/0xc0 [ 203.273276] ? __netlink_sendskb+0x50/0x50 [ 203.273278] ? mod_objcg_state+0xee/0x310 [ 203.273282] ? __dentry_kill+0x114/0x170 [ 203.273286] ? get_max_files+0x10/0x10 [ 203.273288] __sys_sendmsg+0x57/0xa0 [ 203.273290] do_syscall_64+0x37/0x80 [ 203.273295] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 203.273296] RIP: 0033:0x7f8edf96e278 [ 203.273298] Code: 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 8d 05 25 63 2c 00 8b 00 85 c0 75 17 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 41 54 41 89 d4 55 [ 203.273300] RSP: 002b:00007ffcb8bdac08 EFLAGS: 00000246 ORIG_RAX: 000000000000002e [ 203.273303] RAX: ffffffffffffffda RBX: 000000006115e0ae RCX: 00007f8edf96e278 [ 203.273304] RDX: 0000000000000000 RSI: 00007ffcb8bdac70 RDI: 0000000000000003 [ 203.273305] RBP: 0000000000000000 R08: 0000000000000001 R09: 00007ffcb8bda5b0 [ 203.273306] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001 [ 203.273306] R13: 0000555e10092020 R14: 0000000000000000 R15: 0000000000000005 Fix this by only locking when changing the netdev->dev_addr. Also, make sure to restore the old netdev->dev_addr on any failures. Fixes: 3ba7f53f8bf1 ("ice-linux: don't remove netdev->dev_addr from uc sync list") Signed-off-by: Brett Creeley <brett.creeley@intel.com> --- drivers/net/ethernet/intel/ice/ice_main.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)