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? Eric _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers