Hi Ben,
On Fri, May 02, 2014 at 07:34:58AM -0700, Ben Pfaff wrote:
> On Fri, May 02, 2014 at 05:41:35PM +0900, Simon Horman wrote:
> > This is to allow a recirculation id to be associated with a rule
> > in the case that its actions cause recirculation.
> >
> > In such a case if the recirc_id field is non-zero then that value should be
> > used, otherwise a value should be obtained using
> > ofproto_dpif_alloc_recirc_id and saved in recirc_id field.
> >
> > When destructing the rule if the recirc_id field is non-zero then
> > the associated internal flow should be deleted.
> >
> > This is in preparation for using the same helper as part of support
> > for using recirculation in conjunction series of actions including
> > with MPLS actions that are currently not able to be translated.
> >
> > Signed-off-by: Simon Horman <ho...@verge.net.au>
>
> Has this been tested? It appears that rule_dpif_get_recirc_id()
> requires rule->up.mutex:
I have exercised the code that uses this code.
But not specifically the locking portions.
I will look into this.
>
> > +uint32_t
> > +rule_dpif_get_recirc_id(struct rule_dpif *rule)
> > + OVS_REQUIRES(rule->up.mutex)
> > +{
> > + if (!rule->recirc_id) {
> > + struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
> > +
> > + rule_dpif_set_recirc_id(rule,
> > ofproto_dpif_alloc_recirc_id(ofproto));
> > + }
> > + return rule->recirc_id;
> > +}
>
> but that rule_dpif_get_recirc_id() sometimes calls rule_set_recirc_id(),
> which attempts to acquire rule->up.mutex:
>
> > +/* Sets 'rule''s recirculation id. */
> > +void
> > +rule_set_recirc_id(struct rule *rule_, uint32_t id)
> > +{
> > + struct rule_dpif *rule = rule_dpif_cast(rule_);
> > +
> > + ovs_mutex_lock(&rule->up.mutex);
> > + rule_dpif_set_recirc_id(rule, id);
> > + ovs_mutex_unlock(&rule->up.mutex);
> > +}
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
