This is a prepatory step for adding group mod support for bundles in a following patch.
Signed-off-by: Jarno Rajahalme <ja...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev