Implement support for version negotiation using hello messages. This

also prevents connections from switching the version in the middle of
the operation.

tweak from and ok reyk@

5 files changed, 200 insertions, 19 deletions

diff --git a/usr.sbin/switchd/ofp.c b/usr.sbin/switchd/ofp.c
index 7b8fadbd82a..02fd26ab16e 100644
--- a/usr.sbin/switchd/ofp.c
+++ b/usr.sbin/switchd/ofp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ofp.c,v 1.15 2016/11/04 22:27:08 reyk Exp $	*/
+/*	$OpenBSD: ofp.c,v 1.16 2016/11/22 17:21:56 rzalamena Exp $	*/
 
 /*
  * Copyright (c) 2013-2016 Reyk Floeter <reyk@openbsd.org>
@@ -132,6 +132,13 @@ ofp_input(struct switch_connection *con, struct ibuf *ibuf)
 		return (-1);
 	}
 
+	if (con->con_version != OFP_V_0 &&
+	    oh->oh_version != con->con_version) {
+		log_debug("wrong version %d, expected %d",
+		    oh->oh_version, con->con_version);
+		return (-1);
+	}
+
 	switch (oh->oh_version) {
 	case OFP_V_1_0:
 		if (ofp10_input(sc, con, oh, ibuf) != 0)
@@ -166,6 +173,10 @@ ofp_open(struct privsep *ps, struct switch_connection *con)
 	    __func__, con->con_id, con->con_instance,
 	    sw == NULL ? 0 : sw->sw_id);
 
+	/* Send the hello with the latest version we support. */
+	if (ofp_send_hello(ps->ps_env, con, OFP_V_1_3) == -1)
+		return (-1);
+
 	return (0);
 }
 
diff --git a/usr.sbin/switchd/ofp10.c b/usr.sbin/switchd/ofp10.c
index 56b7dfdee25..c990442f596 100644
--- a/usr.sbin/switchd/ofp10.c
+++ b/usr.sbin/switchd/ofp10.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ofp10.c,v 1.16 2016/11/21 18:19:51 rzalamena Exp $	*/
+/*	$OpenBSD: ofp10.c,v 1.17 2016/11/22 17:21:56 rzalamena Exp $	*/
 
 /*
  * Copyright (c) 2013-2016 Reyk Floeter <reyk@openbsd.org>
@@ -60,7 +60,7 @@ int	 ofp10_validate_packet_out(struct switchd *,
 	    struct ofp_header *, struct ibuf *);
 
 struct ofp_callback ofp10_callbacks[] = {
-	{ OFP10_T_HELLO,		ofp10_hello, NULL },
+	{ OFP10_T_HELLO,		ofp10_hello, ofp_validate_hello },
 	{ OFP10_T_ERROR,		NULL, ofp10_validate_error },
 	{ OFP10_T_ECHO_REQUEST,		ofp10_echo_request, NULL },
 	{ OFP10_T_ECHO_REPLY,		NULL, NULL },
@@ -262,13 +262,8 @@ ofp10_hello(struct switchd *sc, struct switch_connection *con,
 		return (-1);
 	}
 
-	/* Echo back the received Hello packet */
-	oh->oh_version = OFP_V_1_0;
-	oh->oh_length = htons(sizeof(*oh));
-	oh->oh_xid = htonl(con->con_xidnxt++);
-	if (ofp10_validate(sc, &con->con_local, &con->con_peer, oh, NULL) != 0)
+	if (ofp_recv_hello(sc, con, oh, ibuf) == -1)
 		return (-1);
-	ofp_output(con, oh, NULL);
 
 #if 0
 	(void)write(fd, &oh, sizeof(oh));
diff --git a/usr.sbin/switchd/ofp13.c b/usr.sbin/switchd/ofp13.c
index 38adacb258b..43ed45f1640 100644
--- a/usr.sbin/switchd/ofp13.c
+++ b/usr.sbin/switchd/ofp13.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ofp13.c,v 1.39 2016/11/21 19:33:12 rzalamena Exp $	*/
+/*	$OpenBSD: ofp13.c,v 1.40 2016/11/22 17:21:56 rzalamena Exp $	*/
 
 /*
  * Copyright (c) 2013-2016 Reyk Floeter <reyk@openbsd.org>
@@ -109,7 +109,7 @@ int	 ofp13_tablemiss_sendctrl(struct switchd *, struct switch_connection *,
 	    uint8_t);
 
 struct ofp_callback ofp13_callbacks[] = {
-	{ OFP_T_HELLO,			ofp13_hello, NULL },
+	{ OFP_T_HELLO,			ofp13_hello, ofp_validate_hello },
 	{ OFP_T_ERROR,			NULL, ofp13_validate_error },
 	{ OFP_T_ECHO_REQUEST,		ofp13_echo_request, NULL },
 	{ OFP_T_ECHO_REPLY,		NULL, NULL },
@@ -639,13 +639,8 @@ ofp13_hello(struct switchd *sc, struct switch_connection *con,
 		return (-1);
 	}
 
-	/* Echo back the received Hello packet */
-	oh->oh_version = OFP_V_1_3;
-	oh->oh_length = htons(sizeof(*oh));
-	oh->oh_xid = htonl(con->con_xidnxt++);
-	if (ofp13_validate(sc, &con->con_local, &con->con_peer, oh, NULL) != 0)
+	if (ofp_recv_hello(sc, con, oh, ibuf) == -1)
 		return (-1);
