[ovs-dev] [PATCH RFC 6/8] ofproto: Protect rule_actions with RCU.
Ben Pfaff
blp at nicira.com
Thu Jan 23 00:11:56 UTC 2014
---
ofproto/connmgr.c | 1 +
ofproto/ofproto-dpif-xlate.c | 9 +++--
ofproto/ofproto-dpif.c | 4 ++-
ofproto/ofproto-provider.h | 16 ++++-----
ofproto/ofproto.c | 76 ++++++++++++++++++------------------------
5 files changed, 49 insertions(+), 57 deletions(-)
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index b7c16d2..b9227e8 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -20,6 +20,7 @@
#include <errno.h>
#include <stdlib.h>
+#include <urcu-qsbr.h>
#include "coverage.h"
#include "fail-open.h"
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index c5e6600..998db6b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -17,6 +17,7 @@
#include "ofproto/ofproto-dpif-xlate.h"
#include <errno.h>
+#include <urcu-qsbr.h>
#include "bfd.h"
#include "bitmap.h"
@@ -1845,9 +1846,10 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule)
ctx->resubmits++;
ctx->recurse++;
ctx->rule = rule;
+ rcu_read_lock();
actions = rule_dpif_get_actions(rule);
do_xlate_actions(actions->ofpacts, actions->ofpacts_len, ctx);
- rule_actions_unref(actions);
+ rcu_read_unlock();
ctx->rule = old_rule;
ctx->recurse--;
}
@@ -3108,6 +3110,7 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
ofpacts = xin->ofpacts;
ofpacts_len = xin->ofpacts_len;
} else if (ctx.rule) {
+ rcu_read_lock();
actions = rule_dpif_get_actions(ctx.rule);
ofpacts = actions->ofpacts;
ofpacts_len = actions->ofpacts_len;
@@ -3242,7 +3245,9 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
flow_wildcards_clear_non_packet_fields(wc);
out:
- rule_actions_unref(actions);
+ if (actions) {
+ rcu_read_unlock();
+ }
rule_dpif_unref(rule);
}
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index cc1e9d5..50e2445 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -20,6 +20,7 @@
#include "ofproto/ofproto-provider.h"
#include <errno.h>
+#include <urcu-qsbr.h>
#include "bfd.h"
#include "bond.h"
@@ -3483,6 +3484,7 @@ trace_format_rule(struct ds *result, int level, const struct rule_dpif *rule)
cls_rule_format(&rule->up.cr, result);
ds_put_char(result, '\n');
+ rcu_read_lock();
actions = rule_dpif_get_actions(rule);
ds_put_char_multiple(result, '\t', level);
@@ -3490,7 +3492,7 @@ trace_format_rule(struct ds *result, int level, const struct rule_dpif *rule)
ofpacts_format(actions->ofpacts, actions->ofpacts_len, result);
ds_put_char(result, '\n');
- rule_actions_unref(actions);
+ rcu_read_unlock();
}
static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 19d1551..2e3b38c 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.
@@ -33,6 +33,7 @@
* structures for details.
*/
+#include <urcu-call-rcu.h>
#include "cfm.h"
#include "classifier.h"
#include "guarded-list.h"
@@ -376,7 +377,7 @@ struct rule {
/* OpenFlow actions. See struct rule_actions for more thread-safety
* notes. */
- struct rule_actions *actions OVS_GUARDED;
+ 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);
@@ -398,10 +399,7 @@ 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);
+struct rule_actions *rule_get_actions(const struct rule *);
/* Returns true if 'rule' is an OpenFlow 1.3 "table-miss" rule, false
* otherwise.
@@ -426,10 +424,9 @@ 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 rcu_head rcu_head;
struct ofpact *ofpacts; /* Sequence of "struct ofpacts". */
unsigned int ofpacts_len; /* Size of 'ofpacts', in bytes. */
uint32_t provider_meter_id; /* Datapath meter_id, or UINT32_MAX. */
@@ -437,8 +434,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 b2cdb10..73087d1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -22,6 +22,7 @@
#include <stdbool.h>
#include <stdlib.h>
#include <unistd.h>
+#include <urcu-qsbr.h>
#include "bitmap.h"
#include "byte-order.h"
#include "classifier.h"
@@ -1905,11 +1906,13 @@ 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;
+
+ rcu_read_lock();
+ actions = rule_get_actions(rule);
+ must_add = !ofpacts_equal(actions->ofpacts, actions->ofpacts_len,
ofpacts, ofpacts_len);
- ovs_mutex_unlock(&rule->mutex);
+ rcu_read_unlock();
} else {
must_add = true;
}
@@ -2542,23 +2545,8 @@ 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;
+ return rcu_dereference(rule->actions);
}
static void
@@ -2566,7 +2554,7 @@ 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->actions);
ovs_mutex_destroy(&rule->mutex);
ovs_refcount_destroy(&rule->ref_count);
rule->ofproto->ofproto_class->rule_dealloc(rule);
@@ -2584,7 +2572,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
@@ -2594,24 +2581,23 @@ 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 rcu_head *rcu_head)
{
- if (actions) {
- ovs_refcount_ref(&actions->ref_count);
- }
+ struct rule_actions *actions
+ = CONTAINER_OF(rcu_head, struct rule_actions, rcu_head);
+
+ 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) {
+ call_rcu(&actions->rcu_head, rule_actions_destroy_cb);
}
}
@@ -3563,7 +3549,8 @@ handle_flow_stats_request(struct ofconn *ofconn,
created = rule->created;
used = rule->used;
modified = rule->modified;
- actions = rule_get_actions__(rule);
+ rcu_read_lock();
+ actions = rule_get_actions(rule);
flags = rule->flags;
ovs_mutex_unlock(&rule->mutex);
@@ -3581,7 +3568,7 @@ handle_flow_stats_request(struct ofconn *ofconn,
fs.flags = flags;
ofputil_append_flow_stats_reply(&fs, &replies);
- rule_actions_unref(actions);
+ rcu_read_unlock();
}
rule_collection_unref(&rules);
@@ -3603,7 +3590,8 @@ flow_stats_ds(struct rule *rule, struct ds *results)
&packet_count, &byte_count);
ovs_mutex_lock(&rule->mutex);
- actions = rule_get_actions__(rule);
+ rcu_read_lock();
+ actions = rule_get_actions(rule);
created = rule->created;
ovs_mutex_unlock(&rule->mutex);
@@ -3621,7 +3609,7 @@ flow_stats_ds(struct rule *rule, struct ds *results)
ds_put_cstr(results, "\n");
- rule_actions_unref(actions);
+ rcu_read_unlock();
}
/* Adds a pretty-printed description of all flows to 'results', including
@@ -4025,7 +4013,9 @@ 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);
+ rcu_assign_pointer(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);
@@ -4122,9 +4112,7 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
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);
+ rcu_assign_pointer(rule->actions, new_actions);
rule->ofproto->ofproto_class->rule_modify_actions(rule,
reset_counters);
@@ -6207,11 +6195,11 @@ ofopgroup_complete(struct ofopgroup *group)
ovs_mutex_lock(&rule->mutex);
old_actions = rule->actions;
- rule->actions = op->actions;
+ rcu_assign_pointer(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;
}
@@ -6297,7 +6285,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);
}
--
1.7.10.4
More information about the dev
mailing list