On 9/2/16 8:45 AM, Eric Dumazet wrote: > On Fri, 2016-09-02 at 08:30 -0600, David Ahern wrote: > >> This check duplicates what netdev_rx_handler_register does. Why not >> move the call to netdev_rx_handler_register here and then call >> unregister on failure paths? > > As soon as you call netdev_rx_handler_register(), incoming packets will > hit your driver and we'll likely crash since the enslaving is not done > yet. > > Really think about RCU, we do rcu_assign_pointer() because we want all > prior changes being committed to memory before 'enabling' readers to see > the updated rcu protected pointer. > > There are 9 call sites where netdev_rx_handler_register() is used. > > We can get rid of the extra check in netdev_rx_handler_register() once > all of them are using netdev_is_rx_handler_busy() > > Since this patch takes care of bonding only, we need to keep the > existing check in netdev_rx_handler_register() > > Anyway, we are speaking of control function, an extra check is simply > safer, like all the ASSERT_RTNL() we do have... >
I hit this same problem yesterday but with the bridge. I forgot I had a macvlan device on an interface and tried to enslave it to a bridge. It failed with EBUSY without crashing the kernel so it is one example that handles the conflict, and the bridge also calls the register before the enslaving is done.
