From 2e3fa633aad59f9687e2b6e3aad3b0a308aa89fc Mon Sep 17 00:00:00 2001 From: Simon Horman Date: Mon, 30 Jul 2012 11:02:59 +0900 Subject: [PATCH] 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 Signed-off-by: Ben Pfaff --- include/openflow/openflow-common.h | 8 +- lib/ofp-msgs.c | 47 +++++++--- lib/ofp-util.c | 142 ++++++++++++++++++++++------- lib/ofp-util.h | 9 +- lib/vconn-provider.h | 5 +- lib/vconn.c | 5 +- lib/vconn.h | 5 +- ofproto/ofproto.c | 5 +- 8 files changed, 164 insertions(+), 62 deletions(-) diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h index 202ba1ed..0bca0d26 100644 --- 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 48ecafc6..07ee87a3 100644 --- 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 8d0f9938..a943b4ec 100644 --- 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(); } } @@ -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 c87d6488..9230b36f 100644 --- 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 29408524..54ec2e6b 100644 --- a/lib/vconn-provider.h +++ b/lib/vconn-provider.h @@ -22,6 +22,7 @@ #include #include "vconn.h" +#include "openflow/openflow-common.h" /* 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 8f4052cc..caa1bccf 100644 --- 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 ee4a5681..fd8bb253 100644 --- 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 5fa57d85..0f24fd01 100644 --- 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); -- 2.30.2