diff options
author | Ben Pfaff <blp@ovn.org> | 2018-08-29 11:30:13 -0700 |
---|---|---|
committer | Ben Pfaff <blp@ovn.org> | 2019-01-15 16:59:37 -0800 |
commit | 4e413ac88d4841d7e17de3e36bba3fa12796e938 (patch) | |
tree | f2b60dfccac0bec0393e261721cfa78f99458a98 /ofproto | |
parent | d1a227ecef81773a748320cee733334d21b484ce (diff) | |
download | openvswitch-4e413ac88d4841d7e17de3e36bba3fa12796e938.tar.gz |
ovs-vswitchd: Implement OFPT_TABLE_FEATURES table modification request.
This allows a controller to change the name of OpenFlow flow tables in the
OVS software switch.
CC: Brad Cowie <brad@cowie.nz>
Acked-by: Justin Pettit <jpettit@ovn.org>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Diffstat (limited to 'ofproto')
-rw-r--r-- | ofproto/ofproto-dpif.c | 1 | ||||
-rw-r--r-- | ofproto/ofproto-provider.h | 19 | ||||
-rw-r--r-- | ofproto/ofproto.c | 284 |
3 files changed, 279 insertions, 25 deletions
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 2dc5778e1..14fe6fc8a 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -6029,6 +6029,7 @@ const struct ofproto_class ofproto_dpif_class = { type_get_memory_usage, flush, query_tables, + NULL, /* modify_tables */ set_tables_version, port_alloc, port_construct, diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 4fd8cb14e..074edfc11 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -221,6 +221,7 @@ struct oftable { enum oftable_flags flags; struct classifier cls; /* Contains "struct rule"s. */ char *name; /* Table name exposed via OpenFlow, or NULL. */ + int name_level; /* 0=name unset, 1=via OF, 2=via OVSDB. */ /* Maximum number of flows or UINT_MAX if there is no limit besides any * limit imposed by resource limitations. */ @@ -934,6 +935,24 @@ struct ofproto_class { struct ofputil_table_features *features, struct ofputil_table_stats *stats); + /* Helper for the OFPT_TABLE_FEATURES request. + * + * A controller is requesting that the table features be updated from 'old' + * to 'new', where 'old' is the features currently in use as previously + * initialized by 'query_tables'. + * + * If this function is nonnull, then it should either update the table + * features or return an OpenFlow error. The update should be + * all-or-nothing. + * + * If this function is null, then only updates that eliminate table + * features will be allowed. Such updates have no actual effect. This + * implementation is acceptable because OpenFlow says that a table's + * features may be a superset of those requested. */ + enum ofperr (*modify_tables)(struct ofproto *ofproto, + const struct ofputil_table_features *old, + const struct ofputil_table_features *new); + /* Sets the current tables version the provider should use for classifier * lookups. This must be called with a new version number after each set * of flow table changes has been completed, so that datapath revalidation diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index bdc3ac684..d60decad8 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -85,7 +85,9 @@ const enum mf_field_id default_prefix_fields[2] = static void oftable_init(struct oftable *); static void oftable_destroy(struct oftable *); -static void oftable_set_name(struct oftable *, const char *name); +static void oftable_set_name(struct oftable *, const char *name, int level); +static bool oftable_may_set_name(const struct oftable *, + const char *name, int level); static enum ofperr evict_rules_from_table(struct oftable *) OVS_REQUIRES(ofproto_mutex); @@ -1473,7 +1475,7 @@ ofproto_configure_table(struct ofproto *ofproto, int table_id, ovs_assert(table_id >= 0 && table_id < ofproto->n_tables); table = &ofproto->tables[table_id]; - oftable_set_name(table, s->name); + oftable_set_name(table, s->name, 2); if (table->flags & OFTABLE_READONLY) { return; @@ -3249,6 +3251,8 @@ query_tables(struct ofproto *ofproto, atomic_read_relaxed(&ofproto->tables[i].miss_config, &f->miss_config); f->max_entries = 1000000; + f->any_properties = true; + bool more_tables = false; for (int j = i + 1; j < ofproto->n_tables; j++) { if (!(ofproto->tables[j].flags & OFTABLE_HIDDEN)) { @@ -3784,33 +3788,237 @@ handle_table_stats_request(struct ofconn *ofconn, return 0; } -static enum ofperr -handle_table_features_request(struct ofconn *ofconn, - const struct ofp_header *request) +/* OpenFlow 1.5.1 section 7.3.5.18.1 "Table Features request and reply" + * says: + * + * If a table feature included in the request has an empty list of + * properties, the list of properties for that flow table is unchanged and + * only the other features of that flow table are updated. + * + * This function copies the "list of properties" from '*src' to '*dst'. */ +static void +copy_properties(struct ofputil_table_features *dst, + const struct ofputil_table_features *src) +{ + dst->any_properties = src->any_properties; + if (src->any_properties) { + dst->nonmiss = src->nonmiss; + dst->miss = src->miss; + dst->match = src->match; + dst->mask = src->mask; + dst->wildcard = src->wildcard; + } +} + +/* Attempts to change the table features of the ofproto backing 'ofconn' to + * those specified in the table features request in 'msgs', given that the + * features are currently those in 'old'. + * + * Returns 0 if successful, an OpenFlow error if the caller should send an + * error message for the request as a whole, or -1 if the function already sent + * an error message for some message in 'msgs'. */ +static int +handle_table_features_change(struct ofconn *ofconn, + const struct ovs_list *msgs, + const struct ofputil_table_features old[]) { struct ofproto *ofproto = ofconn_get_ofproto(ofconn); - struct ofpbuf msg = ofpbuf_const_initializer(request, - ntohs(request->length)); - ofpraw_pull_assert(&msg); - if (msg.size || ofpmp_more(request)) { + + enum ofp15_table_features_command command = OFPTFC15_REPLACE; + struct ofputil_table_features new[255]; + + unsigned long int seen[BITMAP_N_LONGS(255)]; + memset(seen, 0, sizeof seen); + + struct ofpbuf *msg; + int n = 0; + LIST_FOR_EACH (msg, list_node, msgs) { + for (;;) { + struct ofputil_table_features tf; + int retval = ofputil_decode_table_features(msg, &tf, NULL); + if (retval == EOF) { + break; + } else if (retval) { + ofconn_send_error(ofconn, msg->header, retval); + return -1; + } + + /* Get command from first request. */ + if (!n) { + command = tf.command; + } + n++; + + /* Avoid duplicate tables. */ + if (bitmap_is_set(seen, tf.table_id)) { + VLOG_INFO_RL(&rl, "duplicate table %"PRIu8, tf.table_id); + ofconn_send_error(ofconn, msg->header, + OFPERR_NXTFFC_DUP_TABLE); + return -1; + } + bitmap_set1(seen, tf.table_id); + + /* Save table. */ + new[tf.table_id] = tf; + } + } + + if (!n) { + return 0; + } + + for (size_t i = 0; i < ofproto->n_tables; i++) { + if (ofproto->tables[i].flags & OFTABLE_HIDDEN) { + if (bitmap_is_set(seen, i)) { + VLOG_INFO_RL(&rl, "can't modify hidden table %"PRIuSIZE, i); + return OFPERR_OFPTFFC_EPERM; + } + + new[i] = old[i]; + bitmap_set1(seen, i); + } + } + + switch (command) { + case OFPTFC15_REPLACE: + break; + + case OFPTFC15_MODIFY: + for (size_t i = 0; i < ofproto->n_tables; i++) { + if (!bitmap_is_set(seen, i)) { + new[i] = old[i]; + bitmap_set1(seen, i); + } else if (!new[i].any_properties) { + copy_properties(&new[i], &old[i]); + } + } + break; + + case OFPTFC15_ENABLE: + case OFPTFC15_DISABLE: + /* It really isn't clear what these commands are supposed to do in an + * Open vSwitch context. OVS doesn't have a concept of tables that + * exist but are not in the pipeline, and OVS table ids are always + * sequential from 0. */ return OFPERR_OFPTFFC_EPERM; } + /* Make sure that the new number of tables is the same as the old number, + * because we don't support changing the number of tables or disabling + * tables. */ + int n_tables = bitmap_scan(seen, 0, 0, 255); + bool skipped_tables = bitmap_scan(seen, 1, n_tables, 255) != 255; + if (n_tables != ofproto->n_tables || skipped_tables) { + if (skipped_tables) { + VLOG_INFO_RL(&rl, "can't disable table %d", n_tables); + } else { + VLOG_INFO_RL(&rl, "can't change number of tables from %d to %d", + ofproto->n_tables, n_tables); + } + return (n_tables > ofproto->n_tables + ? OFPERR_OFPTFFC_TOO_MANY + : OFPERR_OFPTFFC_EPERM); + } + + /* OpenFlow 1.5.1 section 7.3.5.18.1 "Table Features request and reply" + * says: + * + * "All fields in ofp_table_features may be requested to be changed by + * the controller with the exception of the max_entries field, this is + * read only and returned by the switch." + * + * so forbid the controller from attempting to change it. + * + * (This seems like a particularly arbitrary prohibition since OVS could + * easily implement such a feature. Whatever.) */ + for (size_t i = 0; i < n_tables; i++) { + if (old[i].max_entries != new[i].max_entries) { + VLOG_INFO_RL(&rl, "can't change max_entries"); + return OFPERR_OFPTFFC_EPERM; + } + } + + /* Check that we can set table names. */ + for (size_t i = 0; i < n_tables; i++) { + if (!oftable_may_set_name(&ofproto->tables[i], new[i].name, 1)) { + const char *name = ofproto->tables[i].name; + VLOG_INFO_RL(&rl, "can't change name of table %"PRIuSIZE" " + "to %s because it is already set to %s via OVSDB", + i, new[i].name, name ? name : "\"\""); + return OFPERR_OFPTFFC_EPERM; + } + } + + /* Ask the provider to update its features. + * + * If the provider can't update features, just make sure that the + * controller isn't asking to enable new features. OpenFlow says it's OK + * if a superset of the requested features are actually enabled. */ + if (ofproto->ofproto_class->modify_tables) { + enum ofperr error = ofproto->ofproto_class->modify_tables(ofproto, + old, new); + if (error) { + VLOG_INFO_RL(&rl, "can't change table features"); + return error; + } + } else { + for (size_t i = 0; i < n_tables; i++) { + if (!ofputil_table_features_are_superset(&old[i], &new[i])) { + VLOG_INFO_RL(&rl, "can't increase table features " + "for table %"PRIuSIZE, i); + return OFPERR_OFPTFFC_EPERM; + } + } + } + + /* Update table names. */ + for (size_t i = 0; i < n_tables; i++) { + oftable_set_name(&ofproto->tables[i], new[i].name, 1); + } + + return 0; +} + +static void +handle_table_features_request(struct ofconn *ofconn, + const struct ovs_list *msgs) +{ + struct ofproto *ofproto = ofconn_get_ofproto(ofconn); + + struct ofpbuf *msg = ofpbuf_from_list(ovs_list_back(msgs)); + const struct ofp_header *request = msg->data; + ofpraw_pull_assert(msg); + + /* Update the table features configuration, if requested. */ struct ofputil_table_features *features; query_tables(ofproto, &features, NULL); + if (!ovs_list_is_singleton(msgs) || msg->size) { + int retval = handle_table_features_change(ofconn, msgs, features); + free(features); + if (retval) { + if (retval < 0) { + /* handle_table_features_change() already sent an error. */ + } else { + ofconn_send_error(ofconn, request, retval); + } + return; + } + + /* Features may have changed, re-query. */ + query_tables(ofproto, &features, NULL); + } + /* Reply the controller with the table configuration. */ struct ovs_list replies; ofpmp_init(&replies, request); for (size_t i = 0; i < ofproto->n_tables; i++) { if (!(ofproto->tables[i].flags & OFTABLE_HIDDEN)) { - ofputil_append_table_features_reply(&features[i], &replies); + ofputil_append_table_features(&features[i], NULL, &replies); } } ofconn_send_replies(ofconn, &replies); free(features); - - return 0; } /* Returns the vacancy of 'oftable', a number that ranges from 0 (if the table @@ -8195,7 +8403,7 @@ handle_single_part_openflow(struct ofconn *ofconn, const struct ofp_header *oh, return handle_table_stats_request(ofconn, oh); case OFPTYPE_TABLE_FEATURES_STATS_REQUEST: - return handle_table_features_request(ofconn, oh); + OVS_NOT_REACHED(); case OFPTYPE_TABLE_DESC_REQUEST: return handle_table_desc_request(ofconn, oh); @@ -8306,7 +8514,9 @@ handle_openflow(struct ofconn *ofconn, const struct ovs_list *msgs) enum ofptype type; enum ofperr error = ofptype_decode(&type, msg->data); if (!error) { - if (!ovs_list_is_short(msgs)) { + if (type == OFPTYPE_TABLE_FEATURES_STATS_REQUEST) { + handle_table_features_request(ofconn, msgs); + } else if (!ovs_list_is_short(msgs)) { error = OFPERR_OFPBRC_BAD_STAT; } else { error = handle_single_part_openflow(ofconn, msg->data, type); @@ -8631,26 +8841,50 @@ oftable_destroy(struct oftable *table) free(table->name); } -/* Changes the name of 'table' to 'name'. If 'name' is NULL or the empty - * string, then 'table' will use its default name. +/* Changes the name of 'table' to 'name'. Null or empty string 'name' unsets + * the name. + * + * 'level' should be 1 if the name is being set via OpenFlow, or 2 if the name + * is being set via OVSDB. Higher levels get precedence. * * This only affects the name exposed for a table exposed through the OpenFlow * OFPST_TABLE (as printed by "ovs-ofctl dump-tables"). */ static void -oftable_set_name(struct oftable *table, const char *name) -{ - if (name && name[0]) { - int len = strnlen(name, OFP_MAX_TABLE_NAME_LEN); - if (!table->name || strncmp(name, table->name, len)) { +oftable_set_name(struct oftable *table, const char *name, int level) +{ + int len = name ? strnlen(name, OFP_MAX_TABLE_NAME_LEN) : 0; + if (level >= table->name_level) { + if (name) { + if (name[0]) { + if (!table->name || strncmp(name, table->name, len)) { + free(table->name); + table->name = xmemdup0(name, len); + } + } else { + free(table->name); + table->name = NULL; + } + table->name_level = level; + } else if (table->name_level == level) { free(table->name); - table->name = xmemdup0(name, len); + table->name = NULL; + table->name_level = 0; } - } else { - free(table->name); - table->name = NULL; } } +/* Returns true if oftable_set_name(table, name, level) would be effective, + * false otherwise. */ +static bool +oftable_may_set_name(const struct oftable *table, const char *name, int level) +{ + return (level >= table->name_level + || !name + || !table->name + || !strncmp(name, table->name, + strnlen(name, OFP_MAX_TABLE_NAME_LEN))); +} + /* oftables support a choice of two policies when adding a rule would cause the * number of flows in the table to exceed the configured maximum number: either * they can refuse to add the new flow or they can evict some existing flow. |