I applied this commit to branch-2.1 and it will be in the next release from that branch.
Sorry it took me so long to get back to this. On Wed, Sep 17, 2014 at 05:17:17PM -0400, Anup Khadka wrote: > Yes, I got a crash with 100 rules which led me to inspect the code. > > The collect_rule function inside collect_rules_loose returns > OFPROTO_POSTPONE if rule->pending is non-zero (this is possible if > the ofproto vendor class in not done inserting the rules). > > In such cases, collect_rules_loose function will call a free on rules. > > Without your commit in July, delete_flows_loose will call free again on the > rules structure. > > This is what was happening in my code base too. I was using OVS 2.1. > > Thanks, > Anup > > > > > > > On Wed, Sep 17, 2014 at 5:01 PM, Ben Pfaff <[email protected]> wrote: > > > According to the commit message, the bug could not cause a real problem > > in practice. Do you see a way that it could? > > > > On Wed, Sep 17, 2014 at 04:50:04PM -0400, Anup Khadka wrote: > > > Looks like this code was added in July: > > > https://github.com/openvswitch/ovs/commit/bfd3dbf6a0c978ceb20faf292bca51 > > > 3a63e2b68c > > > I was using an older code-base. > > > Thanks, > > > Anup > > > > > > On Wed, Sep 17, 2014 at 4:37 PM, Ben Pfaff <[email protected]> wrote: > > > > > > > On Wed, Sep 17, 2014 at 02:58:50PM -0400, Anup Khadka wrote: > > > > > On Tue, Sep 16, 2014 at 3:30 PM, Anup Khadka <[email protected]> > > > > wrote: > > > > > > > > > > > It looks like OVS tries to double-free in delete_flows_loose if the > > > > > > rules->rules (inside struct rule_collection *rules is not equal to > > > > > > rules->stub). > > > > > > > > > > > > A little more detail: > > > > > > In the function delete_flows_loose, the call to the function > > > > > > collect_rules_loose takes care of freeing rules (again struct > > > > > > rule_collection *rules) if there is any error while collecting the > > > > rule. > > > > > > > > > > > > The function returns back to delete_flows_loose where it calls > > > > > > rule_collection_destroy again. > > > > > > > > > > > > Because rules->rules is still not rules->stab, it attempts to free > > the > > > > > > rules structure again, resulting in a double-free. > > > > > > > > > > > > Perhaps rules->rules can be set to rules->stab inside > > > > > > rule_collection_destroy function after its freed. Or perhaps, > > > > > > rule_collection_destroy should only be called from > > delete_flows_loose > > > > if > > > > > > there is no error, or perhaps collect_rules_loose should not take > > care > > > > of > > > > > > freeing the data structure. > > > > > > > > rule_collection_destroy() already reinitializes 'rules' after it > > > > destroys it: > > > > > > > > void > > > > rule_collection_destroy(struct rule_collection *rules) > > > > { > > > > if (rules->rules != rules->stub) { > > > > free(rules->rules); > > > > } > > > > > > > > /* Make repeated destruction harmless. */ > > > > rule_collection_init(rules); > > > > } > > > > > > _______________________________________________ discuss mailing list [email protected] http://openvswitch.org/mailman/listinfo/discuss
