[ovs-dev] [PATCH 3/3] ofproto-dpif-xlate: Implement RCU locking in ofproto-dpif-xlate.
wryan at vmware.com
Thu May 15 18:16:40 UTC 2014
Here's my newest patch addressing this issue:
Also, the second idea to resolve this issue which I discussed offline with Ethan was to have the netdev struct have a pointer to its subclass (i.e. netdev with name 'p0' has a pointer to its dummy subclass). Thus, when switching types, we could use the RCU userspace API.
However, this solution wasn't quite as simple as advertised as the netdev_open function is used in many places with the following logic: open the netdev with the following name 'p0' if it exists, otherwise create 'p0' with this new type (netdev_open in netdev_get_in4_by_name is a good example of this). Its likely that we would have to have 2 different netdev_open's here, one which do the logic as explained in the previous sentence and one which will add a type check if 'p0' exists and if it exists, create 'p0' with the new type. In my opinion, this makes the code more complicated as there are two versions of netdev_open or requires a deeper refactoring of the netdev API.
Anyways, let me know what your thoughts are on this and if this sounds good, I'll resubmit my RCU locking patch corrected for this change.
Member of Technical Staff
wryan at vmware.com
3401 Hillview Avenue, Palo Alto, CA
On May 9, 2014, at 11:48 AM, Alex Wang <alexw at nicira.com> wrote:
> Great to see this series.
> Based on the offline discussion with Ryan, we think it is not ideal to enforce the
> double-standard that xlate module cannot take reference to netdev while other
> modules (e.g. bfd/cfm) must. This makes the code more complicated.
> The root cause of this double-standard is the need to guarantee that netdev shash
> does not contains the netdev when it is certain to be removed. Currently, the rcu_postponed
> xport deletion may cause the netdev still in the shash and prevent the creation of
> new netdev with same name
> One idea is to move the "removal of netdev from shash" logic into a separate function.
> And this function should be called whenever we are sure the netdev will be removed (hope
> there are not too many places and it is not invovled)
> We will discuss this further, and update the thread with the direction to go.
> +static void
> +xlate_xport_copy(struct xbridge *xbridge, struct xbundle *xbundle,
> + struct xport *xport)
> + struct skb_priority_to_dscp *pdscp, *new_pdscp;
> + struct xport *new_xport = xzalloc(sizeof *xport);
> + new_xport->ofport = xport->ofport;
> + new_xport->ofp_port = xport->ofp_port;
> + new_xport->xbridge = xbridge;
> + xlate_xport_init(new_xport);
> + xlate_xport_set(new_xport, xport->odp_port, xport->cfm, xport->bfd,
> + xport->stp_port_no, xport->config, xport->state,
> + xport->is_tunnel, xport->may_enable);
> + /* Note that RCU read threads do not take a reference to netdev since this
> + * blocks creation of netdevs with the same name after deletion or type
> + * modification of the netdev */
> + new_xport->netdev = xport->netdev;
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the dev