From: Ben Pfaff <blp@nicira.com>
Date: Wed, 10 Sep 2008 20:22:25 +0000 (-0700)
Subject: Make switches send error messages when they receive a bad request.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=39136b0fb9a89fa1489ef30eccfef115e584a74d;p=openvswitch

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

diff --git a/datapath/datapath.c b/datapath/datapath.c
index c2ffc4a1..4cea5a83 100644
--- 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 f13a6722..1e1f2bf5 100644
--- 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 97aceb9c..5d57fb16 100644
--- 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 e7d15893..0309b4c4 100644
--- 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 1f705b84..c709fb79 100644
--- 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;
 }
 
 /* Packet buffering. */