Create specific types for ofp and odp port

Until now, datapath ports and openflow ports were both represented by
unsigned integers of various sizes. With implicit conversions, etc., it is
easy to mix them up and use one where the other is expected.  This commit
creates two typedefs, ofp_port_t and odp_port_t.  Both of these two types
are marked by "__attribute__((bitwise))" so that sparse can be used to
detect any misuse.

Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

Author: yew011

include/openflow/openflow-1.0.h

@@ -28,28 +28,22 @@
  * 0xff00...0xfff7  "reserved" but not assigned a meaning by OpenFlow 1.0
  * 0xfff8...0xffff  "reserved" OFPP_* ports with assigned meanings
  */
-enum ofp_port {
-    /* Ranges. */
-    OFPP_MAX        = 0xff00,   /* Maximum number of physical switch ports. */
-    OFPP_FIRST_RESV = 0xfff8,   /* First assigned reserved port number. */
-    OFPP_LAST_RESV  = 0xffff,   /* Last assigned reserved port number. */
-
-    /* Reserved output "ports". */
-    OFPP_IN_PORT    = 0xfff8,  /* Send the packet out the input port.  This
-                                  virtual port must be explicitly used
-                                  in order to send back out of the input
-                                  port. */
-    OFPP_TABLE      = 0xfff9,  /* Perform actions in flow table.
-                                  NB: This can only be the destination
-                                  port for packet-out messages. */
-    OFPP_NORMAL     = 0xfffa,  /* Process with normal L2/L3 switching. */
-    OFPP_FLOOD      = 0xfffb,  /* All physical ports except input port and
-                                  those disabled by STP. */
-    OFPP_ALL        = 0xfffc,  /* All physical ports except input port. */
-    OFPP_CONTROLLER = 0xfffd,  /* Send to controller. */
-    OFPP_LOCAL      = 0xfffe,  /* Local openflow "port". */
-    OFPP_NONE       = 0xffff   /* Not associated with a physical port. */
-};
+
+/* Ranges. */
+#define OFPP_MAX        OFP_PORT_C(0xff00) /* Max # of switch ports. */
+#define OFPP_FIRST_RESV OFP_PORT_C(0xfff8) /* First assigned reserved port. */
+#define OFPP_LAST_RESV  OFP_PORT_C(0xffff) /* Last assigned reserved port. */
+
+/* Reserved output "ports". */
+#define OFPP_IN_PORT    OFP_PORT_C(0xfff8) /* Where the packet came in. */
+#define OFPP_TABLE      OFP_PORT_C(0xfff9) /* Perform actions in flow table. */
+#define OFPP_NORMAL     OFP_PORT_C(0xfffa) /* Process with normal L2/L3. */
+#define OFPP_FLOOD      OFP_PORT_C(0xfffb) /* All ports except input port and
+                                            * ports disabled by STP. */
+#define OFPP_ALL        OFP_PORT_C(0xfffc) /* All ports except input port. */
+#define OFPP_CONTROLLER OFP_PORT_C(0xfffd) /* Send to controller. */
+#define OFPP_LOCAL      OFP_PORT_C(0xfffe) /* Local openflow "port". */
+#define OFPP_NONE       OFP_PORT_C(0xffff) /* Not associated with any port. */
 
 /* OpenFlow 1.0 specific capabilities supported by the datapath (struct
  * ofp_switch_features, member capabilities). */

include/openflow/openflow-1.1.h

@@ -67,8 +67,8 @@
  * an OpenFlow 1.0 reserved port number to or from, respectively, the
  * corresponding OpenFlow 1.1 reserved port number.
  */
-#define OFPP11_MAX    0xffffff00
-#define OFPP11_OFFSET (OFPP11_MAX - OFPP_MAX)
+#define OFPP11_MAX    OFP11_PORT_C(0xffffff00)
+#define OFPP11_OFFSET 0xffff0000    /* OFPP11_MAX - OFPP_MAX */
 
 /* Reserved wildcard port used only for flow mod (delete) and flow stats
  * requests. Selects all flows regardless of output port

include/openvswitch/types.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2011 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -60,4 +60,16 @@ typedef struct {
         ovs_be32 hi, lo;
 } ovs_32aligned_be64;
 
+/* ofp_port_t represents the port number of a OpenFlow switch.
+ * odp_port_t represents the port number on the datapath.
+ * ofp11_port_t represents the OpenFlow-1.1 port number. */
+typedef uint16_t OVS_BITWISE ofp_port_t;
+typedef uint32_t OVS_BITWISE odp_port_t;
+typedef uint32_t OVS_BITWISE ofp11_port_t;
+
+/* Macro functions that cast int types to ofp/odp/ofp11 types. */
+#define OFP_PORT_C(X) ((OVS_FORCE ofp_port_t) (X))
+#define ODP_PORT_C(X) ((OVS_FORCE odp_port_t) (X))
+#define OFP11_PORT_C(X) ((OVS_FORCE ofp11_port_t) (X))
+
 #endif /* openvswitch/types.h */

