Adding groups support for bundles is simpler if also groups are
modified under ofproto_mutex.
Eliminate the search for rules when deleting a group so that we will
not keep the mutex for too long.
Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
---
include/openvswitch/ofp-actions.h | 40 +++++++
ofproto/ofproto-provider.h | 10 +-
ofproto/ofproto.c | 234 +++++++++++++++++++++++++-------------
3 files changed, 204 insertions(+), 80 deletions(-)
diff --git a/include/openvswitch/ofp-actions.h
b/include/openvswitch/ofp-actions.h
index 0b8ccbb..6355eed 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -214,12 +214,45 @@ ofpact_end(const struct ofpact *ofpacts, size_t
ofpacts_len)
return (void *) ((uint8_t *) ofpacts + ofpacts_len);
}
+static inline const struct ofpact *
+ofpact_find_type(const struct ofpact *a, enum ofpact_type type,
+ const struct ofpact * const end)
+{
+ while (a < end) {
+ if (a->type == type) {
+ return a;
+ }
+ a = ofpact_next(a);
+ }
+ return NULL;
+}
+
+static inline const struct ofpact *
+ofpact_find_type_flattened(const struct ofpact *a, enum ofpact_type type,
+ const struct ofpact * const end)
+{
+ while (a < end) {
+ if (a->type == type) {
+ return a;
+ }
+ a = ofpact_next_flattened(a);
+ }
+ return NULL;
+}
+
/* Assigns POS to each ofpact, in turn, in the OFPACTS_LEN bytes of ofpacts
* starting at OFPACTS. */
#define OFPACT_FOR_EACH(POS, OFPACTS, OFPACTS_LEN) \
for ((POS) = (OFPACTS); (POS) < ofpact_end(OFPACTS, OFPACTS_LEN); \
(POS) = ofpact_next(POS))
+#define OFPACT_FOR_EACH_TYPE(POS, TYPE, OFPACTS, OFPACTS_LEN) \
+ for ((POS) = ofpact_find_type(OFPACTS, TYPE, \
+ ofpact_end(OFPACTS, OFPACTS_LEN)); \
+ (POS); \
+ (POS) = ofpact_find_type(ofpact_next(POS), TYPE, \
+ ofpact_end(OFPACTS, OFPACTS_LEN)))
+
/* Assigns POS to each ofpact, in turn, in the OFPACTS_LEN bytes of ofpacts
* starting at OFPACTS.
*
@@ -228,6 +261,13 @@ ofpact_end(const struct ofpact *ofpacts, size_t
ofpacts_len)
#define OFPACT_FOR_EACH_FLATTENED(POS, OFPACTS, OFPACTS_LEN) \
for ((POS) = (OFPACTS); (POS) < ofpact_end(OFPACTS, OFPACTS_LEN); \
(POS) = ofpact_next_flattened(POS))
+
+#define OFPACT_FOR_EACH_TYPE_FLATTENED(POS, TYPE, OFPACTS, OFPACTS_LEN) \
+ for ((POS) = ofpact_find_type_flattened(OFPACTS, TYPE, \
+ ofpact_end(OFPACTS, OFPACTS_LEN)); \
+ (POS); \
+ (POS) = ofpact_find_type_flattened(ofpact_next_flattened(POS), TYPE, \
+ ofpact_end(OFPACTS, OFPACTS_LEN)))
/* Action structure for each OFPACT_*. */
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index dd014ea..8a898a5 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -124,7 +124,6 @@ struct ofproto {
int min_mtu; /* Current MTU of non-internal ports. */
/* Groups. */
- struct ovs_rwlock groups_rwlock;
struct cmap groups; /* Contains "struct ofgroup"s. */
uint32_t n_groups[4] OVS_GUARDED; /* # of existing groups of each type. */
struct ofputil_group_features ogf;
@@ -437,6 +436,7 @@ struct rule_actions {
* action whose flags include NX_LEARN_F_DELETE_LEARNED. */
bool has_meter;
bool has_learn_with_delete;
+ bool has_groups;
/* Actions. */
uint32_t ofpacts_len; /* Size of 'ofpacts', in bytes. */
@@ -455,11 +455,14 @@ struct rule_collection {
size_t n; /* Number of rules collected. */
size_t capacity; /* Number of rules that will fit in 'rules'. */
- struct rule *stub[64]; /* Preallocated rules to avoid malloc(). */
+ struct rule *stub[5]; /* Preallocated rules to avoid malloc(). */
};
void rule_collection_init(struct rule_collection *);
void rule_collection_add(struct rule_collection *, struct rule *);
+void rule_collection_remove(struct rule_collection *, struct rule *);
+void rule_collection_move(struct rule_collection *to,
+ struct rule_collection *from);
void rule_collection_ref(struct rule_collection *) OVS_REQUIRES(ofproto_mutex);
void rule_collection_unref(struct rule_collection *);
void rule_collection_destroy(struct rule_collection *);
@@ -513,6 +516,7 @@ struct ofgroup {
const struct ofproto *ofproto; /* The ofproto that contains this group. */
const uint32_t group_id;
const enum ofp11_group_type type; /* One of OFPGT_*. */
+ bool being_deleted; /* Group removal has begun. */
const long long int created; /* Creation time. */
const long long int modified; /* Time of last modification. */
@@ -522,6 +526,8 @@ struct ofgroup {
const uint32_t n_buckets;
const struct ofputil_group_props props;
+
+ struct rule_collection rules OVS_GUARDED; /* Referring rules. */
};
struct ofgroup *ofproto_group_lookup(const struct ofproto *ofproto,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 83719a5..7542fc3 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -296,7 +296,8 @@ static void send_buffered_packet(const struct
flow_mod_requester *,
static bool ofproto_group_exists(const struct ofproto *, uint32_t group_id);
static enum ofperr add_group(struct ofproto *,
- const struct ofputil_group_mod *);
+ const struct ofputil_group_mod *)
+ OVS_REQUIRES(ofproto_mutex);
static void handle_openflow(struct ofconn *, const struct ofpbuf *);
static enum ofperr ofproto_flow_mod_start(struct ofproto *,
struct ofproto_flow_mod *)
@@ -559,7 +560,6 @@ ofproto_create(const char *datapath_name, const char
*datapath_type,
ofproto->connmgr = connmgr_create(ofproto, datapath_name, datapath_name);
guarded_list_init(&ofproto->rule_executes);
ofproto->min_mtu = INT_MAX;
- ovs_rwlock_init(&ofproto->groups_rwlock);
cmap_init(&ofproto->groups);
ovs_mutex_unlock(&ofproto_mutex);
ofproto->ogf.types = 0xf;
@@ -1575,7 +1575,8 @@ ofproto_flush__(struct ofproto *ofproto)
ovs_mutex_unlock(&ofproto_mutex);
}
-static void delete_group(struct ofproto *ofproto, uint32_t group_id);
+static void delete_group(struct ofproto *ofproto, uint32_t group_id)
+ OVS_REQUIRES(ofproto_mutex);
static void
ofproto_destroy__(struct ofproto *ofproto)
@@ -1586,7 +1587,6 @@ ofproto_destroy__(struct ofproto *ofproto)
destroy_rule_executes(ofproto);
guarded_list_destroy(&ofproto->rule_executes);
- ovs_rwlock_destroy(&ofproto->groups_rwlock);
cmap_destroy(&ofproto->groups);
hmap_remove(&all_ofprotos, &ofproto->hmap_node);
@@ -2991,8 +2991,10 @@ group_destroy_cb(struct ofgroup *group)
void
ofproto_group_unref(struct ofgroup *group)
+ OVS_NO_THREAD_SAFETY_ANALYSIS
{
if (group && ovs_refcount_unref_relaxed(&group->ref_count) == 1) {
+ ovs_assert(group->rules.n == 0);
ovsrcu_postpone(group_destroy_cb, group);
}
}
@@ -3009,9 +3011,12 @@ rule_actions_create(const struct ofpact *ofpacts, size_t
ofpacts_len)
actions = xmalloc(sizeof *actions + ofpacts_len);
actions->ofpacts_len = ofpacts_len;
- actions->has_meter = ofpacts_get_meter(ofpacts, ofpacts_len) != 0;
memcpy(actions->ofpacts, ofpacts, ofpacts_len);
-
+ actions->has_meter = ofpacts_get_meter(ofpacts, ofpacts_len) != 0;
+ actions->has_groups =
+ (ofpact_find_type_flattened(ofpacts, OFPACT_GROUP,
+ ofpact_end(ofpacts, ofpacts_len))
+ != NULL);
actions->has_learn_with_delete = (next_learn_with_delete(actions, NULL)
!= NULL);
@@ -3448,9 +3453,9 @@ ofproto_check_ofpacts(struct ofproto *ofproto,
return OFPERR_OFPMMFC_INVALID_METER;
}
- OFPACT_FOR_EACH_FLATTENED (a, ofpacts, ofpacts_len) {
- if (a->type == OFPACT_GROUP
- && !ofproto_group_exists(ofproto, ofpact_get_GROUP(a)->group_id)) {
+ OFPACT_FOR_EACH_TYPE_FLATTENED (a, OFPACT_GROUP, ofpacts, ofpacts_len) {
+ if (!ofproto_group_exists(ofproto,
+ ofpact_get_GROUP(a)->group_id)) {
return OFPERR_OFPBAC_BAD_OUT_GROUP;
}
}
@@ -3513,6 +3518,8 @@ handle_packet_out(struct ofconn *ofconn, const struct
ofp_header *oh)
0, p->n_tables,
ofconn_get_protocol(ofconn));
if (!error) {
+ /* Should hold ofproto_mutex to guarantee state don't
+ * change between the check and the execution. */
error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len);
if (!error) {
error = p->ofproto_class->packet_out(p, payload, &flow,
@@ -4068,6 +4075,57 @@ rule_collection_add(struct rule_collection *rules,
struct rule *rule)
}
void
+rule_collection_remove(struct rule_collection *rules, struct rule *rule)
+{
+ size_t i;
+
+ for (i = 0; i < rules->n; i++) {
+ if (rules->rules[i] == rule) {
+ break;
+ }
+ }
+ if (i == rules->n) {
+ return;
+ }
+
+ rules->n--;
+ /* Swap the last item in if needed. */
+ if (i != rules->n) {
+ rules->rules[i] = rules->rules[rules->n];
+ }
+
+ /* Shrink? Watermark at '/ 4' to avoid hysteresis, but leave spare
+ * capacity. */
+ if (rules->rules != rules->stub && rules->n <= rules->capacity / 4) {
+ size_t actual_size, new_size;
+
+ actual_size = rules->n * sizeof *rules->rules;
+ rules->capacity /= 2;
+ new_size = rules->capacity * sizeof *rules->rules;
+
+ if (new_size <= sizeof(rules->stub)) {
+ memcpy(rules->stub, rules->rules, actual_size);
+ free(rules->rules);
+ rules->rules = rules->stub;
+ } else {
+ rules->rules = xrealloc(rules->rules, new_size);
+ }
+ }
+}
+
+void
+rule_collection_move(struct rule_collection *to, struct rule_collection *from)
+{
+ ovs_assert(to->n == 0);
+
+ *to = *from;
+ if (from->rules == from->stub) {
+ to->rules = to->stub;
+ }
+ rule_collection_init(from);
+}
+
+void
rule_collection_ref(struct rule_collection *rules)
OVS_REQUIRES(ofproto_mutex)
{
@@ -4126,6 +4184,7 @@ rule_collection_remove_postponed(struct rule_collection
*rules)
if (rules->n > 0) {
if (rules->n == 1) {
ovsrcu_postpone(remove_rule_rcu, rules->rules[0]);
+ rules->n = 0;
} else {
ovsrcu_postpone(remove_rules_rcu, rule_collection_detach(rules));
}
@@ -4939,8 +4998,8 @@ replace_rule_start(struct ofproto *ofproto, ovs_version_t
version,
} else {
table->n_flows++;
}
- /* Insert flow to the classifier, so that later flow_mods may relate
- * to it. This is reversible, in case later errors require this to
+ /* Insert flow to ofproto data structures, so that later flow_mods may
+ * relate to it. This is reversible, in case later errors require this to
* be reverted. */
ofproto_rule_insert__(ofproto, new_rule);
/* Make the new rule visible for classifier lookups only from the next
@@ -4948,8 +5007,9 @@ replace_rule_start(struct ofproto *ofproto, ovs_version_t
version,
classifier_insert(&table->cls, &new_rule->cr, version, conjs, n_conjs);
}
-static void replace_rule_revert(struct ofproto *ofproto,
- struct rule *old_rule, struct rule *new_rule)
+static void
+replace_rule_revert(struct ofproto *ofproto,
+ struct rule *old_rule, struct rule *new_rule)
{
struct oftable *table = &ofproto->tables[new_rule->table_id];
@@ -5454,10 +5514,6 @@ handle_flow_mod(struct ofconn *ofconn, const struct
ofp_header *oh)
u16_to_ofp(ofproto->max_ports),
ofproto->n_tables);
if (!error) {
- error = ofproto_check_ofpacts(ofproto, ofm.fm.ofpacts,
- ofm.fm.ofpacts_len);
- }
- if (!error) {
struct flow_mod_requester req;
req.ofconn = ofconn;
@@ -6235,7 +6291,7 @@ ofproto_group_lookup(const struct ofproto *ofproto,
uint32_t group_id,
return group;
}
-/* Caller should hold 'ofproto->groups_rwlock' if it is important that the
+/* Caller should hold 'ofproto_mutex' if it is important that the
* group is not removed by someone else. */
static bool
ofproto_group_exists(const struct ofproto *ofproto, uint32_t group_id)
@@ -6243,36 +6299,21 @@ ofproto_group_exists(const struct ofproto *ofproto,
uint32_t group_id)
return ofproto_group_lookup__(ofproto, group_id) != NULL;
}
-/* XXX: This is potentially very expensive for large flow tables, and may be
- * called in a loop. Maybe explicitly maintain the count of rules referring to
- * the group instead?. */
-static uint32_t
-group_get_ref_count(struct ofgroup *group)
- OVS_EXCLUDED(ofproto_mutex)
+static void
+group_add_rule(struct ofgroup *group, struct rule *rule)
{
- struct ofproto *ofproto = CONST_CAST(struct ofproto *, group->ofproto);
- struct rule_criteria criteria;
- struct rule_collection rules;
- struct match match;
- enum ofperr error;
- uint32_t count;
-
- match_init_catchall(&match);
- rule_criteria_init(&criteria, 0xff, &match, 0, OVS_VERSION_MAX, htonll(0),
- htonll(0), OFPP_ANY, group->group_id);
- ovs_mutex_lock(&ofproto_mutex);
- error = collect_rules_loose(ofproto, &criteria, &rules);
- ovs_mutex_unlock(&ofproto_mutex);
- rule_criteria_destroy(&criteria);
-
- count = !error && rules.n < UINT32_MAX ? rules.n : UINT32_MAX;
+ rule_collection_add(&group->rules, rule);
+}
- rule_collection_destroy(&rules);
- return count;
+static void
+group_remove_rule(struct ofgroup *group, struct rule *rule)
+{
+ rule_collection_remove(&group->rules, rule);
}
static void
append_group_stats(struct ofgroup *group, struct ovs_list *replies)
+ OVS_REQUIRES(ofproto_mutex)
{
struct ofputil_group_stats ogs;
const struct ofproto *ofproto = group->ofproto;
@@ -6282,7 +6323,7 @@ append_group_stats(struct ofgroup *group, struct ovs_list
*replies)
ogs.bucket_stats = xmalloc(group->n_buckets * sizeof *ogs.bucket_stats);
/* Provider sets the packet and byte counts, we do the rest. */
- ogs.ref_count = group_get_ref_count(group);
+ ogs.ref_count = group->rules.n;
ogs.n_buckets = group->n_buckets;
error = (ofproto->ofproto_class->group_get_stats
@@ -6307,25 +6348,26 @@ static void
handle_group_request(struct ofconn *ofconn,
const struct ofp_header *request, uint32_t group_id,
void (*cb)(struct ofgroup *, struct ovs_list *replies))
+ OVS_EXCLUDED(ofproto_mutex)
{
struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
struct ofgroup *group;
struct ovs_list replies;
ofpmp_init(&replies, request);
+ /* Must exclude modifications to guarantee iterating groups. */
+ ovs_mutex_lock(&ofproto_mutex);
if (group_id == OFPG_ALL) {
- /* Must exclude modifications to guarantee iterating all groups. */
- ovs_rwlock_rdlock(&ofproto->groups_rwlock);
CMAP_FOR_EACH (group, cmap_node, &ofproto->groups) {
cb(group, &replies);
}
- ovs_rwlock_unlock(&ofproto->groups_rwlock);
} else {
group = ofproto_group_lookup__(ofproto, group_id);
if (group) {
cb(group, &replies);
}
}
+ ovs_mutex_unlock(&ofproto_mutex);
ofconn_send_replies(ofconn, &replies);
}
@@ -6494,6 +6536,7 @@ init_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm,
*CONST_CAST(long long int *, &((*ofgroup)->created)) = now;
*CONST_CAST(long long int *, &((*ofgroup)->modified)) = now;
ovs_refcount_init(&(*ofgroup)->ref_count);
+ (*ofgroup)->being_deleted = false;
ovs_list_init(&(*ofgroup)->buckets);
ofputil_bucket_clone_list(&(*ofgroup)->buckets, &gm->buckets, NULL);
@@ -6503,6 +6546,7 @@ init_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm,
memcpy(CONST_CAST(struct ofputil_group_props *, &(*ofgroup)->props),
&gm->props, sizeof (struct ofputil_group_props));
+ rule_collection_init(&(*ofgroup)->rules);
/* Construct called BEFORE any locks are held. */
error = ofproto->ofproto_class->group_construct(*ofgroup);
@@ -6518,6 +6562,7 @@ init_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm,
* failure. */
static enum ofperr
add_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm)
+ OVS_REQUIRES(ofproto_mutex)
{
struct ofgroup *ofgroup;
enum ofperr error;
@@ -6528,10 +6573,6 @@ add_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm)
return error;
}
- /* We take the mutex as late as possible to minimize the time we jam any
- * other threads: No visible state changes before acquiring the lock. */
- ovs_rwlock_wrlock(&ofproto->groups_rwlock);
-
if (ofproto->n_groups[gm->type] >= ofproto->ogf.max_groups[gm->type]) {
error = OFPERR_OFPGMFC_OUT_OF_GROUPS;
goto unlock_out;
@@ -6547,11 +6588,9 @@ add_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm)
hash_int(ofgroup->group_id, 0));
ofproto->n_groups[ofgroup->type]++;
- ovs_rwlock_unlock(&ofproto->groups_rwlock);
return 0;
unlock_out:
- ovs_rwlock_unlock(&ofproto->groups_rwlock);
group_destroy_cb(ofgroup);
return error;
}
@@ -6661,6 +6700,7 @@ copy_buckets_for_remove_bucket(const struct ofgroup
*ofgroup,
* the xlate module hold a pointer to the group. */
static enum ofperr
modify_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm)
+ OVS_REQUIRES(ofproto_mutex)
{
struct ofgroup *ofgroup, *new_ofgroup, *retiring;
enum ofperr error;
@@ -6672,7 +6712,6 @@ modify_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm)
retiring = new_ofgroup;
- ovs_rwlock_wrlock(&ofproto->groups_rwlock);
ofgroup = ofproto_group_lookup__(ofproto, gm->group_id);
if (!ofgroup) {
error = OFPERR_OFPGMFC_UNKNOWN_GROUP;
@@ -6713,6 +6752,8 @@ modify_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm)
/* Replace ofgroup in ofproto's groups hash map with new_ofgroup. */
cmap_replace(&ofproto->groups, &ofgroup->cmap_node,
&new_ofgroup->cmap_node, hash_int(ofgroup->group_id, 0));
+ /* Transfer rules. */
+ rule_collection_move(&new_ofgroup->rules, &ofgroup->rules);
if (ofgroup->type != new_ofgroup->type) {
ofproto->n_groups[ofgroup->type]--;
@@ -6721,7 +6762,6 @@ modify_group(struct ofproto *ofproto, const struct
ofputil_group_mod *gm)
out:
ofproto_group_unref(retiring);
- ovs_rwlock_unlock(&ofproto->groups_rwlock);
return error;
}
@@ -6729,53 +6769,50 @@ out:
* exist yet and modifies it otherwise */
static enum ofperr
add_or_modify_group(struct ofproto *ofproto, const struct ofputil_group_mod
*gm)
+ OVS_REQUIRES(ofproto_mutex)
{
enum ofperr error;
bool exists;
exists = ofproto_group_exists(ofproto, gm->group_id);
- /* XXX: Race: Should hold groups_rwlock here. */
-
if (!exists) {
error = add_group(ofproto, gm);
} else {
error = modify_group(ofproto, gm);
}
+
return error;
}
static void
delete_group__(struct ofproto *ofproto, struct ofgroup *ofgroup)
- OVS_RELEASES(ofproto->groups_rwlock)
+ OVS_REQUIRES(ofproto_mutex)
{
- struct match match;
- struct ofproto_flow_mod ofm;
+ /* Makes flow deletion code leave the rule pointers in group's 'rules'
+ * intact, so that we can later refer to the rules deleted due to the group
+ * deletion. Rule pointers will be removed from all other groups, if any,
+ * so we will never try to delete the same rule twice. */
+ ofgroup->being_deleted = true;
- /* Delete all flow entries containing this group in a group action */
- match_init_catchall(&match);
- flow_mod_init(&ofm.fm, &match, 0, NULL, 0, OFPFC_DELETE);
- ofm.fm.delete_reason = OFPRR_GROUP_DELETE;
- ofm.fm.out_group = ofgroup->group_id;
- ofm.fm.table_id = OFPTT_ALL;
- handle_flow_mod__(ofproto, &ofm, NULL);
+ /* Delete all flow entries containing this group in a group action. */
+ delete_flows__(&ofgroup->rules, OFPRR_GROUP_DELETE, NULL);
cmap_remove(&ofproto->groups, &ofgroup->cmap_node,
hash_int(ofgroup->group_id, 0));
/* No-one can find this group any more, but current users may continue to
* use it. */
ofproto->n_groups[ofgroup->type]--;
- ovs_rwlock_unlock(&ofproto->groups_rwlock);
ofproto_group_unref(ofgroup);
}
/* Implements OFPGC11_DELETE. */
static void
delete_group(struct ofproto *ofproto, uint32_t group_id)
+ OVS_REQUIRES(ofproto_mutex)
{
struct ofgroup *ofgroup;
- ovs_rwlock_wrlock(&ofproto->groups_rwlock);
if (group_id == OFPG_ALL) {
for (;;) {
struct cmap_node *node = cmap_first(&ofproto->groups);
@@ -6783,10 +6820,7 @@ delete_group(struct ofproto *ofproto, uint32_t group_id)
break;
}
ofgroup = CONTAINER_OF(node, struct ofgroup, cmap_node);
- delete_group__(ofproto, ofgroup); /* Releases the mutex. */
- /* Lock for each node separately, so that we will not jam the
- * other threads for too long time. */
- ovs_rwlock_wrlock(&ofproto->groups_rwlock);
+ delete_group__(ofproto, ofgroup);
}
} else {
CMAP_FOR_EACH_WITH_HASH (ofgroup, cmap_node,
@@ -6797,7 +6831,6 @@ delete_group(struct ofproto *ofproto, uint32_t group_id)
}
}
}
- ovs_rwlock_unlock(&ofproto->groups_rwlock);
}
/* Delete all groups from 'ofproto'.
@@ -6806,12 +6839,16 @@ delete_group(struct ofproto *ofproto, uint32_t group_id)
* function. */
void
ofproto_group_delete_all(struct ofproto *ofproto)
+ OVS_EXCLUDED(ofproto_mutex)
{
+ ovs_mutex_lock(&ofproto_mutex);
delete_group(ofproto, OFPG_ALL);
+ ovs_mutex_unlock(&ofproto_mutex);
}
static enum ofperr
handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh)
+ OVS_EXCLUDED(ofproto_mutex)
{
struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
struct ofputil_group_mod gm;
@@ -6827,6 +6864,7 @@ handle_group_mod(struct ofconn *ofconn, const struct
ofp_header *oh)
return error;
}
+ ovs_mutex_lock(&ofproto_mutex);
switch (gm.command) {
case OFPGC11_ADD:
error = add_group(ofproto, &gm);
@@ -6860,6 +6898,7 @@ handle_group_mod(struct ofconn *ofconn, const struct
ofp_header *oh)
}
error = OFPERR_OFPGMFC_BAD_COMMAND;
}
+ ovs_mutex_unlock(&ofproto_mutex);
if (!error) {
struct ofputil_requestforward rf;
@@ -6984,6 +7023,13 @@ static enum ofperr
ofproto_flow_mod_start(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ /* Must check actions while holding ofproto_mutex to avoid a race. */
+ enum ofperr error = ofproto_check_ofpacts(ofproto, ofm->fm.ofpacts,
+ ofm->fm.ofpacts_len);
+ if (error) {
+ return error;
+ }
+
switch (ofm->fm.command) {
case OFPFC_ADD:
return add_flow_start(ofproto, ofm);
@@ -7263,11 +7309,6 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
ofproto->n_tables);
/* Move actions to heap. */
bmsg->ofm.fm.ofpacts = ofpbuf_steal_data(&ofpacts);
-
- if (!error && bmsg->ofm.fm.ofpacts_len) {
- error = ofproto_check_ofpacts(ofproto, bmsg->ofm.fm.ofpacts,
- bmsg->ofm.fm.ofpacts_len);
- }
} else {
OVS_NOT_REACHED();
}
@@ -7980,6 +8021,20 @@ ofproto_rule_insert__(struct ofproto *ofproto, struct
rule *rule)
if (actions->has_meter) {
meter_insert_rule(rule);
}
+ if (actions->has_groups) {
+ const struct ofpact *a;
+
+ OFPACT_FOR_EACH_TYPE_FLATTENED (a, OFPACT_GROUP, actions->ofpacts,
+ actions->ofpacts_len) {
+ struct ofgroup *group;
+
+ group = ofproto_group_lookup(ofproto,
+ ofpact_get_GROUP(a)->group_id, false);
+ ovs_assert(group != NULL);
+ group_add_rule(group, rule);
+ }
+ }
+
rule->removed = false;
}
@@ -8002,6 +8057,29 @@ ofproto_rule_remove__(struct ofproto *ofproto, struct
rule *rule)
ovs_list_init(&rule->meter_list_node);
}
+ /* Remove the rule from any groups, except from the group that is being
+ * deleted, if any. */
+ const struct rule_actions *actions = rule_get_actions(rule);
+
+ if (actions->has_groups) {
+ const struct ofpact *a;
+
+ OFPACT_FOR_EACH_TYPE_FLATTENED (a, OFPACT_GROUP, actions->ofpacts,
+ actions->ofpacts_len) {
+ struct ofgroup *group;
+
+ group = ofproto_group_lookup(ofproto,
+ ofpact_get_GROUP(a)->group_id, false);
+ ovs_assert(group);
+
+ /* Leave the rule for the group that is being deleted, if any,
+ * as we still need the list of rules for clean-up. */
+ if (!group->being_deleted) {
+ group_remove_rule(group, rule);
+ }
+ }
+ }
+
rule->removed = true;
}
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
