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. > So unless there is a reason for this change beyond general cleanup I > would prefer not to think about it potential weirdness, and keep the > code the way it is. > > I seem to remember a conversation about this synchronize_net when the > code was merged as well so if we are going to change it, let's look > up those arguments if we can and see if there was something useful > said. > > Eric > > > _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers