From: Ben Pfaff Date: Fri, 13 Apr 2012 21:09:10 +0000 (-0700) Subject: ofproto-dpif: Make it easier to credit statistics for resubmits. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=112bc5f46f04c46e1d4fbfaa93f76ea39b39b66e;p=openvswitch ofproto-dpif: Make it easier to credit statistics for resubmits. Until now, crediting statistics to OpenFlow rules due to "resubmit" actions has required setting up a "resubmit hook" with a callback function and auxiliary data. This commit makes it easier to do, by adding a member to struct action_xlate_ctx that specifies statistics to credit to each resubmitted rule. This commit includes one small behavioral change as an optimization. Previously, rule_execute() translated the rule twice: once to get the ODP actions, then a second time after executing the ODP actions to credit statistics to the rules. After this commit, rule_execute() translates the rule only once, crediting statistics as a side effect. The difference only becomes visible when executing the actions fails: previously the statistics would not be incremented, after this commit they will be. It is very unusual for executing actions to fail (generally this indicates a bug) so I'm not concerned about it. Signed-off-by: Ben Pfaff --- diff --git a/lib/dpif.c b/lib/dpif.c index a1bd59b0..7cf5fba4 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -674,8 +674,7 @@ dpif_port_poll_wait(const struct dpif *dpif) } /* Extracts the flow stats for a packet. The 'flow' and 'packet' - * arguments must have been initialized through a call to flow_extract(). - */ + * arguments must have been initialized through a call to flow_extract(). */ void dpif_flow_stats_extract(const struct flow *flow, const struct ofpbuf *packet, struct dpif_flow_stats *stats) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 99993868..ed87762c 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -105,10 +105,10 @@ static struct rule_dpif *rule_dpif_cast(const struct rule *rule) static struct rule_dpif *rule_dpif_lookup(struct ofproto_dpif *, const struct flow *, uint8_t table); +static void rule_credit_stats(struct rule_dpif *, + const struct dpif_flow_stats *); static void flow_push_stats(struct rule_dpif *, const struct flow *, - uint64_t packets, uint64_t bytes, - long long int used); - + const struct dpif_flow_stats *); static tag_type rule_calculate_tag(const struct flow *, const struct flow_wildcards *, uint32_t basis); @@ -225,13 +225,23 @@ struct action_xlate_ctx { * timeouts.) */ uint8_t tcp_flags; - /* If nonnull, called just before executing a resubmit action. In - * addition, disables logging of traces when the recursion depth is - * exceeded. + /* If nonnull, flow translation calls this function just before executing a + * resubmit or OFPP_TABLE action. In addition, disables logging of traces + * when the recursion depth is exceeded. + * + * 'rule' is the rule being submitted into. It will be null if the + * resubmit or OFPP_TABLE action didn't find a matching rule. * * This is normally null so the client has to set it manually after * calling action_xlate_ctx_init(). */ - void (*resubmit_hook)(struct action_xlate_ctx *, struct rule_dpif *); + void (*resubmit_hook)(struct action_xlate_ctx *, struct rule_dpif *rule); + + /* If nonnull, flow translation credits the specified statistics to each + * rule reached through a resubmit or OFPP_TABLE action. + * + * This is normally null so the client has to set it manually after + * calling action_xlate_ctx_init(). */ + const struct dpif_flow_stats *resubmit_stats; /* xlate_actions() initializes and uses these members. The client might want * to look at them after it returns. */ @@ -3767,68 +3777,52 @@ facet_reset_counters(struct facet *facet) static void facet_push_stats(struct facet *facet) { - uint64_t new_packets, new_bytes; + struct dpif_flow_stats stats; assert(facet->packet_count >= facet->prev_packet_count); assert(facet->byte_count >= facet->prev_byte_count); assert(facet->used >= facet->prev_used); - new_packets = facet->packet_count - facet->prev_packet_count; - new_bytes = facet->byte_count - facet->prev_byte_count; + stats.n_packets = facet->packet_count - facet->prev_packet_count; + stats.n_bytes = facet->byte_count - facet->prev_byte_count; + stats.used = facet->used; + stats.tcp_flags = 0; - if (new_packets || new_bytes || facet->used > facet->prev_used) { + if (stats.n_packets || stats.n_bytes || facet->used > facet->prev_used) { facet->prev_packet_count = facet->packet_count; facet->prev_byte_count = facet->byte_count; facet->prev_used = facet->used; - flow_push_stats(facet->rule, &facet->flow, - new_packets, new_bytes, facet->used); + flow_push_stats(facet->rule, &facet->flow, &stats); update_mirror_stats(ofproto_dpif_cast(facet->rule->up.ofproto), - facet->mirrors, new_packets, new_bytes); + facet->mirrors, stats.n_packets, stats.n_bytes); } } -struct ofproto_push { - struct action_xlate_ctx ctx; - uint64_t packets; - uint64_t bytes; - long long int used; -}; - static void -push_resubmit(struct action_xlate_ctx *ctx, struct rule_dpif *rule) +rule_credit_stats(struct rule_dpif *rule, const struct dpif_flow_stats *stats) { - struct ofproto_push *push = CONTAINER_OF(ctx, struct ofproto_push, ctx); - - if (rule) { - rule->packet_count += push->packets; - rule->byte_count += push->bytes; - ofproto_rule_update_used(&rule->up, push->used); - } + rule->packet_count += stats->n_packets; + rule->byte_count += stats->n_bytes; + ofproto_rule_update_used(&rule->up, stats->used); } /* Pushes flow statistics to the rules which 'flow' resubmits into given * 'rule''s actions and mirrors. */ static void flow_push_stats(struct rule_dpif *rule, - const struct flow *flow, uint64_t packets, uint64_t bytes, - long long int used) + const struct flow *flow, const struct dpif_flow_stats *stats) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto); - struct ofproto_push push; - - push.packets = packets; - push.bytes = bytes; - push.used = used; + struct action_xlate_ctx ctx; - ofproto_rule_update_used(&rule->up, used); + ofproto_rule_update_used(&rule->up, stats->used); - action_xlate_ctx_init(&push.ctx, ofproto, flow, flow->vlan_tci, rule, + action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, rule, 0, NULL); - push.ctx.resubmit_hook = push_resubmit; - xlate_actions_for_side_effects(&push.ctx, - rule->up.actions, rule->up.n_actions); + ctx.resubmit_stats = stats; + xlate_actions_for_side_effects(&ctx, rule->up.actions, rule->up.n_actions); } /* Subfacets. */ @@ -4262,22 +4256,24 @@ rule_execute(struct rule *rule_, const struct flow *flow, struct rule_dpif *rule = rule_dpif_cast(rule_); struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto); - size_t size = packet->size; + struct dpif_flow_stats stats; struct action_xlate_ctx ctx; uint64_t odp_actions_stub[1024 / 8]; struct ofpbuf odp_actions; + dpif_flow_stats_extract(flow, packet, &stats); + rule_credit_stats(rule, &stats); + ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof odp_actions_stub); action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, - rule, packet_get_tcp_flags(packet, flow), packet); + rule, stats.tcp_flags, packet); + ctx.resubmit_stats = &stats; xlate_actions(&ctx, rule->up.actions, rule->up.n_actions, &odp_actions); - if (execute_odp_actions(ofproto, flow, odp_actions.data, - odp_actions.size, packet)) { - rule->packet_count++; - rule->byte_count += size; - flow_push_stats(rule, flow, 1, size, time_msec()); - } + + execute_odp_actions(ofproto, flow, odp_actions.data, + odp_actions.size, packet); + ofpbuf_uninit(&odp_actions); return 0; @@ -4540,6 +4536,10 @@ xlate_table_action(struct action_xlate_ctx *ctx, if (rule) { struct rule_dpif *old_rule = ctx->rule; + if (ctx->resubmit_stats) { + rule_credit_stats(rule, ctx->resubmit_stats); + } + ctx->recurse++; ctx->rule = rule; do_xlate_actions(rule->up.actions, rule->up.n_actions, ctx); @@ -5140,6 +5140,7 @@ action_xlate_ctx_init(struct action_xlate_ctx *ctx, ctx->may_learn = packet != NULL; ctx->tcp_flags = tcp_flags; ctx->resubmit_hook = NULL; + ctx->resubmit_stats = NULL; } /* Translates the 'n_in' "union ofp_action"s in 'in' into datapath actions in @@ -5951,28 +5952,26 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet, ofproto->max_ports); if (!error) { struct odputil_keybuf keybuf; + struct dpif_flow_stats stats; + struct ofpbuf key; + struct action_xlate_ctx ctx; uint64_t odp_actions_stub[1024 / 8]; struct ofpbuf odp_actions; - struct ofproto_push push; ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); odp_flow_key_from_flow(&key, flow); - action_xlate_ctx_init(&push.ctx, ofproto, flow, flow->vlan_tci, NULL, - packet_get_tcp_flags(packet, flow), packet); + dpif_flow_stats_extract(flow, packet, &stats); - /* Ensure that resubmits in 'ofp_actions' get accounted to their - * matching rules. */ - push.packets = 1; - push.bytes = packet->size; - push.used = time_msec(); - push.ctx.resubmit_hook = push_resubmit; + action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, NULL, + packet_get_tcp_flags(packet, flow), packet); + ctx.resubmit_stats = &stats; ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof odp_actions_stub); - xlate_actions(&push.ctx, ofp_actions, n_ofp_actions, &odp_actions); + xlate_actions(&ctx, ofp_actions, n_ofp_actions, &odp_actions); dpif_execute(ofproto->dpif, key.data, key.size, odp_actions.data, odp_actions.size, packet); ofpbuf_uninit(&odp_actions);