From bb61b33de6d6b8488aabf76419371a13ddecde36 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 23 Aug 2013 11:03:55 -0700 Subject: [PATCH] ofproto-dpif-xlate: Refactor xlate_table_action() to avoid Clang warnings. I get a bunch of thread-safety warnings with the latest Clang without this patch, because Clang is smart enough to see locking and unlocking but not smart enough to figure out the relationships. This refactoring avoids the warnings. I first noticed these warnings with Clang 1:3.4~svn188890-1~exp1. I previously used version 1:3.4~svn187484-1~exp1. Signed-off-by: Ben Pfaff Acked-by: Jarno Rajahalme --- ofproto/ofproto-dpif-xlate.c | 55 ++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 45786756c..54fabfe43 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1663,6 +1663,25 @@ compose_output_action(struct xlate_ctx *ctx, ofp_port_t ofp_port) compose_output_action__(ctx, ofp_port, true); } +static void +xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule) + OVS_RELEASES(rule->up.evict) +{ + struct rule_dpif *old_rule = ctx->rule; + + if (ctx->xin->resubmit_stats) { + rule_credit_stats(rule, ctx->xin->resubmit_stats); + } + + ctx->recurse++; + ctx->rule = rule; + do_xlate_actions(rule->up.ofpacts, rule->up.ofpacts_len, ctx); + ctx->rule = old_rule; + ctx->recurse--; + + rule_release(rule); +} + static void xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id, bool may_packet_in) @@ -1671,28 +1690,28 @@ xlate_table_action(struct xlate_ctx *ctx, struct rule_dpif *rule; ofp_port_t old_in_port = ctx->xin->flow.in_port.ofp_port; uint8_t old_table_id = ctx->table_id; + bool got_rule; ctx->table_id = table_id; - /* Look up a flow with 'in_port' as the input port. */ + /* Look up a flow with 'in_port' as the input port. Then restore the + * original input port (otherwise OFPP_NORMAL and OFPP_IN_PORT will + * have surprising behavior). */ ctx->xin->flow.in_port.ofp_port = in_port; - rule_dpif_lookup_in_table(ctx->xbridge->ofproto, &ctx->xin->flow, - &ctx->xout->wc, table_id, &rule); - - /* Restore the original input port. Otherwise OFPP_NORMAL and - * OFPP_IN_PORT will have surprising behavior. */ + got_rule = rule_dpif_lookup_in_table(ctx->xbridge->ofproto, + &ctx->xin->flow, &ctx->xout->wc, + table_id, &rule); ctx->xin->flow.in_port.ofp_port = old_in_port; if (ctx->xin->resubmit_hook) { ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse); } - if (rule == NULL && may_packet_in) { + if (got_rule) { + xlate_recursively(ctx, rule); + } else if (may_packet_in) { struct xport *xport; - /* Makes clang's thread safety analysis happy. */ - rule_release(rule); - /* XXX * check if table configuration flags * OFPTC_TABLE_MISS_CONTROLLER, default. @@ -1704,23 +1723,9 @@ xlate_table_action(struct xlate_ctx *ctx, ctx->xbridge->miss_rule, ctx->xbridge->no_packet_in_rule); ovs_rwlock_rdlock(&rule->up.evict); + xlate_recursively(ctx, rule); } - if (rule && ctx->xin->resubmit_stats) { - rule_credit_stats(rule, ctx->xin->resubmit_stats); - } - - if (rule) { - struct rule_dpif *old_rule = ctx->rule; - - ctx->recurse++; - ctx->rule = rule; - do_xlate_actions(rule->up.ofpacts, rule->up.ofpacts_len, ctx); - ctx->rule = old_rule; - ctx->recurse--; - } - rule_release(rule); - ctx->table_id = old_table_id; } else { static struct vlog_rate_limit recurse_rl = VLOG_RATE_LIMIT_INIT(1, 1); -- 2.43.0