On Fri, Mar 08, 2013 at 03:50:12PM +0200, Jarno Rajahalme wrote: > Remove recursion from GOTO_TABLE processing in do_xlate_actions(). > This allows packet processing pipelines built with goto table be > longer and not interact with each other via the resubmit recursion limit. > > Signed-off-by: Jarno Rajahalme <jarno.rajaha...@nsn.com>
Thank you, this is a nice refinement. This looks very close to correct to me. However, I don't think that the handling of the 'evictable' flags is quite right. The basic rule for 'evictable' is that we need to make sure that it is set for any rule that we're currently "using" in some way (one example is iterating over its actions) to tell "learn" actions not to delete that rule. As I read your patch, it didn't do this for rules reached via "goto_table". (Probably because the purpose of 'evictable' really isn't obvious.) I applied the following incremental to fix that. It seems correct, so I'll apply it to master soon, but please check my work. Thanks, Ben. diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 6628aa6..d911080 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -6305,7 +6305,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, { bool was_evictable = true; const struct ofpact *a; - struct rule_dpif *orig_rule = ctx->rule; if (ctx->rule) { /* Don't let the rule we're working on get evicted underneath us. */ @@ -6509,7 +6508,12 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, rule = ctx_rule_hooks(ctx, rule, true); if (rule) { + if (ctx->rule) { + ctx->rule->up.evictable = was_evictable; + } ctx->rule = rule; + was_evictable = rule->up.evictable; + rule->up.evictable = false; /* Tail recursion removal. */ ofpacts = rule->up.ofpacts; @@ -6522,7 +6526,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, } out: - ctx->rule = orig_rule; if (ctx->rule) { ctx->rule->up.evictable = was_evictable; } _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev