On Wed, Apr 14, 2021 at 11:56:08PM +0200, Alexander Bluhm wrote:
> Hi,
>
> On powerpc64 regress/usr.sbin/bgpd/config fails. It parses a config
> file, writes bgpd's config to stdout and compares it with an expected
> output.
>
> On powerpc64 the order of the set of communities is different.
>
> The parser uses memcmp(3) to sort a struct of integers. This depends
> of the endianess. The correct way is to compare the integer fields
> in native byte order.
>
> With this diff, the resulting order is the same on i386 and poerpc64.
> Also on powerpc64 the fix does not change bgpd's output. memcmp(3)
> and field comparison are equivalent on big endian architectures.
>
> ok?
OK claudio@
There are more places where communities are memcmp (filterset_cmp(),
filterset_equal()) but in these cases only equality matters.
filterset_add() needs to ensure that equal filterset lists are always
sorted the same way.
> Index: usr.sbin/bgpd/parse.y
> ===================================================================
> RCS file: /mount/openbsd/cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.414
> diff -u -p -r1.414 parse.y
> --- usr.sbin/bgpd/parse.y 2 Mar 2021 09:45:07 -0000 1.414
> +++ usr.sbin/bgpd/parse.y 14 Apr 2021 21:09:47 -0000
> @@ -3558,6 +3558,28 @@ symget(const char *nam)
> }
>
> static int
> +cmpcommunity(struct community *a, struct community *b)
> +{
> + if (a->flags > b->flags)
> + return 1;
> + if (a->flags < b->flags)
> + return -1;
> + if (a->data1 > b->data1)
> + return 1;
> + if (a->data1 < b->data1)
> + return -1;
> + if (a->data2 > b->data2)
> + return 1;
> + if (a->data2 < b->data2)
> + return -1;
> + if (a->data3 > b->data3)
> + return 1;
> + if (a->data3 < b->data3)
> + return -1;
> + return 0;
> +}
> +
> +static int
> getcommunity(char *s, int large, u_int32_t *val, u_int32_t *flag)
> {
> long long max = USHRT_MAX;
> @@ -4396,16 +4418,17 @@ filterset_add(struct filter_set_head *sh
> switch (s->type) {
> case ACTION_SET_COMMUNITY:
> case ACTION_DEL_COMMUNITY:
> - if (memcmp(&s->action.community,
> - &t->action.community,
> - sizeof(s->action.community)) < 0) {
> + switch (cmpcommunity(&s->action.community,
> + &t->action.community)) {
> + case -1:
> TAILQ_INSERT_BEFORE(t, s, entry);
> return;
> - } else if (memcmp(&s->action.community,
> - &t->action.community,
> - sizeof(s->action.community)) == 0)
> + case 0:
> break;
> - continue;
> + case 1:
> + continue;
> + }
> + break;
> case ACTION_SET_NEXTHOP:
> /* only last nexthop per AF matters */
> if (s->action.nexthop.aid <
> Index: regress/usr.sbin/bgpd/config/bgpd.conf.10.ok
> ===================================================================
> RCS file:
> /mount/openbsd/cvs/src/regress/usr.sbin/bgpd/config/bgpd.conf.10.ok,v
> retrieving revision 1.6
> diff -u -p -r1.6 bgpd.conf.10.ok
> --- regress/usr.sbin/bgpd/config/bgpd.conf.10.ok 17 Jul 2019 10:27:50
> -0000 1.6
> +++ regress/usr.sbin/bgpd/config/bgpd.conf.10.ok 14 Apr 2021 21:14:42
> -0000
> @@ -40,4 +40,4 @@ match from any large-community 1234:5678
> match from any large-community 1234:5678:1 large-community 1234:5678:2
> large-community 1234:5678:3
> match from any community 1234:1 large-community 1234:5678:1
> match from any large-community 1234:5678:1 community 1234:1
> -match from any set { community delete 1234:5678 community delete 1234:*
> community delete *:5678 community delete local-as:5678 community delete
> local-as:neighbor-as large-community delete 1234:15:5678 large-community
> delete *:15:5678 large-community delete local-as:15:5678 large-community
> delete local-as:15:* large-community delete local-as:15:neighbor-as
> large-community delete local-as:*:* community 1234:5678 community
> local-as:5678 community local-as:neighbor-as large-community 1234:15:5678
> large-community local-as:15:5678 large-community local-as:15:neighbor-as }
> +match from any set { community delete 1234:5678 large-community delete
> 1234:15:5678 community delete *:5678 large-community delete *:15:5678
> community delete local-as:5678 large-community delete local-as:15:5678
> community delete 1234:* community delete local-as:neighbor-as large-community
> delete local-as:15:* large-community delete local-as:*:* large-community
> delete local-as:15:neighbor-as community 1234:5678 large-community
> 1234:15:5678 community local-as:5678 large-community local-as:15:5678
> community local-as:neighbor-as large-community local-as:15:neighbor-as }
> Index: regress/usr.sbin/bgpd/config/bgpd.conf.11.ok
> ===================================================================
> RCS file:
> /mount/openbsd/cvs/src/regress/usr.sbin/bgpd/config/bgpd.conf.11.ok,v
> retrieving revision 1.5
> diff -u -p -r1.5 bgpd.conf.11.ok
> --- regress/usr.sbin/bgpd/config/bgpd.conf.11.ok 17 Jul 2019 10:27:50
> -0000 1.5
> +++ regress/usr.sbin/bgpd/config/bgpd.conf.11.ok 14 Apr 2021 21:14:09
> -0000
> @@ -33,7 +33,7 @@ match from any ext-community ovs invalid
> match from any ext-community ovs not-found
> match from any ext-community rt 64496:201 ext-community soo 64496:202
> match from any ext-community rt 64496:301 ext-community soo 4200000001:302
> ext-community odi 127.0.0.1:303
> -match from any set { ext-community delete ovs valid ext-community delete odi
> 127.0.0.1:6003 ext-community delete soo 4200000001:6002 ext-community delete
> ort 0x123456789abf ext-community delete rt 64496:6001 ext-community ovs valid
> ext-community odi 127.0.0.1:5003 ext-community soo 4200000001:5002
> ext-community ort 0x123456789abc ext-community rt 64496:5001 }
> +match from any set { ext-community delete ovs valid ext-community delete ort
> 0x123456789abf ext-community delete rt 64496:6001 ext-community delete odi
> 127.0.0.1:6003 ext-community delete soo 4200000001:6002 ext-community ovs
> valid ext-community ort 0x123456789abc ext-community rt 64496:5001
> ext-community odi 127.0.0.1:5003 ext-community soo 4200000001:5002 }
> match from any ext-community * *
> match from any ext-community rt *
> match from any ext-community soo *
> @@ -47,7 +47,7 @@ match from any ext-community rt 127.0.0.
> match from any ext-community soo 64496:*
> match from any ext-community soo 4200000001:*
> match from any ext-community soo 127.0.0.1:*
> -match from any set { ext-community delete odi 127.0.0.1:* ext-community
> delete soo 4200000001:* ext-community delete rt 64496:* ext-community delete
> mac-mob * ext-community delete ovs * ext-community delete rt * ext-community
> delete soo * ext-community delete odi * ext-community delete ort * }
> +match from any set { ext-community delete ort * ext-community delete mac-mob
> * ext-community delete ovs * ext-community delete rt * ext-community delete
> soo * ext-community delete odi * ext-community delete rt 64496:*
> ext-community delete odi 127.0.0.1:* ext-community delete soo 4200000001:* }
> match from any ext-community rt 64496:local-as
> match from any ext-community soo 4200000001:local-as
> match from any ext-community odi 127.0.0.1:local-as
>
--
:wq Claudio
