On Mon, Jan 30, 2023 at 03:29:49PM +0100, Claudio Jeker wrote:
> Extended communities are annoying, especially the ASnum encodings are a
> problem since the same extended community can be encoded in more than one
> way. This results in strange behaviour when used with local-as and/or
> neighbor-as.
>
> This diff changes the match function for any community with masks (either
> a * or local-as/neighbor-as) to work both for two and four byte ASnum
> encoding. Now this change will allow for some impossible encodings but
> because these are impossible the match function will not match and on
> insert the best solution is selected or the insertion fails.
Makes sense, and reads fine.
ok tb
>
> In general I would suggest to stay away from extended communities as much
> as possible. Especially since large-communities solve all these issues.
>
> --
> :wq Claudio
>
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.440
> diff -u -p -r1.440 parse.y
> --- parse.y 24 Jan 2023 14:13:11 -0000 1.440
> +++ parse.y 30 Jan 2023 13:57:05 -0000
> @@ -4083,11 +4083,11 @@ parseextvalue(int type, char *s, uint32_
> } else if (strcmp(s, "neighbor-as") == 0) {
> *flag = COMMUNITY_NEIGHBOR_AS;
> *v = 0;
> - return EXT_COMMUNITY_TRANS_FOUR_AS;
> + return EXT_COMMUNITY_TRANS_TWO_AS;
> } else if (strcmp(s, "local-as") == 0) {
> *flag = COMMUNITY_LOCAL_AS;
> *v = 0;
> - return EXT_COMMUNITY_TRANS_FOUR_AS;
> + return EXT_COMMUNITY_TRANS_TWO_AS;
> } else if ((p = strchr(s, '.')) == NULL) {
> /* AS_PLAIN number (4 or 2 byte) */
> strtonum(s, 0, USHRT_MAX, &errstr);
> Index: rde_community.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_community.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 rde_community.c
> --- rde_community.c 28 Dec 2022 21:30:16 -0000 1.10
> +++ rde_community.c 30 Jan 2023 13:57:06 -0000
> @@ -112,6 +112,7 @@ fc2c(struct community *fc, struct rde_pe
> c->data3 = type << 8 | subtype;
> switch (type & EXT_COMMUNITY_VALUE) {
> case EXT_COMMUNITY_TRANS_TWO_AS:
> + case EXT_COMMUNITY_TRANS_FOUR_AS:
> if ((fc->flags >> 8 & 0xff) == COMMUNITY_ANY)
> break;
>
> @@ -121,11 +122,9 @@ fc2c(struct community *fc, struct rde_pe
> if (apply_flag(fc->data2, fc->flags >> 16, peer,
> &c->data2, m ? &m->data2 : NULL))
> return -1;
> - /* check that values fit */
> - if (c->data1 > USHRT_MAX)
> - return -1;
> + if (m)
> + m->data3 ^= EXT_COMMUNITY_TRANS_FOUR_AS << 8;
> return 0;
> - case EXT_COMMUNITY_TRANS_FOUR_AS:
> case EXT_COMMUNITY_TRANS_IPV4:
> if ((fc->flags >> 8 & 0xff) == COMMUNITY_ANY)
> break;
> @@ -268,7 +267,6 @@ struct rde_peer *peer)
> sizeof(*fc), fast_match) != NULL);
> } else {
> /* slow path */
> -
> if (fc2c(fc, peer, &test, &mask) == -1)
> return 0;
>
> @@ -335,6 +333,24 @@ struct rde_peer *peer)
> } else {
> if (fc2c(fc, peer, &set, NULL) == -1)
> return 0;
> + if ((uint8_t)set.flags == COMMUNITY_TYPE_EXT) {
> + int type = (int)set.data3 >> 8;
> + switch (type & EXT_COMMUNITY_VALUE) {
> + case EXT_COMMUNITY_TRANS_TWO_AS:
> + case EXT_COMMUNITY_TRANS_FOUR_AS:
> + /* check that values fit */
> + if (set.data1 > USHRT_MAX &&
> + set.data2 > USHRT_MAX)
> + return 0;
> + if (set.data1 > USHRT_MAX)
> + set.data3 = (set.data3 & 0xff) |
> + EXT_COMMUNITY_TRANS_FOUR_AS << 8;
> + else
> + set.data3 = (set.data3 & 0xff) |
> + EXT_COMMUNITY_TRANS_TWO_AS << 8;
> + break;
> + }
> + }
> insert_community(comm, &set);
> }
> return 1;
>
