Acked-by: Jarno Rajahalme <[email protected]> > On Aug 2, 2015, at 11:51 AM, Ben Pfaff <[email protected]> wrote: > > Until now, flow translation has had to use try_ref to take a reference on > a rule, because a competing thread might have released the last reference > and done an RCU-postponed deletion. Since classifier versioning was > introduced, however, the release of the last reference is itself > RCU-postponed, which means that it is always safe to take the reference > directly. > > Changing try_ref to ref means that taking a reference can't fail, which > allows the caller to take a reference in cases where the need to take a > reference was previously passed along a call chain, which simplifies some > code. > > CC: Jarno Rajahalme <[email protected]> > Signed-off-by: Ben Pfaff <[email protected]> > --- > ofproto/ofproto-dpif-xlate.c | 5 +++-- > ofproto/ofproto-dpif.c | 38 ++++++++++---------------------------- > ofproto/ofproto-dpif.h | 10 ---------- > 3 files changed, 13 insertions(+), 40 deletions(-) > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index e2596d9..a34b5f9 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -3133,7 +3133,6 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t > in_port, uint8_t table_id, > rule = rule_dpif_lookup_from_table(ctx->xbridge->ofproto, > ctx->tables_version, > &ctx->xin->flow, ctx->xin->wc, > - ctx->xin->xcache != NULL, > ctx->xin->resubmit_stats, > &ctx->table_id, in_port, > may_packet_in, honor_table_miss); > @@ -3152,6 +3151,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t > in_port, uint8_t table_id, > > entry = xlate_cache_add_entry(ctx->xin->xcache, XC_RULE); > entry->u.rule = rule; > + rule_dpif_ref(rule); > } > xlate_recursively(ctx, rule); > } > @@ -4912,7 +4912,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out > *xout) > if (!xin->ofpacts && !ctx.rule) { > ctx.rule = rule_dpif_lookup_from_table( > ctx.xbridge->ofproto, ctx.tables_version, flow, xin->wc, > - ctx.xin->xcache != NULL, ctx.xin->resubmit_stats, &ctx.table_id, > + ctx.xin->resubmit_stats, &ctx.table_id, > flow->in_port.ofp_port, true, true); > if (ctx.xin->resubmit_stats) { > rule_dpif_credit_stats(ctx.rule, ctx.xin->resubmit_stats); > @@ -4922,6 +4922,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out > *xout) > > entry = xlate_cache_add_entry(ctx.xin->xcache, XC_RULE); > entry->u.rule = ctx.rule; > + rule_dpif_ref(ctx.rule); > } > > if (OVS_UNLIKELY(ctx.xin->resubmit_hook)) { > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 70faa13..30db4fe 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -3765,29 +3765,19 @@ ofproto_dpif_get_tables_version(struct ofproto_dpif > *ofproto OVS_UNUSED) > } > > /* The returned rule (if any) 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. > + * period. If the rule needs to stay around longer, the caller should take > + * a reference. > * > * 'flow' is non-const to allow for temporary modifications during the lookup. > * Any changes are restored before returning. */ > static struct rule_dpif * > rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, cls_version_t version, > uint8_t table_id, struct flow *flow, > - struct flow_wildcards *wc, bool take_ref) > + struct flow_wildcards *wc) > { > struct classifier *cls = &ofproto->up.tables[table_id].cls; > - const struct cls_rule *cls_rule; > - struct rule_dpif *rule; > - > - do { > - cls_rule = classifier_lookup(cls, version, flow, wc); > - > - rule = rule_dpif_cast(rule_from_cls_rule(cls_rule)); > - > - /* Try again if the rule was released before we get the reference. */ > - } while (rule && take_ref && !rule_dpif_try_ref(rule)); > - > - return rule; > + return rule_dpif_cast(rule_from_cls_rule(classifier_lookup(cls, version, > + flow, wc))); > } > > /* Look up 'flow' in 'ofproto''s classifier version 'version', starting from > @@ -3807,9 +3797,8 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, > cls_version_t version, > * '*table_id'. > * > * 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. > + * RCU quiescent period. If the '*rule' needs to stay around longer, the > + * caller must take a reference. > * > * 'in_port' allows the lookup to take place as if the in port had the value > * 'in_port'. This is needed for resubmit action support. > @@ -3819,7 +3808,7 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, > cls_version_t version, > struct rule_dpif * > rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto, > cls_version_t version, struct flow *flow, > - struct flow_wildcards *wc, bool take_ref, > + struct flow_wildcards *wc, > const struct dpif_flow_stats *stats, > uint8_t *table_id, ofp_port_t in_port, > bool may_packet_in, bool honor_table_miss) > @@ -3842,9 +3831,6 @@ rule_dpif_lookup_from_table(struct ofproto_dpif > *ofproto, > /* Must be OFPC_FRAG_DROP (we don't have OFPC_FRAG_REASM). > * Use the drop_frags_rule (which cannot disappear). */ > rule = ofproto->drop_frags_rule; > - if (take_ref) { > - rule_dpif_ref(rule); > - } > if (stats) { > struct oftable *tbl = &ofproto->up.tables[*table_id]; > unsigned long orig; > @@ -3871,8 +3857,7 @@ 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, version, next_id, flow, wc, > - take_ref); > + rule = rule_dpif_lookup_in_table(ofproto, version, next_id, flow, > wc); > if (stats) { > struct oftable *tbl = &ofproto->up.tables[next_id]; > unsigned long orig; > @@ -3911,9 +3896,6 @@ rule_dpif_lookup_from_table(struct ofproto_dpif > *ofproto, > rule = ofproto->miss_rule; > } > } > - if (take_ref) { > - rule_dpif_ref(rule); > - } > out: > /* Restore port numbers, as they may have been modified above. */ > flow->tp_src = old_tp_src; > @@ -5520,7 +5502,7 @@ ofproto_dpif_add_internal_flow(struct ofproto_dpif > *ofproto, > rule = rule_dpif_lookup_in_table(ofproto, > ofproto_dpif_get_tables_version(ofproto), > TBL_INTERNAL, &ofm.fm.match.flow, > - &ofm.fm.match.wc, false); > + &ofm.fm.match.wc); > if (rule) { > *rulep = &rule->up; > } else { > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > index 0209565..69ca54c 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -102,7 +102,6 @@ cls_version_t ofproto_dpif_get_tables_version(struct > ofproto_dpif *); > struct rule_dpif *rule_dpif_lookup_from_table(struct ofproto_dpif *, > cls_version_t, struct flow *, > struct flow_wildcards *, > - bool take_ref, > const struct dpif_flow_stats *, > uint8_t *table_id, > ofp_port_t in_port, > @@ -200,15 +199,6 @@ static inline void rule_dpif_ref(struct rule_dpif *rule) > } > } > > -static inline bool rule_dpif_try_ref(struct rule_dpif *rule) > -{ > - if (rule) { > - return ofproto_rule_try_ref(RULE_CAST(rule)); > - } > - return false; > -} > - > - > static inline void rule_dpif_unref(struct rule_dpif *rule) > { > if (rule) { > -- > 2.1.3 >
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