-	ofp_output(con, oh, NULL);
 
 	/* Ask for switch features so we can get more information. */
 	if (ofp13_featuresrequest(sc, con) == -1)
diff --git a/usr.sbin/switchd/ofp_common.c b/usr.sbin/switchd/ofp_common.c
index 019f3c652ca..8fdcbd067d2 100644
--- a/usr.sbin/switchd/ofp_common.c
+++ b/usr.sbin/switchd/ofp_common.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ofp_common.c,v 1.7 2016/11/17 13:10:26 reyk Exp $	*/
+/*	$OpenBSD: ofp_common.c,v 1.8 2016/11/22 17:21:56 rzalamena Exp $	*/
 
 /*
  * Copyright (c) 2013-2016 Reyk Floeter <reyk@openbsd.org>
@@ -43,6 +43,8 @@
 #include "switchd.h"
 #include "ofp_map.h"
 
+int		ofp_setversion(struct switch_connection *, int);
+
 int
 ofp_validate_header(struct switchd *sc,
     struct sockaddr_storage *src, struct sockaddr_storage *dst,
@@ -118,6 +120,176 @@ ofp_output(struct switch_connection *con, struct ofp_header *oh,
 	return (0);
 }
 
+int
+ofp_send_hello(struct switchd *sc, struct switch_connection *con, int version)
+{
+	struct ofp_hello_element_header	*he;
+	struct ofp_header		*oh;
+	struct ibuf			*ibuf;
+	size_t				 hstart, hend;
+	uint32_t			*bmp;
+	int				 rv = -1;
+
+	if ((ibuf = ibuf_static()) == NULL ||
+	    (oh = ibuf_advance(ibuf, sizeof(*oh))) == NULL ||
+	    (he = ibuf_advance(ibuf, sizeof(*he))) == NULL)
+		goto done;
+
+	/* Write down all versions we support. */
+	hstart = ibuf->wpos;
+	if ((bmp = ibuf_advance(ibuf, sizeof(*bmp))) == NULL)
+		goto done;
+
+	*bmp = htonl((1 << OFP_V_1_0) | (1 << OFP_V_1_3));
+	hend = ibuf->wpos;
+
+	/* Fill the headers. */
+	oh->oh_version = version;
+	oh->oh_type = OFP_T_HELLO;
+	oh->oh_length = htons(ibuf_length(ibuf));
+	oh->oh_xid = htonl(con->con_xidnxt++);
+	he->he_type = htons(OFP_HELLO_T_VERSION_BITMAP);
+	he->he_length = htons(sizeof(*he) + (hend - hstart));
+
+	if (ofp_validate(sc, &con->con_local, &con->con_peer, oh, ibuf,
+	    version) != 0)
+		goto done;
+
+	rv = ofp_output(con, NULL, ibuf);
+
+ done:
+	ibuf_free(ibuf);
+	return (rv);
+}
+
+int
+ofp_validate_hello(struct switchd *sc,
+    struct sockaddr_storage *src, struct sockaddr_storage *dst,
+    struct ofp_header *oh, struct ibuf *ibuf)
+{
+	struct ofp_hello_element_header	*he;
+	uint32_t			*bmp;
+	off_t				 poff;
+	int				 helen, i, ver;
+
+	/* No extra element headers. */
+	if (ntohs(oh->oh_length) == sizeof(*oh))
+		return (0);
+
+	/* Test for supported element headers. */
+	if ((he = ibuf_seek(ibuf, sizeof(*oh), sizeof(*he))) == NULL)
+		return (-1);
+	if (he->he_type != htons(OFP_HELLO_T_VERSION_BITMAP))
+		return (-1);
+
+	log_debug("\tversion bitmap:");
+
+	/* Validate header sizes. */
+	helen = ntohs(he->he_length);
+	if (helen < (int)sizeof(*he))
+		return (-1);
+	else if (helen == sizeof(*he))
+		return (0);
+
+	helen -= sizeof(*he);
+	/* Invalid bitmap size. */
+	if ((helen % sizeof(*bmp)) != 0)
+		return (-1);
+
+	ver = 0;
+	poff = sizeof(*oh) + sizeof(*he);
+	while (helen > 0) {
+		if ((bmp = ibuf_seek(ibuf, poff, sizeof(*bmp))) == NULL)
+			return (-1);
+
+		for (i = 0; i < 32; i++, ver++) {
+			if ((ntohl(*bmp) & (1 << i)) == 0)
+				continue;
+
+			log_debug("\t\tversion %s",
+			    print_map(ver, ofp_v_map));
+		}
+
+		helen -= sizeof(*bmp);
+		poff += sizeof(*bmp);
+	}
+
+	return (0);
+}
+
+int
+ofp_setversion(struct switch_connection *con, int version)
+{
+	switch (version) {
+	case OFP_V_1_0:
+	case OFP_V_1_3:
+		con->con_version = version;
+		return (0);
+
+	default:
+		return (-1);
+	}
+}
+
+int
+ofp_recv_hello(struct switchd *sc, struct switch_connection *con,
+    struct ofp_header *oh, struct ibuf *ibuf)
+{
+	struct ofp_hello_element_header	*he;
+	uint32_t			*bmp;
+	off_t				 poff;
+	int				 helen, i, ver;
+
+	/* No extra element headers, just use the header version. */
+	if (ntohs(oh->oh_length) == sizeof(*oh))
+		return (ofp_setversion(con, oh->oh_version));
+
+	/* Read the element header. */
+	if ((he = ibuf_seek(ibuf, sizeof(*oh), sizeof(*he))) == NULL)
+		return (-1);
+
+	/* We don't support anything else than the version bitmap. */
+	if (he->he_type != htons(OFP_HELLO_T_VERSION_BITMAP))
+		return (-1);
+
+	/* Validate header sizes. */
+	helen = ntohs(he->he_length);
+	if (helen < (int)sizeof(*he))
+		return (-1);
+	else if (helen == sizeof(*he))
+		return (ofp_setversion(con, oh->oh_version));
+
+	helen -= sizeof(*he);
+	/* Invalid bitmap size. */
+	if ((helen % sizeof(*bmp)) != 0)
+		return (-1);
+
+	ver = 0;
+	poff = sizeof(*oh) + sizeof(*he);
+
+	/* Loop through the bitmaps and choose the higher version. */
+	while (helen > 0) {
+		if ((bmp = ibuf_seek(ibuf, poff, sizeof(*bmp))) == NULL)
+			return (-1);
+
+		for (i = 0; i < 32; i++, ver++) {
+			if ((ntohl(*bmp) & (1 << i)) == 0)
+				continue;
+
+			ofp_setversion(con, ver);
+		}
+
+		helen -= sizeof(*bmp);
+		poff += sizeof(*bmp);
+	}
+
+	/* Check if we have set any version, otherwise fallback. */
+	if (con->con_version == OFP_V_0)
+		return (ofp_setversion(con, oh->oh_version));
+
+	return (0);
+}
+
 /* Appends an action with just the generic header. */
 int
 action_new(struct ibuf *ibuf, uint16_t type)
