secchan: Disallow port numbers not supported by datapath.

The OpenFlow protocol supports port numbers from 0 to 0xff00.
The datapath protocol supports port numbers from 0 to 0x100.

The datapath will reject port numbers in the range 0x100 to 0xff00, but the
secchan was not screening these out.  Thus, attempting to add a flow or
send a packet to one of these ports would result in a kernel EINVAL error,
instead of a more sensible OpenFlow error.  This commit remedies the
situation.

Thanks to Justin for pointing out the issue.

diff --git a/lib/vconn.c b/lib/vconn.c
index 6af44214186d1d5b78836fa16da406899aa83caf..e9afac27ecfb73e9c4b8b6ddc4e4ae6734731b35 100644 (file)
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -1089,7 +1089,7 @@ check_ofp_message_array(const struct ofp_header *msg, uint8_t type,
 
 int
 check_ofp_packet_out(const struct ofp_header *oh, struct ofpbuf *data,
-                     int *n_actionsp)
+                     int *n_actionsp, int max_ports)
 {
     const struct ofp_packet_out *opo;
     unsigned int actions_len, n_actions;
@@ -1120,7 +1120,7 @@ check_ofp_packet_out(const struct ofp_header *oh, struct ofpbuf *data,
 
     n_actions = actions_len / sizeof(union ofp_action);
     error = validate_actions((const union ofp_action *) opo->actions,
-                             n_actions);
+                             n_actions, max_ports);
     if (error) {
         return error;
     }
@@ -1193,7 +1193,7 @@ check_action_exact_len(const union ofp_action *a, unsigned int len,
 }
 
 static int
-check_action_port(int port)
+check_action_port(int port, int max_ports)
 {
     switch (port) {
     case OFPP_IN_PORT:
@@ -1206,7 +1206,7 @@ check_action_port(int port)
         return 0;
 
     default:
-        if (port >= 0 && port < OFPP_MAX) {
+        if (port >= 0 && port < max_ports) {
             return 0;
         }
         VLOG_WARN_RL(&bad_ofmsg_rl, "unknown output port %x", port);
@@ -1235,13 +1235,13 @@ check_nicira_action(const union ofp_action *a, unsigned int len)
 }
 
 static int
-check_action(const union ofp_action *a, unsigned int len)
+check_action(const union ofp_action *a, unsigned int len, int max_ports)
 {
     int error;
 
     switch (a->type) {
     case OFPAT_OUTPUT:
-        error = check_action_port(ntohs(a->output.port));
+        error = check_action_port(ntohs(a->output.port), max_ports);
         if (error) {
             return error;
         }
@@ -1286,7 +1286,8 @@ check_action(const union ofp_action *a, unsigned int len)
 }
 
 int
-validate_actions(const union ofp_action *actions, size_t n_actions)
+validate_actions(const union ofp_action *actions, size_t n_actions,
+                 int max_ports)
 {
     const union ofp_action *a;
 
@@ -1302,7 +1303,7 @@ validate_actions(const union ofp_action *actions, size_t n_actions)
                         n_slots, slots_left);
             return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN);
         }
-        error = check_action(a, len);
+        error = check_action(a, len, max_ports);
         if (error) {
             return error;
         }

diff --git a/lib/vconn.h b/lib/vconn.h
index 54893e06e970c4339a68c8258f409f2fee743bb5..44736d6107b74293646ff3a6c88370ef3cbf9362 100644 (file)
--- a/lib/vconn.h
+++ b/lib/vconn.h
@@ -114,7 +114,7 @@ int check_ofp_message_array(const struct ofp_header *, uint8_t type,
                             size_t size, size_t array_elt_size,
                             size_t *n_array_elts);
 int check_ofp_packet_out(const struct ofp_header *, struct ofpbuf *data,
-                         int *n_actions);
+                         int *n_actions, int max_ports);
 
 struct flow_stats_iterator {
     const uint8_t *pos, *end;
@@ -130,7 +130,8 @@ const union ofp_action *actions_first(struct actions_iterator *,
                                       const union ofp_action *,
                                       size_t n_actions);
 const union ofp_action *actions_next(struct actions_iterator *);
-int validate_actions(const union ofp_action *, size_t n_actions);
+int validate_actions(const union ofp_action *, size_t n_actions,
+                     int max_ports);
 
 void normalize_match(struct ofp_match *);
 

diff --git a/secchan/ofproto.c b/secchan/ofproto.c
index 534d001f851cf9f4e7ceaf5dd6fd645b0acf04ec..9a70f3e19d6e8b887dbcc60bf816f08ab3a29f52 100644 (file)
--- a/secchan/ofproto.c
+++ b/secchan/ofproto.c
@@ -213,6 +213,7 @@ struct ofproto {
     struct port_array ports;    /* Index is ODP port nr; ofport->opp.port_no is
                                  * OFP port nr. */
     struct shash port_by_name;
+    uint32_t max_ports;
 
     /* Configuration. */
     struct switch_status *switch_status;
@@ -275,6 +276,7 @@ ofproto_create(const char *datapath, const struct ofhooks *ofhooks, void *aux,
                struct ofproto **ofprotop)
 {
     struct dpifmon *dpifmon;
+    struct odp_stats stats;
     struct ofproto *p;
     struct dpif dpif;
     int error;
@@ -287,6 +289,13 @@ ofproto_create(const char *datapath, const struct ofhooks *ofhooks, void *aux,
         VLOG_ERR("failed to open datapath %s: %s", datapath, strerror(error));
         return error;
     }
+    error = dpif_get_dp_stats(&dpif, &stats);
+    if (error) {
+        VLOG_ERR("failed to obtain stats for datapath %s: %s",
+                 datapath, strerror(error));
+        dpif_close(&dpif);
+        return error;
+    }
     error = dpif_set_listen_mask(&dpif, ODPL_MISS | ODPL_ACTION);
     if (error) {
         VLOG_ERR("failed to listen on datapath %s: %s",
@@ -321,6 +330,7 @@ ofproto_create(const char *datapath, const struct ofhooks *ofhooks, void *aux,
     p->dpifmon = dpifmon;
     port_array_init(&p->ports);
     shash_init(&p->port_by_name);
+    p->max_ports = stats.max_ports;
 
     /* Initialize submodules. */
     p->switch_status = switch_status_create(p);
@@ -2099,7 +2109,7 @@ handle_packet_out(struct ofproto *p, struct ofconn *ofconn,
     flow_t flow;
     int error;
 
-    error = check_ofp_packet_out(oh, &payload, &n_actions);
+    error = check_ofp_packet_out(oh, &payload, &n_actions, p->max_ports);
     if (error) {
         return error;
     }
@@ -2721,7 +2731,7 @@ handle_flow_mod(struct ofproto *p, struct ofconn *ofconn,
     }
 
     error = validate_actions((const union ofp_action *) ofm->actions,
-                             n_actions);
+                             n_actions, p->max_ports);
     if (error) {
         return error;
     }