On Fri, Oct 11, 2013 at 1:03 PM, Alexei Starovoitov <a...@plumgrid.com> wrote: > 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)
Yes, this seems safer. Is the check for NETREG_UNREGISTERING in dp_device_event() still needed given that we are checking the event? > ovs_netdev_destroy_dev() name instead ovs_netdev_unlink_dev() name? How about detach_dev? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
