From: Claudio Jeker Subject: bgpd: flip bgpid and clusterid to host byte order To: tech@openbsd.org Date: Mon, 20 May 2024 15:42:43 +0200 Inspired by a discussion with tb@. I think keeping those values in host byte order makes more sense. Especially since remote_bgpid in the RDE is already in host byte order. I hope I got all usages of bgpid, remore_bgpid and clusterid and converted them correctly. Regress still pass which is a good first step :) -- :wq Claudio Index: bgpctl/bgpctl.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v diff -u -p -r1.305 bgpctl.c --- bgpctl/bgpctl.c 1 Feb 2024 11:37:10 -0000 1.305 +++ bgpctl/bgpctl.c 20 May 2024 13:31:04 -0000 @@ -1548,15 +1548,16 @@ show_mrt_capabilities(struct ibuf *b) static void show_mrt_open(struct ibuf *b) { + struct in_addr ina; + uint32_t bgpid; uint16_t short_as, holdtime; uint8_t version, optparamlen; - struct in_addr bgpid; /* length check up to optparamlen already happened */ if (ibuf_get_n8(b, &version) == -1 || ibuf_get_n16(b, &short_as) == -1 || ibuf_get_n16(b, &holdtime) == -1 || - ibuf_get(b, &bgpid, sizeof(bgpid)) == -1 || + ibuf_get_n32(b, &bgpid) == -1 || ibuf_get_n8(b, &optparamlen) == -1) { trunc: printf("truncated message"); @@ -1564,8 +1565,9 @@ show_mrt_open(struct ibuf *b) } printf("\n "); + ina.s_addr = htonl(bgpid); printf("Version: %d AS: %u Holdtime: %u BGP Id: %s Paramlen: %u", - version, short_as, holdtime, inet_ntoa(bgpid), optparamlen); + version, short_as, holdtime, inet_ntoa(ina), optparamlen); if (optparamlen != ibuf_size(b)) { /* XXX missing support for RFC9072 */ printf("optional parameter length mismatch"); Index: bgpctl/output.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpctl/output.c,v diff -u -p -r1.50 output.c --- bgpctl/output.c 31 Jan 2024 11:23:20 -0000 1.50 +++ bgpctl/output.c 20 May 2024 13:31:16 -0000 @@ -299,7 +299,7 @@ show_neighbor_full(struct peer *p, struc printf("\n"); if (p->state == STATE_ESTABLISHED) { - ina.s_addr = p->remote_bgpid; + ina.s_addr = htonl(p->remote_bgpid); printf(" BGP version 4, remote router-id %s", inet_ntoa(ina)); printf("%s\n", fmt_auth_method(p->auth.method)); Index: bgpctl/output_json.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpctl/output_json.c,v diff -u -p -r1.43 output_json.c --- bgpctl/output_json.c 24 Apr 2024 10:42:09 -0000 1.43 +++ bgpctl/output_json.c 20 May 2024 13:31:24 -0000 @@ -324,7 +324,7 @@ json_neighbor(struct peer *p, struct par log_addr(&p->conf.remote_addr), p->conf.remote_masklen); if (p->state == STATE_ESTABLISHED) { struct in_addr ina; - ina.s_addr = p->remote_bgpid; + ina.s_addr = htonl(p->remote_bgpid); json_do_string("bgpid", inet_ntoa(ina)); } json_do_string("state", statenames[p->state]); Index: bgpd/config.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/config.c,v diff -u -p -r1.108 config.c --- bgpd/config.c 16 Aug 2023 08:26:35 -0000 1.108 +++ bgpd/config.c 20 May 2024 13:20:41 -0000 @@ -466,7 +466,7 @@ get_bgpid(void) struct ifaddrs *ifap, *ifa; uint32_t ip = 0, cur, localnet; - localnet = htonl(INADDR_LOOPBACK & IN_CLASSA_NET); + localnet = INADDR_LOOPBACK & IN_CLASSA_NET; if (getifaddrs(&ifap) == -1) fatal("getifaddrs"); @@ -476,9 +476,10 @@ get_bgpid(void) ifa->ifa_addr->sa_family != AF_INET) continue; cur = ((struct sockaddr_in *)ifa->ifa_addr)->sin_addr.s_addr; + cur = ntohl(cur); if ((cur & localnet) == localnet) /* skip 127/8 */ continue; - if (ntohl(cur) > ntohl(ip)) + if (cur > ip) ip = cur; } freeifaddrs(ifap); Index: bgpd/mrt.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v diff -u -p -r1.116 mrt.c --- bgpd/mrt.c 14 Jul 2023 10:30:53 -0000 1.116 +++ bgpd/mrt.c 20 May 2024 13:21:06 -0000 @@ -824,7 +824,7 @@ mrt_dump_v2_hdr(struct mrt *mrt, struct return (-1); } - if (ibuf_add(buf, &conf->bgpid, sizeof(conf->bgpid)) == -1) + if (ibuf_add_n32(buf, conf->bgpid) == -1) goto fail; nlen = strlen(mrt->rib); if (nlen > 0) Index: bgpd/parse.y =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v diff -u -p -r1.462 parse.y --- bgpd/parse.y 24 Apr 2024 10:41:34 -0000 1.462 +++ bgpd/parse.y 20 May 2024 13:26:31 -0000 @@ -742,7 +742,7 @@ conf_main : AS as4number { yyerror("router-id must be an IPv4 address"); YYERROR; } - conf->bgpid = $2.v4.s_addr; + conf->bgpid = ntohl($2.v4.s_addr); } | HOLDTIME NUMBER { if ($2 < MIN_HOLDTIME || $2 > USHRT_MAX) { @@ -2236,14 +2236,14 @@ peeropts : REMOTEAS as4number { YYERROR; } if ((conf->flags & BGPD_FLAG_REFLECTOR) && - conf->clusterid != $2.v4.s_addr) { + conf->clusterid != ntohl($2.v4.s_addr)) { yyerror("only one route reflector " "cluster allowed"); YYERROR; } conf->flags |= BGPD_FLAG_REFLECTOR; curpeer->conf.reflector_client = 1; - conf->clusterid = $2.v4.s_addr; + conf->clusterid = ntohl($2.v4.s_addr); } | DEPEND ON STRING { if (strlcpy(curpeer->conf.if_depend, $3, Index: bgpd/printconf.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v diff -u -p -r1.172 printconf.c --- bgpd/printconf.c 24 Apr 2024 10:41:34 -0000 1.172 +++ bgpd/printconf.c 20 May 2024 13:29:00 -0000 @@ -383,7 +383,7 @@ print_mainconf(struct bgpd_config *conf) printf("AS %s", log_as(conf->as)); if (conf->as > USHRT_MAX && conf->short_as != AS_TRANS) printf(" %u", conf->short_as); - ina.s_addr = conf->bgpid; + ina.s_addr = htonl(conf->bgpid); printf("\nrouter-id %s\n", inet_ntoa(ina)); printf("socket \"%s\"\n", conf->csock); @@ -800,7 +800,7 @@ print_peer(struct peer_config *p, struct if (conf->clusterid == 0) printf("%s\troute-reflector\n", c); else { - ina.s_addr = conf->clusterid; + ina.s_addr = htonl(conf->clusterid); printf("%s\troute-reflector %s\n", c, inet_ntoa(ina)); } Index: bgpd/rde.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v diff -u -p -r1.624 rde.c --- bgpd/rde.c 20 Mar 2024 09:35:46 -0000 1.624 +++ bgpd/rde.c 20 May 2024 13:28:50 -0000 @@ -2647,48 +2647,49 @@ rde_reflector(struct rde_peer *peer, str /* check for originator id if eq router_id drop */ if ((a = attr_optget(asp, ATTR_ORIGINATOR_ID)) != NULL) { - if (memcmp(&conf->bgpid, a->data, sizeof(conf->bgpid)) == 0) { + id = htonl(conf->bgpid); + if (memcmp(&id, a->data, sizeof(id)) == 0) { /* this is coming from myself */ asp->flags |= F_ATTR_LOOP; return; } } else if (conf->flags & BGPD_FLAG_REFLECTOR) { if (peer->conf.ebgp) - id = conf->bgpid; + id = htonl(conf->bgpid); else id = htonl(peer->remote_bgpid); if (attr_optadd(asp, ATTR_OPTIONAL, ATTR_ORIGINATOR_ID, - &id, sizeof(uint32_t)) == -1) + &id, sizeof(id)) == -1) fatalx("attr_optadd failed but impossible"); } /* check for own id in the cluster list */ if (conf->flags & BGPD_FLAG_REFLECTOR) { + id = htonl(conf->clusterid); if ((a = attr_optget(asp, ATTR_CLUSTER_LIST)) != NULL) { - for (len = 0; len < a->len; - len += sizeof(conf->clusterid)) + for (len = 0; len < a->len; len += sizeof(id)) /* check if coming from my cluster */ - if (memcmp(&conf->clusterid, a->data + len, - sizeof(conf->clusterid)) == 0) { + if (memcmp(&id, a->data + len, + sizeof(id)) == 0) { asp->flags |= F_ATTR_LOOP; return; } /* prepend own clusterid by replacing attribute */ - len = a->len + sizeof(conf->clusterid); + len = a->len + sizeof(id); if (len < a->len) fatalx("rde_reflector: cluster-list overflow"); if ((p = malloc(len)) == NULL) fatal("rde_reflector"); - memcpy(p, &conf->clusterid, sizeof(conf->clusterid)); - memcpy(p + sizeof(conf->clusterid), a->data, a->len); + memcpy(p, &id, sizeof(id)); + memcpy(p + sizeof(id), a->data, a->len); attr_free(asp, a); if (attr_optadd(asp, ATTR_OPTIONAL, ATTR_CLUSTER_LIST, p, len) == -1) fatalx("attr_optadd failed but impossible"); free(p); } else if (attr_optadd(asp, ATTR_OPTIONAL, ATTR_CLUSTER_LIST, - &conf->clusterid, sizeof(conf->clusterid)) == -1) + &id, sizeof(id)) == -1) fatalx("attr_optadd failed but impossible"); } } @@ -3584,11 +3585,11 @@ rde_reload_done(void) nconf = NULL; /* sync peerself with conf */ - peerself->remote_bgpid = ntohl(conf->bgpid); + peerself->remote_bgpid = conf->bgpid; peerself->conf.local_as = conf->as; peerself->conf.remote_as = conf->as; peerself->conf.remote_addr.aid = AID_INET; - peerself->conf.remote_addr.v4.s_addr = conf->bgpid; + peerself->conf.remote_addr.v4.s_addr = htonl(conf->bgpid); peerself->conf.remote_masklen = 32; peerself->short_as = conf->short_as; Index: bgpd/rde_peer.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v diff -u -p -r1.36 rde_peer.c --- bgpd/rde_peer.c 20 Mar 2024 09:35:46 -0000 1.36 +++ bgpd/rde_peer.c 20 May 2024 13:24:58 -0000 @@ -422,7 +422,7 @@ peer_up(struct rde_peer *peer, struct se peer->stats.prefix_out_cnt = 0; peer->state = PEER_DOWN; } - peer->remote_bgpid = ntohl(sup->remote_bgpid); + peer->remote_bgpid = sup->remote_bgpid; peer->short_as = sup->short_as; peer->remote_addr = sup->remote_addr; peer->local_v4_addr = sup->local_v4_addr; Index: bgpd/session.c =================================================================== RCS file: /cvs/src/usr.sbin/bgpd/session.c,v diff -u -p -r1.477 session.c --- bgpd/session.c 20 May 2024 10:01:52 -0000 1.477 +++ bgpd/session.c 20 May 2024 13:36:53 -0000 @@ -1602,7 +1602,7 @@ session_open(struct peer *p) errs += ibuf_add_n16(buf->buf, p->conf.local_short_as); errs += ibuf_add_n16(buf->buf, holdtime); /* is already in network byte order */ - errs += ibuf_add(buf->buf, &conf->bgpid, sizeof(conf->bgpid)); + errs += ibuf_add_n32(buf->buf, conf->bgpid); errs += ibuf_add_n8(buf->buf, optparamlen); if (extlen) { @@ -2228,8 +2228,7 @@ parse_open(struct peer *peer) change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN); return (-1); } - /* strore remote_bgpid in network byte order */ - peer->remote_bgpid = htonl(bgpid); + peer->remote_bgpid = bgpid; if (optparamlen != 0) { struct ibuf oparams, op; @@ -2332,8 +2331,10 @@ parse_open(struct peer *peer) /* on iBGP sessions check for bgpid collision */ if (!peer->conf.ebgp && peer->remote_bgpid == conf->bgpid) { - log_peer_warnx(&peer->conf, "peer BGPID %u conflicts with ours", - ntohl(bgpid)); + struct in_addr ina; + ina.s_addr = htonl(bgpid); + log_peer_warnx(&peer->conf, "peer BGPID %s conflicts with ours", + inet_ntoa(ina)); session_notification(peer, ERR_OPEN, ERR_OPEN_BGPID, NULL); change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN); return (-1);