Re: [PATCH net v1] net: defer __dev_set_promiscuity() to avoid sleeping in atomic context

[Jakub Kicinski]

On Thu, 19 Feb 2026 10:59:01 -0800 Stanislav Fomichev wrote:
> On 02/18, Jakub Kicinski wrote:
> > On Tue, 17 Feb 2026 17:10:36 -0800 Stanislav Fomichev wrote:  
> > > > 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 bug    
> > > 
> > > Can 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)

> 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.