diff --git a/usr.sbin/switchd/switchd.h b/usr.sbin/switchd/switchd.h
index 8b3f73e9523..fb6748688ae 100644
--- a/usr.sbin/switchd/switchd.h
+++ b/usr.sbin/switchd/switchd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: switchd.h,v 1.25 2016/11/18 16:49:35 reyk Exp $	*/
+/*	$OpenBSD: switchd.h,v 1.26 2016/11/22 17:21:56 rzalamena Exp $	*/
 
 /*
  * Copyright (c) 2013-2016 Reyk Floeter <reyk@openbsd.org>
@@ -93,6 +93,7 @@ struct switch_connection {
 	struct sockaddr_storage	 con_local;
 	in_port_t		 con_port;
 	uint32_t		 con_xidnxt;
+	int			 con_version;
 
 	struct event		 con_ev;
 	struct ibuf		*con_rbuf;
@@ -347,6 +348,13 @@ int		 oflowmod_instructionclose(struct oflowmod_ctx *);
 int		 oflowmod_state(struct oflowmod_ctx *,
 		    unsigned int, unsigned int);
 int		 oflowmod_err(struct oflowmod_ctx *, const char *, int);
+int		 ofp_validate_hello(struct switchd *,
+		    struct sockaddr_storage *, struct sockaddr_storage *,
+		    struct ofp_header *, struct ibuf *);
+int		 ofp_recv_hello(struct switchd *, struct switch_connection *,
+		    struct ofp_header *, struct ibuf *);
+int		 ofp_send_hello(struct switchd *, struct switch_connection *,
+		    int);
 
 /* ofcconn.c */
 void		 ofcconn(struct privsep *, struct privsep_proc *);