> On 22.11.2016, at 16:12, Rafael Zalamena <[email protected]> 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 -0000 1.15
> +++ ofp.c 22 Nov 2016 14:55:12 -0000
> @@ -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 -0000 1.16
> +++ ofp10.c 22 Nov 2016 15:08:02 -0000
> @@ -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->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));
> 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 -0000 1.39
> +++ ofp13.c 22 Nov 2016 14:49:27 -0000
> @@ -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->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)
> 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 -0000 1.7
> +++ ofp_common.c 22 Nov 2016 14:53:45 -0000
> @@ -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,
> @@ -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 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, vmajor, vminor;
> +
> + /* 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 = 9;
> + 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;
> +
> + vmajor = (ver / 10);
> + vminor = ver % 10;
> + log_debug("\t\tv%d.%d", vmajor, vminor);
this should be
log_debug("\t\tversion %s", print_map(ofp_v_map, ver));
no need for vmajor/vminor
> + }
> +
> + 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);
> }
> Index: switchd.h
> ===================================================================
> RCS file: /home/obsdcvs/src/usr.sbin/switchd/switchd.h,v
> retrieving revision 1.25
> diff -u -p -r1.25 switchd.h
> --- switchd.h 18 Nov 2016 16:49:35 -0000 1.25
> +++ switchd.h 22 Nov 2016 14:54:36 -0000
> @@ -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 o
> 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 *);
>
