Make switches send error messages when they receive a bad request.

diff --git a/datapath/datapath.c b/datapath/datapath.c
index c2ffc4a..4cea5a8 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -947,7 +947,7 @@ EXPORT_SYMBOL(dp_send_flow_expired);
 
 int
 dp_send_error_msg(struct datapath *dp, const struct sender *sender, 
-               uint16_t type, uint16_t code, const uint8_t *data, size_t len)
+               uint16_t type, uint16_t code, const void *data, size_t len)
 {
        struct sk_buff *skb;
        struct ofp_error_msg *oem;
@@ -1550,6 +1550,8 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
         * struct genl_ops.  This kluge supports earlier versions also. */
        cb->done = dp_genl_openflow_done;
 
+       sender.pid = NETLINK_CB(cb->skb).pid;
+       sender.seq = cb->nlh->nlmsg_seq;
        if (!cb->args[0]) {
                struct nlattr *attrs[DP_GENL_A_MAX + 1];
                struct ofp_stats_request *rq;
@@ -1575,13 +1577,21 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
                        return -EINVAL;
 
                rq = nla_data(va);
-               type = ntohs(rq->type);
-               if (rq->header.version != OFP_VERSION
-                   || rq->header.type != OFPT_STATS_REQUEST
-                   || ntohs(rq->header.length) != len
-                   || type >= ARRAY_SIZE(stats)
-                   || !stats[type].dump)
+               sender.xid = type = ntohs(rq->type);
+               if (rq->header.version != OFP_VERSION) {
+                       dp_send_error_msg(dp, &sender, OFPET_BAD_REQUEST,
+                                         OFPBRC_BAD_VERSION, rq, len);
+                       return -EINVAL;
+               }
+               if (rq->header.type != OFPT_STATS_REQUEST
+                   || ntohs(rq->header.length) != len)
+                       return -EINVAL;
+
+               if (type >= ARRAY_SIZE(stats) || !stats[type].dump) {
+                       dp_send_error_msg(dp, &sender, OFPET_BAD_REQUEST,
+                                         OFPBRC_BAD_STAT, rq, len);
                        return -EINVAL;
+               }
 
                s = &stats[type];
                body_len = len - offsetof(struct ofp_stats_request, body);
@@ -1600,6 +1610,7 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
                        cb->args[4] = (long) state;
                }
        } else if (cb->args[0] == 1) {
+               sender.xid = cb->args[3];
                dp_idx = cb->args[1];
                s = &stats[cb->args[2]];
 
@@ -1610,10 +1621,6 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
                return 0;
        }
 
-       sender.xid = cb->args[3];
-       sender.pid = NETLINK_CB(cb->skb).pid;
-       sender.seq = cb->nlh->nlmsg_seq;
-
        osr = put_openflow_headers(dp, skb, OFPT_STATS_REPLY, &sender,
                                   &max_openflow_len);
        if (IS_ERR(osr))

diff --git a/datapath/datapath.h b/datapath/datapath.h
index f13a672..1e1f2bf 100644 (file)
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -96,7 +96,7 @@ int dp_send_config_reply(struct datapath *, const struct sender *);
 int dp_send_flow_expired(struct datapath *, struct sw_flow *,
                         enum ofp_flow_expired_reason);
 int dp_send_error_msg(struct datapath *, const struct sender *, 
-                       uint16_t, uint16_t, const uint8_t *, size_t);
+                       uint16_t, uint16_t, const void *, size_t);
 int dp_update_port_flags(struct datapath *dp, const struct ofp_port_mod *opm);
 int dp_send_echo_reply(struct datapath *, const struct sender *,
                       const struct ofp_header *);

diff --git a/datapath/forward.c b/datapath/forward.c
index 97aceb9..5d57fb1 100644 (file)
--- a/datapath/forward.c
+++ b/datapath/forward.c
@@ -551,21 +551,28 @@ fwd_control_input(struct sw_chain *chain, const struct sender *sender,
                },
        };
 
-       const struct openflow_packet *pkt;
        struct ofp_header *oh;
 
        oh = (struct ofp_header *) msg;
-       if (oh->version != OFP_VERSION || oh->type >= ARRAY_SIZE(packets)
-               || ntohs(oh->length) > length)
+       if (oh->version != OFP_VERSION) {
+               dp_send_error_msg(chain->dp, sender, OFPET_BAD_REQUEST,
+                                 OFPBRC_BAD_VERSION, msg, length);
+               return -EINVAL;
+       }
+       if (ntohs(oh->length) > length)
                return -EINVAL;
 
-       pkt = &packets[oh->type];
-       if (!pkt->handler)
-               return -ENOSYS;
-       if (length < pkt->min_size)
-               return -EFAULT;
-
-       return pkt->handler(chain, sender, msg);
+       if (oh->type < ARRAY_SIZE(packets)) {
+               const struct openflow_packet *pkt = &packets[oh->type];
+               if (pkt->handler) {
+                       if (length < pkt->min_size)
+                               return -EFAULT;
+                       return pkt->handler(chain, sender, msg);
+               }
+       }
+       dp_send_error_msg(chain->dp, sender, OFPET_BAD_REQUEST,
+                         OFPBRC_BAD_TYPE, msg, length);
+       return -EINVAL;
 }
 
 /* Packet buffering. */

diff --git a/include/openflow.h b/include/openflow.h
index e7d1589..0309b4c 100644 (file)
--- a/include/openflow.h
+++ b/include/openflow.h
@@ -451,6 +451,18 @@ struct ofp_flow_expired {
 };
 OFP_ASSERT(sizeof(struct ofp_flow_expired) == 72);
 
+enum ofp_error_type {
+    OFPET_BAD_REQUEST           /* Request was not understood. */
+};
+
+enum ofp_bad_request_code {
+    OFPBRC_BAD_VERSION,         /* ofp_header.version not supported. */
+    OFPBRC_BAD_TYPE,            /* ofp_header.type not supported. */
+    OFPBRC_BAD_STAT,            /* ofp_stats_request.type not supported. */
+    OFPBRC_BAD_VENDOR           /* Vendor not supported (in ofp_vendor or
+                                 * ofp_stats_request or ofp_stats_reply). */
+};
+
 /* Error message (datapath -> controller). */
 struct ofp_error_msg {
     struct ofp_header header;

diff --git a/switch/datapath.c b/switch/datapath.c
index 1f705b8..c709fb7 100644 (file)
--- a/switch/datapath.c
+++ b/switch/datapath.c
@@ -793,7 +793,7 @@ send_flow_expired(struct datapath *dp, struct sw_flow *flow,
 
 void
 dp_send_error_msg(struct datapath *dp, const struct sender *sender,
-        uint16_t type, uint16_t code, const uint8_t *data, size_t len)
+                  uint16_t type, uint16_t code, const void *data, size_t len)
 {
     struct buffer *buffer;
     struct ofp_error_msg *oem;
@@ -1575,6 +1575,8 @@ recv_stats_request(struct datapath *dp, const struct sender *sender,
 
     type = ntohs(rq->type);
     if (type >= ARRAY_SIZE(stats) || !stats[type].dump) {
+        dp_send_error_msg(dp, sender, OFPET_BAD_REQUEST, OFPBRC_BAD_STAT,
+                          rq, rq_len);
         VLOG_WARN_RL(&rl, "received stats request of unknown type %d", type);
         return -EINVAL;
     }
@@ -1677,21 +1679,24 @@ fwd_control_input(struct datapath *dp, const struct sender *sender,
         },
     };
 
-    const struct openflow_packet *pkt;
     struct ofp_header *oh;
 
     oh = (struct ofp_header *) msg;
     assert(oh->version == OFP_VERSION);
-    if (oh->type >= ARRAY_SIZE(packets) || ntohs(oh->length) > length)
+    if (ntohs(oh->length) > length)
         return -EINVAL;
 
-    pkt = &packets[oh->type];
-    if (!pkt->handler)
-        return -ENOSYS;
-    if (length < pkt->min_size)
-        return -EFAULT;
-
-    return pkt->handler(dp, sender, msg);
+    if (oh->type < ARRAY_SIZE(packets)) {
+        const struct openflow_packet *pkt = &packets[oh->type];
+        if (pkt->handler) {
+            if (length < pkt->min_size)
+                return -EFAULT;
+            return pkt->handler(dp, sender, msg);
+        }
+    }
+    dp_send_error_msg(dp, sender, OFPET_BAD_REQUEST, OFPBRC_BAD_TYPE,
+                      msg, length);
+    return -EINVAL;
 }
 \f
 /* Packet buffering. */