[ovs-dev] [PATCH] dpif-netdev: Wait for threads to quiesce before freeing port.

[Ben Pfaff]

I haven't looked at the code yet, but one restriction on
ovsrcu_synchronize() is that the current thread can't have active
pointers to any RCU-protected data (they can get freed).  Is that safe
here?

On Thu, Mar 19, 2015 at 01:43:54PM -0700, Jarno Rajahalme wrote:
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> Ben may also want to have a look, as commits 98de6bebb (dpif-netdev: Fix another use-after-free in port_unref().) and 87400a3d4cc4a (dpif-netdev: Fix use-after-free in port_unref().) by him have fixed earlier problems related to this. I did not check the changes in these earlier patches, but it is possible that ovsrcu_synchronize() is a more general solution to this problem, as this makes the port lookups essentially RCU-protected. Maybe we should document this fact in some comments?
> 
>   Jarno
> 
> > On Mar 19, 2015, at 9:44 AM, Daniele Di Proietto <diproiettod at vmware.com> wrote:
> > 
> > port_unref() shouldn't immediately free the port and close the netdev,
> > because otherwise threads that still have a pointer to the port would
> > crash.
> > 
> > This commit fixes the problem by introducing an ovsrcu_synchronize()
> > call in port_unref().
> > 
> > I chose not to use ovsrcu_postpone(), because postponing the removal of
> > the 'netdev' would cause other issues, such as not being possible to
> > reuse the same name for another netdev.
> > 
> > Found the crash while developing new code.
> > 
> > Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
> > ---
> > lib/dpif-netdev.c | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index f01fecb..2546e5e 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -994,6 +994,8 @@ port_unref(struct dp_netdev_port *port)
> >         int n_rxq = netdev_n_rxq(port->netdev);
> >         int i;
> > 
> > +        ovsrcu_synchronize();
> > +
> >         netdev_close(port->netdev);
> >         netdev_restore_flags(port->sf);
> > 
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev