[ovs-dev] [threaded-learning v2 09/25] ofproto: Remove soon-to-be-invalid optimizations.

Ben Pfaff blp at nicira.com
Tue Sep 24 03:09:24 UTC 2013


On Tue, Sep 24, 2013 at 11:41:23AM +0900, YAMAMOTO Takashi wrote:
> > @@ -1788,26 +1815,21 @@ ofproto_delete_flow(struct ofproto *ofproto,
> >      struct classifier *cls = &ofproto->tables[0].cls;
> >      struct rule *rule;
> >  
> > +    /* First do a cheap check whether the rule we're looking for has already
> > +     * been deleted.  If so, then we're done. */
> >      ovs_rwlock_rdlock(&cls->rwlock);
> >      rule = rule_from_cls_rule(classifier_find_match_exactly(cls, target,
> >                                                              priority));
> >      ovs_rwlock_unlock(&cls->rwlock);
> >      if (!rule) {
> > -        /* No such rule -> success. */
> > -        return true;
> > -    } else if (rule->pending) {
> > -        /* An operation on the rule is already pending -> failure.
> > -         * Caller must retry later if it's important. */
> > -        return false;
> > -    } else {
> > -        /* Initiate deletion -> success. */
> > -        ovs_rwlock_wrlock(&cls->rwlock);
> > -        ofproto_rule_delete(ofproto, cls, rule);
> > -        ovs_rwlock_unlock(&cls->rwlock);
> > -
> >          return true;
> >      }
> >  
> > +    /* Fall back to a executing a full flow mod.  We can't optimize this at all
> > +     * because we didn't take enough locks above to ensure that the flow table
> > +     * didn't already change beneath us.  */
> > +    return simple_flow_mod(ofproto, target, priority, NULL, 0,
> > +                           OFPFC_DELETE_STRICT) != OFPROTO_POSTPONE;
> >  }
> 
> this seems wrong to me.  ofproto_delete_flow is used to remove hidden rules.
> the new code ends up to use collect_rules_strict which skips hidden rules.

Good point, thank you.



More information about the dev mailing list