lib/bundle.c

@@ -35,14 +35,14 @@
 
 VLOG_DEFINE_THIS_MODULE(bundle);
 
-static uint16_t
+static ofp_port_t
 execute_ab(const struct ofpact_bundle *bundle,
-           bool (*slave_enabled)(uint16_t ofp_port, void *aux), void *aux)
+           bool (*slave_enabled)(ofp_port_t ofp_port, void *aux), void *aux)
 {
     size_t i;
 
     for (i = 0; i < bundle->n_slaves; i++) {
-        uint16_t slave = bundle->slaves[i];
+        ofp_port_t slave = bundle->slaves[i];
         if (slave_enabled(slave, aux)) {
             return slave;
         }
@@ -51,10 +51,10 @@ execute_ab(const struct ofpact_bundle *bundle,
     return OFPP_NONE;
 }
 
-static uint16_t
+static ofp_port_t
 execute_hrw(const struct ofpact_bundle *bundle,
             const struct flow *flow, struct flow_wildcards *wc,
-            bool (*slave_enabled)(uint16_t ofp_port, void *aux), void *aux)
+            bool (*slave_enabled)(ofp_port_t ofp_port, void *aux), void *aux)
 {
     uint32_t flow_hash, best_hash;
     int best, i;
@@ -85,10 +85,11 @@ execute_hrw(const struct ofpact_bundle *bundle,
  * calculate the result.  Uses 'slave_enabled' to determine if the slave
  * designated by 'ofp_port' is up.  Returns the chosen slave, or
  * OFPP_NONE if none of the slaves are acceptable. */
-uint16_t
+ofp_port_t
 bundle_execute(const struct ofpact_bundle *bundle,
                const struct flow *flow, struct flow_wildcards *wc,
-               bool (*slave_enabled)(uint16_t ofp_port, void *aux), void *aux)
+               bool (*slave_enabled)(ofp_port_t ofp_port, void *aux),
+               void *aux)
 {
     switch (bundle->algorithm) {
     case NX_BD_ALG_HRW:
@@ -186,7 +187,7 @@ bundle_from_openflow(const struct nx_action_bundle *nab,
 }
 
 enum ofperr
-bundle_check(const struct ofpact_bundle *bundle, int max_ports,
+bundle_check(const struct ofpact_bundle *bundle, ofp_port_t max_ports,
              const struct flow *flow)
 {
     static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -200,7 +201,7 @@ bundle_check(const struct ofpact_bundle *bundle, int max_ports,
     }
 
     for (i = 0; i < bundle->n_slaves; i++) {
-        uint16_t ofp_port = bundle->slaves[i];
+        ofp_port_t ofp_port = bundle->slaves[i];
         enum ofperr error;
 
         error = ofputil_check_output_port(ofp_port, max_ports);
@@ -246,7 +247,7 @@ bundle_to_nxast(const struct ofpact_bundle *bundle, struct ofpbuf *openflow)
 
     slaves = ofpbuf_put_zeros(openflow, slaves_len);
     for (i = 0; i < bundle->n_slaves; i++) {
-        slaves[i] = htons(bundle->slaves[i]);
+        slaves[i] = htons(ofp_to_u16(bundle->slaves[i]));
     }
 }
 
@@ -271,7 +272,7 @@ bundle_parse__(const char *s, char **save_ptr,
     bundle = ofpact_put_BUNDLE(ofpacts);
 
     for (;;) {
-        uint16_t slave_port;
+        ofp_port_t slave_port;
         char *slave;
 
         slave = strtok_r(NULL, ", []", save_ptr);

lib/bundle.h

@@ -35,13 +35,13 @@ struct ofpbuf;
  *
  * See include/openflow/nicira-ext.h for NXAST_BUNDLE specification. */
 
-uint16_t bundle_execute(const struct ofpact_bundle *, const struct flow *,
+ofp_port_t bundle_execute(const struct ofpact_bundle *, const struct flow *,
                         struct flow_wildcards *wc,
-                        bool (*slave_enabled)(uint16_t ofp_port, void *aux),
+                        bool (*slave_enabled)(ofp_port_t ofp_port, void *aux),
                         void *aux);
 enum ofperr bundle_from_openflow(const struct nx_action_bundle *,
                                  struct ofpbuf *ofpact);
-enum ofperr bundle_check(const struct ofpact_bundle *, int max_ports,
+enum ofperr bundle_check(const struct ofpact_bundle *, ofp_port_t max_ports,
                          const struct flow *);
 void bundle_to_nxast(const struct ofpact_bundle *, struct ofpbuf *of10);
 void bundle_parse(const char *, struct ofpbuf *ofpacts);

lib/dpif-linux.c

@@ -169,7 +169,8 @@ static int dpif_linux_init(void);
 static void open_dpif(const struct dpif_linux_dp *, struct dpif **);
 static bool dpif_linux_nln_parse(struct ofpbuf *, void *);
 static void dpif_linux_port_changed(const void *vport, void *dpif);
-static uint32_t dpif_linux_port_get_pid(const struct dpif *, uint32_t port_no);
+static uint32_t dpif_linux_port_get_pid(const struct dpif *,
+                                        odp_port_t port_no);
 
 static void dpif_linux_vport_to_ofpbuf(const struct dpif_linux_vport *,
                                        struct ofpbuf *);
@@ -261,7 +262,7 @@ open_dpif(const struct dpif_linux_dp *dp, struct dpif **dpifp)
 static void
 destroy_channels(struct dpif_linux *dpif)
 {
-    int i;
+    unsigned int i;
 
     if (dpif->epoll_fd < 0) {
         return;
@@ -280,7 +281,7 @@ destroy_channels(struct dpif_linux *dpif)
         dpif_linux_vport_init(&vport_request);
         vport_request.cmd = OVS_VPORT_CMD_SET;
         vport_request.dp_ifindex = dpif->dp_ifindex;
-        vport_request.port_no = i;
+        vport_request.port_no = u32_to_odp(i);
         vport_request.upcall_pid = &upcall_pid;
         dpif_linux_vport_transact(&vport_request, NULL, NULL);
 
@@ -300,19 +301,20 @@ destroy_channels(struct dpif_linux *dpif)
 }
 
 static int
-add_channel(struct dpif_linux *dpif, uint32_t port_no, struct nl_sock *sock)
+add_channel(struct dpif_linux *dpif, odp_port_t port_no, struct nl_sock *sock)
 {
     struct epoll_event event;
+    uint32_t port_idx = odp_to_u32(port_no);
 
     if (dpif->epoll_fd < 0) {
         return 0;
     }
 
     /* We assume that the datapath densely chooses port numbers, which
      * can therefore be used as an index into an array of channels. */
-    if (port_no >= dpif->uc_array_size) {
-        int new_size = port_no + 1;
-        int i;
+    if (port_idx >= dpif->uc_array_size) {
+        uint32_t new_size = port_idx + 1;
+        uint32_t i;
 
         if (new_size > MAX_PORTS) {
             VLOG_WARN_RL(&error_rl, "%s: datapath port %"PRIu32" too big",
@@ -333,29 +335,30 @@ add_channel(struct dpif_linux *dpif, uint32_t port_no, struct nl_sock *sock)
 
     memset(&event, 0, sizeof event);
     event.events = EPOLLIN;
-    event.data.u32 = port_no;
+    event.data.u32 = port_idx;
     if (epoll_ctl(dpif->epoll_fd, EPOLL_CTL_ADD, nl_sock_fd(sock),
                   &event) < 0) {
         return errno;
     }
 
-    nl_sock_destroy(dpif->channels[port_no].sock);
-    dpif->channels[port_no].sock = sock;
-    dpif->channels[port_no].last_poll = LLONG_MIN;
+    nl_sock_destroy(dpif->channels[port_idx].sock);
+    dpif->channels[port_idx].sock = sock;
+    dpif->channels[port_idx].last_poll = LLONG_MIN;
 
     return 0;
 }
 
 static void
-del_channel(struct dpif_linux *dpif, uint32_t port_no)
+del_channel(struct dpif_linux *dpif, odp_port_t port_no)
 {
     struct dpif_channel *ch;
+    uint32_t port_idx = odp_to_u32(port_no);
 
-    if (dpif->epoll_fd < 0 || port_no >= dpif->uc_array_size) {
+    if (dpif->epoll_fd < 0 || port_idx >= dpif->uc_array_size) {
         return;
     }
 
-    ch = &dpif->channels[port_no];
+    ch = &dpif->channels[port_idx];
     if (!ch->sock) {
         return;
     }
@@ -482,7 +485,7 @@ netdev_to_ovs_vport_type(const struct netdev *netdev)
 
 static int
 dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
-                    uint32_t *port_nop)
+                    odp_port_t *port_nop)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
     const struct netdev_tunnel_config *tnl_cfg;
@@ -541,7 +544,7 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
         VLOG_DBG("%s: assigning port %"PRIu32" to netlink pid %"PRIu32,
                  dpif_name(dpif_), reply.port_no, upcall_pid);
     } else {
-        if (error == EBUSY && *port_nop != UINT32_MAX) {
+        if (error == EBUSY && *port_nop != ODPP_NONE) {
             VLOG_INFO("%s: requested port %"PRIu32" is in use",
                       dpif_name(dpif_), *port_nop);
         }
@@ -573,7 +576,7 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
 }
 
 static int
-dpif_linux_port_del(struct dpif *dpif_, uint32_t port_no)
+dpif_linux_port_del(struct dpif *dpif_, odp_port_t port_no)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
     struct dpif_linux_vport vport;
@@ -591,7 +594,7 @@ dpif_linux_port_del(struct dpif *dpif_, uint32_t port_no)
 }
 
 static int
-dpif_linux_port_query__(const struct dpif *dpif, uint32_t port_no,
+dpif_linux_port_query__(const struct dpif *dpif, odp_port_t port_no,
                         const char *port_name, struct dpif_port *dpif_port)
 {
     struct dpif_linux_vport request;
@@ -622,7 +625,7 @@ dpif_linux_port_query__(const struct dpif *dpif, uint32_t port_no,
 }
 
 static int
-dpif_linux_port_query_by_number(const struct dpif *dpif, uint32_t port_no,
+dpif_linux_port_query_by_number(const struct dpif *dpif, odp_port_t port_no,
                                 struct dpif_port *dpif_port)
 {
     return dpif_linux_port_query__(dpif, port_no, NULL, dpif_port);
@@ -635,23 +638,24 @@ dpif_linux_port_query_by_name(const struct dpif *dpif, const char *devname,
     return dpif_linux_port_query__(dpif, 0, devname, dpif_port);
 }
 
-static int
+static odp_port_t
 dpif_linux_get_max_ports(const struct dpif *dpif OVS_UNUSED)
 {
-    return MAX_PORTS;
+    return u32_to_odp(MAX_PORTS);
 }
 
 static uint32_t
-dpif_linux_port_get_pid(const struct dpif *dpif_, uint32_t port_no)
+dpif_linux_port_get_pid(const struct dpif *dpif_, odp_port_t port_no)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
+    uint32_t port_idx = odp_to_u32(port_no);
 
     if (dpif->epoll_fd < 0) {
         return 0;
     } else {
-        /* The UINT32_MAX "reserved" port number uses the "ovs-system"'s
+        /* The ODPP_NONE "reserved" port number uses the "ovs-system"'s
          * channel, since it is not heavily loaded. */
-        int idx = (port_no >= dpif->uc_array_size) ? 0 : port_no;
+        uint32_t idx = port_idx >= dpif->uc_array_size ? 0 : port_idx;
         return nl_sock_pid(dpif->channels[idx].sock);
     }
 }
@@ -1562,7 +1566,7 @@ dpif_linux_vport_from_ofpbuf(struct dpif_linux_vport *vport,
 
     vport->cmd = genl->cmd;
     vport->dp_ifindex = ovs_header->dp_ifindex;
-    vport->port_no = nl_attr_get_u32(a[OVS_VPORT_ATTR_PORT_NO]);
+    vport->port_no = nl_attr_get_odp_port(a[OVS_VPORT_ATTR_PORT_NO]);
     vport->type = nl_attr_get_u32(a[OVS_VPORT_ATTR_TYPE]);
     vport->name = nl_attr_get_string(a[OVS_VPORT_ATTR_NAME]);
     if (a[OVS_VPORT_ATTR_UPCALL_PID]) {
@@ -1592,8 +1596,8 @@ dpif_linux_vport_to_ofpbuf(const struct dpif_linux_vport *vport,
     ovs_header = ofpbuf_put_uninit(buf, sizeof *ovs_header);
     ovs_header->dp_ifindex = vport->dp_ifindex;
 
-    if (vport->port_no != UINT32_MAX) {
-        nl_msg_put_u32(buf, OVS_VPORT_ATTR_PORT_NO, vport->port_no);
+    if (vport->port_no != ODPP_NONE) {
+        nl_msg_put_odp_port(buf, OVS_VPORT_ATTR_PORT_NO, vport->port_no);
     }
 
     if (vport->type != OVS_VPORT_TYPE_UNSPEC) {
@@ -1624,7 +1628,7 @@ void
 dpif_linux_vport_init(struct dpif_linux_vport *vport)
 {
     memset(vport, 0, sizeof *vport);
-    vport->port_no = UINT32_MAX;
+    vport->port_no = ODPP_NONE;
 }
 
 /* Executes 'request' in the kernel datapath.  If the command fails, returns a

lib/dpif-linux.h

@@ -22,6 +22,8 @@
 #include <stdint.h>
 #include <linux/openvswitch.h>
 
+#include "flow.h"
+
 struct ofpbuf;
 
 struct dpif_linux_vport {
@@ -30,7 +32,7 @@ struct dpif_linux_vport {
 
     /* ovs_vport header. */
     int dp_ifindex;
-    uint32_t port_no;                      /* UINT32_MAX if unknown. */
+    odp_port_t port_no;                    /* ODPP_NONE if unknown. */
     enum ovs_vport_type type;
 
     /* Attributes.