Re: [PATCH net v1] net: defer __dev_set_promiscuity() to avoid sleeping in atomic context
From: Stanislav Fomichev <hidden>
Date: 2026-02-20 00:30:45
Also in:
lkml
On 02/19, Jakub Kicinski wrote:
quoted hunk ↗ jump to hunk
On Thu, 19 Feb 2026 10:59:01 -0800 Stanislav Fomichev wrote:quoted
On 02/18, Jakub Kicinski wrote:quoted
On Tue, 17 Feb 2026 17:10:36 -0800 Stanislav Fomichev wrote:quoted
quoted
Reproducer: ip link add dummy0 type dummy ip link add team0 type team ip link set dummy0 master team0 ip link set team0 up ip link add bridge0 type bridge vlan_filtering 1 ip link set bridge0 up ip link set team0 master bridge0 ip link add macsec0 link bridge0 type macsec ip link set macsec0 up # triggers the bugCan you add it as a selftest under selftests/drivers/net/team/?Stan, this "fix" may work for the promisc flag but won't we have the same problem with sync'ing the address list? Looks like team will do: - team_set_rx_mode() - dev_uc_sync_multiple() - __dev_set_rx_mode(port->dev) so AFAICT we're calling ndo_set_rx_mode without holding the instance lock?Not sure I understand your trace without more details about the hierarchy.Team on top of a ops-locked netdev - team_set_rx_mode() # set_rx_mode on team - dev_uc_sync_multiple() - __dev_set_rx_mode(port->dev) # calls ndo_set_rx_mode on ops-locked # netdev without holding the inst. lock IOW this will fire:diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c index 6285fbefe38a..77991f62bffc 100644 --- a/drivers/net/netdevsim/netdev.c +++ b/drivers/net/netdevsim/netdev.c@@ -184,6 +184,7 @@ static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev) static void nsim_set_rx_mode(struct net_device *dev) { + netdev_assert_locked(dev); } static int nsim_change_mtu(struct net_device *dev, int new_mtu)
Ah, you're saying in general.. Yeah, agreed, for the instance locked devices not grabbing an instance lock for these paths looks problematic.
quoted
But you have a point, per netdevices.rst ndo_set_rx_mode is synchronized via netif_addr_lock and we are breaking that with this patch.. :-( (so I don't think we need an instance lock if we keep netif_addr_lock?) For this particular issue, maybe we can do something similar to net_todo_list? Instead of changing the promisc for !FLT under right here right now, move it to the rtnl_unlock? Not sure how important the ordering is..Not sure. Another alternative is to implement the long standing idea of having an async / sleeping version of ndo_set_rx_mode() orchestrated by the core. Because a lot of drivers need to sleep, anyway, so they just schedule a work from that callback. Then we can say old ndo_set_rx_mode is under netif_addr_lock. ndo_set_rx_mode_async is under instance lock.
That sounds like a better plan going forward, but gonna need a bunch of work to redo the addr lock it seems? We can start with moving promisc into rtnl_unlock to unblock that "bridge vlan_filtering 1" and I can look into adding an instance lock for set_rx_mode.. LMK if you prefer me to focus on the latter and don't waste time on the former.