Re: switchd(8): negotiate versions with hello

[Reyk Floeter]

> On 22.11.2016, at 16:12, Rafael Zalamena  wrote:
> 
> Teach switchd(8) how to negotiate protocol version using the hello bitmap
> header. This way switchd(8) is able to fallback or use higher version using
> the bitmap.
> 
> This diff also prevents connections from switching version in the middle of
> the operation.
> 
> This is the first step before adding a state machine to switchd(8): move the
> hello to a common function and make it step into the first state: HELLO_WAIT.
> (next diff)
> 
> ok?

Two comments below, otherwise OK.

Reyk

> 
> Index: ofp.c
> ===
> RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 ofp.c
> --- ofp.c 4 Nov 2016 22:27:08 -   1.15
> +++ ofp.c 22 Nov 2016 14:55:12 -
> @@ -132,6 +132,13 @@ ofp_input(struct switch_connection *con,
>   return (-1);
>   }
> 
> + if (con->con_version != OFP_V_0 &&

This should be handled by the state machine later, but OK for now.

(<= HELLO_WAIT accepts hello and any version, > HELLO_WAIT doesn't)

> + 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)
> @@ -165,6 +172,10 @@ ofp_open(struct privsep *ps, struct swit
>   log_info("%s: new connection %u.%u from switch %u",
>   __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);
> }
> Index: ofp10.c
> ===
> RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp10.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 ofp10.c
> --- ofp10.c   21 Nov 2016 18:19:51 -  1.16
> +++ ofp10.c   22 Nov 2016 15:08:02 -
> @@ -60,7 +60,7 @@ int  ofp10_validate_packet_out(struct sw
>   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 s
>   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_local, >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, , sizeof(oh));
> Index: ofp13.c
> ===
> RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp13.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 ofp13.c
> --- ofp13.c   21 Nov 2016 19:33:12 -  1.39
> +++ ofp13.c   22 Nov 2016 14:49:27 -
> @@ -109,7 +109,7 @@ intofp13_tablemiss_sendctrl(struct swi
>   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 s
>   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_local, >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)
> Index: ofp_common.c
> ===
> RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp_common.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 ofp_common.c
> --- ofp_common.c  17 Nov 2016 13:10:26 -  1.7
> +++ ofp_common.c  22 Nov 2016 14:53:45 -
> @@ -43,6 +43,8 @@
> #include "switchd.h"
> #include "ofp_map.h"
> 
> +int  ofp_setversion(struct switch_connection *, int);
> +
> int
> 

switchd(8): negotiate versions with hello

[Rafael Zalamena]

Teach switchd(8) how to negotiate protocol version using the hello bitmap
header. This way switchd(8) is able to fallback or use higher version using
the bitmap.

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

This is the first step before adding a state machine to switchd(8): move the
hello to a common function and make it step into the first state: HELLO_WAIT.
(next diff)

ok?

Index: ofp.c
===
RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp.c,v
retrieving revision 1.15
diff -u -p -r1.15 ofp.c
--- ofp.c   4 Nov 2016 22:27:08 -   1.15
+++ ofp.c   22 Nov 2016 14:55:12 -
@@ -132,6 +132,13 @@ ofp_input(struct switch_connection *con,
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)
@@ -165,6 +172,10 @@ ofp_open(struct privsep *ps, struct swit
log_info("%s: new connection %u.%u from switch %u",
__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);
 }
Index: ofp10.c
===
RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp10.c,v
retrieving revision 1.16
diff -u -p -r1.16 ofp10.c
--- ofp10.c 21 Nov 2016 18:19:51 -  1.16
+++ ofp10.c 22 Nov 2016 15:08:02 -
@@ -60,7 +60,7 @@ intofp10_validate_packet_out(struct sw
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 s
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_local, >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, , sizeof(oh));
Index: ofp13.c
===
RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.39
diff -u -p -r1.39 ofp13.c
--- ofp13.c 21 Nov 2016 19:33:12 -  1.39
+++ ofp13.c 22 Nov 2016 14:49:27 -
@@ -109,7 +109,7 @@ int  ofp13_tablemiss_sendctrl(struct swi
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 s
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_local, >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)
Index: ofp_common.c
===
RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp_common.c,v
retrieving revision 1.7
diff -u -p -r1.7 ofp_common.c
--- ofp_common.c17 Nov 2016 13:10:26 -  1.7
+++ ofp_common.c22 Nov 2016 14:53:45 -
@@ -43,6 +43,8 @@
 #include "switchd.h"
 #include "ofp_map.h"
 
+intofp_setversion(struct switch_connection *, int);
+
 int
 ofp_validate_header(struct switchd *sc,
 struct sockaddr_storage *src, struct sockaddr_storage *dst,
@@ -114,6 +116,177 @@ ofp_output(struct switch_connection *con
}
 
ofrelay_write(con, buf);
+
+   return (0);
+}
+
+int
+ofp_send_hello(struct switchd *sc, struct switch_connection *con, int version)
+{
+   struct