--- ofproto/connmgr.c | 1 + ofproto/ofproto-dpif-xlate.c | 9 +++-- ofproto/ofproto-dpif.c | 7 ++-- ofproto/ofproto-provider.h | 14 +++----- ofproto/ofproto.c | 81 ++++++++++++++++-------------------------- 5 files changed, 48 insertions(+), 64 deletions(-)
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index 02da1f6..9f69c3f 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 9313a7f..9b24bdb 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" @@ -1707,9 +1708,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--; } @@ -2780,6 +2782,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; @@ -2883,7 +2886,9 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout) memset(&wc->masks.regs, 0, sizeof wc->masks.regs); 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 e53bb25..dcf6184 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" @@ -3971,6 +3972,7 @@ facet_is_controller_flow(struct facet *facet) bool is_controller; rule_dpif_lookup(ofproto, &facet->flow, NULL, &rule); + rcu_read_lock(); actions = rule_dpif_get_actions(rule); rule_dpif_unref(rule); @@ -3979,7 +3981,7 @@ facet_is_controller_flow(struct facet *facet) is_controller = ofpacts_len > 0 && ofpacts->type == OFPACT_CONTROLLER && ofpact_next(ofpacts) >= ofpact_end(ofpacts, ofpacts_len); - rule_actions_unref(actions); + rcu_read_unlock(); return is_controller; } @@ -5052,6 +5054,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); @@ -5059,7 +5062,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 de566e3..c00edfc 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -33,6 +33,7 @@ * structures for details. */ +#include <urcu-call-rcu.h> #include "cfm.h" #include "classifier.h" #include "guarded-list.h" @@ -371,7 +372,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); @@ -393,10 +394,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 *); /* A set of actions within a "struct rule". * @@ -409,10 +407,9 @@ struct rule_actions *rule_get_actions__(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 { - atomic_uint 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. */ @@ -420,8 +417,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 fe004b5..3185586 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" @@ -1850,11 +1851,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; } @@ -2474,23 +2477,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 @@ -2498,7 +2486,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); rule->ofproto->ofproto_class->rule_dealloc(rule); } @@ -2515,7 +2503,6 @@ rule_actions_create(const struct ofproto *ofproto, struct rule_actions *actions; actions = xmalloc(sizeof *actions); - atomic_init(&actions->ref_count, 1); actions->ofpacts = xmemdup(ofpacts, ofpacts_len); actions->ofpacts_len = ofpacts_len; actions->provider_meter_id @@ -2525,33 +2512,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) { - unsigned int orig; + struct rule_actions *actions + = CONTAINER_OF(rcu_head, struct rule_actions, rcu_head); - atomic_add(&actions->ref_count, 1, &orig); - ovs_assert(orig != 0); - } + 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) { - unsigned int orig; - - atomic_sub(&actions->ref_count, 1, &orig); - if (orig == 1) { - free(actions->ofpacts); - free(actions); - } else { - ovs_assert(orig != 0); - } + call_rcu(&actions->rcu_head, rule_actions_destroy_cb); } } @@ -3502,7 +3479,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); @@ -3520,7 +3498,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); @@ -3542,7 +3520,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); @@ -3560,7 +3539,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 @@ -3972,7 +3951,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); @@ -4076,9 +4057,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); @@ -6032,11 +6011,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; } @@ -6122,7 +6101,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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev