[ovs-dev] [PATCH repost] Better abstract OFPT_SET_CONFIG and OFPT_GET_CONFIG_REPLY, make stricter.

Ben Pfaff blp at ovn.org
Mon Dec 21 23:38:31 UTC 2015


From: Ben Pfaff <blp at nicira.com>

The OFPT_SET_CONFIG and OFPT_GET_CONFIG_REPLY messages, which have the
same format, have a 'flags' field in which OpenFlow defines some bits,
which change somewhat from one version to another, and does not define
others.  Until now, Open vSwitch has not abstracted these messages at all
and has ignored the bits that OpenFlow leaves undefined.  This commit
abstracts the messages in the same way as other OpenFlow messages and
validates in OFPT_SET_CONFIG messages that the undefined bits are set to
zero.

OpenFlow 1.1 and 1.2, but not OpenFlow 1.0, define a flag named
OFPC_INVALID_TTL_TO_CONTROLLER.  Open vSwitch has until now also
implemented this as an extension to OpenFlow 1.0, and this commit retains
that extension.

Reported-by: Manpreet Singh <er.manpreet25 at gmail.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
This was previously posted near the beginning of November:
	https://patchwork.ozlabs.org/patch/539744/
but that posting did not attract any reviews.

 lib/learning-switch.c      |   9 ++--
 lib/ofp-print.c            |  43 +++++++++++++------
 lib/ofp-util.c             | 101 ++++++++++++++++++++++++++++++++++++++++-----
 lib/ofp-util.h             |  39 +++++++++++++++--
 ofproto/ofproto-dpif.c     |  12 +++---
 ofproto/ofproto-provider.h |  22 +++++-----
 ofproto/ofproto.c          |  49 ++++++++++------------
 utilities/ovs-ofctl.8.in   |   2 +
 utilities/ovs-ofctl.c      |  93 ++++++++++++++++++++++-------------------
 9 files changed, 250 insertions(+), 120 deletions(-)

diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index a45cf98..47f0a7a 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -469,7 +469,6 @@ static void
 send_features_request(struct lswitch *sw)
 {
     struct ofpbuf *b;
-    struct ofp_switch_config *osc;
     int ofp_version = rconn_get_version(sw->rconn);
 
     ovs_assert(ofp_version > 0 && ofp_version < 0xff);
@@ -479,10 +478,10 @@ send_features_request(struct lswitch *sw)
     queue_tx(sw, b);
 
     /* Send OFPT_SET_CONFIG. */
-    b = ofpraw_alloc(OFPRAW_OFPT_SET_CONFIG, ofp_version, sizeof *osc);
-    osc = ofpbuf_put_zeros(b, sizeof *osc);
-    osc->miss_send_len = htons(OFP_DEFAULT_MISS_SEND_LEN);
-    queue_tx(sw, b);
+    struct ofputil_switch_config config = {
+        .miss_send_len = OFP_DEFAULT_MISS_SEND_LEN
+    };
+    queue_tx(sw, ofputil_encode_set_config(&config, ofp_version));
 }
 
 static void
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 930b01a..a089dd2 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -496,25 +496,39 @@ ofp_print_switch_features(struct ds *string, const struct ofp_header *oh)
 }
 
 static void
