openflow: Add enum ofp_version

Use an enum for ofp_version in ofp-util and ofp-msg.
This in conjunction with the use of switch() statements
allows the compiler to warn when a new ofp_version isn't handled.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h
index 202ba1e..0bca0d2 100644 (file)
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -71,9 +71,11 @@
 /* The most significant bit being set in the version field indicates an
  * experimental OpenFlow version.
  */
-#define OFP10_VERSION   0x01
-#define OFP11_VERSION   0x02
-#define OFP12_VERSION   0x03
+enum ofp_version {
+    OFP10_VERSION = 0x01,
+    OFP11_VERSION = 0x02,
+    OFP12_VERSION = 0x03,
+};
 
 #define OFP_MAX_TABLE_NAME_LEN 32
 #define OFP_MAX_PORT_NAME_LEN  16

diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c
index 48ecafc..07ee87a 100644 (file)
--- a/lib/ofp-msgs.c
+++ b/lib/ofp-msgs.c
@@ -259,11 +259,17 @@ ofphdrs_decode_assert(struct ofphdrs *hdrs,
 static bool
 ofphdrs_is_stat(const struct ofphdrs *hdrs)
 {
-    return (hdrs->version == OFP10_VERSION
-            ? (hdrs->type == OFPT10_STATS_REQUEST ||
-               hdrs->type == OFPT10_STATS_REPLY)
-            : (hdrs->type == OFPT11_STATS_REQUEST ||
-               hdrs->type == OFPT11_STATS_REPLY));
+    switch ((enum ofp_version) hdrs->version) {
+    case OFP10_VERSION:
+        return (hdrs->type == OFPT10_STATS_REQUEST ||
+                hdrs->type == OFPT10_STATS_REPLY);
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+        return (hdrs->type == OFPT11_STATS_REQUEST ||
+                hdrs->type == OFPT11_STATS_REPLY);
+    }
+
+    return false;
 }
 
 size_t
@@ -273,20 +279,25 @@ ofphdrs_len(const struct ofphdrs *hdrs)
         return sizeof(struct nicira_header);
     }
 
