It is cleaner to not use ofp_bundle_entry for non-bundle flow mods.
To address this, the new struct ofproto_flow_mod combines an
ofputil_flow_mod and the necessary execution context for executing the
start, revert, and finish phases of the flow mod, which all were
previously members of struct ofp_bundle_entry.
This also simplifies many of the function prototypes introduced with
the OF 1.4 bundles code. However, in case of learn action execution
this approach requires a new copy of the ofputil_flow_mod. This could
be avoided by making struct ofproto_flow_mod more complex, but it
seems not worth the complication.
As part of carving out the execution context from ofp_bundle_entry to
ofproto_flow_mod, the 'version' member is now also in
ofproto_flow_mod, as it makes sense for flow mods, but not for port
mods. Now that the functions operate on the version also get the full
execution context, they use 'version' instead of
'ofproto->tables_version'. This allows ofproto->tables_version to be
changed only when a new version is committed.
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
ofproto/bundles.c | 2 +-
ofproto/bundles.h | 14 +--
ofproto/ofproto-dpif.c | 79 +++++++------
ofproto/ofproto-dpif.h | 3 +-
ofproto/ofproto-provider.h | 18 ++-
ofproto/ofproto.c | 278 ++++++++++++++++++++++----------------------
6 files changed, 206 insertions(+), 188 deletions(-)
diff --git a/ofproto/bundles.c b/ofproto/bundles.c
index 686d61f..003b20b 100644
--- a/ofproto/bundles.c
+++ b/ofproto/bundles.c
@@ -67,7 +67,7 @@ ofp_bundle_remove__(struct ofconn *ofconn, struct ofp_bundle
*bundle,
LIST_FOR_EACH_POP (msg, node, &bundle->msg_list) {
if (success && msg->type == OFPTYPE_FLOW_MOD) {
/* Tell connmgr about successful flow mods. */
- ofconn_report_flow_mod(ofconn, msg->fm.command);
+ ofconn_report_flow_mod(ofconn, msg->ofm.fm.command);
}
ofp_bundle_entry_free(msg);
}
diff --git a/ofproto/bundles.h b/ofproto/bundles.h
index 65717df..1b2e333 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -34,18 +34,11 @@ extern "C" {
struct ofp_bundle_entry {
struct ovs_list node;
enum ofptype type; /* OFPTYPE_FLOW_MOD or OFPTYPE_PORT_MOD. */
- long long version; /* Version in which the changes take
- * effect. */
union {
- struct ofputil_flow_mod fm; /* 'fm.ofpacts' must be malloced. */
- struct ofputil_port_mod pm;
+ struct ofproto_flow_mod ofm; /* ofm.fm.ofpacts must be malloced. */
+ struct ofproto_port_mod opm;
};
- /* Used during commit. */
- struct ofport *port; /* Affected port. */
- struct rule_collection old_rules; /* Affected rules. */
- struct rule_collection new_rules; /* Replacement rules. */
-
/* OpenFlow header and some of the message contents for error reporting. */
struct ofp_header ofp_msg[DIV_ROUND_UP(64, sizeof(struct ofp_header))];
};
@@ -83,7 +76,6 @@ ofp_bundle_entry_alloc(enum ofptype type, const struct
ofp_header *oh)
struct ofp_bundle_entry *entry = xmalloc(sizeof *entry);
entry->type = type;
- entry->version = 0;
/* Max 64 bytes for error reporting. */
memcpy(entry->ofp_msg, oh, MIN(ntohs(oh->length), sizeof entry->ofp_msg));
@@ -96,7 +88,7 @@ ofp_bundle_entry_free(struct ofp_bundle_entry *entry)
{
if (entry) {
if (entry->type == OFPTYPE_FLOW_MOD) {
- free(entry->fm.ofpacts);
+ free(entry->ofm.fm.ofpacts);
}
free(entry);
}
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 13d2f21..0a84525 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -392,9 +392,18 @@ static struct shash init_ofp_ports =
SHASH_INITIALIZER(&init_ofp_ports);
* it. */
void
ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto,
- struct ofputil_flow_mod *fm)
+ const struct ofputil_flow_mod *fm)
{
- ofproto_flow_mod(&ofproto->up, fm);
+ struct ofproto_flow_mod ofm;
+
+ /* Multiple threads may do this for the same 'fm' at the same time.
+ * Allocate ofproto_flow_mod with execution context from stack.
+ *
+ * Note: This copy could be avoided by making ofproto_flow_mod more
+ * complex, but that may not be desireable, and a learn action is not that
+ * fast to begin with. */
+ ofm.fm = *fm;
+ ofproto_flow_mod(&ofproto->up, &ofm);
}
/* Appends 'pin' to the queue of "packet ins" to be sent to the controller.
@@ -5477,28 +5486,28 @@ ofproto_dpif_add_internal_flow(struct ofproto_dpif
*ofproto,
const struct ofpbuf *ofpacts,
struct rule **rulep)
{
- struct ofputil_flow_mod fm;
+ struct ofproto_flow_mod ofm;
struct rule_dpif *rule;
int error;
- fm.match = *match;
- fm.priority = priority;
- fm.new_cookie = htonll(0);
- fm.cookie = htonll(0);
- fm.cookie_mask = htonll(0);
- fm.modify_cookie = false;
- fm.table_id = TBL_INTERNAL;
- fm.command = OFPFC_ADD;
- fm.idle_timeout = idle_timeout;
- fm.hard_timeout = 0;
- fm.importance = 0;
- fm.buffer_id = 0;
- fm.out_port = 0;
- fm.flags = OFPUTIL_FF_HIDDEN_FIELDS | OFPUTIL_FF_NO_READONLY;
- fm.ofpacts = ofpacts->data;
- fm.ofpacts_len = ofpacts->size;
-
- error = ofproto_flow_mod(&ofproto->up, &fm);
+ ofm.fm.match = *match;
+ ofm.fm.priority = priority;
+ ofm.fm.new_cookie = htonll(0);
+ ofm.fm.cookie = htonll(0);
+ ofm.fm.cookie_mask = htonll(0);
+ ofm.fm.modify_cookie = false;
+ ofm.fm.table_id = TBL_INTERNAL;
+ ofm.fm.command = OFPFC_ADD;
+ ofm.fm.idle_timeout = idle_timeout;
+ ofm.fm.hard_timeout = 0;
+ ofm.fm.importance = 0;
+ ofm.fm.buffer_id = 0;
+ ofm.fm.out_port = 0;
+ ofm.fm.flags = OFPUTIL_FF_HIDDEN_FIELDS | OFPUTIL_FF_NO_READONLY;
+ ofm.fm.ofpacts = ofpacts->data;
+ ofm.fm.ofpacts_len = ofpacts->size;
+
+ error = ofproto_flow_mod(&ofproto->up, &ofm);
if (error) {
VLOG_ERR_RL(&rl, "failed to add internal flow (%s)",
ofperr_to_string(error));
@@ -5508,8 +5517,8 @@ ofproto_dpif_add_internal_flow(struct ofproto_dpif
*ofproto,
rule = rule_dpif_lookup_in_table(ofproto,
ofproto_dpif_get_tables_version(ofproto),
- TBL_INTERNAL, &fm.match.flow,
- &fm.match.wc, false);
+ TBL_INTERNAL, &ofm.fm.match.flow,
+ &ofm.fm.match.wc, false);
if (rule) {
*rulep = &rule->up;
} else {
@@ -5522,20 +5531,20 @@ int
ofproto_dpif_delete_internal_flow(struct ofproto_dpif *ofproto,
struct match *match, int priority)
{
- struct ofputil_flow_mod fm;
+ struct ofproto_flow_mod ofm;
int error;
- fm.match = *match;
- fm.priority = priority;
- fm.new_cookie = htonll(0);
- fm.cookie = htonll(0);
- fm.cookie_mask = htonll(0);
- fm.modify_cookie = false;
- fm.table_id = TBL_INTERNAL;
- fm.flags = OFPUTIL_FF_HIDDEN_FIELDS | OFPUTIL_FF_NO_READONLY;
- fm.command = OFPFC_DELETE_STRICT;
-
- error = ofproto_flow_mod(&ofproto->up, &fm);
+ ofm.fm.match = *match;
+ ofm.fm.priority = priority;
+ ofm.fm.new_cookie = htonll(0);
+ ofm.fm.cookie = htonll(0);
+ ofm.fm.cookie_mask = htonll(0);
+ ofm.fm.modify_cookie = false;
+ ofm.fm.table_id = TBL_INTERNAL;
+ ofm.fm.flags = OFPUTIL_FF_HIDDEN_FIELDS | OFPUTIL_FF_NO_READONLY;
+ ofm.fm.command = OFPFC_DELETE_STRICT;
+
+ error = ofproto_flow_mod(&ofproto->up, &ofm);
if (error) {
VLOG_ERR_RL(&rl, "failed to delete internal flow (%s)",
ofperr_to_string(error));
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index bb6df5e..4af21cc 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -168,7 +168,8 @@ void ofproto_dpif_send_packet_in(struct ofproto_dpif *,
struct ofproto_packet_in *);
bool ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *);
int ofproto_dpif_send_packet(const struct ofport_dpif *, struct dp_packet *);
-void ofproto_dpif_flow_mod(struct ofproto_dpif *, struct ofputil_flow_mod *);
+void ofproto_dpif_flow_mod(struct ofproto_dpif *,
+ const struct ofputil_flow_mod *);
struct rule_dpif *ofproto_dpif_refresh_rule(struct rule_dpif *);
struct ofport_dpif *odp_port_to_ofport(const struct dpif_backer *, odp_port_t);
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index e5739b0..ac2f99f 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1755,7 +1755,23 @@ extern const struct ofproto_class ofproto_dpif_class;
int ofproto_class_register(const struct ofproto_class *);
int ofproto_class_unregister(const struct ofproto_class *);
-enum ofperr ofproto_flow_mod(struct ofproto *, struct ofputil_flow_mod *)
+/* flow_mod with execution context. */
+struct ofproto_flow_mod {
+ struct ofputil_flow_mod fm;
+
+ cls_version_t version; /* Version in which changes take
+ * effect. */
+ struct rule_collection old_rules; /* Affected rules. */
+ struct rule_collection new_rules; /* Replacement rules. */
+};
+
+/* port_mod with execution context. */
+struct ofproto_port_mod {
+ struct ofputil_port_mod pm;
+ struct ofport *port; /* Affected port. */
+};
+
+enum ofperr ofproto_flow_mod(struct ofproto *, struct ofproto_flow_mod *)
OVS_EXCLUDED(ofproto_mutex);
void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
const struct ofpact *ofpacts, size_t ofpacts_len)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c794f07..e40a80e 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -260,9 +260,8 @@ static enum ofperr replace_rule_create(struct ofproto *,
struct rule **new_rule)
OVS_REQUIRES(ofproto_mutex);
-static void replace_rule_start(struct ofproto *,
- struct rule *old_rule,
- struct rule *new_rule,
+static void replace_rule_start(struct ofproto *, cls_version_t version,
+ struct rule *old_rule, struct rule *new_rule,
struct cls_conjunction *, size_t n_conjs)
OVS_REQUIRES(ofproto_mutex);
@@ -292,17 +291,15 @@ static bool ofproto_group_exists(const struct ofproto
*ofproto,
OVS_EXCLUDED(ofproto->groups_rwlock);
static enum ofperr add_group(struct ofproto *, struct ofputil_group_mod *);
static void handle_openflow(struct ofconn *, const struct ofpbuf *);
-static enum ofperr do_bundle_flow_mod_start(struct ofproto *,
- struct ofputil_flow_mod *,
- struct ofp_bundle_entry *)
+static enum ofperr ofproto_flow_mod_start(struct ofproto *,
+ struct ofproto_flow_mod *)
OVS_REQUIRES(ofproto_mutex);
-static void do_bundle_flow_mod_finish(struct ofproto *,
- struct ofputil_flow_mod *,
- const struct flow_mod_requester *,
- struct ofp_bundle_entry *)
+static void ofproto_flow_mod_finish(struct ofproto *,
+ struct ofproto_flow_mod *,
+ const struct flow_mod_requester *)
OVS_REQUIRES(ofproto_mutex);
static enum ofperr handle_flow_mod__(struct ofproto *,
- struct ofputil_flow_mod *,
+ struct ofproto_flow_mod *,
const struct flow_mod_requester *)
OVS_EXCLUDED(ofproto_mutex);
static void calc_duration(long long int start, long long int now,
@@ -2057,11 +2054,11 @@ simple_flow_mod(struct ofproto *ofproto,
const struct ofpact *ofpacts, size_t ofpacts_len,
enum ofp_flow_mod_command command)
{
- struct ofputil_flow_mod fm;
+ struct ofproto_flow_mod ofm;
- flow_mod_init(&fm, match, priority, ofpacts, ofpacts_len, command);
+ flow_mod_init(&ofm.fm, match, priority, ofpacts, ofpacts_len, command);
- return handle_flow_mod__(ofproto, &fm, NULL);
+ return handle_flow_mod__(ofproto, &ofm, NULL);
}
/* Adds a flow to OpenFlow flow table 0 in 'p' that matches 'cls_rule' and
@@ -2113,9 +2110,11 @@ ofproto_add_flow(struct ofproto *ofproto, const struct
match *match,
* This is a helper function for in-band control and fail-open and the "learn"
* action. */
enum ofperr
-ofproto_flow_mod(struct ofproto *ofproto, struct ofputil_flow_mod *fm)
+ofproto_flow_mod(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_EXCLUDED(ofproto_mutex)
{
+ struct ofputil_flow_mod *fm = &ofm->fm;
+
/* Optimize for the most common case of a repeated learn action.
* If an identical flow already exists we only need to update its
* 'modified' time. */
@@ -2156,7 +2155,7 @@ ofproto_flow_mod(struct ofproto *ofproto, struct
ofputil_flow_mod *fm)
}
}
- return handle_flow_mod__(ofproto, fm, NULL);
+ return handle_flow_mod__(ofproto, ofm, NULL);
}
/* Searches for a rule with matching criteria exactly equal to 'target' in
@@ -4457,10 +4456,12 @@ get_conjunctions(const struct ofputil_flow_mod *fm,
*
* The caller retains ownership of 'fm->ofpacts'. */
static enum ofperr
-add_flow_start(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- struct rule **old_rule, struct rule **new_rule)
+add_flow_start(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ struct ofputil_flow_mod *fm = &ofm->fm;
+ struct rule **old_rule = &ofm->old_rules.stub[0];
+ struct rule **new_rule = &ofm->new_rules.stub[0];
struct oftable *table;
struct cls_rule cr;
struct rule *rule;
@@ -4506,7 +4507,7 @@ add_flow_start(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
return OFPERR_OFPBRC_EPERM;
}
- cls_rule_init(&cr, &fm->match, fm->priority, ofproto->tables_version + 1);
+ cls_rule_init(&cr, &fm->match, fm->priority, ofm->version);
/* Check for the existence of an identical rule.
* This will not return rules earlier marked for removal. */
@@ -4545,7 +4546,7 @@ add_flow_start(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
}
get_conjunctions(fm, &conjs, &n_conjs);
- replace_rule_start(ofproto, rule, *new_rule, conjs, n_conjs);
+ replace_rule_start(ofproto, ofm->version, rule, *new_rule, conjs, n_conjs);
free(conjs);
return 0;
@@ -4553,10 +4554,13 @@ add_flow_start(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
/* Revert the effects of add_flow_start(). */
static void
-add_flow_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- struct rule *old_rule, struct rule *new_rule)
+add_flow_revert(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ struct ofputil_flow_mod *fm = &ofm->fm;
+ struct rule *old_rule = ofm->old_rules.stub[0];
+ struct rule *new_rule = ofm->new_rules.stub[0];
+
if (old_rule && fm->delete_reason == OFPRR_EVICTION) {
/* Revert the eviction. */
eviction_group_add_rule(old_rule);
@@ -4567,11 +4571,13 @@ add_flow_revert(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
/* To be called after version bump. */
static void
-add_flow_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- const struct flow_mod_requester *req,
- struct rule *old_rule, struct rule *new_rule)
+add_flow_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
+ const struct flow_mod_requester *req)
OVS_REQUIRES(ofproto_mutex)
{
+ struct ofputil_flow_mod *fm = &ofm->fm;
+ struct rule *old_rule = ofm->old_rules.stub[0];
+ struct rule *new_rule = ofm->new_rules.stub[0];
struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
replace_rule_finish(ofproto, fm, req, old_rule, new_rule, &dead_cookies);
@@ -4683,7 +4689,7 @@ replace_rule_create(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
}
static void
-replace_rule_start(struct ofproto *ofproto,
+replace_rule_start(struct ofproto *ofproto, cls_version_t version,
struct rule *old_rule, struct rule *new_rule,
struct cls_conjunction *conjs, size_t n_conjs)
{
@@ -4692,8 +4698,7 @@ replace_rule_start(struct ofproto *ofproto,
/* 'old_rule' may be either an evicted rule or replaced rule. */
if (old_rule) {
/* Mark the old rule for removal in the next version. */
- cls_rule_make_invisible_in_version(&old_rule->cr,
- ofproto->tables_version + 1);
+ cls_rule_make_invisible_in_version(&old_rule->cr, version);
} else {
table->n_flows++;
}
@@ -4792,11 +4797,12 @@ replace_rule_finish(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
}
static enum ofperr
-modify_flows_start__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- struct rule_collection *old_rules,
- struct rule_collection *new_rules)
+modify_flows_start__(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ struct ofputil_flow_mod *fm = &ofm->fm;
+ struct rule_collection *old_rules = &ofm->old_rules;
+ struct rule_collection *new_rules = &ofm->new_rules;
enum ofperr error;
rule_collection_init(new_rules);
@@ -4812,8 +4818,7 @@ modify_flows_start__(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
struct rule *new_rule;
struct cls_rule cr;
- cls_rule_clone_in_version(&cr, &old_rule->cr,
- ofproto->tables_version + 1);
+ cls_rule_clone_in_version(&cr, &old_rule->cr, ofm->version);
error = replace_rule_create(ofproto, fm, &cr, old_rule->table_id,
old_rule, &new_rule);
if (!error) {
@@ -4828,15 +4833,14 @@ modify_flows_start__(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
get_conjunctions(fm, &conjs, &n_conjs);
for (i = 0; i < old_rules->n; i++) {
- replace_rule_start(ofproto, old_rules->rules[i],
+ replace_rule_start(ofproto, ofm->version, old_rules->rules[i],
new_rules->rules[i], conjs, n_conjs);
}
free(conjs);
} else if (!(fm->cookie_mask != htonll(0)
|| fm->new_cookie == OVS_BE64_MAX)) {
/* No match, add a new flow. */
- error = add_flow_start(ofproto, fm, &old_rules->rules[0],
- &new_rules->rules[0]);
+ error = add_flow_start(ofproto, ofm);
if (!error) {
ovs_assert(fm->delete_reason == OFPRR_EVICTION
|| !old_rules->rules[0]);
@@ -4855,11 +4859,11 @@ modify_flows_start__(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
* 'ofconn' is used to retrieve the packet buffer specified in fm->buffer_id,
* if any. */
static enum ofperr
-modify_flows_start_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- struct rule_collection *old_rules,
- struct rule_collection *new_rules)
+modify_flows_start_loose(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ struct ofputil_flow_mod *fm = &ofm->fm;
+ struct rule_collection *old_rules = &ofm->old_rules;
struct rule_criteria criteria;
enum ofperr error;
@@ -4871,7 +4875,7 @@ modify_flows_start_loose(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
rule_criteria_destroy(&criteria);
if (!error) {
- error = modify_flows_start__(ofproto, fm, old_rules, new_rules);
+ error = modify_flows_start__(ofproto, ofm);
}
if (error) {
@@ -4881,14 +4885,15 @@ modify_flows_start_loose(struct ofproto *ofproto,
struct ofputil_flow_mod *fm,
}
static void
-modify_flows_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- struct rule_collection *old_rules,
- struct rule_collection *new_rules)
+modify_flows_revert(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ struct rule_collection *old_rules = &ofm->old_rules;
+ struct rule_collection *new_rules = &ofm->new_rules;
+
/* Old rules were not changed yet, only need to revert new rules. */
if (old_rules->n == 0 && new_rules->n == 1) {
- add_flow_revert(ofproto, fm, old_rules->rules[0], new_rules->rules[0]);
+ add_flow_revert(ofproto, ofm);
} else if (old_rules->n > 0) {
for (size_t i = 0; i < old_rules->n; i++) {
replace_rule_revert(ofproto, old_rules->rules[i],
@@ -4900,15 +4905,16 @@ modify_flows_revert(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
}
static void
-modify_flows_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- const struct flow_mod_requester *req,
- struct rule_collection *old_rules,
- struct rule_collection *new_rules)
+modify_flows_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
+ const struct flow_mod_requester *req)
OVS_REQUIRES(ofproto_mutex)
{
+ struct ofputil_flow_mod *fm = &ofm->fm;
+ struct rule_collection *old_rules = &ofm->old_rules;
+ struct rule_collection *new_rules = &ofm->new_rules;
+
if (old_rules->n == 0 && new_rules->n == 1) {
- add_flow_finish(ofproto, fm, req, old_rules->rules[0],
- new_rules->rules[0]);
+ add_flow_finish(ofproto, ofm, req);
} else if (old_rules->n > 0) {
struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
@@ -4929,11 +4935,11 @@ modify_flows_finish(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
/* Implements OFPFC_MODIFY_STRICT. Returns 0 on success or an OpenFlow error
* code on failure. */
static enum ofperr
-modify_flow_start_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- struct rule_collection *old_rules,
- struct rule_collection *new_rules)
+modify_flow_start_strict(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ struct ofputil_flow_mod *fm = &ofm->fm;
+ struct rule_collection *old_rules = &ofm->old_rules;
struct rule_criteria criteria;
enum ofperr error;
@@ -4947,7 +4953,7 @@ modify_flow_start_strict(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
if (!error) {
/* collect_rules_strict() can return max 1 rule. */
- error = modify_flows_start__(ofproto, fm, old_rules, new_rules);
+ error = modify_flows_start__(ofproto, ofm);
}
if (error) {
@@ -4959,7 +4965,7 @@ modify_flow_start_strict(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
/* OFPFC_DELETE implementation. */
static void
-delete_flows_start__(struct ofproto *ofproto,
+delete_flows_start__(struct ofproto *ofproto, cls_version_t version,
const struct rule_collection *rules)
OVS_REQUIRES(ofproto_mutex)
{
@@ -4968,8 +4974,7 @@ delete_flows_start__(struct ofproto *ofproto,
struct oftable *table = &ofproto->tables[rule->table_id];
table->n_flows--;
- cls_rule_make_invisible_in_version(&rule->cr,
- ofproto->tables_version + 1);
+ cls_rule_make_invisible_in_version(&rule->cr, version);
}
}
@@ -5015,7 +5020,7 @@ delete_flows__(struct rule_collection *rules,
if (rules->n) {
struct ofproto *ofproto = rules->rules[0]->ofproto;
- delete_flows_start__(ofproto, rules);
+ delete_flows_start__(ofproto, ofproto->tables_version + 1, rules);
ofproto_bump_tables_version(ofproto);
delete_flows_finish__(ofproto, rules, reason, req);
ofmonitor_flush(ofproto->connmgr);
@@ -5024,11 +5029,11 @@ delete_flows__(struct rule_collection *rules,
/* Implements OFPFC_DELETE. */
static enum ofperr
-delete_flows_start_loose(struct ofproto *ofproto,
- const struct ofputil_flow_mod *fm,
- struct rule_collection *rules)
+delete_flows_start_loose(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ const struct ofputil_flow_mod *fm = &ofm->fm;
+ struct rule_collection *rules = &ofm->old_rules;
struct rule_criteria criteria;
enum ofperr error;
@@ -5041,17 +5046,18 @@ delete_flows_start_loose(struct ofproto *ofproto,
rule_criteria_destroy(&criteria);
if (!error) {
- delete_flows_start__(ofproto, rules);
+ delete_flows_start__(ofproto, ofm->version, rules);
}
return error;
}
static void
-delete_flows_revert(struct ofproto *ofproto,
- struct rule_collection *rules)
+delete_flows_revert(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ struct rule_collection *rules = &ofm->old_rules;
+
for (size_t i = 0; i < rules->n; i++) {
struct rule *rule = rules->rules[i];
struct oftable *table = &ofproto->tables[rule->table_id];
@@ -5067,21 +5073,22 @@ delete_flows_revert(struct ofproto *ofproto,
static void
delete_flows_finish(struct ofproto *ofproto,
- const struct ofputil_flow_mod *fm,
- const struct flow_mod_requester *req,
- struct rule_collection *rules)
+ struct ofproto_flow_mod *ofm,
+ const struct flow_mod_requester *req)
OVS_REQUIRES(ofproto_mutex)
{
- delete_flows_finish__(ofproto, rules, fm->delete_reason, req);
+ delete_flows_finish__(ofproto, &ofm->old_rules, ofm->fm.delete_reason,
+ req);
}
/* Implements OFPFC_DELETE_STRICT. */
static enum ofperr
delete_flow_start_strict(struct ofproto *ofproto,
- const struct ofputil_flow_mod *fm,
- struct rule_collection *rules)
+ struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
+ const struct ofputil_flow_mod *fm = &ofm->fm;
+ struct rule_collection *rules = &ofm->old_rules;
struct rule_criteria criteria;
enum ofperr error;
@@ -5094,7 +5101,7 @@ delete_flow_start_strict(struct ofproto *ofproto,
rule_criteria_destroy(&criteria);
if (!error) {
- delete_flows_start__(ofproto, rules);
+ delete_flows_start__(ofproto, ofm->version, rules);
}
return error;
@@ -5186,7 +5193,7 @@ handle_flow_mod(struct ofconn *ofconn, const struct
ofp_header *oh)
OVS_EXCLUDED(ofproto_mutex)
{
struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
- struct ofputil_flow_mod fm;
+ struct ofproto_flow_mod ofm;
uint64_t ofpacts_stub[1024 / 8];
struct ofpbuf ofpacts;
enum ofperr error;
@@ -5197,25 +5204,26 @@ handle_flow_mod(struct ofconn *ofconn, const struct
ofp_header *oh)
}
ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
- error = ofputil_decode_flow_mod(&fm, oh, ofconn_get_protocol(ofconn),
+ error = ofputil_decode_flow_mod(&ofm.fm, oh, ofconn_get_protocol(ofconn),
&ofpacts,
u16_to_ofp(ofproto->max_ports),
ofproto->n_tables);
if (!error) {
- error = ofproto_check_ofpacts(ofproto, fm.ofpacts, fm.ofpacts_len);
+ error = ofproto_check_ofpacts(ofproto, ofm.fm.ofpacts,
+ ofm.fm.ofpacts_len);
}
if (!error) {
struct flow_mod_requester req;
req.ofconn = ofconn;
req.request = oh;
- error = handle_flow_mod__(ofproto, &fm, &req);
+ error = handle_flow_mod__(ofproto, &ofm, &req);
}
if (error) {
goto exit_free_ofpacts;
}
- ofconn_report_flow_mod(ofconn, fm.command);
+ ofconn_report_flow_mod(ofconn, ofm.fm.command);
exit_free_ofpacts:
ofpbuf_uninit(&ofpacts);
@@ -5224,18 +5232,18 @@ exit:
}
static enum ofperr
-handle_flow_mod__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+handle_flow_mod__(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
const struct flow_mod_requester *req)
OVS_EXCLUDED(ofproto_mutex)
{
- struct ofp_bundle_entry be;
enum ofperr error;
ovs_mutex_lock(&ofproto_mutex);
- error = do_bundle_flow_mod_start(ofproto, fm, &be);
+ ofm->version = ofproto->tables_version + 1;
+ error = ofproto_flow_mod_start(ofproto, ofm);
if (!error) {
ofproto_bump_tables_version(ofproto);
- do_bundle_flow_mod_finish(ofproto, fm, req, &be);
+ ofproto_flow_mod_finish(ofproto, ofm, req);
}
ofmonitor_flush(ofproto->connmgr);
ovs_mutex_unlock(&ofproto_mutex);
@@ -6458,14 +6466,14 @@ delete_group__(struct ofproto *ofproto, struct ofgroup
*ofgroup)
OVS_RELEASES(ofproto->groups_rwlock)
{
struct match match;
- struct ofputil_flow_mod fm;
+ struct ofproto_flow_mod ofm;
/* Delete all flow entries containing this group in a group action */
match_init_catchall(&match);
- flow_mod_init(&fm, &match, 0, NULL, 0, OFPFC_DELETE);
- fm.delete_reason = OFPRR_GROUP_DELETE;
- fm.out_group = ofgroup->group_id;
- handle_flow_mod__(ofproto, &fm, NULL);
+ 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;
+ handle_flow_mod__(ofproto, &ofm, NULL);
hmap_remove(&ofproto->groups, &ofgroup->hmap_node);
/* No-one can find this group any more. */
@@ -6608,49 +6616,45 @@ handle_table_mod(struct ofconn *ofconn, const struct
ofp_header *oh)
}
static enum ofperr
-do_bundle_flow_mod_start(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- struct ofp_bundle_entry *be)
+ofproto_flow_mod_start(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
- switch (fm->command) {
+ switch (ofm->fm.command) {
case OFPFC_ADD:
- return add_flow_start(ofproto, fm, &be->old_rules.stub[0],
- &be->new_rules.stub[0]);
+ return add_flow_start(ofproto, ofm);
+ /* , &be->old_rules.stub[0],
+ &be->new_rules.stub[0]); */
case OFPFC_MODIFY:
- return modify_flows_start_loose(ofproto, fm, &be->old_rules,
- &be->new_rules);
+ return modify_flows_start_loose(ofproto, ofm);
case OFPFC_MODIFY_STRICT:
- return modify_flow_start_strict(ofproto, fm, &be->old_rules,
- &be->new_rules);
+ return modify_flow_start_strict(ofproto, ofm);
case OFPFC_DELETE:
- return delete_flows_start_loose(ofproto, fm, &be->old_rules);
+ return delete_flows_start_loose(ofproto, ofm);
case OFPFC_DELETE_STRICT:
- return delete_flow_start_strict(ofproto, fm, &be->old_rules);
+ return delete_flow_start_strict(ofproto, ofm);
}
return OFPERR_OFPFMFC_BAD_COMMAND;
}
static void
-do_bundle_flow_mod_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- struct ofp_bundle_entry *be)
+ofproto_flow_mod_revert(struct ofproto *ofproto, struct ofproto_flow_mod *ofm)
OVS_REQUIRES(ofproto_mutex)
{
- switch (fm->command) {
+ switch (ofm->fm.command) {
case OFPFC_ADD:
- add_flow_revert(ofproto, fm, be->old_rules.stub[0],
- be->new_rules.stub[0]);
+ add_flow_revert(ofproto, ofm);
break;
case OFPFC_MODIFY:
case OFPFC_MODIFY_STRICT:
- modify_flows_revert(ofproto, fm, &be->old_rules, &be->new_rules);
+ modify_flows_revert(ofproto, ofm);
break;
case OFPFC_DELETE:
case OFPFC_DELETE_STRICT:
- delete_flows_revert(ofproto, &be->old_rules);
+ delete_flows_revert(ofproto, ofm);
break;
default:
@@ -6659,25 +6663,24 @@ do_bundle_flow_mod_revert(struct ofproto *ofproto,
struct ofputil_flow_mod *fm,
}
static void
-do_bundle_flow_mod_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- const struct flow_mod_requester *req,
- struct ofp_bundle_entry *be)
+ofproto_flow_mod_finish(struct ofproto *ofproto,
+ struct ofproto_flow_mod *ofm,
+ const struct flow_mod_requester *req)
OVS_REQUIRES(ofproto_mutex)
{
- switch (fm->command) {
+ switch (ofm->fm.command) {
case OFPFC_ADD:
- add_flow_finish(ofproto, fm, req, be->old_rules.stub[0],
- be->new_rules.stub[0]);
+ add_flow_finish(ofproto, ofm, req);
break;
case OFPFC_MODIFY:
case OFPFC_MODIFY_STRICT:
- modify_flows_finish(ofproto, fm, req, &be->old_rules, &be->new_rules);
+ modify_flows_finish(ofproto, ofm, req);
break;
case OFPFC_DELETE:
case OFPFC_DELETE_STRICT:
- delete_flows_finish(ofproto, fm, req, &be->old_rules);
+ delete_flows_finish(ofproto, ofm, req);
break;
default:
@@ -6706,7 +6709,7 @@ static enum ofperr
do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
{
struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
- cls_version_t visible_version = ofproto->tables_version;
+ cls_version_t version = ofproto->tables_version + 1;
struct ofp_bundle *bundle;
struct ofp_bundle_entry *be;
enum ofperr error;
@@ -6732,26 +6735,25 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
error = OFPERR_OFPBFC_MSG_FAILED;
} else {
prev_is_port_mod = true;
- error = port_mod_start(ofconn, &be->pm, &be->port);
+ error = port_mod_start(ofconn, &be->opm.pm, &be->opm.port);
}
} else if (be->type == OFPTYPE_FLOW_MOD) {
/* Flow mods between port mods are applied as a single
* version, but the versions are published only after
* we know the commit is successful. */
if (prev_is_port_mod) {
- ++ofproto->tables_version;
+ ++version;
}
prev_is_port_mod = false;
- error = do_bundle_flow_mod_start(ofproto, &be->fm, be);
+ /* Store the version in which the changes should take
+ * effect. */
+ be->ofm.version = version;
+ error = ofproto_flow_mod_start(ofproto, &be->ofm);
} else {
OVS_NOT_REACHED();
}
if (error) {
break;
- } else {
- /* Store the version in which the changes should take
- * effect. */
- be->version = ofproto->tables_version + 1;
}
}
@@ -6765,25 +6767,26 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
/* 2. Revert. Undo all the changes made above. */
LIST_FOR_EACH_REVERSE_CONTINUE(be, node, &bundle->msg_list) {
if (be->type == OFPTYPE_FLOW_MOD) {
- do_bundle_flow_mod_revert(ofproto, &be->fm, be);
+ ofproto_flow_mod_revert(ofproto, &be->ofm);
}
/* Nothing needs to be reverted for a port mod. */
}
} else {
/* 4. Finish. */
LIST_FOR_EACH (be, node, &bundle->msg_list) {
- /* Bump the lookup version to the one of the current message.
- * This makes all the changes in the bundle at this version
- * visible to lookups at once. */
- if (visible_version < be->version) {
- visible_version = be->version;
- ofproto->ofproto_class->set_tables_version(
- ofproto, visible_version);
- }
if (be->type == OFPTYPE_FLOW_MOD) {
struct flow_mod_requester req = { ofconn, be->ofp_msg };
- do_bundle_flow_mod_finish(ofproto, &be->fm, &req, be);
+ /* Bump the lookup version to the one of the current
+ * message. This makes all the changes in the bundle at
+ * this version visible to lookups at once. */
+ if (ofproto->tables_version < be->ofm.version) {
+ ofproto->tables_version = be->ofm.version;
+ ofproto->ofproto_class->set_tables_version(
+ ofproto, ofproto->tables_version);
+ }
+
+ ofproto_flow_mod_finish(ofproto, &be->ofm, &req);
} else if (be->type == OFPTYPE_PORT_MOD) {
/* Perform the actual port mod. This is not atomic, i.e.,
* the effects will be immediately seen by upcall
@@ -6791,14 +6794,11 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
* be noted that port configuration changes can originate
* also from OVSDB changes asynchronously to all upcall
* processing. */
- port_mod_finish(ofconn, &be->pm, be->port);
+ port_mod_finish(ofconn, &be->opm.pm, be->opm.port);
}
}
}
- /* Reset the tables_version. */
- ofproto->tables_version = visible_version;
-
ofmonitor_flush(ofproto->connmgr);
ovs_mutex_unlock(&ofproto_mutex);
@@ -6885,23 +6885,23 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
bmsg = ofp_bundle_entry_alloc(type, badd.msg);
if (type == OFPTYPE_PORT_MOD) {
- error = ofputil_decode_port_mod(badd.msg, &bmsg->pm, false);
+ error = ofputil_decode_port_mod(badd.msg, &bmsg->opm.pm, false);
} else if (type == OFPTYPE_FLOW_MOD) {
struct ofpbuf ofpacts;
uint64_t ofpacts_stub[1024 / 8];
ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
- error = ofputil_decode_flow_mod(&bmsg->fm, badd.msg,
+ error = ofputil_decode_flow_mod(&bmsg->ofm.fm, badd.msg,
ofconn_get_protocol(ofconn),
&ofpacts,
u16_to_ofp(ofproto->max_ports),
ofproto->n_tables);
/* Move actions to heap. */
- bmsg->fm.ofpacts = ofpbuf_steal_data(&ofpacts);
+ bmsg->ofm.fm.ofpacts = ofpbuf_steal_data(&ofpacts);
- if (!error && bmsg->fm.ofpacts_len) {
- error = ofproto_check_ofpacts(ofproto, bmsg->fm.ofpacts,
- bmsg->fm.ofpacts_len);
+ 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();
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
