On 10/04/2016 12:23, "Ben Pfaff" <b...@ovn.org> wrote:
>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? I was only concerned about the use of RCU. The new version should be simpler. Thanks! _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