-    if (hdrs->version == OFP10_VERSION) {
+    switch ((enum ofp_version) hdrs->version) {
+    case OFP10_VERSION:
         if (hdrs->type == OFPT10_STATS_REQUEST ||
             hdrs->type == OFPT10_STATS_REPLY) {
             return (hdrs->stat == OFPST_VENDOR
                     ? sizeof(struct nicira10_stats_msg)
                     : sizeof(struct ofp10_stats_msg));
         }
-    } else {
+        break;
+
+    case OFP11_VERSION:
+    case OFP12_VERSION:
         if (hdrs->type == OFPT11_STATS_REQUEST ||
             hdrs->type == OFPT11_STATS_REPLY) {
             return (hdrs->stat == OFPST_VENDOR
                     ? sizeof(struct nicira11_stats_msg)
                     : sizeof(struct ofp11_stats_msg));
         }
+        break;
     }
 
     return sizeof(struct ofp_header);
@@ -686,12 +697,18 @@ ofpraw_stats_request_to_reply(enum ofpraw raw, uint8_t version)
     enum ofperr error;
 
     hdrs = instance->hdrs;
-    if (hdrs.version == OFP10_VERSION) {
+    switch ((enum ofp_version)hdrs.version) {
+    case OFP10_VERSION:
         assert(hdrs.type == OFPT10_STATS_REQUEST);
         hdrs.type = OFPT10_STATS_REPLY;
-    } else {
+        break;
+    case OFP11_VERSION:
+    case OFP12_VERSION:
         assert(hdrs.type == OFPT11_STATS_REQUEST);
         hdrs.type = OFPT11_STATS_REPLY;
+        break;
+    default:
+        NOT_REACHED();
     }
 
     error = ofpraw_from_ofphdrs(&reply_raw, &hdrs);
@@ -854,9 +871,15 @@ ofpmp_postappend(struct list *replies, size_t start_ofs)
 static ovs_be16 *
 ofpmp_flags__(const struct ofp_header *oh)
 {
-    return (oh->version == OFP10_VERSION
-            ? &((struct ofp10_stats_msg *) oh)->flags
-            : &((struct ofp11_stats_msg *) oh)->flags);
+    switch ((enum ofp_version)oh->version) {
+    case OFP10_VERSION:
+        return &((struct ofp10_stats_msg *) oh)->flags;
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+        return &((struct ofp11_stats_msg *) oh)->flags;
+    default:
+        NOT_REACHED();
+    }
 }
 
 /* Returns the OFPSF_* flags found in the OpenFlow stats header of 'oh', which

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 8d0f993..a943b4e 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -595,18 +595,22 @@ size_t ofputil_n_flow_dump_protocols = ARRAY_SIZE(ofputil_flow_dump_protocols);
  * 1.0, 0x02 for OpenFlow 1.1).  Returns 0 if 'version' is not supported or
  * outside the valid range.  */
 enum ofputil_protocol
-ofputil_protocol_from_ofp_version(int version)
+ofputil_protocol_from_ofp_version(enum ofp_version version)
 {
     switch (version) {
-    case OFP10_VERSION: return OFPUTIL_P_OF10;
-    case OFP12_VERSION: return OFPUTIL_P_OF12;
-    default: return 0;
+    case OFP10_VERSION:
+        return OFPUTIL_P_OF10;
+    case OFP12_VERSION:
+        return OFPUTIL_P_OF12;
+    case OFP11_VERSION:
+    default:
+        return 0;
     }
 }
 
 /* Returns the OpenFlow protocol version number (e.g. OFP10_VERSION,
  * OFP11_VERSION or OFP12_VERSION) that corresponds to 'protocol'. */
-uint8_t
+enum ofp_version
 ofputil_protocol_to_ofp_version(enum ofputil_protocol protocol)
 {
     switch (protocol) {
@@ -2191,10 +2195,17 @@ ofputil_decode_ofp11_port(struct ofputil_phy_port *pp,
 }
 
 static size_t
-ofputil_get_phy_port_size(uint8_t ofp_version)
-{
-    return ofp_version == OFP10_VERSION ? sizeof(struct ofp10_phy_port)
-                                        : sizeof(struct ofp11_port);
+ofputil_get_phy_port_size(enum ofp_version ofp_version)
+{
+    switch (ofp_version) {
+    case OFP10_VERSION:
+        return sizeof(struct ofp10_phy_port);
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+        return sizeof(struct ofp11_port);
+    default:
+        NOT_REACHED();
+    }
 }
 
 static void
@@ -2239,39 +2250,59 @@ ofputil_encode_ofp11_port(const struct ofputil_phy_port *pp,
 }
 
 static void
-ofputil_put_phy_port(uint8_t ofp_version, const struct ofputil_phy_port *pp,
-                     struct ofpbuf *b)
+ofputil_put_phy_port(enum ofp_version ofp_version,
+                     const struct ofputil_phy_port *pp, struct ofpbuf *b)
 {
-    if (ofp_version == OFP10_VERSION) {
+    switch (ofp_version) {
+    case OFP10_VERSION: {
         struct ofp10_phy_port *opp;
         if (b->size + sizeof *opp <= UINT16_MAX) {
             opp = ofpbuf_put_uninit(b, sizeof *opp);
             ofputil_encode_ofp10_phy_port(pp, opp);
         }
-    } else {
+        break;
+    }
+
+    case OFP11_VERSION:
+    case OFP12_VERSION: {
         struct ofp11_port *op;
         if (b->size + sizeof *op <= UINT16_MAX) {
             op = ofpbuf_put_uninit(b, sizeof *op);
             ofputil_encode_ofp11_port(pp, op);
         }
+        break;
+    }
+
+    default:
+        NOT_REACHED();
     }
 }
 
 void
-ofputil_append_port_desc_stats_reply(uint8_t ofp_version,
+ofputil_append_port_desc_stats_reply(enum ofp_version ofp_version,
                                      const struct ofputil_phy_port *pp,
                                      struct list *replies)
 {
-    if (ofp_version == OFP10_VERSION) {
+    switch (ofp_version) {
+    case OFP10_VERSION: {
         struct ofp10_phy_port *opp;
 
         opp = ofpmp_append(replies, sizeof *opp);
         ofputil_encode_ofp10_phy_port(pp, opp);
-    } else {
+        break;
+    }
+
+    case OFP11_VERSION:
+    case OFP12_VERSION: {
         struct ofp11_port *op;
 
         op = ofpmp_append(replies, sizeof *op);
         ofputil_encode_ofp11_port(pp, op);
+        break;
+    }
+
+    default:
+      NOT_REACHED();
     }
 }
 \f
@@ -2452,29 +2483,44 @@ ofputil_encode_switch_features(const struct ofputil_switch_features *features,
 {
     struct ofp_switch_features *osf;
     struct ofpbuf *b;
-    uint8_t version;
+    enum ofp_version version;
+    enum ofpraw raw;
 
     version = ofputil_protocol_to_ofp_version(protocol);
-    b = ofpraw_alloc_xid(version == OFP10_VERSION
-                         ? OFPRAW_OFPT10_FEATURES_REPLY
-                         : OFPRAW_OFPT11_FEATURES_REPLY,
-                         version, xid, 0);
+    switch (version) {
+    case OFP10_VERSION:
+        raw = OFPRAW_OFPT10_FEATURES_REPLY;
+        break;
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+        raw = OFPRAW_OFPT11_FEATURES_REPLY;
+        break;
+    default:
+        NOT_REACHED();
+    }
+    b = ofpraw_alloc_xid(raw, version, xid, 0);
     osf = ofpbuf_put_zeros(b, sizeof *osf);
     osf->datapath_id = htonll(features->datapath_id);
     osf->n_buffers = htonl(features->n_buffers);
     osf->n_tables = features->n_tables;
 
     osf->capabilities = htonl(features->capabilities & OFPC_COMMON);
-    if (version == OFP10_VERSION) {
+    switch (version) {
+    case OFP10_VERSION:
         if (features->capabilities & OFPUTIL_C_STP) {
             osf->capabilities |= htonl(OFPC10_STP);
         }
         osf->actions = encode_action_bits(features->actions, of10_action_bits);
-    } else {
+        break;
+    case OFP11_VERSION:
+    case OFP12_VERSION:
         if (features->capabilities & OFPUTIL_C_GROUP_STATS) {
             osf->capabilities |= htonl(OFPC11_GROUP_STATS);
         }
         osf->actions = encode_action_bits(features->actions, of11_action_bits);
+        break;
+    default:
+        NOT_REACHED();
     }
 
     return b;
@@ -2529,13 +2575,25 @@ ofputil_encode_port_status(const struct ofputil_port_status *ps,
 {
     struct ofp_port_status *ops;
     struct ofpbuf *b;
-    uint8_t version;
+    enum ofp_version version;
+    enum ofpraw raw;
 
     version = ofputil_protocol_to_ofp_version(protocol);
-    b = ofpraw_alloc_xid(version == OFP10_VERSION
-                         ? OFPRAW_OFPT10_PORT_STATUS
-                         : OFPRAW_OFPT11_PORT_STATUS,
-                         version, htonl(0), 0);
+    switch (version) {
+    case OFP10_VERSION:
+        raw = OFPRAW_OFPT10_PORT_STATUS;
+        break;
+
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+        raw = OFPRAW_OFPT11_PORT_STATUS;
+        break;
+
+    default:
+        NOT_REACHED();
+    }
+
+    b = ofpraw_alloc_xid(raw, version, htonl(0), 0);
     ops = ofpbuf_put_zeros(b, sizeof *ops);
     ops->reason = ps->reason;
     ofputil_put_phy_port(version, &ps->desc, b);
@@ -2593,10 +2651,11 @@ struct ofpbuf *
 ofputil_encode_port_mod(const struct ofputil_port_mod *pm,
                         enum ofputil_protocol protocol)
 {
-    uint8_t ofp_version = ofputil_protocol_to_ofp_version(protocol);
+    enum ofp_version ofp_version = ofputil_protocol_to_ofp_version(protocol);
     struct ofpbuf *b;
 
-    if (ofp_version == OFP10_VERSION) {
+    switch (ofp_version) {
+    case OFP10_VERSION: {
         struct ofp10_port_mod *opm;
 
         b = ofpraw_alloc(OFPRAW_OFPT10_PORT_MOD, ofp_version, 0);
@@ -2606,7 +2665,10 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm,
         opm->config = htonl(pm->config & OFPPC10_ALL);
         opm->mask = htonl(pm->mask & OFPPC10_ALL);
         opm->advertise = netdev_port_features_to_ofp10(pm->advertise);
-    } else if (ofp_version == OFP11_VERSION) {
+        break;
+    }
+
+    case OFP11_VERSION: {
         struct ofp11_port_mod *opm;
 
         b = ofpraw_alloc(OFPRAW_OFPT11_PORT_MOD, ofp_version, 0);
@@ -2616,7 +2678,11 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm,
         opm->config = htonl(pm->config & OFPPC11_ALL);
         opm->mask = htonl(pm->mask & OFPPC11_ALL);
         opm->advertise = netdev_port_features_to_ofp11(pm->advertise);
-    } else {
+        break;
+    }
+
+    case OFP12_VERSION:
+    default:
         NOT_REACHED();
     }
 
@@ -3106,16 +3172,22 @@ ofputil_format_port(uint16_t port, struct ds *s)
  * port and returns 0.  If no ports remain to be decoded, returns EOF.
  * On an error, returns a positive OFPERR_* value. */
 int
-ofputil_pull_phy_port(uint8_t ofp_version, struct ofpbuf *b,
+ofputil_pull_phy_port(enum ofp_version ofp_version, struct ofpbuf *b,
                       struct ofputil_phy_port *pp)
 {
-    if (ofp_version == OFP10_VERSION) {
+    switch (ofp_version) {
+    case OFP10_VERSION: {
         const struct ofp10_phy_port *opp = ofpbuf_try_pull(b, sizeof *opp);
         return opp ? ofputil_decode_ofp10_phy_port(pp, opp) : EOF;
-    } else {
+    }
+    case OFP11_VERSION:
+    case OFP12_VERSION: {
         const struct ofp11_port *op = ofpbuf_try_pull(b, sizeof *op);
         return op ? ofputil_decode_ofp11_port(pp, op) : EOF;
     }
+    default:
+        NOT_REACHED();
+    }
 }
 
 /* Given a buffer 'b' that contains an array of OpenFlow ports of type

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index c87d648..9230b36 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -82,8 +82,9 @@ enum ofputil_protocol {
 extern enum ofputil_protocol ofputil_flow_dump_protocols[];
 extern size_t ofputil_n_flow_dump_protocols;
 
-enum ofputil_protocol ofputil_protocol_from_ofp_version(int version);
-uint8_t ofputil_protocol_to_ofp_version(enum ofputil_protocol);
+enum ofputil_protocol
+ofputil_protocol_from_ofp_version(enum ofp_version version);
+enum ofp_version  ofputil_protocol_to_ofp_version(enum ofputil_protocol);
 
 bool ofputil_protocol_is_valid(enum ofputil_protocol);
 enum ofputil_protocol ofputil_protocol_set_tid(enum ofputil_protocol,
@@ -411,7 +412,7 @@ void ofputil_put_switch_features_port(const struct ofputil_phy_port *,
 bool ofputil_switch_features_ports_trunc(struct ofpbuf *b);
 
 /* phy_port helper functions. */
-int ofputil_pull_phy_port(uint8_t ofp_version, struct ofpbuf *,
+int ofputil_pull_phy_port(enum ofp_version ofp_version, struct ofpbuf *,
                           struct ofputil_phy_port *);
 size_t ofputil_count_phy_ports(uint8_t ofp_version, struct ofpbuf *);
 
@@ -483,7 +484,7 @@ uint32_t ofputil_decode_flow_monitor_cancel(const struct ofp_header *);
 struct ofpbuf *ofputil_encode_flow_monitor_cancel(uint32_t id);
 
 /* Encoding OpenFlow stats messages. */
-void ofputil_append_port_desc_stats_reply(uint8_t ofp_version,
+void ofputil_append_port_desc_stats_reply(enum ofp_version ofp_version,
                                           const struct ofputil_phy_port *pp,
                                           struct list *replies);
 

diff --git a/lib/vconn-provider.h b/lib/vconn-provider.h
index 2940852..54ec2e6 100644 (file)
--- a/lib/vconn-provider.h
+++ b/lib/vconn-provider.h
@@ -22,6 +22,7 @@
 
 #include <assert.h>
 #include "vconn.h"
+#include "openflow/openflow-common.h"
 \f
 /* Active virtual connection to an OpenFlow device. */
 
@@ -32,8 +33,8 @@ struct vconn {
     struct vconn_class *class;
     int state;
     int error;
-    int min_version;
-    int version;
+    enum ofp_version min_version;
+    enum ofp_version version;
     ovs_be32 remote_ip;
     ovs_be16 remote_port;
     ovs_be32 local_ip;

diff --git a/lib/vconn.c b/lib/vconn.c
index 8f4052c..caa1bcc 100644 (file)
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -278,7 +278,8 @@ vconn_run_wait(struct vconn *vconn)
 }
 
 int
-vconn_open_block(const char *name, int min_version, struct vconn **vconnp)
+vconn_open_block(const char *name, enum ofp_version min_version,
+                 struct vconn **vconnp)
 {
     struct vconn *vconn;
     int error;
@@ -361,7 +362,7 @@ vconn_get_local_port(const struct vconn *vconn)
  *
  * A vconn that has successfully connected (that is, vconn_connect() or
  * vconn_send() or vconn_recv() has returned 0) always negotiated a version. */
-int
+enum ofp_version
 vconn_get_version(const struct vconn *vconn)
 {
     return vconn->version;

diff --git a/lib/vconn.h b/lib/vconn.h
index ee4a568..fd8bb25 100644 (file)
--- a/lib/vconn.h
+++ b/lib/vconn.h
@@ -41,7 +41,7 @@ ovs_be32 vconn_get_remote_ip(const struct vconn *);
 ovs_be16 vconn_get_remote_port(const struct vconn *);
 ovs_be32 vconn_get_local_ip(const struct vconn *);
 ovs_be16 vconn_get_local_port(const struct vconn *);
-int vconn_get_version(const struct vconn *);
+enum ofp_version vconn_get_version(const struct vconn *);
 int vconn_connect(struct vconn *);
 int vconn_recv(struct vconn *, struct ofpbuf **);
 int vconn_send(struct vconn *, struct ofpbuf *);
@@ -54,7 +54,8 @@ int vconn_transact_multiple_noreply(struct vconn *, struct list *requests,
 void vconn_run(struct vconn *);
 void vconn_run_wait(struct vconn *);
 
-int vconn_open_block(const char *name, int min_version, struct vconn **);
+int vconn_open_block(const char *name, enum ofp_version min_version,
+                     struct vconn **);
 int vconn_send_block(struct vconn *, struct ofpbuf *);
 int vconn_recv_block(struct vconn *, struct ofpbuf **);
 

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 5fa57d8..0f24fd0 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2298,14 +2298,15 @@ handle_port_desc_stats_request(struct ofconn *ofconn,
                                const struct ofp_header *request)
 {
     struct ofproto *p = ofconn_get_ofproto(ofconn);
+    enum ofp_version version;
     struct ofport *port;
     struct list replies;
 
     ofpmp_init(&replies, request);
 
+    version = ofputil_protocol_to_ofp_version(ofconn_get_protocol(ofconn));
     HMAP_FOR_EACH (port, hmap_node, &p->ports) {
-        ofputil_append_port_desc_stats_reply(ofconn_get_protocol(ofconn),
-                                             &port->pp, &replies);
+        ofputil_append_port_desc_stats_reply(version, &port->pp, &replies);
     }
 
     ofconn_send_replies(ofconn, &replies);