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. I think it will not be too risky because the code that calls dpif_netdev_port_del() is high level code that doesn't deal with RCU protected pointers. Of course, things might change. One way to improve the situation would be to use thread-safety annotation to mark all the functions that might quiesce: I've tried doing that, but a lot of functions need to get tagged, like ovs_mutex_cond_wait(), which we already use in the userspace datapath. An easy way to get rid of this problem would be to avoid RCU for ports and storing them in an hmap, but I would like to avoid that. I'll keep thinking about this, maybe we can come up with a better idea. Thanks, Daniele _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
