Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: rde ibuf parser part 2
To:
tech@openbsd.org
Date:
Wed, 24 Jan 2024 15:05:27 +0100

Download raw body.

Thread
Next step of going all ibuf in the RDE update parser. This switches
communities over (community_add(), community_large_add() and
community_ext_add()).

This requires some adjustments in regress which are included.
-- 
:wq Claudio

Index: usr.sbin/bgpd/rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.616 rde.c
--- usr.sbin/bgpd/rde.c	23 Jan 2024 16:13:35 -0000	1.616
+++ usr.sbin/bgpd/rde.c	23 Jan 2024 16:51:57 -0000
@@ -2139,8 +2139,8 @@ rde_attr_parse(struct ibuf *buf, struct 
 		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE,
 		    ATTR_PARTIAL))
 			goto bad_flags;
-		if (community_add(&state->communities, flags, p,
-		    attr_len) == -1) {
+		if (community_add(&state->communities, flags,
+		    &attrbuf) == -1) {
 			/*
 			 * mark update as bad and withdraw all routes as per
 			 * RFC 7606
@@ -2149,14 +2149,13 @@ rde_attr_parse(struct ibuf *buf, struct 
 			log_peer_warnx(&peer->conf, "bad COMMUNITIES, "
 			    "path invalidated and prefix withdrawn");
 		}
-		plen += attr_len;
 		break;
 	case ATTR_LARGE_COMMUNITIES:
 		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE,
 		    ATTR_PARTIAL))
 			goto bad_flags;
-		if (community_large_add(&state->communities, flags, p,
-		    attr_len) == -1) {
+		if (community_large_add(&state->communities, flags,
+		    &attrbuf) == -1) {
 			/*
 			 * mark update as bad and withdraw all routes as per
 			 * RFC 7606
@@ -2165,14 +2164,13 @@ rde_attr_parse(struct ibuf *buf, struct 
 			log_peer_warnx(&peer->conf, "bad LARGE COMMUNITIES, "
 			    "path invalidated and prefix withdrawn");
 		}
-		plen += attr_len;
 		break;
 	case ATTR_EXT_COMMUNITIES:
 		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE,
 		    ATTR_PARTIAL))
 			goto bad_flags;
 		if (community_ext_add(&state->communities, flags,
-		    peer->conf.ebgp, p, attr_len) == -1) {
+		    peer->conf.ebgp, &attrbuf) == -1) {
 			/*
 			 * mark update as bad and withdraw all routes as per
 			 * RFC 7606
@@ -2181,7 +2179,6 @@ rde_attr_parse(struct ibuf *buf, struct 
 			log_peer_warnx(&peer->conf, "bad EXT_COMMUNITIES, "
 			    "path invalidated and prefix withdrawn");
 		}
-		plen += attr_len;
 		break;
 	case ATTR_ORIGINATOR_ID:
 		if (attr_len != 4)
@@ -2338,14 +2335,11 @@ rde_attr_add(struct filterstate *state, 
 
 	switch (type) {
 	case ATTR_COMMUNITIES:
-		return community_add(&state->communities, flags,
-		    ibuf_data(buf), attr_len);
+		return community_add(&state->communities, flags, buf);
 	case ATTR_LARGE_COMMUNITIES:
-		return community_large_add(&state->communities, flags,
-		    ibuf_data(buf), attr_len);
+		return community_large_add(&state->communities, flags, buf);
 	case ATTR_EXT_COMMUNITIES:
-		return community_ext_add(&state->communities, flags, 0,
-		    ibuf_data(buf), attr_len);
+		return community_ext_add(&state->communities, flags, 0, buf);
 	}
 
 	if (attr_optadd(&state->aspath, flags, type, ibuf_data(buf),
Index: usr.sbin/bgpd/rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.298 rde.h
--- usr.sbin/bgpd/rde.h	23 Jan 2024 16:13:35 -0000	1.298
+++ usr.sbin/bgpd/rde.h	23 Jan 2024 16:45:08 -0000
@@ -435,10 +435,9 @@ int	community_set(struct rde_community *
 void	community_delete(struct rde_community *, struct community *,
 	    struct rde_peer *);
 
-int	community_add(struct rde_community *, int, void *, size_t);
-int	community_large_add(struct rde_community *, int, void *, size_t);
-int	community_ext_add(struct rde_community *, int, int, void *, size_t);
-
+int	community_add(struct rde_community *, int, struct ibuf *);
+int	community_large_add(struct rde_community *, int, struct ibuf *);
+int	community_ext_add(struct rde_community *, int, int, struct ibuf *);
 int	community_writebuf(struct rde_community *, uint8_t, int, struct ibuf *);
 
 void			 communities_shutdown(void);
Index: usr.sbin/bgpd/rde_community.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_community.c,v
diff -u -p -r1.14 rde_community.c
--- usr.sbin/bgpd/rde_community.c	10 Oct 2023 14:36:28 -0000	1.14
+++ usr.sbin/bgpd/rde_community.c	23 Jan 2024 16:45:08 -0000
@@ -418,24 +418,23 @@ struct rde_peer *peer)
  * - community_ext_add for ATTR_EXT_COMMUNITIES
  */
 int
-community_add(struct rde_community *comm, int flags, void *buf, size_t len)
+community_add(struct rde_community *comm, int flags, struct ibuf *buf)
 {
 	struct community set = { .flags = COMMUNITY_TYPE_BASIC };
-	uint8_t *b = buf;
-	uint16_t c;
-	size_t l;
+	uint16_t data1, data2;
 
-	if (len == 0 || len % 4 != 0)
+	if (ibuf_size(buf) == 0 || ibuf_size(buf) % 4 != 0)
 		return -1;
 
 	if (flags & ATTR_PARTIAL)
 		comm->flags |= PARTIAL_COMMUNITIES;
 
-	for (l = 0; l < len; l += 4, b += 4) {
-		memcpy(&c, b, sizeof(c));
-		set.data1 = ntohs(c);
-		memcpy(&c, b + 2, sizeof(c));
-		set.data2 = ntohs(c);
+	while (ibuf_size(buf) > 0) {
+		if (ibuf_get_n16(buf, &data1) == -1 ||
+		    ibuf_get_n16(buf, &data2) == -1)
+			return -1;
+		set.data1 = data1;
+		set.data2 = data2;
 		insert_community(comm, &set);
 	}
 
@@ -443,26 +442,21 @@ community_add(struct rde_community *comm
 }
 
 int
-community_large_add(struct rde_community *comm, int flags, void *buf,
-    size_t len)
+community_large_add(struct rde_community *comm, int flags, struct ibuf *buf)
 {
 	struct community set = { .flags = COMMUNITY_TYPE_LARGE };
-	uint8_t *b = buf;
-	size_t l;
 
-	if (len == 0 || len % 12 != 0)
+	if (ibuf_size(buf) == 0 || ibuf_size(buf) % 12 != 0)
 		return -1;
 
 	if (flags & ATTR_PARTIAL)
 		comm->flags |= PARTIAL_LARGE_COMMUNITIES;
 
-	for (l = 0; l < len; l += 12, b += 12) {
-		memcpy(&set.data1, b, sizeof(set.data1));
-		memcpy(&set.data2, b + 4, sizeof(set.data2));
-		memcpy(&set.data3, b + 8, sizeof(set.data3));
-		set.data1 = ntohl(set.data1);
-		set.data2 = ntohl(set.data2);
-		set.data3 = ntohl(set.data3);
+	while (ibuf_size(buf) > 0) {
+		if (ibuf_get_n32(buf, &set.data1) == -1 ||
+		    ibuf_get_n32(buf, &set.data2) == -1 ||
+		    ibuf_get_n32(buf, &set.data3) == -1)
+			return -1;
 		insert_community(comm, &set);
 	}
 
@@ -471,23 +465,22 @@ community_large_add(struct rde_community
 
 int
 community_ext_add(struct rde_community *comm, int flags, int ebgp,
-    void *buf, size_t len)
+    struct ibuf *buf)
 {
 	struct community set = { .flags = COMMUNITY_TYPE_EXT };
-	uint8_t *b = buf, type;
 	uint64_t c;
-	size_t l;
+	uint8_t type;
 
-	if (len == 0 || len % 8 != 0)
+	if (ibuf_size(buf) == 0 || ibuf_size(buf) % 8 != 0)
 		return -1;
 
 	if (flags & ATTR_PARTIAL)
 		comm->flags |= PARTIAL_EXT_COMMUNITIES;
 
-	for (l = 0; l < len; l += 8, b += 8) {
-		memcpy(&c, b, 8);
+	while (ibuf_size(buf) > 0) {
+		if (ibuf_get_n64(buf, &c) == -1)
+			return (-1);
 
-		c = be64toh(c);
 		type = c >> 56;
 		/* filter out non-transitive ext communuties from ebgp peers */
 		if (ebgp && (type & EXT_COMMUNITY_NON_TRANSITIVE))
Index: regress/usr.sbin/bgpd/unittests/rde_community_test.c
===================================================================
RCS file: /cvs/src/regress/usr.sbin/bgpd/unittests/rde_community_test.c,v
diff -u -p -r1.9 rde_community_test.c
--- regress/usr.sbin/bgpd/unittests/rde_community_test.c	11 Oct 2023 07:05:11 -0000	1.9
+++ regress/usr.sbin/bgpd/unittests/rde_community_test.c	24 Jan 2024 14:01:01 -0000
@@ -43,9 +43,9 @@ dump(uint8_t *b, size_t len)
 }
 
 static int
-test_parsing(size_t num, uint8_t *in, size_t inlen, uint8_t *out, size_t outlen)
+test_parsing(size_t num, struct ibuf *in, struct ibuf *out)
 {
-	struct ibuf *buf;
+	struct ibuf *buf, abuf;
 	uint8_t flags, type, attr[256];
 	size_t skip;
 	uint16_t attr_len;
@@ -54,42 +54,40 @@ test_parsing(size_t num, uint8_t *in, si
 	communities_clean(&comm);
 
 	do {
-		flags = in[0];
-		type = in[1];
-		skip = 2;
+		if (ibuf_get_n8(in, &flags) == -1 ||
+		    ibuf_get_n8(in, &type) == -1)
+			goto bad;
 		if (flags & ATTR_EXTLEN) {
-			memcpy(&attr_len, in + 2, sizeof(attr_len));
-			attr_len = ntohs(attr_len);
-			skip += 2;
+			if (ibuf_get_n16(in, &attr_len) == -1)
+				goto bad;
 		} else {
-			attr_len = in[2];
-			skip += 1;
+			uint8_t tmp;
+			if (ibuf_get_n8(in, &tmp) == -1)
+				goto bad;
+			attr_len = tmp;
 		}
-		if (skip + attr_len > inlen) {
+		if (ibuf_get_ibuf(in, attr_len, &abuf) == -1) {
+ bad:
 			printf("Test %zu: attribute parse failure\n", num);
 			return -1;
 		}
 
 		switch (type) {
 		case ATTR_COMMUNITIES:
-			r = community_add(&comm, flags, in + skip, attr_len);
+			r = community_add(&comm, flags, &abuf);
 			break;
 		case ATTR_EXT_COMMUNITIES:
-			r = community_ext_add(&comm, flags, 0, in + skip,
-			    attr_len);
+			r = community_ext_add(&comm, flags, 0, &abuf);
 			break;
 		case ATTR_LARGE_COMMUNITIES:
-			r = community_large_add(&comm, flags, in + skip,
-			    attr_len);
+			r = community_large_add(&comm, flags, &abuf);
 			break;
 		}
 		if (r == -1) {
 			printf("Test %zu: community_add failed\n", num);
 			return -1;
 		}
-		in += skip + attr_len;
-		inlen -= (skip + attr_len);
-	} while (inlen > 0);
+	} while (ibuf_size(in) > 0);
 
 	if ((buf = ibuf_dynamic(0, 4096)) == NULL) {
 		printf("Test %zu: ibuf_dynamic failed\n", num);
@@ -109,19 +107,19 @@ test_parsing(size_t num, uint8_t *in, si
 		return -1;
 	}
 
-	if (ibuf_size(buf) != outlen) {
+	if (ibuf_size(buf) != ibuf_size(out)) {
 		printf("Test %zu: ibuf size value %zd != %zd:",
-		    num, ibuf_size(buf), outlen);
+		    num, ibuf_size(buf), ibuf_size(out));
 		dump(ibuf_data(buf), ibuf_size(buf));
 		printf("expected: ");
-		dump(out, outlen);
+		dump(ibuf_data(out), ibuf_size(out));
 		return -1;
 	}
-	if (memcmp(ibuf_data(buf), out, outlen) != 0) {
+	if (memcmp(ibuf_data(buf), ibuf_data(out), ibuf_size(out)) != 0) {
 		printf("Test %zu: unexpected encoding: ", num);
 		dump(ibuf_data(buf), ibuf_size(buf));
 		printf("expected: ");
-		dump(out, outlen);
+		dump(ibuf_data(out), ibuf_size(out));
 		return -1;
 	}
 
@@ -206,16 +204,17 @@ main(int argc, char *argv[])
 	int error = 0;
 
 	for (t = 0; t < sizeof(vectors) / sizeof(*vectors); t++) {
-		size_t outlen = vectors[t].expsize;
-		uint8_t *out = vectors[t].expected;
+		struct ibuf in, out;
 
-		if (vectors[t].expected == NULL) {
-			outlen = vectors[t].size;
-			out = vectors[t].data;
-		}
+		ibuf_from_buffer(&in, vectors[t].data, vectors[t].size);
+		if (vectors[t].expected == NULL)
+			ibuf_from_buffer(&out,
+			    vectors[t].data, vectors[t].size);
+		else 
+			ibuf_from_buffer(&out,
+			    vectors[t].expected, vectors[t].expsize);
 
-		if (test_parsing(t, vectors[t].data, vectors[t].size,
-		    out, outlen) == -1)
+		if (test_parsing(t, &in, &out) == -1)
 			error = 1;
 	}