On Fri, Oct 11, 2013 at 11:11 AM, Jesse Gross <je...@nicira.com> wrote: > On Thu, Oct 10, 2013 at 9:48 PM, Alexei Starovoitov <a...@plumgrid.com> wrote: >> On Thu, Oct 10, 2013 at 8:56 PM, Jesse Gross <je...@nicira.com> wrote: >>> However, the check dev->reg_state in netdev_destroy() looks racy to >>> me, as it could already be in NETREG_UNREGISTERED even if we already >>> processed this device. >> >> you mean that netdev_destroy() will see reg_state == netreg_unregistered, >> while dp_device_event() didn't see reg_state == netreg_unregistering yet? >> or dp_device_event() saw it, proceeded to do unlink and >> netdev_destroy() ran in parallel? >> well, that's why reg_state == netreg_unregistering check in netdev_destroy() >> is done with rtnl_lock() held. >> reg_state cannot go into netreg_unregistered state skipping >> netreg_unregistering and notifier. >> therefore I don't think it's racy. >> >> In ovs_dp_notify_wq() you're checking for both unregistering and >> unregistered and that makes >> sense, since workq can run after unregistering notifier called and >> netdev_run_todo() >> already changed the state to unregistered. >> But here it's not the case. > > ovs_dp_notify_wq() calls ovs_dp_detach_port(), which indirectly calls > netdev_destroy() so it seems like it actually is the same case to me.
yes. makes sense. how about: - if (netdev_vport->dev->reg_state != NETREG_UNREGISTERING) + if (netdev_vport->dev->priv_flags & IFF_OVS_DATAPATH) or you would prefer (state != unregistering && state != unregistered) check instead? ovs_netdev_destroy_dev() name instead ovs_netdev_unlink_dev() name? Thanks Alexei _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
