@@ -2,6 +2,10 @@ Post-v2.10.0
---------------------
- Linux datapath:
* Support for the kernel versions 4.16.x and 4.17.x.
+ - OpenFlow:
+ * OFPMP_TABLE_FEATURES_REQUEST can now modify table features.
+ - ovs-ofctl:
+ * "mod-table" command can now change OpenFlow table names.
- The environment variable OVS_SYSLOG_METHOD, if set, is now used
as the default syslog method.
- The environment variable OVS_CTL_TIMEOUT, if set, is now used
@@ -674,6 +674,9 @@ enum ofperr {
/* OF1.5+(13,10). Can't handle this many flow tables. */
OFPERR_OFPTFFC_TOO_MANY,
+ /* NX1.3+(44). Table specified multiple times. */
+ OFPERR_NXTFFC_DUP_TABLE,
+
/* ## ------------------ ## */
/* ## OFPET_BAD_PROPERTY ## */
@@ -422,7 +422,7 @@ enum ofpraw {
/* OFPST 1.3+ (11): struct ofp13_meter_features. */
OFPRAW_OFPST13_METER_FEATURES_REPLY,
- /* OFPST 1.3+ (12): void. */
+ /* OFPST 1.3+ (12): uint8_t[8][]. */
OFPRAW_OFPST13_TABLE_FEATURES_REQUEST,
/* OFPST 1.3+ (12): struct ofp13_table_features, uint8_t[8][]. */
@@ -155,7 +155,7 @@ struct ofpbuf *ofputil_encode_table_mod(const struct ofputil_table_mod *,
enum ofputil_protocol);
void ofputil_table_mod_format(struct ds *, const struct ofputil_table_mod *,
const struct ofputil_table_map *);
-char *parse_ofp_table_mod(struct ofputil_table_mod *,
+char *parse_ofp_table_mod(struct ofputil_table_mod *, const char **namep,
const char *table_id, const char *flow_miss_handling,
const struct ofputil_table_map *,
uint32_t *usable_versions)
@@ -271,15 +271,18 @@ struct ofputil_table_features {
struct mf_bitmap wildcard; /* Subset of 'match' that may be wildcarded. */
};
-int ofputil_decode_table_features(struct ofpbuf *,
- struct ofputil_table_features *, bool loose);
+int ofputil_decode_table_features(
+ struct ofpbuf *, struct ofputil_table_features *,
+ struct ofpbuf *raw_properties);
struct ofpbuf *ofputil_encode_table_features_request(enum ofp_version);
struct ofpbuf *ofputil_encode_table_desc_request(enum ofp_version);
-void ofputil_append_table_features_reply(
- const struct ofputil_table_features *tf, struct ovs_list *replies);
+void ofputil_append_table_features(
+ const struct ofputil_table_features *tf,
+ const struct ofpbuf *raw_properties,
+ struct ovs_list *msgs);
void ofputil_table_features_format(
struct ds *, const struct ofputil_table_features *features,
@@ -290,6 +293,10 @@ void ofputil_table_features_format(
void ofputil_table_features_format_finish(struct ds *,
int first_ditto, int last_ditto);
+bool ofputil_table_features_are_superset(
+ const struct ofputil_table_features *super,
+ const struct ofputil_table_features *sub);
+
/* Abstract table stats.
*
* This corresponds to the OpenFlow 1.3 table statistics structure, which only
@@ -58,6 +58,8 @@ int vconn_transact(struct vconn *, struct ofpbuf *, struct ofpbuf **);
int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf **);
int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list *requests,
struct ofpbuf **replyp);
+int vconn_transact_multipart(struct vconn *, struct ovs_list *request,
+ struct ovs_list *replies);
int vconn_dump_flows(struct vconn *, const struct ofputil_flow_stats_request *,
enum ofputil_protocol,
@@ -234,9 +234,8 @@ ofp_print_table_features_reply(struct ds *s, const struct ofp_header *oh)
int first_ditto = -1, last_ditto = -1;
for (int i = 0; ; i++) {
struct ofputil_table_features tf;
- int retval;
-
- retval = ofputil_decode_table_features(&b, &tf, true);
+ struct ofpbuf raw_properties;
+ int retval = ofputil_decode_table_features(&b, &tf, &raw_properties);
if (retval) {
ofputil_table_features_format_finish(s, first_ditto, last_ditto);
return retval != EOF ? retval : 0;
@@ -350,11 +350,15 @@ parse_oxms(struct ofpbuf *payload, bool loose,
/* Converts an OFPMP_TABLE_FEATURES request or reply in 'msg' into an abstract
* ofputil_table_features in 'tf'.
*
- * If 'loose' is true, this function ignores properties and values that it does
- * not understand, as a controller would want to do when interpreting
- * capabilities provided by a switch. If 'loose' is false, this function
- * treats unknown properties and values as an error, as a switch would want to
- * do when interpreting a configuration request made by a controller.
+ * If 'raw_properties' is nonnull function ignores properties and values that
+ * it does not understand, as a controller would want to do when interpreting
+ * capabilities provided by a switch. In this mode, on success, it initializes
+ * 'raw_properties' to contain the properties that were parsed; this allows the
+ * caller to later re-serialize the same properties without change.
+ *
+ * If 'raw_properties' is null, this function treats unknown properties and
+ * values as an error, as a switch would want to do when interpreting a
+ * configuration request made by a controller.
*
* A single OpenFlow message can specify features for multiple tables. Calling
* this function multiple times for a single 'msg' iterates through the tables
@@ -365,8 +369,11 @@ parse_oxms(struct ofpbuf *payload, bool loose,
* a positive "enum ofperr" value. */
int
ofputil_decode_table_features(struct ofpbuf *msg,
- struct ofputil_table_features *tf, bool loose)
+ struct ofputil_table_features *tf,
+ struct ofpbuf *raw_properties)
{
+ bool loose = raw_properties != NULL;
+
memset(tf, 0, sizeof *tf);
if (!msg->header) {
@@ -418,8 +425,11 @@ ofputil_decode_table_features(struct ofpbuf *msg,
struct ofpbuf properties = ofpbuf_const_initializer(ofpbuf_pull(msg, len),
len);
- tf->any_properties = properties.size > 0;
ofpbuf_pull(&properties, sizeof *otf);
+ tf->any_properties = properties.size > 0;
+ if (raw_properties) {
+ *raw_properties = properties;
+ }
uint32_t seen = 0;
while (properties.size > 0) {
struct ofpbuf payload;
@@ -514,7 +524,7 @@ ofputil_decode_table_features(struct ofpbuf *msg,
* OpenFlow 1.5 requires all of them if any property is present. */
if ((seen & OFPTFPT13_REQUIRED) != OFPTFPT13_REQUIRED
&& (tf->any_properties || oh->version < OFP15_VERSION)) {
- VLOG_WARN_RL(&rl, "table features message missing required property");
+ VLOG_WARN_RL(&rl, "table features message missing required property %d", seen);
return OFPERR_OFPTFFC_BAD_FEATURES;
}
@@ -642,16 +652,21 @@ put_table_instruction_features(
OFPTFPT13_APPLY_SETFIELD, miss_offset, version);
}
+/* Appends a table features record to 'msgs', which must be a
+ * OFPT_TABLE_FEATURES request or reply. If 'raw_properties' is nonnull, then
+ * it uses its contents verbatim as the table features properties, ignoring the
+ * corresponding members of 'tf'. */
void
-ofputil_append_table_features_reply(const struct ofputil_table_features *tf,
- struct ovs_list *replies)
+ofputil_append_table_features(const struct ofputil_table_features *tf,
+ const struct ofpbuf *raw_properties,
+ struct ovs_list *msgs)
{
- struct ofpbuf *reply = ofpbuf_from_list(ovs_list_back(replies));
- enum ofp_version version = ofpmp_version(replies);
- size_t start_ofs = reply->size;
+ struct ofpbuf *msg = ofpbuf_from_list(ovs_list_back(msgs));
+ enum ofp_version version = ofpmp_version(msgs);
+ size_t start_ofs = msg->size;
struct ofp13_table_features *otf;
- otf = ofpbuf_put_zeros(reply, sizeof *otf);
+ otf = ofpbuf_put_zeros(msg, sizeof *otf);
otf->table_id = tf->table_id;
otf->command = version >= OFP15_VERSION ? tf->command : 0;
ovs_strlcpy_arrays(otf->name, tf->name);
@@ -667,17 +682,21 @@ ofputil_append_table_features_reply(const struct ofputil_table_features *tf,
}
otf->max_entries = htonl(tf->max_entries);
- put_table_instruction_features(reply, &tf->nonmiss, 0, version);
- put_table_instruction_features(reply, &tf->miss, 1, version);
+ if (raw_properties) {
+ ofpbuf_put(msg, raw_properties->data, raw_properties->size);
+ } else if (tf->any_properties) {
+ put_table_instruction_features(msg, &tf->nonmiss, 0, version);
+ put_table_instruction_features(msg, &tf->miss, 1, version);
- put_fields_property(reply, &tf->match, &tf->mask,
- OFPTFPT13_MATCH, version);
- put_fields_property(reply, &tf->wildcard, NULL,
- OFPTFPT13_WILDCARDS, version);
+ put_fields_property(msg, &tf->match, &tf->mask,
+ OFPTFPT13_MATCH, version);
+ put_fields_property(msg, &tf->wildcard, NULL,
+ OFPTFPT13_WILDCARDS, version);
+ }
- otf = ofpbuf_at_assert(reply, start_ofs, sizeof *otf);
- otf->length = htons(reply->size - start_ofs);
- ofpmp_postappend(replies, start_ofs);
+ otf = ofpbuf_at_assert(msg, start_ofs, sizeof *otf);
+ otf->length = htons(msg->size - start_ofs);
+ ofpmp_postappend(msgs, start_ofs);
}
static enum ofperr
@@ -1236,7 +1255,8 @@ exit:
/* Convert 'table_id' and 'setting' (as described for the "mod-table" command
* in the ovs-ofctl man page) into 'tm' for sending a table_mod command to a
- * switch.
+ * switch. If 'setting' sets the name of the table, puts the new name in
+ * '*namep' (a suffix of 'setting'), otherwise stores NULL.
*
* Stores a bitmap of the OpenFlow versions that are usable for 'tm' into
* '*usable_versions'.
@@ -1244,12 +1264,13 @@ exit:
* Returns NULL if successful, otherwise a malloc()'d string describing the
* error. The caller is responsible for freeing the returned string. */
char * OVS_WARN_UNUSED_RESULT
-parse_ofp_table_mod(struct ofputil_table_mod *tm, const char *table_id,
- const char *setting,
+parse_ofp_table_mod(struct ofputil_table_mod *tm, const char **namep,
+ const char *table_id, const char *setting,
const struct ofputil_table_map *table_map,
uint32_t *usable_versions)
{
*usable_versions = 0;
+ *namep = NULL;
if (!strcasecmp(table_id, "all")) {
tm->table_id = OFPTT_ALL;
} else if (!ofputil_table_from_string(table_id, table_map,
@@ -1293,6 +1314,10 @@ parse_ofp_table_mod(struct ofputil_table_mod *tm, const char *table_id,
} else if (!strcmp(setting, "novacancy")) {
tm->vacancy = OFPUTIL_TABLE_VACANCY_OFF;
*usable_versions = (1 << OFP14_VERSION) | (1u << OFP15_VERSION);
+ } else if (tm->table_id != OFPTT_ALL && !strncmp(setting, "name:", 5)) {
+ *namep = setting + 5;
+ *usable_versions = ((1 << OFP13_VERSION) | (1u << OFP14_VERSION)
+ | (1u << OFP15_VERSION));
} else {
return xasprintf("invalid table_mod setting %s", setting);
}
@@ -1644,6 +1669,106 @@ ofputil_table_features_format_finish(struct ds *s,
ds_put_format(s, " tables %d...%d: ditto\n", first_ditto, last_ditto);
}
}
+
+/* Returns true if the 1-bits in 'super' are a superset of the 1-bits in 'sub',
+ * false otherwise. */
+static bool
+be64_is_superset(ovs_be64 super, ovs_be64 sub)
+{
+ return (super & sub) == sub;
+}
+
+/* Returns true if the 1-bits in 'super' are a superset of the 1-bits in 'sub',
+ * false otherwise. */
+static bool
+u32_is_superset(uint32_t super, uint32_t sub)
+{
+ return (super & sub) == sub;
+}
+
+/* Returns true if the 1-bits in 'super' are a superset of the 1-bits in 'sub',
+ * false otherwise. */
+static bool
+u64_is_superset(uint64_t super, uint64_t sub)
+{
+ return (super & sub) == sub;
+}
+
+/* Returns true if the 1-bits in 'super' are a superset of the 1-bits in 'sub',
+ * false otherwise. */
+static bool
+ulong_is_superset(unsigned long int super, unsigned long int sub)
+{
+ return (super & sub) == sub;
+}
+
+/* Returns true if the 1-bits in 'super' are a superset of the 1-bits in 'sub',
+ * false otherwise. 'super' and 'sub' both have 'n_bits' bits. */
+static bool
+bitmap_is_superset(const unsigned long int *super,
+ const unsigned long int *sub, size_t n_bits)
+{
+ size_t n_longs = bitmap_n_longs(n_bits);
+ for (size_t i = 0; i < n_longs; i++) {
+ if (!ulong_is_superset(super[i], sub[i])) {
+ return false;
+ }
+ }
+ return true;
+}
+
+/* Returns true if the 1-bits in 'super' are a superset of the 1-bits in 'sub',
+ * false otherwise. */
+static bool
+mf_bitmap_is_superset(const struct mf_bitmap *super,
+ const struct mf_bitmap *sub)
+{
+ return bitmap_is_superset(super->bm, sub->bm, MFF_N_IDS);
+}
+
+/* Returns true if 'super' is a superset of 'sub', false otherwise. */
+static bool
+ofputil_table_action_features_is_superset(
+ const struct ofputil_table_action_features *super,
+ const struct ofputil_table_action_features *sub)
+{
+ return (u64_is_superset(super->ofpacts, sub->ofpacts)
+ && mf_bitmap_is_superset(&super->set_fields, &sub->set_fields));
+}
+
+/* Returns true if 'super' is a superset of 'sub', false otherwise. */
+static bool
+ofputil_table_instruction_features_is_superset(
+ const struct ofputil_table_instruction_features *super,
+ const struct ofputil_table_instruction_features *sub)
+{
+ return (bitmap_is_superset(super->next, sub->next, 255)
+ && u32_is_superset(super->instructions, sub->instructions)
+ && ofputil_table_action_features_is_superset(&super->write,
+ &sub->write)
+ && ofputil_table_action_features_is_superset(&super->apply,
+ &sub->apply));
+}
+
+/* Returns true if 'super' is a superset of 'sub', false otherwise. */
+bool
+ofputil_table_features_are_superset(
+ const struct ofputil_table_features *super,
+ const struct ofputil_table_features *sub)
+{
+ return (be64_is_superset(super->metadata_match, sub->metadata_match)
+ && be64_is_superset(super->metadata_write, sub->metadata_write)
+ && super->max_entries >= sub->max_entries
+ && super->supports_eviction >= sub->supports_eviction
+ && super->supports_vacancy_events >= sub->supports_vacancy_events
+ && ofputil_table_instruction_features_is_superset(&super->nonmiss,
+ &sub->nonmiss)
+ && ofputil_table_instruction_features_is_superset(&super->miss,
+ &sub->miss)
+ && mf_bitmap_is_superset(&super->match, &sub->match)
+ && mf_bitmap_is_superset(&super->mask, &sub->mask)
+ && mf_bitmap_is_superset(&super->wildcard, &sub->wildcard));
+}
/* Table stats. */
@@ -934,6 +934,46 @@ vconn_transact_multiple_noreply(struct vconn *vconn, struct ovs_list *requests,
return 0;
}
+/* Sends 'requests' (which should be a multipart request) on 'vconn' and waits
+ * for the replies, which are put into 'replies'. Returns 0 if successful,
+ * otherwise an errno value. */
+int
+vconn_transact_multipart(struct vconn *vconn,
+ struct ovs_list *requests,
+ struct ovs_list *replies)
+{
+ struct ofpbuf *rq = ofpbuf_from_list(ovs_list_front(requests));
+ ovs_be32 send_xid = ((struct ofp_header *) rq->data)->xid;
+
+ ovs_list_init(replies);
+
+ /* Send all the requests. */
+ struct ofpbuf *b, *next;
+ LIST_FOR_EACH_SAFE (b, next, list_node, requests) {
+ ovs_list_remove(&b->list_node);
+ int error = vconn_send_block(vconn, b);
+ if (error) {
+ ofpbuf_delete(b);
+ }
+ }
+
+ /* Receive all the replies. */
+ bool more;
+ do {
+ struct ofpbuf *reply;
+ int error = vconn_recv_xid__(vconn, send_xid, &reply, NULL);
+ if (error) {
+ ofpbuf_list_delete(replies);
+ return error;
+ }
+
+ ovs_list_push_back(replies, &reply->list_node);
+ more = ofpmsg_is_stat_reply(reply->data) && ofpmp_more(reply->data);
+ } while (more);
+
+ return 0;
+}
+
static int
recv_flow_stats_reply(struct vconn *vconn, ovs_be32 send_xid,
struct ofpbuf **replyp,
@@ -6040,6 +6040,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,
@@ -219,6 +219,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. */
@@ -932,6 +933,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
@@ -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);
@@ -1476,7 +1478,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;
@@ -3234,6 +3236,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)) {
@@ -3765,33 +3769,238 @@ 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);
+ 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. */
+ free(features);
+ 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
@@ -8174,7 +8383,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);
@@ -8285,7 +8494,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);
@@ -8610,26 +8821,49 @@ 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
+ || !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.
@@ -2668,6 +2668,19 @@ AT_CLEANUP
AT_SETUP([ofproto - flow table names])
OVS_VSWITCHD_START
add_of_ports br0 1 2
+
+# Set a table name via OpenFlow 1.3 and one via OpenFlow 1.5.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 0 name:xyzzy])
+AT_CHECK([ovs-ofctl -O OpenFlow15 mod-table br0 1 name:quux])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^ table'],
+ [0], [dnl
+ table 0 ("xyzzy"):
+ table 1 ("quux"): ditto
+ tables 2...252: ditto
+ table 253:
+])
+
+# Set some table names via OVSDB.
AT_CHECK(
[ovs-vsctl \
-- --id=@t0 create Flow_Table name=zero \
@@ -2679,6 +2692,16 @@ AT_CHECK(
<1>
<2>
])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^ table'],
+ [0], [dnl
+ table 0 ("zero"):
+ table 1 ("one"): ditto
+ table 2 ("two"): ditto
+ tables 3...252: ditto
+ table 253:
+])
+
+# Check that flow table parsing and dumping uses the names.
AT_DATA([flows.txt], [dnl
table=zero in_port=p2 actions=p1,resubmit(,one)
table=one,in_port=p1,ip,actions=ct(table=two)
@@ -2695,6 +2718,73 @@ AT_CHECK([ovs-ofctl --no-names --no-stats dump-flows br0], [0], [dnl
table=1, ip,in_port=1 actions=ct(table=2)
table=1, arp,in_port=1 actions=resubmit(,2)
])
+
+# Setting the same table names via OpenFlow 1.3 or OpenFlow 1.5 is a no-op.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 0 name:zero])
+AT_CHECK([ovs-ofctl -O OpenFlow15 mod-table br0 1 name:one])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^ table'],
+ [0], [dnl
+ table 0 ("zero"):
+ table 1 ("one"): ditto
+ table 2 ("two"): ditto
+ tables 3...252: ditto
+ table 253:
+])
+
+# Setting different tables names via OpenFlow 1.3 or OpenFlow 1.5 yield errors.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 0 name:xyzzy], 1, [], [stderr])
+AT_CHECK([head -1 stderr], [0], [OFPT_ERROR (OF1.3) (xid=0x5): OFPTFFC_EPERM
+])
+AT_CHECK([ovs-ofctl -O OpenFlow15 mod-table br0 1 name:quux], 1, [], [stderr])
+AT_CHECK([head -1 stderr], [0], [OFPT_ERROR (OF1.5) (xid=0x5): OFPTFFC_EPERM
+])
+
+# But we can still set table names for those not set via OVSDB.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 3 name:three])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^ table'],
+ [0], [dnl
+ table 0 ("zero"):
+ table 1 ("one"): ditto
+ table 2 ("two"): ditto
+ table 3 ("three"): ditto
+ tables 4...252: ditto
+ table 253:
+])
+
+# Unsetting names via OVSDB then setting them via OpenFlow works too.
+AT_CHECK([ovs-vsctl remove bridge br0 Flow_Table 2])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^ table'],
+ [0], [dnl
+ table 0 ("zero"):
+ table 1 ("one"): ditto
+ table 2: ditto
+ table 3 ("three"): ditto
+ tables 4...252: ditto
+ table 253:
+])
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 2 name:foobar])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^ table'],
+ [0], [dnl
+ table 0 ("zero"):
+ table 1 ("one"): ditto
+ table 2 ("foobar"): ditto
+ table 3 ("three"): ditto
+ tables 4...252: ditto
+ table 253:
+])
+
+# We can clear names via OpenFlow, at least if they were set that way.
+AT_CHECK([ovs-ofctl -O OpenFlow13 mod-table br0 2 name:])
+AT_CHECK([ovs-ofctl -O OpenFlow15 dump-table-features br0 |grep '^ table'],
+ [0], [dnl
+ table 0 ("zero"):
+ table 1 ("one"): ditto
+ table 2: ditto
+ table 3 ("three"): ditto
+ tables 4...252: ditto
+ table 253:
+])
+
OVS_VSWITCHD_STOP
AT_CLEANUP
@@ -88,6 +88,17 @@ Send to controller. (This is how an OpenFlow 1.0 switch always
handles packets that do not match any flow in the last table.)
.RE
.IP
+In OpenFlow 1.3 and later (which must be enabled with the \fB\-O\fR
+option) and Open vSwitch 2.11 and later only, \fBmod\-table\fR can
+change the name of a table:
+.RS
+.IP \fBname:\fInew-name\fR
+Changes the name of the table to \fInew-name\fR. (This will be
+ineffective if the name is set via the \fBname\fR column in the
+\fBFlow_Table\fR table in the \fBOpen_vSwitch\fR database as described
+in \fBovs\-vswitchd.conf.db\fR(5).)
+.RE
+.IP
In OpenFlow 1.4 and later (which must be enabled with the \fB\-O\fR
option) only, \fBmod\-table\fR configures the behavior when a
controller attempts to add a flow to a flow table that is full. The
@@ -913,9 +913,9 @@ ofctl_dump_table_features(struct ovs_cmdl_context *ctx)
done = !ofpmp_more(reply->data);
for (;;) {
struct ofputil_table_features tf;
- int retval;
-
- retval = ofputil_decode_table_features(reply, &tf, true);
+ struct ofpbuf raw_properties;
+ int retval = ofputil_decode_table_features(
+ reply, &tf, &raw_properties);
if (retval) {
if (retval != EOF) {
ovs_fatal(0, "decode error: %s",
@@ -1207,6 +1207,7 @@ struct table_iterator {
bool more;
struct ofputil_table_features features;
+ struct ofpbuf raw_properties;
};
/* Initializes 'ti' to prepare for iterating through all of the tables on the
@@ -1248,7 +1249,7 @@ table_iterator_next(struct table_iterator *ti)
ovs_assert(ti->variant == TI_FEATURES);
retval = ofputil_decode_table_features(ti->reply,
&ti->features,
- true);
+ &ti->raw_properties);
}
if (!retval) {
return &ti->features;
@@ -2580,16 +2581,95 @@ fetch_table_desc(struct vconn *vconn, struct ofputil_table_mod *tm,
}
}
+static void
+change_table_name(struct vconn *vconn, uint8_t table_id, const char *new_name)
+{
+ /* Get all tables' features and properties. */
+ struct table {
+ struct ofputil_table_features tf;
+ struct ofpbuf *raw_properties;
+ } *tables[256];
+ memset(tables, 0, sizeof tables);
+
+ struct table_iterator ti;
+ table_iterator_init(&ti, vconn);
+ while (table_iterator_next(&ti)) {
+ struct table *t = tables[ti.features.table_id] = xmalloc(sizeof *t);
+ t->tf = ti.features;
+ t->raw_properties = ofpbuf_clone(&ti.raw_properties);
+ }
+ table_iterator_destroy(&ti);
+
+ /* Change the name for table 'table_id'. */
+ struct table *t = tables[table_id];
+ if (!t) {
+ ovs_fatal(0, "switch does not have table %"PRIu8, table_id);
+ }
+ ovs_strlcpy(t->tf.name, new_name, OFP_MAX_TABLE_NAME_LEN);
+
+ /* Compose the transaction. */
+ enum ofp_version version = vconn_get_version(vconn);
+ struct ovs_list requests = OVS_LIST_INITIALIZER(&requests);
+ struct ofpbuf *tfr = ofputil_encode_table_features_request(version);
+ ovs_list_push_back(&requests, &tfr->list_node);
+ if (version >= OFP15_VERSION) {
+ /* For OpenFlow 1.5, we can use a single OFPTFC15_MODIFY without any
+ * properties. */
+ t->tf.command = OFPTFC15_MODIFY;
+ t->tf.any_properties = false;
+ ofputil_append_table_features(&t->tf, NULL, &requests);
+ } else {
+ /* For OpenFlow 1.3 and 1.4, we have to regurgitate all of the tables
+ * and their properties. */
+ for (size_t i = 0; i < 256; i++) {
+ if (tables[i]) {
+ ofputil_append_table_features(&tables[i]->tf,
+ tables[i]->raw_properties,
+ &requests);
+ }
+ }
+ }
+
+ /* Transact.
+ *
+ * The reply repeats the entire new configuration of the tables, so we
+ * don't bother printing it unless there's an error. */
+ struct ovs_list replies;
+ struct ofpbuf *reply;
+ vconn_transact_multipart(vconn, &requests, &replies);
+ LIST_FOR_EACH (reply, list_node, &replies) {
+ enum ofptype type;
+ enum ofperr error = ofptype_decode(&type, reply->data);
+ if (error) {
+ ovs_fatal(0, "decode error: %s", ofperr_get_name(error));
+ } else if (type == OFPTYPE_ERROR) {
+ ofp_print(stderr, reply->data, reply->size, NULL, NULL,
+ verbosity + 1);
+ exit(1);
+ }
+ }
+ ofpbuf_list_delete(&replies);
+
+ /* Clean up. */
+ for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
+ if (tables[i]) {
+ ofpbuf_delete(tables[i]->raw_properties);
+ free(tables[i]);
+ }
+ }
+}
+
static void
ofctl_mod_table(struct ovs_cmdl_context *ctx)
{
uint32_t usable_versions;
struct ofputil_table_mod tm;
+ const char *name;
struct vconn *vconn;
char *error;
int i;
- error = parse_ofp_table_mod(&tm, ctx->argv[2], ctx->argv[3],
+ error = parse_ofp_table_mod(&tm, &name, ctx->argv[2], ctx->argv[3],
tables_to_accept(ctx->argv[1]),
&usable_versions);
if (error) {
@@ -2607,27 +2687,32 @@ ofctl_mod_table(struct ovs_cmdl_context *ctx)
mask_allowed_ofp_versions(usable_versions);
enum ofputil_protocol protocol = open_vconn(ctx->argv[1], &vconn);
- /* For OpenFlow 1.4+, ovs-ofctl mod-table should not affect table-config
- * properties that the user didn't ask to change, so it is necessary to
- * restore the current configuration of table-config parameters using
- * OFPMP14_TABLE_DESC request. */
- if ((allowed_versions & (1u << OFP14_VERSION)) ||
- (allowed_versions & (1u << OFP15_VERSION))) {
- struct ofputil_table_desc td;
-
- if (tm.table_id == OFPTT_ALL) {
- for (i = 0; i < OFPTT_MAX; i++) {
- tm.table_id = i;
+ if (name) {
+ change_table_name(vconn, tm.table_id, name);
+ } else {
+ /* For OpenFlow 1.4+, ovs-ofctl mod-table should not affect
+ * table-config properties that the user didn't ask to change, so it is
+ * necessary to restore the current configuration of table-config
+ * parameters using OFPMP14_TABLE_DESC request. */
+ if ((allowed_versions & (1u << OFP14_VERSION)) ||
+ (allowed_versions & (1u << OFP15_VERSION))) {
+ struct ofputil_table_desc td;
+
+ if (tm.table_id == OFPTT_ALL) {
+ for (i = 0; i < OFPTT_MAX; i++) {
+ tm.table_id = i;
+ fetch_table_desc(vconn, &tm, &td);
+ transact_noreply(vconn,
+ ofputil_encode_table_mod(&tm, protocol));
+ }
+ } else {
fetch_table_desc(vconn, &tm, &td);
- transact_noreply(vconn,
- ofputil_encode_table_mod(&tm, protocol));
+ transact_noreply(vconn, ofputil_encode_table_mod(&tm,
+ protocol));
}
} else {
- fetch_table_desc(vconn, &tm, &td);
transact_noreply(vconn, ofputil_encode_table_mod(&tm, protocol));
}
- } else {
- transact_noreply(vconn, ofputil_encode_table_mod(&tm, protocol));
}
vconn_close(vconn);
}
This allows a controller to change the name of OpenFlow flow tables in the OVS software switch. CC: Brad Cowie <brad@cowie.nz> Signed-off-by: Ben Pfaff <blp@ovn.org> --- NEWS | 4 + include/openvswitch/ofp-errors.h | 3 + include/openvswitch/ofp-msgs.h | 2 +- include/openvswitch/ofp-table.h | 17 ++- include/openvswitch/vconn.h | 2 + lib/ofp-print.c | 5 +- lib/ofp-table.c | 177 ++++++++++++++++++++---- lib/vconn.c | 40 ++++++ ofproto/ofproto-dpif.c | 1 + ofproto/ofproto-provider.h | 19 +++ ofproto/ofproto.c | 284 +++++++++++++++++++++++++++++++++++---- tests/ofproto.at | 90 +++++++++++++ utilities/ovs-ofctl.8.in | 11 ++ utilities/ovs-ofctl.c | 127 ++++++++++++++--- 14 files changed, 701 insertions(+), 81 deletions(-)