From: Jarno Rajahalme Date: Thu, 24 Apr 2014 15:21:49 +0000 (-0700) Subject: ofproto: Make taking rule reference conditional on lookup. X-Git-Tag: sliver-openvswitch-2.2.90-1~3^2~82 X-Git-Url: http://git.onelab.eu/?p=sliver-openvswitch.git;a=commitdiff_plain;h=1a7c0cd710e19db3ff85606dbfd5fdad964a1eea ofproto: Make taking rule reference conditional on lookup. Prior to this paths the rule lookup functions have always taken a reference on the found rule before returning. Make this conditional, so that unnecessary refs/unrefs can be avoided in a later patch. Signed-off-by: Jarno Rajahalme Acked-by: Ethan Jackson --- diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 248382fc2..c62424a37 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -2057,7 +2057,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id, !skip_wildcards ? &ctx->xout->wc : NULL, honor_table_miss, - &ctx->table_id, &rule); + &ctx->table_id, &rule, true); ctx->xin->flow.in_port.ofp_port = old_in_port; if (ctx->xin->resubmit_hook) { @@ -2090,7 +2090,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id, } choose_miss_rule(config, ctx->xbridge->miss_rule, - ctx->xbridge->no_packet_in_rule, &rule); + ctx->xbridge->no_packet_in_rule, &rule, true); match: if (rule) { @@ -2654,7 +2654,7 @@ xlate_learn_action(struct xlate_ctx *ctx, entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN); entry->u.learn.ofproto = ctx->xin->ofproto; rule_dpif_lookup(ctx->xbridge->ofproto, &ctx->xin->flow, NULL, - &entry->u.learn.rule); + &entry->u.learn.rule, true); } } @@ -3263,7 +3263,7 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout) if (!xin->ofpacts && !ctx.rule) { ctx.table_id = rule_dpif_lookup(ctx.xbridge->ofproto, flow, !xin->skip_wildcards ? wc : NULL, - &rule); + &rule, true); if (ctx.xin->resubmit_stats) { rule_dpif_credit_stats(rule, ctx.xin->resubmit_stats); } diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 983cad513..5669cd184 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3189,10 +3189,16 @@ rule_dpif_get_actions(const struct rule_dpif *rule) * * The return value will be zero unless there was a miss and * OFPTC11_TABLE_MISS_CONTINUE is in effect for the sequence of tables - * where misses occur. */ + * where misses occur. + * + * The rule is returned in '*rule', which is valid at least until the next + * RCU quiescent period. If the '*rule' needs to stay around longer, + * a non-zero 'take_ref' must be passed in to cause a reference to be taken + * on it before this returns. */ uint8_t rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow, - struct flow_wildcards *wc, struct rule_dpif **rule) + struct flow_wildcards *wc, struct rule_dpif **rule, + bool take_ref) { enum rule_dpif_lookup_verdict verdict; enum ofputil_port_config config = 0; @@ -3219,7 +3225,7 @@ rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow, } verdict = rule_dpif_lookup_from_table(ofproto, flow, wc, true, - &table_id, rule); + &table_id, rule, take_ref); switch (verdict) { case RULE_DPIF_LOOKUP_VERDICT_MATCH: @@ -3248,13 +3254,17 @@ rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow, } choose_miss_rule(config, ofproto->miss_rule, - ofproto->no_packet_in_rule, rule); + ofproto->no_packet_in_rule, rule, take_ref); return table_id; } +/* The returned rule is valid at least until the next RCU quiescent period. + * If the '*rule' needs to stay around longer, a non-zero 'take_ref' must be + * passed in to cause a reference to be taken on it before this returns. */ static struct rule_dpif * rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, uint8_t table_id, - const struct flow *flow, struct flow_wildcards *wc) + const struct flow *flow, struct flow_wildcards *wc, + bool take_ref) { struct classifier *cls = &ofproto->up.tables[table_id].cls; const struct cls_rule *cls_rule; @@ -3288,7 +3298,9 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, uint8_t table_id, } rule = rule_dpif_cast(rule_from_cls_rule(cls_rule)); - rule_dpif_ref(rule); + if (take_ref) { + rule_dpif_ref(rule); + } fat_rwlock_unlock(&cls->rwlock); return rule; @@ -3323,13 +3335,19 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, uint8_t table_id, * * - RULE_DPIF_LOOKUP_VERDICT_DEFAULT if no rule was found, * 'honor_table_miss' is true and a table miss configuration has - * not been specified in this case. */ + * not been specified in this case. + * + * The rule is returned in '*rule', which is valid at least until the next + * RCU quiescent period. If the '*rule' needs to stay around longer, + * a non-zero 'take_ref' must be passed in to cause a reference to be taken + * on it before this returns. */ enum rule_dpif_lookup_verdict rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto, const struct flow *flow, struct flow_wildcards *wc, bool honor_table_miss, - uint8_t *table_id, struct rule_dpif **rule) + uint8_t *table_id, struct rule_dpif **rule, + bool take_ref) { uint8_t next_id; @@ -3338,7 +3356,8 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto, next_id++, next_id += (next_id == TBL_INTERNAL)) { *table_id = next_id; - *rule = rule_dpif_lookup_in_table(ofproto, *table_id, flow, wc); + *rule = rule_dpif_lookup_in_table(ofproto, *table_id, flow, wc, + take_ref); if (*rule) { return RULE_DPIF_LOOKUP_VERDICT_MATCH; } else if (!honor_table_miss) { @@ -3365,13 +3384,21 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto, /* Given a port configuration (specified as zero if there's no port), chooses * which of 'miss_rule' and 'no_packet_in_rule' should be used in case of a - * flow table miss. */ + * flow table miss. + * + * The rule is returned in '*rule', which is valid at least until the next + * RCU quiescent period. If the '*rule' needs to stay around longer, + * a reference must be taken on it (rule_dpif_ref()). + */ void choose_miss_rule(enum ofputil_port_config config, struct rule_dpif *miss_rule, - struct rule_dpif *no_packet_in_rule, struct rule_dpif **rule) + struct rule_dpif *no_packet_in_rule, struct rule_dpif **rule, + bool take_ref) { *rule = config & OFPUTIL_PC_NO_PACKET_IN ? no_packet_in_rule : miss_rule; - rule_dpif_ref(*rule); + if (take_ref) { + rule_dpif_ref(*rule); + } } void @@ -4197,7 +4224,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow, if (ofpacts) { rule = NULL; } else { - rule_dpif_lookup(ofproto, flow, &trace.wc, &rule); + rule_dpif_lookup(ofproto, flow, &trace.wc, &rule, false); trace_format_rule(ds, 0, rule); if (rule == ofproto->miss_rule) { @@ -4253,8 +4280,6 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow, xlate_out_uninit(&trace.xout); } - - rule_dpif_unref(rule); } /* Store the current ofprotos in 'ofproto_shash'. Returns a sorted list @@ -4819,9 +4844,8 @@ ofproto_dpif_add_internal_flow(struct ofproto_dpif *ofproto, } rule = rule_dpif_lookup_in_table(ofproto, TBL_INTERNAL, &match->flow, - &match->wc); + &match->wc, false); if (rule) { - rule_dpif_unref(rule); *rulep = &rule->up; } else { OVS_NOT_REACHED(); diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index eb4787c03..8af66458e 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -87,14 +87,16 @@ size_t ofproto_dpif_get_max_mpls_depth(const struct ofproto_dpif *); bool ofproto_dpif_get_enable_recirc(const struct ofproto_dpif *); uint8_t rule_dpif_lookup(struct ofproto_dpif *, struct flow *, - struct flow_wildcards *, struct rule_dpif **rule); + struct flow_wildcards *, struct rule_dpif **rule, + bool take_ref); enum rule_dpif_lookup_verdict rule_dpif_lookup_from_table(struct ofproto_dpif *, const struct flow *, struct flow_wildcards *, bool force_controller_on_miss, uint8_t *table_id, - struct rule_dpif **rule); + struct rule_dpif **rule, + bool take_ref); void rule_dpif_ref(struct rule_dpif *); void rule_dpif_unref(struct rule_dpif *); @@ -117,7 +119,7 @@ void rule_dpif_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout, void choose_miss_rule(enum ofputil_port_config, struct rule_dpif *miss_rule, struct rule_dpif *no_packet_in_rule, - struct rule_dpif **rule); + struct rule_dpif **rule, bool take_ref); bool group_dpif_lookup(struct ofproto_dpif *ofproto, uint32_t group_id, struct group_dpif **group);