[ovs-dev] [urcu 4/8] ofproto: Use RCU to protect rule_actions.
Ben Pfaff
blp at nicira.com
Thu Mar 6 07:12:59 UTC 2014
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
ofproto/connmgr.c | 5 +-
ofproto/ofproto-dpif-xlate.c | 2 -
ofproto/ofproto-dpif.c | 2 -
ofproto/ofproto-provider.h | 19 ++++---
ofproto/ofproto.c | 120 ++++++++++++++++++-------------------------
5 files changed, 63 insertions(+), 85 deletions(-)
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 033ab7d..0058309 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -2043,8 +2043,9 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
ovs_mutex_unlock(&rule->mutex);
if (flags & NXFMF_ACTIONS) {
- fu.ofpacts = rule->actions->ofpacts;
- fu.ofpacts_len = rule->actions->ofpacts_len;
+ struct rule_actions *actions = rule_get_actions(rule);
+ fu.ofpacts = actions->ofpacts;
+ fu.ofpacts_len = actions->ofpacts_len;
} else {
fu.ofpacts = NULL;
fu.ofpacts_len = 0;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index eb4931e..5201b93 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1836,7 +1836,6 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule)
ctx->rule = rule;
actions = rule_dpif_get_actions(rule);
do_xlate_actions(actions->ofpacts, actions->ofpacts_len, ctx);
- rule_actions_unref(actions);
ctx->rule = old_rule;
ctx->recurse--;
}
@@ -3171,7 +3170,6 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
}
out:
- rule_actions_unref(actions);
rule_dpif_unref(rule);
}
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8c43ee9..1c779ee 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3587,8 +3587,6 @@ trace_format_rule(struct ds *result, int level, const struct rule_dpif *rule)
ds_put_cstr(result, "OpenFlow actions=");
ofpacts_format(actions->ofpacts, actions->ofpacts_len, result);
ds_put_char(result, '\n');
-
- rule_actions_unref(actions);
}
static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2c72fbc..956e593 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -43,6 +43,7 @@
#include "ofp-util.h"
#include "ofproto/ofproto.h"
#include "ovs-atomic.h"
+#include "ovs-rcu.h"
#include "ovs-thread.h"
#include "shash.h"
#include "simap.h"
@@ -374,7 +375,7 @@ struct rule {
/* OpenFlow actions. See struct rule_actions for more thread-safety
* notes. */
- struct rule_actions *actions OVS_GUARDED;
+ OVSRCU_TYPE(struct rule_actions *) actions;
/* In owning meter's 'rules' list. An empty list if there is no meter. */
struct list meter_list_node OVS_GUARDED_BY(ofproto_mutex);
@@ -403,10 +404,11 @@ struct rule {
void ofproto_rule_ref(struct rule *);
void ofproto_rule_unref(struct rule *);
-struct rule_actions *rule_get_actions(const struct rule *rule)
- OVS_EXCLUDED(rule->mutex);
-struct rule_actions *rule_get_actions__(const struct rule *rule)
- OVS_REQUIRES(rule->mutex);
+static inline struct rule_actions *
+rule_get_actions(const struct rule *rule)
+{
+ return ovsrcu_get(struct rule_actions *, &rule->actions);
+}
/* Returns true if 'rule' is an OpenFlow 1.3 "table-miss" rule, false
* otherwise.
@@ -431,8 +433,6 @@ rule_is_table_miss(const struct rule *rule)
* 'rule' is the rule for which 'rule->actions == actions') or that owns a
* reference to 'actions->ref_count' (or both). */
struct rule_actions {
- struct ovs_refcount ref_count;
-
/* These members are immutable: they do not change during the struct's
* lifetime. */
struct ofpact *ofpacts; /* Sequence of "struct ofpacts". */
@@ -442,8 +442,7 @@ struct rule_actions {
struct rule_actions *rule_actions_create(const struct ofproto *,
const struct ofpact *, size_t);
-void rule_actions_ref(struct rule_actions *);
-void rule_actions_unref(struct rule_actions *);
+void rule_actions_destroy(struct rule_actions *);
/* A set of rules to which an OpenFlow operation applies. */
struct rule_collection {
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 19e7091..ce4e578 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -43,6 +43,7 @@
#include "ofproto-provider.h"
#include "openflow/nicira-ext.h"
#include "openflow/openflow.h"
+#include "ovs-rcu.h"
#include "packets.h"
#include "pinsched.h"
#include "pktbuf.h"
@@ -1897,11 +1898,9 @@ ofproto_add_flow(struct ofproto *ofproto, const struct match *match,
rule = rule_from_cls_rule(classifier_find_match_exactly(
&ofproto->tables[0].cls, match, priority));
if (rule) {
- ovs_mutex_lock(&rule->mutex);
- must_add = !ofpacts_equal(rule->actions->ofpacts,
- rule->actions->ofpacts_len,
+ struct rule_actions *actions = rule_get_actions(rule);
+ must_add = !ofpacts_equal(actions->ofpacts, actions->ofpacts_len,
ofpacts, ofpacts_len);
- ovs_mutex_unlock(&rule->mutex);
} else {
must_add = true;
}
@@ -1945,14 +1944,16 @@ ofproto_flow_mod(struct ofproto *ofproto, struct ofputil_flow_mod *fm)
/* Reading many of the rule fields and writing on 'modified'
* requires the rule->mutex. Also, rule->actions may change
* if rule->mutex is not held. */
+ const struct rule_actions *actions;
+
ovs_mutex_lock(&rule->mutex);
+ actions = rule_get_actions(rule);
if (rule->idle_timeout == fm->idle_timeout
&& rule->hard_timeout == fm->hard_timeout
&& rule->flags == (fm->flags & OFPUTIL_FF_STATE)
&& (!fm->modify_cookie || (fm->new_cookie == rule->flow_cookie))
&& ofpacts_equal(fm->ofpacts, fm->ofpacts_len,
- rule->actions->ofpacts,
- rule->actions->ofpacts_len)) {
+ actions->ofpacts, actions->ofpacts_len)) {
/* Rule already exists and need not change, only update the
modified timestamp. */
rule->modified = time_msec();
@@ -2574,33 +2575,12 @@ ofproto_rule_unref(struct rule *rule)
}
}
-struct rule_actions *
-rule_get_actions(const struct rule *rule)
- OVS_EXCLUDED(rule->mutex)
-{
- struct rule_actions *actions;
-
- ovs_mutex_lock(&rule->mutex);
- actions = rule_get_actions__(rule);
- ovs_mutex_unlock(&rule->mutex);
-
- return actions;
-}
-
-struct rule_actions *
-rule_get_actions__(const struct rule *rule)
- OVS_REQUIRES(rule->mutex)
-{
- rule_actions_ref(rule->actions);
- return rule->actions;
-}
-
static void
ofproto_rule_destroy__(struct rule *rule)
OVS_NO_THREAD_SAFETY_ANALYSIS
{
cls_rule_destroy(CONST_CAST(struct cls_rule *, &rule->cr));
- rule_actions_unref(rule->actions);
+ rule_actions_destroy(rule_get_actions(rule));
ovs_mutex_destroy(&rule->mutex);
ovs_refcount_destroy(&rule->ref_count);
rule->ofproto->ofproto_class->rule_dealloc(rule);
@@ -2618,7 +2598,6 @@ rule_actions_create(const struct ofproto *ofproto,
struct rule_actions *actions;
actions = xmalloc(sizeof *actions);
- ovs_refcount_init(&actions->ref_count);
actions->ofpacts = xmemdup(ofpacts, ofpacts_len);
actions->ofpacts_len = ofpacts_len;
actions->provider_meter_id
@@ -2628,24 +2607,20 @@ rule_actions_create(const struct ofproto *ofproto,
return actions;
}
-/* Increments 'actions''s ref_count. */
-void
-rule_actions_ref(struct rule_actions *actions)
+static void
+rule_actions_destroy_cb(struct rule_actions *actions)
{
- if (actions) {
- ovs_refcount_ref(&actions->ref_count);
- }
+ free(actions->ofpacts);
+ free(actions);
}
/* Decrements 'actions''s ref_count and frees 'actions' if the ref_count
* reaches 0. */
void
-rule_actions_unref(struct rule_actions *actions)
+rule_actions_destroy(struct rule_actions *actions)
{
- if (actions && ovs_refcount_unref(&actions->ref_count) == 1) {
- ovs_refcount_destroy(&actions->ref_count);
- free(actions->ofpacts);
- free(actions);
+ if (actions) {
+ ovsrcu_postpone(rule_actions_destroy_cb, actions);
}
}
@@ -2655,9 +2630,13 @@ static bool
ofproto_rule_has_out_port(const struct rule *rule, ofp_port_t port)
OVS_REQUIRES(ofproto_mutex)
{
- return (port == OFPP_ANY
- || ofpacts_output_to_port(rule->actions->ofpacts,
- rule->actions->ofpacts_len, port));
+ if (port == OFPP_ANY) {
+ return true;
+ } else {
+ const struct rule_actions *actions = rule_get_actions(rule);
+ return ofpacts_output_to_port(actions->ofpacts,
+ actions->ofpacts_len, port);
+ }
}
/* Returns true if 'rule' has group and equals group_id. */
@@ -2665,9 +2644,13 @@ static bool
ofproto_rule_has_out_group(const struct rule *rule, uint32_t group_id)
OVS_REQUIRES(ofproto_mutex)
{
- return (group_id == OFPG11_ANY
- || ofpacts_output_to_group(rule->actions->ofpacts,
- rule->actions->ofpacts_len, group_id));
+ if (group_id == OFPG_ANY) {
+ return true;
+ } else {
+ const struct rule_actions *actions = rule_get_actions(rule);
+ return ofpacts_output_to_group(actions->ofpacts,
+ actions->ofpacts_len, group_id);
+ }
}
/* Returns true if a rule related to 'op' has an OpenFlow OFPAT_OUTPUT or
@@ -3595,7 +3578,7 @@ handle_flow_stats_request(struct ofconn *ofconn,
fs.hard_timeout = rule->hard_timeout;
created = rule->created;
modified = rule->modified;
- actions = rule_get_actions__(rule);
+ actions = rule_get_actions(rule);
flags = rule->flags;
ovs_mutex_unlock(&rule->mutex);
@@ -3613,8 +3596,6 @@ handle_flow_stats_request(struct ofconn *ofconn,
fs.flags = flags;
ofputil_append_flow_stats_reply(&fs, &replies);
-
- rule_actions_unref(actions);
}
rule_collection_unref(&rules);
@@ -3636,7 +3617,7 @@ flow_stats_ds(struct rule *rule, struct ds *results)
&byte_count, &used);
ovs_mutex_lock(&rule->mutex);
- actions = rule_get_actions__(rule);
+ actions = rule_get_actions(rule);
created = rule->created;
ovs_mutex_unlock(&rule->mutex);
@@ -3653,8 +3634,6 @@ flow_stats_ds(struct rule *rule, struct ds *results)
ofpacts_format(actions->ofpacts, actions->ofpacts_len, results);
ds_put_cstr(results, "\n");
-
- rule_actions_unref(actions);
}
/* Adds a pretty-printed description of all flows to 'results', including
@@ -4059,7 +4038,8 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
*CONST_CAST(uint8_t *, &rule->table_id) = table - ofproto->tables;
rule->flags = fm->flags & OFPUTIL_FF_STATE;
- rule->actions = rule_actions_create(ofproto, fm->ofpacts, fm->ofpacts_len);
+ ovsrcu_set(&rule->actions,
+ rule_actions_create(ofproto, fm->ofpacts, fm->ofpacts_len));
list_init(&rule->meter_list_node);
rule->eviction_group = NULL;
list_init(&rule->expirable);
@@ -4110,6 +4090,7 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
error = OFPERR_OFPBRC_EPERM;
for (i = 0; i < rules->n; i++) {
struct rule *rule = rules->rules[i];
+ const struct rule_actions *actions;
struct ofoperation *op;
bool actions_changed;
bool reset_counters;
@@ -4123,9 +4104,10 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
continue;
}
+ actions = rule_get_actions(rule);
actions_changed = !ofpacts_equal(fm->ofpacts, fm->ofpacts_len,
- rule->actions->ofpacts,
- rule->actions->ofpacts_len);
+ actions->ofpacts,
+ actions->ofpacts_len);
op = ofoperation_create(group, rule, type, 0);
@@ -4152,13 +4134,11 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
if (actions_changed || reset_counters) {
struct rule_actions *new_actions;
- op->actions = rule->actions;
+ op->actions = rule_get_actions(rule);
new_actions = rule_actions_create(ofproto,
fm->ofpacts, fm->ofpacts_len);
- ovs_mutex_lock(&rule->mutex);
- rule->actions = new_actions;
- ovs_mutex_unlock(&rule->mutex);
+ ovsrcu_set(&rule->actions, new_actions);
rule->ofproto->ofproto_class->rule_modify_actions(rule,
reset_counters);
@@ -4739,7 +4719,7 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
if (!(flags & NXFMF_ACTIONS)) {
actions = NULL;
} else if (!op) {
- actions = rule->actions;
+ actions = rule_get_actions(rule);
} else {
/* An operation is in progress. Use the previous version of the flow's
* actions, so that when the operation commits we report the change. */
@@ -4749,11 +4729,11 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
case OFOPERATION_MODIFY:
case OFOPERATION_REPLACE:
- actions = op->actions ? op->actions : rule->actions;
+ actions = op->actions ? op->actions : rule_get_actions(rule);
break;
case OFOPERATION_DELETE:
- actions = rule->actions;
+ actions = rule_get_actions(rule);
break;
default:
@@ -6243,12 +6223,12 @@ ofopgroup_complete(struct ofopgroup *group)
struct rule_actions *old_actions;
ovs_mutex_lock(&rule->mutex);
- old_actions = rule->actions;
- rule->actions = op->actions;
+ old_actions = rule_get_actions(rule);
+ ovsrcu_set(&rule->actions, op->actions);
ovs_mutex_unlock(&rule->mutex);
op->actions = NULL;
- rule_actions_unref(old_actions);
+ rule_actions_destroy(old_actions);
}
rule->flags = op->flags;
}
@@ -6334,7 +6314,7 @@ ofoperation_destroy(struct ofoperation *op)
hmap_remove(&group->ofproto->deletions, &op->hmap_node);
}
list_remove(&op->group_node);
- rule_actions_unref(op->actions);
+ rule_actions_destroy(op->actions);
free(op);
}
@@ -6817,6 +6797,7 @@ oftable_insert_rule(struct rule *rule)
{
struct ofproto *ofproto = rule->ofproto;
struct oftable *table = &ofproto->tables[rule->table_id];
+ struct rule_actions *actions;
bool may_expire;
ovs_mutex_lock(&rule->mutex);
@@ -6829,9 +6810,10 @@ oftable_insert_rule(struct rule *rule)
cookies_insert(ofproto, rule);
- if (rule->actions->provider_meter_id != UINT32_MAX) {
- uint32_t meter_id = ofpacts_get_meter(rule->actions->ofpacts,
- rule->actions->ofpacts_len);
+ actions = rule_get_actions(rule);
+ if (actions->provider_meter_id != UINT32_MAX) {
+ uint32_t meter_id = ofpacts_get_meter(actions->ofpacts,
+ actions->ofpacts_len);
struct meter *meter = ofproto->meters[meter_id];
list_insert(&meter->rules, &rule->meter_list_node);
}
--
1.8.5.3
More information about the dev
mailing list