[ovs-dev] [next2 7/7] ofproto: Make rule construction and destruction more symmetric.

Ben Pfaff blp at nicira.com
Wed May 11 20:55:55 UTC 2011


On Wed, May 11, 2011 at 01:50:37PM -0700, Ethan Jackson wrote:
> > +/* Inserts 'rule' into 'cls'. ?Until 'rule' is removed from 'cls', the caller
> > + * must not modify or free it.
> > + *
> > + * 'cls' must not contain an identical rule (including wildcards, values of
> > + * fixed fields, and priority). ?Use classifier_find_rule_exactly() to find
> > + * such a rule.
> > + *
> > + * Returns NULL if 'cls' does not contain a rule with an identical key, after
> > + * inserting the new rule. ?In this case, no rules are displaced by the new
> > + * rule, even rules that cannot have any effect because the new rule matches a
> > + * superset of their flows and has higher priority. */
> 
> This final paragraph bitrotted in the copy. classifier_insert()
> doesn't return anything.

Thanks for pointing this out.

I deleted the final paragraph.

> > + ? ?classifier_remove(&ofproto->up.cls, &rule->up.cr);
> > ? ? LIST_FOR_EACH_SAFE (facet, next_facet, list_node, &rule->facets) {
> > ? ? ? ? facet_revalidate(ofproto, facet);
> > ? ? }
> > -}
> > -
> > -static void
> > -rule_remove(struct rule *rule_)
> > -{
> > - ? ?struct rule_dpif *rule = rule_dpif_cast(rule_);
> > - ? ?struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
> > -
> > ? ? ofproto->need_revalidate = true;
> > - ? ?classifier_remove(&ofproto->up.cls, &rule->up.cr);
> > ?}
> 
> Why do we need to facet_revalidate each of 'rule''s facets given that
> we set the ofproto->need_revalidate flag?  Are we worried that the
> facet's will point to a destructed parent rule and cause problems?

That's right.  We can't tolerate a facet with a NULL rule or a facet
whose rule is a pointer to freed memory.



More information about the dev mailing list