-ofp_print_switch_config(struct ds *string, const struct ofp_switch_config *osc)
+ofp_print_switch_config(struct ds *string,
+                        const struct ofputil_switch_config *config)
 {
-    enum ofp_config_flags flags;
+    ds_put_format(string, " frags=%s",
+                  ofputil_frag_handling_to_string(config->frag));
 
-    flags = ntohs(osc->flags);
-
-    ds_put_format(string, " frags=%s", ofputil_frag_handling_to_string(flags));
-    flags &= ~OFPC_FRAG_MASK;
-
-    if (flags & OFPC_INVALID_TTL_TO_CONTROLLER) {
+    if (config->invalid_ttl_to_controller > 0) {
         ds_put_format(string, " invalid_ttl_to_controller");
-        flags &= ~OFPC_INVALID_TTL_TO_CONTROLLER;
     }
 
-    if (flags) {
-        ds_put_format(string, " ***unknown flags 0x%04"PRIx16"***", flags);
+    ds_put_format(string, " miss_send_len=%"PRIu16"\n", config->miss_send_len);
+}
+
+static void
+ofp_print_set_config(struct ds *string, const struct ofp_header *oh)
+{
+    struct ofputil_switch_config config;
+    enum ofperr error;
+
+    error = ofputil_decode_set_config(oh, &config);
+    if (error) {
+        ofp_print_error(string, error);
+        return;
     }
+    ofp_print_switch_config(string, &config);
+}
 
-    ds_put_format(string, " miss_send_len=%"PRIu16"\n", ntohs(osc->miss_send_len));
+static void
+ofp_print_get_config_reply(struct ds *string, const struct ofp_header *oh)
+{
+    struct ofputil_switch_config config;
+    ofputil_decode_get_config_reply(oh, &config);
+    ofp_print_switch_config(string, &config);
 }
 
 static void print_wild(struct ds *string, const char *leader, int is_wild,
@@ -3215,8 +3229,11 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw,
         break;
 
     case OFPTYPE_GET_CONFIG_REPLY:
+        ofp_print_get_config_reply(string, oh);
+        break;
+
     case OFPTYPE_SET_CONFIG:
-        ofp_print_switch_config(string, ofpmsg_body(oh));
+        ofp_print_set_config(string, oh);
         break;
 
     case OFPTYPE_PACKET_IN:
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index d7e9ccc..650e451 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -4068,6 +4068,84 @@ ofputil_append_port_desc_stats_reply(const struct ofputil_phy_port *pp,
     ofpmp_postappend(replies, start_ofs);
 }
 
+/* ofputil_switch_config */
+
+/* Decodes 'oh', which must be an OFPT_GET_CONFIG_REPLY or OFPT_SET_CONFIG
+ * message, into 'config'.  Returns false if 'oh' contained any flags that
+ * aren't specified in its version of OpenFlow, true otherwise. */
+static bool
+ofputil_decode_switch_config(const struct ofp_header *oh,
+                             struct ofputil_switch_config *config)
+{
+    const struct ofp_switch_config *osc;
+    struct ofpbuf b;
+
+    ofpbuf_use_const(&b, oh, ntohs(oh->length));
+    ofpraw_pull_assert(&b);
+    osc = ofpbuf_pull(&b, sizeof *osc);
+
+    config->frag = ntohs(osc->flags) & OFPC_FRAG_MASK;
+    config->miss_send_len = ntohs(osc->miss_send_len);
+
+    ovs_be16 valid_mask = htons(OFPC_FRAG_MASK);
+    if (oh->version < OFP13_VERSION) {
+        const ovs_be16 ttl_bit = htons(OFPC_INVALID_TTL_TO_CONTROLLER);
+        valid_mask |= ttl_bit;
+        config->invalid_ttl_to_controller = (osc->flags & ttl_bit) != 0;
+    } else {
+        config->invalid_ttl_to_controller = -1;
+    }
+
+    return !(osc->flags & ~valid_mask);
+}
+
+void
+ofputil_decode_get_config_reply(const struct ofp_header *oh,
+                                struct ofputil_switch_config *config)
+{
+    ofputil_decode_switch_config(oh, config);
+}
+
+enum ofperr
+ofputil_decode_set_config(const struct ofp_header *oh,
+                          struct ofputil_switch_config *config)
+{
+    return (ofputil_decode_switch_config(oh, config)
+            ? 0
+            : OFPERR_OFPSCFC_BAD_FLAGS);
+}
+
+static struct ofpbuf *
+ofputil_put_switch_config(const struct ofputil_switch_config *config,
+                          struct ofpbuf *b)
+{
+    const struct ofp_header *oh = b->data;
+    struct ofp_switch_config *osc = ofpbuf_put_zeros(b, sizeof *osc);
+    osc->flags = htons(config->frag);
+    if (config->invalid_ttl_to_controller > 0 && oh->version < OFP13_VERSION) {
+        osc->flags |= htons(OFPC_INVALID_TTL_TO_CONTROLLER);
+    }
+    osc->miss_send_len = htons(config->miss_send_len);
+    return b;
+}
+
+struct ofpbuf *
+ofputil_encode_get_config_reply(const struct ofp_header *request,
+                                const struct ofputil_switch_config *config)
+{
+    struct ofpbuf *b = ofpraw_alloc_reply(OFPRAW_OFPT_GET_CONFIG_REPLY,
+                                          request, 0);
+    return ofputil_put_switch_config(config, b);
+}
+
+struct ofpbuf *
+ofputil_encode_set_config(const struct ofputil_switch_config *config,
+                          enum ofp_version version)
+{
+    struct ofpbuf *b = ofpraw_alloc(OFPRAW_OFPT_SET_CONFIG, version, 0);
+    return ofputil_put_switch_config(config, b);
+}
+
 /* ofputil_switch_features */
 
 #define OFPC_COMMON (OFPC_FLOW_STATS | OFPC_TABLE_STATS | OFPC_PORT_STATS | \
@@ -6447,29 +6525,30 @@ ofputil_encode_barrier_request(enum ofp_version ofp_version)
 }
 
 const char *
-ofputil_frag_handling_to_string(enum ofp_config_flags flags)
+ofputil_frag_handling_to_string(enum ofputil_frag_handling frag)
 {
-    switch (flags & OFPC_FRAG_MASK) {
-    case OFPC_FRAG_NORMAL:   return "normal";
-    case OFPC_FRAG_DROP:     return "drop";
-    case OFPC_FRAG_REASM:    return "reassemble";
-    case OFPC_FRAG_NX_MATCH: return "nx-match";
+    switch (frag) {
+    case OFPUTIL_FRAG_NORMAL:   return "normal";
+    case OFPUTIL_FRAG_DROP:     return "drop";
+    case OFPUTIL_FRAG_REASM:    return "reassemble";
+    case OFPUTIL_FRAG_NX_MATCH: return "nx-match";
     }
 
     OVS_NOT_REACHED();
 }
 
 bool
-ofputil_frag_handling_from_string(const char *s, enum ofp_config_flags *flags)
+ofputil_frag_handling_from_string(const char *s,
+                                  enum ofputil_frag_handling *frag)
 {
     if (!strcasecmp(s, "normal")) {
-        *flags = OFPC_FRAG_NORMAL;
+        *frag = OFPUTIL_FRAG_NORMAL;
     } else if (!strcasecmp(s, "drop")) {
-        *flags = OFPC_FRAG_DROP;
+        *frag = OFPUTIL_FRAG_DROP;
     } else if (!strcasecmp(s, "reassemble")) {
-        *flags = OFPC_FRAG_REASM;
+        *frag = OFPUTIL_FRAG_REASM;
     } else if (!strcasecmp(s, "nx-match")) {
-        *flags = OFPC_FRAG_NX_MATCH;
+        *frag = OFPUTIL_FRAG_NX_MATCH;
     } else {
         return false;
     }
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index c0541d4..1917318 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -490,6 +490,41 @@ enum ofperr ofputil_decode_packet_out(struct ofputil_packet_out *,
 struct ofpbuf *ofputil_encode_packet_out(const struct ofputil_packet_out *,
                                          enum ofputil_protocol protocol);
 
+enum ofputil_frag_handling {
+    OFPUTIL_FRAG_NORMAL = OFPC_FRAG_NORMAL,    /* No special handling. */
+    OFPUTIL_FRAG_DROP = OFPC_FRAG_DROP,        /* Drop fragments. */
+    OFPUTIL_FRAG_REASM = OFPC_FRAG_REASM,      /* Reassemble (if supported). */
+    OFPUTIL_FRAG_NX_MATCH = OFPC_FRAG_NX_MATCH /* Match on frag bits. */
+};
+
+const char *ofputil_frag_handling_to_string(enum ofputil_frag_handling);
+bool ofputil_frag_handling_from_string(const char *,
+                                       enum ofputil_frag_handling *);
+
+/* Abstract struct ofp_switch_config. */
+struct ofputil_switch_config {
+    /* Fragment handling. */
+    enum ofputil_frag_handling frag;
+
+    /* 0: Do not send packet to controller when decrementing invalid IP TTL.
+     * 1: Do send packet to controller when decrementing invalid IP TTL.
+     * -1: Unspecified (only OpenFlow 1.1 and 1.2 support this setting. */
+    int invalid_ttl_to_controller;
+
+    /* Maximum bytes of packet to send to controller on miss. */
+    uint16_t miss_send_len;
+};
+
+void ofputil_decode_get_config_reply(const struct ofp_header *,
+                                     struct ofputil_switch_config *);
+struct ofpbuf *ofputil_encode_get_config_reply(
+    const struct ofp_header *request, const struct ofputil_switch_config *);
+
+enum ofperr ofputil_decode_set_config(const struct ofp_header *,
+                                      struct ofputil_switch_config *);
+struct ofpbuf *ofputil_encode_set_config(
+    const struct ofputil_switch_config *, enum ofp_version);
+
 enum ofputil_port_config {
     /* OpenFlow 1.0 and 1.1 share these values for these port config bits. */
     OFPUTIL_PC_PORT_DOWN    = 1 << 0, /* Port is administratively down. */
@@ -1034,10 +1069,6 @@ struct ofpbuf *make_echo_request(enum ofp_version);
 struct ofpbuf *make_echo_reply(const struct ofp_header *rq);
 
 struct ofpbuf *ofputil_encode_barrier_request(enum ofp_version);
-
-const char *ofputil_frag_handling_to_string(enum ofp_config_flags);
-bool ofputil_frag_handling_from_string(const char *, enum ofp_config_flags *);
-
 
 /* Actions. */
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6561c65..eed863e 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -303,7 +303,7 @@ struct ofproto_dpif {
     /* Special OpenFlow rules. */
     struct rule_dpif *miss_rule; /* Sends flow table misses to controller. */
     struct rule_dpif *no_packet_in_rule; /* Drops flow table misses. */
-    struct rule_dpif *drop_frags_rule; /* Used in OFPC_FRAG_DROP mode. */
+    struct rule_dpif *drop_frags_rule; /* Used in OFPUTIL_FRAG_DROP mode. */
 
     /* Bridging. */
     struct netflow *netflow;
@@ -3903,13 +3903,13 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
     /* We always unwildcard nw_frag (for IP), so they
      * need not be unwildcarded here. */
     if (flow->nw_frag & FLOW_NW_FRAG_ANY
-        && ofproto->up.frag_handling != OFPC_FRAG_NX_MATCH) {
-        if (ofproto->up.frag_handling == OFPC_FRAG_NORMAL) {
+        && ofproto->up.frag_handling != OFPUTIL_FRAG_NX_MATCH) {
+        if (ofproto->up.frag_handling == OFPUTIL_FRAG_NORMAL) {
             /* We must pretend that transport ports are unavailable. */
             flow->tp_src = htons(0);
             flow->tp_dst = htons(0);
         } else {
-            /* Must be OFPC_FRAG_DROP (we don't have OFPC_FRAG_REASM).
+            /* Must be OFPUTIL_FRAG_DROP (we don't have OFPUTIL_FRAG_REASM).
              * Use the drop_frags_rule (which cannot disappear). */
             rule = ofproto->drop_frags_rule;
             if (stats) {
@@ -4401,10 +4401,10 @@ get_datapath_version(const struct ofproto *ofproto_)
 
 static bool
 set_frag_handling(struct ofproto *ofproto_,
-                  enum ofp_config_flags frag_handling)
+                  enum ofputil_frag_handling frag_handling)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
-    if (frag_handling != OFPC_FRAG_REASM) {
+    if (frag_handling != OFPUTIL_FRAG_REASM) {
         ofproto->backer->need_revalidate = REV_RECONFIGURE;
         return true;
     } else {
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2a6bd91..b6aac0a 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -78,7 +78,7 @@ struct ofproto {
     char *sw_desc;              /* Software version (NULL for default). */
     char *serial_desc;          /* Serial number (NULL for default). */
     char *dp_desc;              /* Datapath description (NULL for default). */
-    enum ofp_config_flags frag_handling; /* One of OFPC_*.  */
+    enum ofputil_frag_handling frag_handling;
 
     /* Datapath. */
     struct hmap ports;          /* Contains "struct ofport"s. */
@@ -1235,22 +1235,22 @@ struct ofproto_class {
      * which takes one of the following values, with the corresponding
      * meanings:
      *
-     *  - OFPC_FRAG_NORMAL: The switch should treat IP fragments the same way
-     *    as other packets, omitting TCP and UDP port numbers (always setting
-     *    them to 0).
+     *  - OFPUTIL_FRAG_NORMAL: The switch should treat IP fragments the same
+     *    way as other packets, omitting TCP and UDP port numbers (always
+     *    setting them to 0).
      *
-     *  - OFPC_FRAG_DROP: The switch should drop all IP fragments without
+     *  - OFPUTIL_FRAG_DROP: The switch should drop all IP fragments without
      *    passing them through the flow table.
      *
-     *  - OFPC_FRAG_REASM: The switch should reassemble IP fragments before
+     *  - OFPUTIL_FRAG_REASM: The switch should reassemble IP fragments before
      *    passing packets through the flow table.
      *
-     *  - OFPC_FRAG_NX_MATCH (a Nicira extension): Similar to OFPC_FRAG_NORMAL,
-     *    except that TCP and UDP port numbers should be included in fragments
-     *    with offset 0.
+     *  - OFPUTIL_FRAG_NX_MATCH (a Nicira extension): Similar to
+     *    OFPUTIL_FRAG_NORMAL, except that TCP and UDP port numbers should be
+     *    included in fragments with offset 0.
      *
      * Implementations are not required to support every mode.
-     * OFPC_FRAG_NORMAL is the default mode when an ofproto is created.
+     * OFPUTIL_FRAG_NORMAL is the default mode when an ofproto is created.
      *
      * At the time of the call to ->set_frag_handling(), the current mode is
      * available in 'ofproto->frag_handling'.  ->set_frag_handling() returns
@@ -1260,7 +1260,7 @@ struct ofproto_class {
      * reflect the new mode.
      */
     bool (*set_frag_handling)(struct ofproto *ofproto,
-                              enum ofp_config_flags frag_handling);
+                              enum ofputil_frag_handling frag_handling);
 
     /* Implements the OpenFlow OFPT_PACKET_OUT command.  The datapath should
      * execute the 'ofpacts_len' bytes of "struct ofpacts" in 'ofpacts'.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c5bd949..db4063f 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -544,7 +544,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->sw_desc = NULL;
     ofproto->serial_desc = NULL;
     ofproto->dp_desc = NULL;
-    ofproto->frag_handling = OFPC_FRAG_NORMAL;
+    ofproto->frag_handling = OFPUTIL_FRAG_NORMAL;
     hmap_init(&ofproto->ports);
     hmap_init(&ofproto->ofport_usage);
     shash_init(&ofproto->port_by_name);
@@ -3255,23 +3255,13 @@ handle_features_request(struct ofconn *ofconn, const struct ofp_header *oh)
 static enum ofperr
 handle_get_config_request(struct ofconn *ofconn, const struct ofp_header *oh)
 {
-    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    struct ofp_switch_config *osc;
-    enum ofp_config_flags flags;
-    struct ofpbuf *buf;
+    struct ofputil_switch_config config;
+    config.frag = ofconn_get_ofproto(ofconn)->frag_handling;
+    config.invalid_ttl_to_controller
+        = ofconn_get_invalid_ttl_to_controller(ofconn);
+    config.miss_send_len = ofconn_get_miss_send_len(ofconn);
 
-    /* Send reply. */
-    buf = ofpraw_alloc_reply(OFPRAW_OFPT_GET_CONFIG_REPLY, oh, 0);
-    osc = ofpbuf_put_uninit(buf, sizeof *osc);
-    flags = ofproto->frag_handling;
-    /* OFPC_INVALID_TTL_TO_CONTROLLER is deprecated in OF 1.3 */
-    if (oh->version < OFP13_VERSION
-        && ofconn_get_invalid_ttl_to_controller(ofconn)) {
-        flags |= OFPC_INVALID_TTL_TO_CONTROLLER;
-    }
-    osc->flags = htons(flags);
-    osc->miss_send_len = htons(ofconn_get_miss_send_len(ofconn));
-    ofconn_send_reply(ofconn, buf);
+    ofconn_send_reply(ofconn, ofputil_encode_get_config_reply(oh, &config));
 
     return 0;
 }
@@ -3279,16 +3269,20 @@ handle_get_config_request(struct ofconn *ofconn, const struct ofp_header *oh)
 static enum ofperr
 handle_set_config(struct ofconn *ofconn, const struct ofp_header *oh)
 {
-    const struct ofp_switch_config *osc = ofpmsg_body(oh);
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    uint16_t flags = ntohs(osc->flags);
+    struct ofputil_switch_config config;
+    enum ofperr error;
+
+    error = ofputil_decode_set_config(oh, &config);
+    if (error) {
+        return error;
+    }
 
     if (ofconn_get_type(ofconn) != OFCONN_PRIMARY
         || ofconn_get_role(ofconn) != OFPCR12_ROLE_SLAVE) {
-        enum ofp_config_flags cur = ofproto->frag_handling;
-        enum ofp_config_flags next = flags & OFPC_FRAG_MASK;
+        enum ofputil_frag_handling cur = ofproto->frag_handling;
+        enum ofputil_frag_handling next = config.frag;
 
-        ovs_assert((cur & OFPC_FRAG_MASK) == cur);
         if (cur != next) {
             if (ofproto->ofproto_class->set_frag_handling(ofproto, next)) {
                 ofproto->frag_handling = next;
@@ -3299,12 +3293,13 @@ handle_set_config(struct ofconn *ofconn, const struct ofp_header *oh)
             }
         }
     }
-    /* OFPC_INVALID_TTL_TO_CONTROLLER is deprecated in OF 1.3 */
-    ofconn_set_invalid_ttl_to_controller(ofconn,
-             (oh->version < OFP13_VERSION
-              && flags & OFPC_INVALID_TTL_TO_CONTROLLER));
 
-    ofconn_set_miss_send_len(ofconn, ntohs(osc->miss_send_len));
+    if (config.invalid_ttl_to_controller >= 0) {
+        ofconn_set_invalid_ttl_to_controller(ofconn,
+                                             config.invalid_ttl_to_controller);
+    }
+
+    ofconn_set_miss_send_len(ofconn, config.miss_send_len);
 
     return 0;
 }
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 0f0b1f7..a9ac31f 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -513,6 +513,8 @@ If \fBinvalid_ttl\fR is passed, \fBovs\-ofctl\fR sends an OpenFlow ``set
 configuration'' message at connection setup time that requests
 \fBINVALID_TTL_TO_CONTROLLER\fR, so that \fBovs\-ofctl monitor\fR can
 receive ``packet-in'' messages when TTL reaches zero on \fBdec_ttl\fR action.
+Only OpenFlow 1.1 and 1.2 support \fBinvalid_ttl\fR; Open vSwitch also
+implements it for OpenFlow 1.0 as an extension.
 .IP
 \fBwatch:\fR[\fB\fIspec\fR...] causes \fBovs\-ofctl\fR to send a
 ``monitor request'' Nicira extension message to the switch at
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 0d57f85..291738c 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -656,9 +656,8 @@ transact_noreply(struct vconn *vconn, struct ofpbuf *request)
 }
 
 static void
-fetch_switch_config(struct vconn *vconn, struct ofp_switch_config *config_)
+fetch_switch_config(struct vconn *vconn, struct ofputil_switch_config *config)
 {
-    struct ofp_switch_config *config;
     struct ofpbuf *request;
     struct ofpbuf *reply;
     enum ofptype type;
@@ -668,25 +667,20 @@ fetch_switch_config(struct vconn *vconn, struct ofp_switch_config *config_)
     run(vconn_transact(vconn, request, &reply),
         "talking to %s", vconn_get_name(vconn));
 
-    if (ofptype_pull(&type, reply) || type != OFPTYPE_GET_CONFIG_REPLY) {
+    if (ofptype_decode(&type, reply->data)
+        || type != OFPTYPE_GET_CONFIG_REPLY) {
         ovs_fatal(0, "%s: bad reply to config request", vconn_get_name(vconn));
     }
-
-    config = ofpbuf_pull(reply, sizeof *config);
-    *config_ = *config;
-
+    ofputil_decode_get_config_reply(reply->data, config);
     ofpbuf_delete(reply);
 }
 
 static void
-set_switch_config(struct vconn *vconn, const struct ofp_switch_config *config)
+set_switch_config(struct vconn *vconn,
+                  const struct ofputil_switch_config *config)
 {
-    struct ofpbuf *request;
-
-    request = ofpraw_alloc(OFPRAW_OFPT_SET_CONFIG, vconn_get_version(vconn), 0);
-    ofpbuf_put(request, config, sizeof *config);
-
-    transact_noreply(vconn, request);
+    enum ofp_version version = vconn_get_version(vconn);
+    transact_noreply(vconn, ofputil_encode_set_config(config, version));
 }
 
 static void
@@ -1426,27 +1420,20 @@ set_packet_in_format(struct vconn *vconn,
 static int
 monitor_set_invalid_ttl_to_controller(struct vconn *vconn)
 {
-    struct ofp_switch_config config;
-    enum ofp_config_flags flags;
+    struct ofputil_switch_config config;
 
     fetch_switch_config(vconn, &config);
-    flags = ntohs(config.flags);
-    if (!(flags & OFPC_INVALID_TTL_TO_CONTROLLER)) {
-        /* Set the invalid ttl config. */
-        flags |= OFPC_INVALID_TTL_TO_CONTROLLER;
-
-        config.flags = htons(flags);
+    if (!config.invalid_ttl_to_controller) {
+        config.invalid_ttl_to_controller = 1;
         set_switch_config(vconn, &config);
 
         /* Then retrieve the configuration to see if it really took.  OpenFlow
-         * doesn't define error reporting for bad modes, so this is all we can
-         * do. */
+         * has ill-defined error reporting for bad flags, so this is about the
+         * best we can do. */
         fetch_switch_config(vconn, &config);
-        flags = ntohs(config.flags);
-        if (!(flags & OFPC_INVALID_TTL_TO_CONTROLLER)) {
+        if (!config.invalid_ttl_to_controller) {
             ovs_fatal(0, "setting invalid_ttl_to_controller failed (this "
-                      "switch probably doesn't support mode)");
-            return -EOPNOTSUPP;
+                      "switch probably doesn't support this flag)");
         }
     }
     return 0;
@@ -1707,15 +1694,37 @@ ofctl_monitor(struct ovs_cmdl_context *ctx)
     int i;
     enum ofputil_protocol usable_protocols;
 
+    /* If the user wants the invalid_ttl_to_controller feature, limit the
+     * OpenFlow versions to those that support that feature.  (Support in
+     * OpenFlow 1.0 is an Open vSwitch extension.) */
+    for (i = 2; i < ctx->argc; i++) {
+        if (!strcmp(ctx->argv[i], "invalid_ttl")) {
+            uint32_t usable_versions = ((1u << OFP10_VERSION) |
+                                        (1u << OFP11_VERSION) |
+                                        (1u << OFP12_VERSION));
+            uint32_t allowed_versions = get_allowed_ofp_versions();
+            if (!(allowed_versions & usable_versions)) {
+                struct ds versions = DS_EMPTY_INITIALIZER;
+                ofputil_format_version_bitmap_names(&versions,
+                                                    usable_versions);
+                ovs_fatal(0, "invalid_ttl requires one of the OpenFlow "
+                          "versions %s but none is enabled (use -O)",
+                          ds_cstr(&versions));
+            }
+            mask_allowed_ofp_versions(usable_versions);
+            break;
+        }
+    }
+
     open_vconn(ctx->argv[1], &vconn);
     for (i = 2; i < ctx->argc; i++) {
         const char *arg = ctx->argv[i];
 
         if (isdigit((unsigned char) *arg)) {
-            struct ofp_switch_config config;
+            struct ofputil_switch_config config;
 
             fetch_switch_config(vconn, &config);
-            config.miss_send_len = htons(atoi(arg));
+            config.miss_send_len = atoi(arg);
             set_switch_config(vconn, &config);
         } else if (!strcmp(arg, "invalid_ttl")) {
             monitor_set_invalid_ttl_to_controller(vconn);
@@ -2052,43 +2061,41 @@ ofctl_mod_table(struct ovs_cmdl_context *ctx)
 static void
 ofctl_get_frags(struct ovs_cmdl_context *ctx)
 {
-    struct ofp_switch_config config;
+    struct ofputil_switch_config config;
     struct vconn *vconn;
 
     open_vconn(ctx->argv[1], &vconn);
     fetch_switch_config(vconn, &config);
-    puts(ofputil_frag_handling_to_string(ntohs(config.flags)));
+    puts(ofputil_frag_handling_to_string(config.frag));
     vconn_close(vconn);
 }
 
 static void
 ofctl_set_frags(struct ovs_cmdl_context *ctx)
 {
-    struct ofp_switch_config config;
-    enum ofp_config_flags mode;
+    struct ofputil_switch_config config;
+    enum ofputil_frag_handling frag;
     struct vconn *vconn;
-    ovs_be16 flags;
 
-    if (!ofputil_frag_handling_from_string(ctx->argv[2], &mode)) {
+    if (!ofputil_frag_handling_from_string(ctx->argv[2], &frag)) {
         ovs_fatal(0, "%s: unknown fragment handling mode", ctx->argv[2]);
     }
 
     open_vconn(ctx->argv[1], &vconn);
     fetch_switch_config(vconn, &config);
-    flags = htons(mode) | (config.flags & htons(~OFPC_FRAG_MASK));
-    if (flags != config.flags) {
+    if (frag != config.frag) {
         /* Set the configuration. */
-        config.flags = flags;
+        config.frag = frag;
         set_switch_config(vconn, &config);
 
         /* Then retrieve the configuration to see if it really took.  OpenFlow
-         * doesn't define error reporting for bad modes, so this is all we can
-         * do. */
+         * has ill-defined error reporting for bad flags, so this is about the
+         * best we can do. */
         fetch_switch_config(vconn, &config);
-        if (flags != config.flags) {
+        if (frag != config.frag) {
             ovs_fatal(0, "%s: setting fragment handling mode failed (this "
                       "switch probably doesn't support mode \"%s\")",
-                      ctx->argv[1], ofputil_frag_handling_to_string(mode));
+                      ctx->argv[1], ofputil_frag_handling_to_string(frag));
         }
     }
     vconn_close(vconn);
-- 
2.1.3




More information about the dev mailing list