Add support for port mods in an OpenFlow 1.4 bundle, as required for
the minimum support level by the OpenFlow 1.4 specification. If the
bundle includes port mods, it may not specify the OFPBF_ATOMIC flag.
Port mods and flow mods in a bundle are always applied in order and
the consecutive flow mods between port mods are made available to
lookups atomically.
Note that ovs-ofctl does not support creating bundles with port mods.
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
NEWS | 11 +++---
OPENFLOW-1.1+.md | 6 +++
ofproto/bundles.h | 12 ++++--
ofproto/ofproto.c | 106 +++++++++++++++++++++++++++++++++++++++--------------
4 files changed, 99 insertions(+), 36 deletions(-)
diff --git a/NEWS b/NEWS
index f171cfc..c3524b6 100644
--- a/NEWS
+++ b/NEWS
@@ -32,11 +32,12 @@ Post-v2.3.0
commands are now redundant and will be removed in a future
release. See ovs-vswitchd(8) for details.
- OpenFlow:
- * OpenFlow 1.4 bundles are now supported, but for flow mod
- messages only. Both 'atomic' and 'ordered' bundle flags are
- trivially supported, as all bundled messages are executed in
- the order they were added and all flow table modifications are
- now atomic to the datapath.
+ * OpenFlow 1.4 bundles are now supported for flow mods and port
+ mods. For flow mods, both 'atomic' and 'ordered' bundle flags
+ are trivially supported, as all bundled messages are executed
+ in the order they were added and all flow table modifications
+ are now atomic to the datapath. Port mods may not appear in
+ atomic bundles, as port status modifications are not atomic.
* IPv6 flow label and neighbor discovery fields are now modifiable.
* OpenFlow 1.5 extended registers are now supported.
* The OpenFlow 1.5 actset_output field is now supported.
diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md
index 7911406..0b2f0f9 100644
--- a/OPENFLOW-1.1+.md
+++ b/OPENFLOW-1.1+.md
@@ -148,6 +148,12 @@ parallel in OVS.
Transactional modification. OpenFlow 1.4 requires to support
flow_mods and port_mods in a bundle if bundle is supported.
(Not related to OVS's 'ofbundle' stuff.)
+ Implemented as an OpenFlow 1.4 feature. Only flow_mods and
+ port_mods are supported in a bundle. If the bundle includes port
+ mods, it may not specify the OFPBF_ATOMIC flag. Nevertheless,
+ port mods and flow mods in a bundle are always applied in order
+ and consecutive flow mods between port mods are made available to
+ lookups atomically.
[EXT-230]
[optional for OF1.4+]
diff --git a/ofproto/bundles.h b/ofproto/bundles.h
index 98cb2ba..3723a5d 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -34,14 +34,17 @@ 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_miniflow_mod fm; /* 'fm.ofpacts' must be malloced. */
struct ofputil_port_mod pm;
};
/* Used during commit. */
+ struct ofport *port; /* Affected port. */
struct rule_collection old_rules; /* Affected rules. */
- struct rule_collection new_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))];
@@ -63,7 +66,7 @@ struct ofp_bundle {
};
static inline struct ofp_bundle_entry *ofp_bundle_entry_alloc(
- enum ofptype type, const struct ofp_header *oh);
+ enum ofptype type, long long version, const struct ofp_header *oh);
static inline void ofp_bundle_entry_free(struct ofp_bundle_entry *);
enum ofperr ofp_bundle_open(struct ofconn *, uint32_t id, uint16_t flags);
@@ -75,7 +78,8 @@ enum ofperr ofp_bundle_add_message(struct ofconn *, uint32_t
id,
void ofp_bundle_remove__(struct ofconn *, struct ofp_bundle *, bool success);
static inline struct ofp_bundle_entry *
-ofp_bundle_entry_alloc(enum ofptype type, const struct ofp_header *oh)
+ofp_bundle_entry_alloc(enum ofptype type, long long version,
+ const struct ofp_header *oh)
{
struct ofp_bundle_entry *entry = xmalloc(sizeof *entry);
@@ -83,6 +87,8 @@ ofp_bundle_entry_alloc(enum ofptype type, const struct
ofp_header *oh)
if (type == OFPTYPE_FLOW_MOD) {
minimatch_init_catchall(&entry->fm.match);
}
+ entry->version = version;
+
/* Max 64 bytes for error reporting. */
memcpy(entry->ofp_msg, oh, MIN(ntohs(oh->length), sizeof entry->ofp_msg));
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index a07b0fe..7df7bef 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3436,9 +3436,34 @@ update_port_config(struct ofconn *ofconn, struct ofport
*port,
}
static enum ofperr
-handle_port_mod(struct ofconn *ofconn, const struct ofp_header *oh)
+port_mod_start(struct ofconn *ofconn, struct ofputil_port_mod *pm,
+ struct ofport **port)
{
struct ofproto *p = ofconn_get_ofproto(ofconn);
+
+ *port = ofproto_get_port(p, pm->port_no);
+ if (!*port) {
+ return OFPERR_OFPPMFC_BAD_PORT;
+ }
+ if (!eth_addr_equals((*port)->pp.hw_addr, pm->hw_addr)) {
+ return OFPERR_OFPPMFC_BAD_HW_ADDR;
+ }
+ return 0;
+}
+
+static void
+port_mod_finish(struct ofconn *ofconn, struct ofputil_port_mod *pm,
+ struct ofport *port)
+{
+ update_port_config(ofconn, port, pm->config, pm->mask);
+ if (pm->advertise) {
+ netdev_set_advertisements(port->netdev, pm->advertise);
+ }
+}
+
+static enum ofperr
+handle_port_mod(struct ofconn *ofconn, const struct ofp_header *oh)
+{
struct ofputil_port_mod pm;
struct ofport *port;
enum ofperr error;
@@ -3453,18 +3478,11 @@ handle_port_mod(struct ofconn *ofconn, const struct
ofp_header *oh)
return error;
}
- port = ofproto_get_port(p, pm.port_no);
- if (!port) {
- return OFPERR_OFPPMFC_BAD_PORT;
- } else if (!eth_addr_equals(port->pp.hw_addr, pm.hw_addr)) {
- return OFPERR_OFPPMFC_BAD_HW_ADDR;
- } else {
- update_port_config(ofconn, port, pm.config, pm.mask);
- if (pm.advertise) {
- netdev_set_advertisements(port->netdev, pm.advertise);
- }
+ error = port_mod_start(ofconn, &pm, &port);
+ if (!error) {
+ port_mod_finish(ofconn, &pm, port);
}
- return 0;
+ return error;
}
static enum ofperr
@@ -6723,12 +6741,10 @@ do_bundle_flow_mod_finish(struct ofproto *ofproto,
* possible. No visible changes were made, so rollback is minimal (remove
* added invisible rules, restore visibility of rules marked for removal).
*
- * 3. Bump the version visible to lookups.
- *
- * 4. Finish: Insert replacement rules to the ofproto provider. Remove replaced
- * and deleted rules from ofproto data structures, and Schedule postponed
- * removal of deleted rules from the classifier. Send notifications, buffered
- * packets, etc.
+ * 3. Finish: Make the changes visible for lookups. Insert replacement rules to
+ * the ofproto provider. Remove replaced and deleted rules from ofproto data
+ * structures, and Schedule postponed removal of deleted rules from the
+ * classifier. Send notifications, buffered packets, etc.
*/
static enum ofperr
do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
@@ -6736,6 +6752,7 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
struct ofp_bundle *bundle;
struct ofp_bundle_entry *be;
+ long long start_version = ofproto->tables_version;
enum ofperr error;
bundle = ofconn_get_bundle(ofconn, id);
@@ -6746,15 +6763,30 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
if (bundle->flags != flags) {
error = OFPERR_OFPBFC_BAD_FLAGS;
} else {
+ bool prev_is_port_mod = false;
+
error = 0;
ovs_mutex_lock(&ofproto_mutex);
/* 1. Begin. */
LIST_FOR_EACH (be, node, &bundle->msg_list) {
+
if (be->type == OFPTYPE_PORT_MOD) {
- /* Not supported yet. */
- error = OFPERR_OFPBFC_MSG_FAILED;
+ /* Our port mods are not atomic. */
+ if (flags & OFPBF_ATOMIC) {
+ error = OFPERR_OFPBFC_MSG_FAILED;
+ } else {
+ prev_is_port_mod = true;
+ error = port_mod_start(ofconn, &be->pm, &be->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;
+ }
+ prev_is_port_mod = false;
error = do_bundle_flow_mod_start(ofproto, &be->fm, be);
} else {
OVS_NOT_REACHED();
@@ -6763,6 +6795,13 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
break;
}
}
+
+ /* Restore the tables_version. In a successful commit we will publish
+ * the versions used explicitly. In a failed commit we must restore
+ * 'ofproto->tables_version' before any of the reverts, as they depend
+ * on the original value. */
+ ofproto->tables_version = start_version;
+
if (error) {
/* Send error referring to the original message. */
if (error) {
@@ -6775,21 +6814,31 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
if (be->type == OFPTYPE_FLOW_MOD) {
do_bundle_flow_mod_revert(ofproto, &be->fm, be);
}
+ /* Nothing needs to be reverted for a port mod. */
}
} else {
- /* 3. Bump the version. This makes all the changes in the bundle
- * visible to the lookups at once. For this to work an upcall must
- * read the tables_version once at the beginning and keep using the
- * same version number for the whole duration of the upcall
- * processing. */
- ofproto_bump_tables_version(ofproto);
-
/* 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 (ofproto->tables_version < be->version) {
+ ofproto->tables_version = be->version;
+ ofproto->ofproto_class->set_tables_version(
+ ofproto, ofproto->tables_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);
+ } 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
+ * processing regardless of the lookup version. It should
+ * 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);
}
}
}
@@ -6860,6 +6909,7 @@ handle_bundle_control(struct ofconn *ofconn, const struct
ofp_header *oh)
static enum ofperr
handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh)
{
+ struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
struct ofputil_bundle_add_msg badd;
struct ofp_bundle_entry *be;
enum ofptype type;
@@ -6875,7 +6925,7 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
return error;
}
- be = ofp_bundle_entry_alloc(type, badd.msg);
+ be = ofp_bundle_entry_alloc(type, ofproto->tables_version + 1, badd.msg);
if (type == OFPTYPE_PORT_MOD) {
error = ofputil_decode_port_mod(badd.msg, &be->pm, false);
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
