[ovs-dev] [PATCH] ofproto-dpif: Use a regular ref instead of try_ref for rule translation.
Ben Pfaff
blp at nicira.com
Sun Aug 2 18:51:32 UTC 2015
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 <jrajahalme at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
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
More information about the dev
mailing list