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 | 7 ++- ofproto/ofproto-provider.h | 3 +- ofproto/ofproto.c | 112 ++++++++++++++++++++++++++++++++------------ 5 files changed, 101 insertions(+), 38 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..ba4b1b1 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))]; @@ -83,6 +86,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 = 0; + /* Max 64 bytes for error reporting. */ memcpy(entry->ofp_msg, oh, MIN(ntohs(oh->length), sizeof entry->ofp_msg)); diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 335c843..f78c051 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -93,8 +93,9 @@ struct ofproto { long long int eviction_group_timer; /* For rate limited reheapification. */ struct oftable *tables; int n_tables; - long long tables_version; /* Controls which rules are visible to + long long visible_version; /* Controls which rules are visible to * table lookups. */ + long long tables_version; /* Used during commits. */ /* Rules indexed on their cookie values, in all flow tables. */ struct hindex cookies OVS_GUARDED_BY(ofproto_mutex); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index ca7be4f..d69629d 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -498,9 +498,9 @@ ofproto_enumerate_names(const char *type, struct sset *names) static void ofproto_bump_tables_version(struct ofproto *ofproto) { - ++ofproto->tables_version; + ofproto->visible_version = ++ofproto->tables_version; ofproto->ofproto_class->set_tables_version(ofproto, - ofproto->tables_version); + ofproto->visible_version); } int @@ -557,6 +557,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type, ofproto->tables = NULL; ofproto->n_tables = 0; ofproto->tables_version = CLS_MIN_VERSION; + ofproto->visible_version = 0; hindex_init(&ofproto->cookies); hmap_init(&ofproto->learned_cookies); list_init(&ofproto->expirable); @@ -3444,9 +3445,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; @@ -3461,18 +3487,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 @@ -4706,7 +4725,7 @@ replace_rule_start(struct ofproto *ofproto, /* Mark the old rule for removal in the next version. */ cls_rule_make_invisible_in_version(&old_rule->cr, ofproto->tables_version + 1, - ofproto->tables_version); + ofproto->visible_version); } else { table->n_flows++; } @@ -4987,7 +5006,7 @@ delete_flows_start__(struct ofproto *ofproto, table->n_flows--; cls_rule_make_invisible_in_version(&rule->cr, ofproto->tables_version + 1, - ofproto->tables_version); + ofproto->visible_version); } } @@ -6734,12 +6753,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) @@ -6757,23 +6774,42 @@ 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(); } if (error) { break; + } else { + /* Store the version in which the changes should take + * effect. */ + be->version = ofproto->tables_version + 1; } } + if (error) { /* Send error referring to the original message. */ if (error) { @@ -6786,24 +6822,38 @@ 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->visible_version < be->version) { + ofproto->visible_version = be->version; + ofproto->ofproto_class->set_tables_version( + ofproto, 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); + } 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); } } } + + /* Reset the tables_version. */ + ofproto->tables_version = ofproto->visible_version; + ofmonitor_flush(ofproto->connmgr); ovs_mutex_unlock(&ofproto_mutex); -- 1.7.10.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev