[ovs-dev] [PATCH v2 10/26] ofproto: Make groups versioned.
Jarno Rajahalme
jarno at ovn.org
Fri Jul 29 00:56:02 UTC 2016
This is a prepatory step for adding group mod support for bundles in a
following patch.
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
---
ofproto/ofproto-dpif-xlate.c | 5 +-
ofproto/ofproto-dpif.c | 4 +-
ofproto/ofproto-dpif.h | 3 +-
ofproto/ofproto-provider.h | 11 +++-
ofproto/ofproto.c | 150 ++++++++++++++++++++++++++++++-------------
5 files changed, 122 insertions(+), 51 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ca468c6..a847557 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1503,7 +1503,8 @@ group_is_alive(const struct xlate_ctx *ctx, uint32_t group_id, int depth)
{
struct group_dpif *group;
- group = group_dpif_lookup(ctx->xbridge->ofproto, group_id, false);
+ group = group_dpif_lookup(ctx->xbridge->ofproto, group_id,
+ ctx->tables_version, false);
if (group) {
return group_first_live_bucket(ctx, group, depth) != NULL;
}
@@ -3611,7 +3612,7 @@ xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)
/* Take ref only if xcache exists. */
group = group_dpif_lookup(ctx->xbridge->ofproto, group_id,
- ctx->xin->xcache);
+ ctx->tables_version, ctx->xin->xcache);
if (!group) {
/* XXX: Should set ctx->error ? */
return true;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6fa76c2..0f00cef 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4366,10 +4366,10 @@ group_get_stats(const struct ofgroup *group_, struct ofputil_group_stats *ogs)
* a reference to the group. */
struct group_dpif *
group_dpif_lookup(struct ofproto_dpif *ofproto, uint32_t group_id,
- bool take_ref)
+ ovs_version_t version, bool take_ref)
{
struct ofgroup *ofgroup = ofproto_group_lookup(&ofproto->up, group_id,
- take_ref);
+ version, take_ref);
return ofgroup ? group_dpif_cast(ofgroup) : NULL;
}
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 4c6f088..f1e1209 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -137,7 +137,8 @@ void group_dpif_credit_stats(struct group_dpif *,
struct ofputil_bucket *,
const struct dpif_flow_stats *);
struct group_dpif *group_dpif_lookup(struct ofproto_dpif *ofproto,
- uint32_t group_id, bool take_ref);
+ uint32_t group_id, ovs_version_t version,
+ bool take_ref);
const struct ovs_list *group_dpif_get_buckets(const struct group_dpif *group);
enum ofp11_group_type group_dpif_get_type(const struct group_dpif *group);
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 7ad1d16..c6300e7 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -49,6 +49,7 @@
#include "openvswitch/shash.h"
#include "simap.h"
#include "timeval.h"
+#include "versions.h"
struct match;
struct ofputil_flow_mod;
@@ -584,6 +585,9 @@ void ofproto_rule_reduce_timeouts(struct rule *rule, uint16_t idle_timeout,
struct ofgroup {
struct cmap_node cmap_node; /* In ofproto's "groups" cmap. */
+ /* Group versioning. */
+ struct versions versions;
+
/* Number of references.
*
* This is needed to keep track of references to the group in the xlate
@@ -597,7 +601,7 @@ struct ofgroup {
/* No lock is needed to protect the fields below since they are not
* modified after construction. */
- const struct ofproto *ofproto; /* The ofproto that contains this group. */
+ struct ofproto * const 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. */
@@ -615,7 +619,8 @@ struct ofgroup {
};
struct ofgroup *ofproto_group_lookup(const struct ofproto *ofproto,
- uint32_t group_id, bool take_ref);
+ uint32_t group_id, ovs_version_t version,
+ bool take_ref);
void ofproto_group_ref(struct ofgroup *);
bool ofproto_group_try_ref(struct ofgroup *);
@@ -1918,6 +1923,8 @@ struct ofproto_port_mod {
struct ofproto_group_mod {
struct ofputil_group_mod gm;
+ ovs_version_t version; /* Version in which changes take
+ * effect. */
struct ofgroup *new_group; /* New group. */
struct group_collection old_groups; /* Affected groups. */
};
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 2037d89..8ebbad0 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2993,6 +2993,45 @@ ofproto_group_unref(struct ofgroup *group)
}
}
+static void
+remove_group_rcu__(struct ofgroup *group)
+ OVS_REQUIRES(ofproto_mutex)
+{
+ struct ofproto *ofproto = group->ofproto;
+
+ ovs_assert(!versions_visible_in_version(&group->versions, OVS_VERSION_MAX));
+
+ cmap_remove(&ofproto->groups, &group->cmap_node,
+ hash_int(group->group_id, 0));
+ ofproto_group_unref(group);
+}
+
+static void
+remove_group_rcu(struct ofgroup *group)
+ OVS_EXCLUDED(ofproto_mutex)
+{
+ ovs_mutex_lock(&ofproto_mutex);
+ remove_group_rcu__(group);
+ ovs_mutex_unlock(&ofproto_mutex);
+}
+
+/* Removes and deletes groups from a NULL-terminated array of group
+ * pointers. */
+static void
+remove_groups_rcu(struct ofgroup **groups)
+ OVS_EXCLUDED(ofproto_mutex)
+{
+ struct ofgroup **orig_groups = groups;
+ struct ofgroup *group;
+
+ ovs_mutex_lock(&ofproto_mutex);
+ while ((group = *groups++)) {
+ remove_group_rcu__(group);
+ }
+ ovs_mutex_unlock(&ofproto_mutex);
+ free(orig_groups);
+}
+
static uint32_t get_provider_meter_id(const struct ofproto *,
uint32_t of_meter_id);
@@ -4165,6 +4204,23 @@ rule_collection_remove_postponed(struct rule_collection *rules)
}
}
+/* Schedules postponed removal of groups, destroys 'groups'. */
+static void
+group_collection_remove_postponed(struct group_collection *groups)
+ OVS_REQUIRES(ofproto_mutex)
+{
+ if (group_collection_n(groups) > 0) {
+ if (group_collection_n(groups) == 1) {
+ ovsrcu_postpone(remove_group_rcu,
+ group_collection_groups(groups)[0]);
+ group_collection_init(groups);
+ } else {
+ ovsrcu_postpone(remove_groups_rcu,
+ group_collection_detach(groups));
+ }
+ }
+}
+
/* Checks whether 'rule' matches 'c' and, if so, adds it to 'rules'. This
* function verifies most of the criteria in 'c' itself, but the caller must
* check 'c->cr' itself.
@@ -6225,13 +6281,15 @@ handle_meter_request(struct ofconn *ofconn, const struct ofp_header *request,
/* Returned group is RCU protected. */
static struct ofgroup *
-ofproto_group_lookup__(const struct ofproto *ofproto, uint32_t group_id)
+ofproto_group_lookup__(const struct ofproto *ofproto, uint32_t group_id,
+ ovs_version_t version)
{
struct ofgroup *group;
CMAP_FOR_EACH_WITH_HASH (group, cmap_node, hash_int(group_id, 0),
&ofproto->groups) {
- if (group->group_id == group_id) {
+ if (group->group_id == group_id
+ && versions_visible_in_version(&group->versions, version)) {
return group;
}
}
@@ -6245,11 +6303,11 @@ ofproto_group_lookup__(const struct ofproto *ofproto, uint32_t group_id)
* a reference to the group. */
struct ofgroup *
ofproto_group_lookup(const struct ofproto *ofproto, uint32_t group_id,
- bool take_ref)
+ ovs_version_t version, bool take_ref)
{
struct ofgroup *group;
- group = ofproto_group_lookup__(ofproto, group_id);
+ group = ofproto_group_lookup__(ofproto, group_id, version);
if (group && take_ref) {
/* Not holding a lock, so it is possible that another thread releases
* the last reference just before we manage to get one. */
@@ -6263,7 +6321,7 @@ ofproto_group_lookup(const struct ofproto *ofproto, uint32_t group_id,
static bool
ofproto_group_exists(const struct ofproto *ofproto, uint32_t group_id)
{
- return ofproto_group_lookup__(ofproto, group_id) != NULL;
+ return ofproto_group_lookup__(ofproto, group_id, OVS_VERSION_MAX) != NULL;
}
static void
@@ -6329,7 +6387,7 @@ handle_group_request(struct ofconn *ofconn,
cb(group, &replies);
}
} else {
- group = ofproto_group_lookup__(ofproto, group_id);
+ group = ofproto_group_lookup__(ofproto, group_id, OVS_VERSION_MAX);
if (group) {
cb(group, &replies);
}
@@ -6479,7 +6537,7 @@ handle_queue_get_config_request(struct ofconn *ofconn,
static enum ofperr
init_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm,
- struct ofgroup **ofgroup)
+ ovs_version_t version, struct ofgroup **ofgroup)
{
enum ofperr error;
const long long int now = time_msec();
@@ -6497,7 +6555,7 @@ init_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm,
return OFPERR_OFPGMFC_OUT_OF_GROUPS;
}
- (*ofgroup)->ofproto = ofproto;
+ *CONST_CAST(struct ofproto **, &(*ofgroup)->ofproto) = ofproto;
*CONST_CAST(uint32_t *, &((*ofgroup)->group_id)) = gm->group_id;
*CONST_CAST(enum ofp11_group_type *, &(*ofgroup)->type) = gm->type;
*CONST_CAST(long long int *, &((*ofgroup)->created)) = now;
@@ -6515,6 +6573,10 @@ init_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm,
&gm->props, sizeof (struct ofputil_group_props));
rule_collection_init(&(*ofgroup)->rules);
+ /* Make group visible from 'version'. */
+ (*ofgroup)->versions = VERSIONS_INITIALIZER(version,
+ OVS_VERSION_NOT_REMOVED);
+
/* Construct called BEFORE any locks are held. */
error = ofproto->ofproto_class->group_construct(*ofgroup);
if (error) {
@@ -6543,7 +6605,7 @@ add_group_start(struct ofproto *ofproto, struct ofproto_group_mod *ogm)
}
/* Allocate new group and initialize it. */
- error = init_group(ofproto, &ogm->gm, &ogm->new_group);
+ error = init_group(ofproto, &ogm->gm, ogm->version, &ogm->new_group);
if (!error) {
/* Insert new group. */
cmap_insert(&ofproto->groups, &ogm->new_group->cmap_node,
@@ -6664,7 +6726,8 @@ modify_group_start(struct ofproto *ofproto, struct ofproto_group_mod *ogm)
struct ofgroup *new_group;
enum ofperr error;
- old_group = ofproto_group_lookup__(ofproto, ogm->gm.group_id);
+ old_group = ofproto_group_lookup__(ofproto, ogm->gm.group_id,
+ OVS_VERSION_MAX);
if (!old_group) {
return OFPERR_OFPGMFC_UNKNOWN_GROUP;
}
@@ -6675,7 +6738,7 @@ modify_group_start(struct ofproto *ofproto, struct ofproto_group_mod *ogm)
return OFPERR_OFPGMFC_OUT_OF_GROUPS;
}
- error = init_group(ofproto, &ogm->gm, &ogm->new_group);
+ error = init_group(ofproto, &ogm->gm, ogm->version, &ogm->new_group);
if (error) {
return error;
}
@@ -6699,10 +6762,11 @@ modify_group_start(struct ofproto *ofproto, struct ofproto_group_mod *ogm)
group_collection_add(&ogm->old_groups, old_group);
- /* Replace ofgroup in ofproto's groups hash map with new_ofgroup. */
- cmap_replace(&ofproto->groups, &old_group->cmap_node,
- &new_group->cmap_node, hash_int(new_group->group_id, 0));
-
+ /* Mark the old group for deletion. */
+ versions_set_remove_version(&old_group->versions, ogm->version);
+ /* Insert replacement group. */
+ cmap_insert(&ofproto->groups, &new_group->cmap_node,
+ hash_int(new_group->group_id, 0));
/* Transfer rules. */
rule_collection_move(&new_group->rules, &old_group->rules);
@@ -6736,8 +6800,8 @@ add_or_modify_group_start(struct ofproto *ofproto,
}
static void
-delete_group_start(struct ofproto *ofproto, struct group_collection *groups,
- struct ofgroup *group)
+delete_group_start(struct ofproto *ofproto, ovs_version_t version,
+ struct group_collection *groups, struct ofgroup *group)
OVS_REQUIRES(ofproto_mutex)
{
/* Makes flow deletion code leave the rule pointers in 'group->rules'
@@ -6747,9 +6811,9 @@ delete_group_start(struct ofproto *ofproto, struct group_collection *groups,
group->being_deleted = true;
/* Mark all the referring groups for deletion. */
- delete_flows_start__(ofproto, ofproto->tables_version + 1,
- &group->rules);
+ delete_flows_start__(ofproto, version, &group->rules);
group_collection_add(groups, group);
+ versions_set_remove_version(&group->versions, version);
ofproto->n_groups[group->type]--;
}
@@ -6761,11 +6825,7 @@ delete_group_finish(struct ofproto *ofproto, struct ofgroup *group)
* action. */
delete_flows_finish__(ofproto, &group->rules, OFPRR_GROUP_DELETE, NULL);
- cmap_remove(&ofproto->groups, &group->cmap_node,
- hash_int(group->group_id, 0));
- /* No-one can find this group any more, but current users may
- * continue to use it. */
- ofproto_group_unref(group);
+ /* Group removal is postponed by the caller. */
}
/* Implements OFPGC11_DELETE. */
@@ -6777,16 +6837,15 @@ delete_groups_start(struct ofproto *ofproto, struct ofproto_group_mod *ogm)
if (ogm->gm.group_id == OFPG_ALL) {
CMAP_FOR_EACH (group, cmap_node, &ofproto->groups) {
- delete_group_start(ofproto, &ogm->old_groups, group);
+ if (versions_visible_in_version(&group->versions, ogm->version)) {
+ delete_group_start(ofproto, ogm->version, &ogm->old_groups,
+ group);
+ }
}
} else {
- CMAP_FOR_EACH_WITH_HASH (group, cmap_node,
- hash_int(ogm->gm.group_id, 0),
- &ofproto->groups) {
- if (group->group_id == ogm->gm.group_id) {
- delete_group_start(ofproto, &ogm->old_groups, group);
- return;
- }
+ group = ofproto_group_lookup__(ofproto, ogm->gm.group_id, ogm->version);
+ if (group) {
+ delete_group_start(ofproto, ogm->version, &ogm->old_groups, group);
}
}
}
@@ -6846,23 +6905,19 @@ ofproto_group_mod_finish(struct ofproto *ofproto,
struct ofgroup *new_group = ogm->new_group;
struct ofgroup *old_group;
- if (!new_group) {
- /* Delete old groups. */
- ofproto_bump_tables_version(ofproto);
- GROUP_COLLECTION_FOR_EACH(old_group, &ogm->old_groups) {
- delete_group_finish(ofproto, old_group);
- }
- } else if (group_collection_n(&ogm->old_groups)) {
+ if (new_group && group_collection_n(&ogm->old_groups)) {
/* Modify a group. */
ovs_assert(group_collection_n(&ogm->old_groups) == 1);
- old_group = group_collection_groups(&ogm->old_groups)[0];
/* XXX: OK to lose old group's stats? */
ofproto->ofproto_class->group_modify(new_group);
+ }
- ofproto_group_unref(old_group);
+ /* Delete old groups. */
+ GROUP_COLLECTION_FOR_EACH(old_group, &ogm->old_groups) {
+ delete_group_finish(ofproto, old_group);
}
- group_collection_destroy(&ogm->old_groups);
+ group_collection_remove_postponed(&ogm->old_groups);
if (req) {
struct ofputil_requestforward rf;
@@ -6887,7 +6942,9 @@ ofproto_group_delete_all(struct ofproto *ofproto)
ogm.gm.group_id = OFPG_ALL;
ovs_mutex_lock(&ofproto_mutex);
+ ogm.version = ofproto->tables_version + 1;
ofproto_group_mod_start(ofproto, &ogm);
+ ofproto_bump_tables_version(ofproto);
ofproto_group_mod_finish(ofproto, &ogm, NULL);
ovs_mutex_unlock(&ofproto_mutex);
}
@@ -6911,9 +6968,12 @@ handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh)
}
ovs_mutex_lock(&ofproto_mutex);
+ ogm.version = ofproto->tables_version + 1;
error = ofproto_group_mod_start(ofproto, &ogm);
if (!error) {
struct openflow_mod_requester req = { ofconn, oh };
+
+ ofproto_bump_tables_version(ofproto);
ofproto_group_mod_finish(ofproto, &ogm, &req);
}
ofmonitor_flush(ofproto->connmgr);
@@ -8046,7 +8106,8 @@ ofproto_rule_insert__(struct ofproto *ofproto, struct rule *rule)
struct ofgroup *group;
group = ofproto_group_lookup(ofproto,
- ofpact_get_GROUP(a)->group_id, false);
+ ofpact_get_GROUP(a)->group_id,
+ OVS_VERSION_MAX, false);
ovs_assert(group != NULL);
group_add_rule(group, rule);
}
@@ -8086,7 +8147,8 @@ ofproto_rule_remove__(struct ofproto *ofproto, struct rule *rule)
struct ofgroup *group;
group = ofproto_group_lookup(ofproto,
- ofpact_get_GROUP(a)->group_id, false);
+ ofpact_get_GROUP(a)->group_id,
+ OVS_VERSION_MAX, false);
ovs_assert(group);
/* Leave the rule for the group that is being deleted, if any,
--
2.1.4
More information about the dev
mailing list