Noticed this morning working on something else. The non-transitive
extended community support is fairly messed up. In my case (set
ext-community rt 1:1) it causes the community to be skipped.
First of all the check in non_transitive_community() is messed up. It uses
the wrong data field, does a unneccessary ntohl and the logic seems also
to be reversed.
While looking at this I decided to fix the various switch statements on
type to mask out the two high bits with this extending ext community types
should be a bit easier.
While there be strict from where bgpd accepts non-transitive
ext-communities. Now non-transitive ext communities are filtered in and out
for ebgp sessions. So non-transitive ext communities only pass on ibgp
sessions.
For the rde_attr_add() the system defaults to ibgp since this is for
network announcements.
There is also a trivial fix to the bgpd unittests that I excluded here.
With this ext-communities are properly sent on updates.
OK?
--
:wq Claudio
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.545
diff -u -p -r1.545 rde.c
--- rde.c 6 May 2022 15:51:09 -0000 1.545
+++ rde.c 25 May 2022 12:30:44 -0000
@@ -1917,8 +1917,8 @@ bad_flags:
if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE,
ATTR_PARTIAL))
goto bad_flags;
- if (community_ext_add(&state->communities, flags, p,
- attr_len) == -1) {
+ if (community_ext_add(&state->communities, flags,
+ peer->conf.ebgp, p, attr_len) == -1) {
/*
* mark update as bad and withdraw all routes as per
* RFC 7606
@@ -2062,8 +2062,8 @@ rde_attr_add(struct filterstate *state,
return community_large_add(&state->communities, flags, p,
attr_len);
case ATTR_EXT_COMMUNITIES:
- return community_ext_add(&state->communities, flags, p,
- attr_len);
+ return community_ext_add(&state->communities, flags, 0,
+ p, attr_len);
}
if (attr_optadd(&state->aspath, flags, type, p, attr_len) == -1)
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.251
diff -u -p -r1.251 rde.h
--- rde.h 22 Mar 2022 10:53:08 -0000 1.251
+++ rde.h 25 May 2022 12:30:44 -0000
@@ -459,7 +459,7 @@ void community_delete(struct rde_communi
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, void *, size_t);
+int community_ext_add(struct rde_community *, int, int, void *, size_t);
int community_write(struct rde_community *, void *, uint16_t);
int community_large_write(struct rde_community *, void *, uint16_t);
Index: rde_community.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_community.c,v
retrieving revision 1.4
diff -u -p -r1.4 rde_community.c
--- rde_community.c 6 Feb 2022 09:51:19 -0000 1.4
+++ rde_community.c 25 May 2022 12:30:44 -0000
@@ -99,7 +99,7 @@ fc2c(struct community *fc, struct rde_pe
subtype = fc->data3 & 0xff;
c->data3 = type << 8 | subtype;
- switch (type) {
+ switch (type & EXT_COMMUNITY_VALUE) {
case -1:
/* special case for 'ext-community rt *' */
if ((fc->flags >> 8 & 0xff) != COMMUNITY_ANY ||
@@ -141,7 +141,6 @@ fc2c(struct community *fc, struct rde_pe
return 0;
case EXT_COMMUNITY_TRANS_OPAQUE:
case EXT_COMMUNITY_TRANS_EVPN:
- case EXT_COMMUNITY_NON_TRANS_OPAQUE:
if ((fc->flags >> 8 & 0xff) == COMMUNITY_ANY)
break;
@@ -244,10 +243,9 @@ insert_community(struct rde_community *c
}
static int
-non_transitive_community(struct community *c)
+non_transitive_ext_community(struct community *c)
{
- if ((uint8_t)c->flags == COMMUNITY_TYPE_EXT &&
- !((ntohl(c->data1) >> 24) & EXT_COMMUNITY_NON_TRANSITIVE))
+ if ((c->data3 >> 8) & EXT_COMMUNITY_NON_TRANSITIVE)
return 1;
return 0;
}
@@ -404,7 +402,8 @@ community_large_add(struct rde_community
}
int
-community_ext_add(struct rde_community *comm, int flags, void *buf, size_t len)
+community_ext_add(struct rde_community *comm, int flags, int ebgp,
+ void *buf, size_t len)
{
struct community set = { .flags = COMMUNITY_TYPE_EXT };
uint8_t *b = buf, type;
@@ -422,11 +421,13 @@ community_ext_add(struct rde_community *
c = be64toh(c);
type = c >> 56;
- switch (type) {
+ /* filter out non-transitive ext communuties from ebgp peers */
+ if (ebgp && (type & EXT_COMMUNITY_NON_TRANSITIVE))
+ continue;
+ switch (type & EXT_COMMUNITY_VALUE) {
case EXT_COMMUNITY_TRANS_TWO_AS:
case EXT_COMMUNITY_TRANS_OPAQUE:
case EXT_COMMUNITY_TRANS_EVPN:
- case EXT_COMMUNITY_NON_TRANS_OPAQUE:
set.data1 = c >> 32 & 0xffff;
set.data2 = c;
break;
@@ -564,7 +565,7 @@ community_ext_write(struct rde_community
for (l = 0; l < comm->nentries; l++)
if ((uint8_t)comm->communities[l].flags ==
COMMUNITY_TYPE_EXT && !(ebgp &&
- non_transitive_community(&comm->communities[l])))
+ non_transitive_ext_community(&comm->communities[l])))
n++;
if (n == 0)
@@ -580,13 +581,12 @@ community_ext_write(struct rde_community
for (l = 0; l < comm->nentries; l++) {
cp = comm->communities + l;
if ((uint8_t)cp->flags == COMMUNITY_TYPE_EXT && !(ebgp &&
- non_transitive_community(cp))) {
+ non_transitive_ext_community(cp))) {
ext = (uint64_t)cp->data3 << 48;
- switch (cp->data3 >> 8) {
+ switch ((cp->data3 >> 8) & EXT_COMMUNITY_VALUE) {
case EXT_COMMUNITY_TRANS_TWO_AS:
case EXT_COMMUNITY_TRANS_OPAQUE:
case EXT_COMMUNITY_TRANS_EVPN:
- case EXT_COMMUNITY_NON_TRANS_OPAQUE:
ext |= ((uint64_t)cp->data1 & 0xffff) << 32;
ext |= (uint64_t)cp->data2;
break;
@@ -672,11 +672,10 @@ community_writebuf(struct ibuf *buf, str
continue;
ext = (uint64_t)cp->data3 << 48;
- switch (cp->data3 >> 8) {
+ switch ((cp->data3 >> 8) & EXT_COMMUNITY_VALUE) {
case EXT_COMMUNITY_TRANS_TWO_AS:
case EXT_COMMUNITY_TRANS_OPAQUE:
case EXT_COMMUNITY_TRANS_EVPN:
- case EXT_COMMUNITY_NON_TRANS_OPAQUE:
ext |= ((uint64_t)cp->data1 & 0xffff) << 32;
ext |= (uint64_t)cp->data2;
break;
@@ -919,7 +918,7 @@ community_to_rd(struct community *fc, ui
if (fc2c(fc, NULL, &c, NULL) == -1)
return -1;
- switch (c.data3 >> 8) {
+ switch ((c.data3 >> 8) & EXT_COMMUNITY_VALUE) {
case EXT_COMMUNITY_TRANS_TWO_AS:
rd = (0ULL << 48);
rd |= ((uint64_t)c.data1 & 0xffff) << 32;
