Eric W. Biederman wrote: > Daniel Lezcano <daniel.lezcano@xxxxxxx> writes: > > >> Eric W. Biederman wrote: >> >>> Daniel Lezcano <daniel.lezcano@xxxxxxx> writes: >>> >>> >>>> Hmm, at the first glance I would say it is useless but perhaps there is a >>>> >> trick >> >>>> here I do not understand. >>>> Eric, is there any particular reason to call synchronize_net before exiting >>>> >> the >> >>>> dev_change_net_namespace function ? >>>> >>>> >>> I haven't thought about that part of the code path in detail in a long >>> time. dev_change_net_namespace() is a condensed version of >>> register_netdevice() unregister_netdevice(). With the calls down into >>> the driver removed. >>> >>> On a side note. It looks like we now cope with: >>> call_netdevice_notifiers(NETDEV_REGISTER, dev); failing in >>> register_netdev, but no one updated dev_change_net_namespace to handle >>> the change, looks like a real pain to cope with. >>> >>> As for the synchronize_net, and in response to the original >>> comment as best as I can tell we do have things being being >>> deleted that are at least candidates for synchronize_net. >>> >>> dev_addr_discard(dev); >>> dev_net_set(dev, net); >>> netdev_unregister_kobject(dev); >>> >>> We very much do access dev->net with only rcu protection. >>> >>> Hmm. >>> >>> It looks like I originally took the second synchronize_net from what >>> became rollback_registered, which happens just before we start freeing >>> the netdevice. >>> >>> The nastiest case that I can envision is if we happen to receive a >>> packet (on another cpu) for the network device that we are moving, >>> just after it has registered in the new network namespace. If we read >>> the old network namespace and forward it up the network stack in that >>> context I can imagine it being a recipe for all kinds of strange >>> non-deterministic behavior. >>> >>> >> The code does: >> >> dev_close >> dev_deactive >> synchronize_rcu >> synchronize_net >> ... >> dev_shutdown >> ... >> synchronize_net >> >> The network device can no longer receive packets after dev_deactive, no ? >> The first synchronize_net will wait for the outstanding packets to be delivered >> to the upper layer and we change the nd_net field after. >> Your scenario makes sense for the first synchronize_net but I am not sure that >> can happen if we remove the second synchronize_net. >> > > Good point. Visibility is key. What can find us after we > call list_netdevice() ? Aren't there some pieces of code that > do for_each_netdevice under the rcu lock? > AFAIR, no. for_each_netdev is protected by rtnl_lock. _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers