On Fri, Apr 08, 2016 at 03:12:59AM +0000, Daniele Di Proietto wrote: > > > On 01/04/2016 09:52, "Jarno Rajahalme" <ja...@ovn.org> wrote: > > > > >> On Mar 30, 2016, at 8:08 PM, Daniele Di Proietto > >><diproiet...@vmware.com> wrote: > >> > >> > >> On 30/03/2016 16:01, "Ben Pfaff" <b...@ovn.org> wrote: > >> > >>> (I'm taking a look at this patch specifically because Daniele asked me; > >>> I'm not planning to review the whole series.) > >>> > >>> On Mon, Mar 28, 2016 at 12:41:40PM -0700, Daniele Di Proietto wrote: > >>>> The dpif-netdev datapath keeps ports in a cmap which is written only > >>>>by > >>>> the main thread (holding port_mutex), but which is read concurrently > >>>>by > >>>> many threads (most notably the pmd threads). > >>>> > >>>> When removing ports from the datapath we should postpone the deletion, > >>>> otherwise another thread might access invalid memory while reading the > >>>> cmap. > >>>> > >>>> This commit splits do_port_del() in do_port_remove() and > >>>> do_port_destroy(): the former removes the port from the cmap, while > >>>>the > >>>> latter reclaims the memory and drops the reference to the underlying > >>>> netdev. > >>> > >>> s/del_port/port_del/ here: > >> > >> Thanks, changed > >> > >>> > >>>> dpif_netdev_del_port() now uses ovsrcu_synchronize() before calling > >>>> do_port_destroy(), to avoid memory corruption in concurrent readers. > >>> > >>> ovsrcu_synchronize() requires that nothing in the thread that calls it > >>> is relying on RCU to keep objects around. That means that no caller of > >>> dfpi_port_del()--there are a few of them--can rely on it. This is > >>> usually a risky assumption, especially because this assumption can > >>> change later. Is there reason to believe that it isn't important in > >>>all > >>> of these cases? > >> > >> I agree that's risky, but I think it's the only way to keep the ports > >>RCU > >> protected, because a port needs to be effectively deleted before > >> dpif_netdev_port_del() can return. > >> > > > >If this is because otherwise a following port_add can fail, as the old > >port is still around, maybe we could make the highest possible level of > >port_add detect the failure and then rcu_synchronize and try again? Would > >that work? > > > > Jarno > > After some thought I decided to avoid using RCU for ports. I'll send an > updated > series soon.
Well, that makes the discussion a little easier ;-) Thanks. Do you want to me to review anything in the new version? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
