On Mon, Nov 21, 2011 at 10:09:47AM -0800, Jesse Gross wrote: > On Mon, Nov 21, 2011 at 10:00 AM, Ben Pfaff <b...@nicira.com> wrote: > > On Sat, Nov 19, 2011 at 02:11:48PM -0800, Jesse Gross wrote: > >> We currently have a wrapper to protect the datapath ports array. > >> However, this can lead to confusion over exactly what lock is > >> protecting the access (either RTNL or RCU). ??This removes the > >> wrapper in favor of directly accessing the data, which also has > >> the benefit of being less permissive about what lock we allow so > >> it can be restricted to the one that we expect. > >> > >> Signed-off-by: Jesse Gross <je...@nicira.com> > > > >> @@ -132,7 +126,7 @@ static int get_dpifindex(struct datapath *dp) > >> > >> ?? ?? ?? rcu_read_lock(); > >> > >> - ?? ?? local = get_vport_protected(dp, OVSP_LOCAL); > >> + ?? ?? local = rcu_dereference(dp->ports[OVSP_LOCAL]); > >> ?? ?? ?? if (local) > >> ?? ?? ?? ?? ?? ?? ?? ifindex = local->ops->get_ifindex(local); > >> ?? ?? ?? else > > > > get_dpifindex() is called from dp_fill_ifinfo(), which holds RTNL but > > I don't see that it holds RCU. > > It's called from other places as well that don't hold RTNL, such as > dp_upcall(). We do hold RCU read lock inside of this function (see > the snippet above) and that's all that is really needed to protect > access to the vport since we are returning an integer and not a > pointer to the protected data.
Somehow I missed the rcu_read_lock() in get_dpifindex() itself. Thanks. Acked-by: Ben Pfaff <b...@nicira.com> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
