Handling AGGREGATOR in v2.0.7
Hi all, I found a bit strange behavior of handling AGGREGATOR attributes in BIRD v2.0.7 route server. When BIRD receives a route containing AGGREGATOR attribute from 4byte-AS capable peer, it sends the route with strange AGGREGATOR to 4byte-AS incapable peer. On the other hand, BIRD can sends AS4_AGGREGATOR attribute correctly. Here is a scenario what I did. In the following simple topology, Peer1 and BIRD RS are 4byte-AS capable and Peer2 is 2byte-AS support only. Peer1 --(r1)--> BIRD RS --(r1’)--> Peer2 Peer1 advertise a route r1 which contains AGGREGATOR attribute. (omitting ATOMIC_AGGREGATE here) ------ r1: AGGREGATOR: - Aggregator AS: 650001 (in 4byte) - Aggregator Origin: x.x.x.x ------ Then, the expected route r1’ advertised from RS to Peer2 should be like: ------ r1’: AGGREGATOR: - Aggregator AS: 23456 (AS_TRANS, in 2byte) - Aggregator Origin: x.x.x.x AS4_AGGREGATOR: - Aggregator AS: 650001 (in 4byte) - Aggregator Origin: x.x.x.x ------ But as far as I see, BIRD sends the route including AGGREGATOR like: ------ r1’: AGGREGATOR: - Aggregator AS: 0 (in 2byte) #here - Aggregator Origin: y.y.y.y #and here, not match to original origin x.x.x.x AS4_AGGREGATOR: - Aggregator AS: 650001 (in 4byte) - Aggregator Origin: x.x.x.x ------ This may not confirm to the section 4.2.2 in https://tools.ietf.org/html/rfc6793. And also, when the AS number in original AGGREGATOR is a “mappable” 4byte-AS number, I see that the AS number in AGGREGATOR advertised from BIRD is set as 65535 (0xFFFF). Then I tried a small patch, this fixed my problem. But I’m not sure if it’s appropriate. ------------------------------------------ diff --git a/nest/attrs.h b/nest/attrs.h index 6fb0a8fa..e0399708 100644 --- a/nest/attrs.h +++ b/nest/attrs.h @@ -111,7 +111,7 @@ static inline struct adata * aggregator_to_old(struct linpool *pool, const struct adata *a) { struct adata *d = lp_alloc_adata(pool, 8); - put_u32(d->data, 0xFFFF); + put_u32(d->data, AS_TRANS); memcpy(d->data + 4, a->data + 4, 4); return d; } diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index b8921363..f2a4d5b2 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -562,6 +562,7 @@ bgp_encode_aggregator(struct bgp_write_state *s, eattr *a, byte *buf, uint size) /* Prepare 16-bit AGGREGATOR (from 32-bit one) in a temporary buffer */ byte *dst = alloca(6); len = aggregator_32to16(dst, data); + data = dst; } return bgp_put_attr(buf, size, BA_AGGREGATOR, a->flags, data, len); ------------------------------------------ If further information is needed I will be happy to provide it. Best regards, ----- Nasato Goto JPNAP / INTERNET MULTIFEED CO. goto@mfeed.ad.jp -----
On Tue, Apr 14, 2020 at 02:45:29PM +0000, Nasato Goto wrote:
Hi all,
I found a bit strange behavior of handling AGGREGATOR attributes in BIRD v2.0.7 route server. When BIRD receives a route containing AGGREGATOR attribute from 4byte-AS capable peer, it sends the route with strange AGGREGATOR to 4byte-AS incapable peer. On the other hand, BIRD can sends AS4_AGGREGATOR attribute correctly.
Here is a scenario what I did. In the following simple topology, Peer1 and BIRD RS are 4byte-AS capable and Peer2 is 2byte-AS support only.
Peer1 --(r1)--> BIRD RS --(r1’)--> Peer2
...
Then I tried a small patch, this fixed my problem. But I’m not sure if it’s appropriate.
Hi Thanks for the bugreport and the patch. It is fully appropriate, the second part is a clear omission on my side. Will merge it. Just wondering whether you noticed this issue during testing or during operation (so there are still some 2B-AS-only implementations in the wild.
------------------------------------------ diff --git a/nest/attrs.h b/nest/attrs.h index 6fb0a8fa..e0399708 100644 --- a/nest/attrs.h +++ b/nest/attrs.h @@ -111,7 +111,7 @@ static inline struct adata * aggregator_to_old(struct linpool *pool, const struct adata *a) { struct adata *d = lp_alloc_adata(pool, 8); - put_u32(d->data, 0xFFFF); + put_u32(d->data, AS_TRANS); memcpy(d->data + 4, a->data + 4, 4); return d; } diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index b8921363..f2a4d5b2 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -562,6 +562,7 @@ bgp_encode_aggregator(struct bgp_write_state *s, eattr *a, byte *buf, uint size) /* Prepare 16-bit AGGREGATOR (from 32-bit one) in a temporary buffer */ byte *dst = alloca(6); len = aggregator_32to16(dst, data); + data = dst; }
return bgp_put_attr(buf, size, BA_AGGREGATOR, a->flags, data, len); ------------------------------------------
If further information is needed I will be happy to provide it.
Best regards,
----- Nasato Goto JPNAP / INTERNET MULTIFEED CO. goto@mfeed.ad.jp -----
-- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Hi Ondrej, Thank you! Could you tell me will it be merged on next release, v2.0.8?
Just wondering whether you noticed this issue during testing or during operation (so there are still some 2B-AS-only implementations in the wild. I found this through our test suite. We faced with this kind of issue in production few years ago. Currently, we still have only few customers who don't support 4byte-AS.
Best regards, ----- Nasato Goto JPNAP / INTERNET MULTIFEED CO. goto@mfeed.ad.jp ----- 2020/04/15 0:12 に、"Ondrej Zajicek" <santiago@crfreenet.org> を書き込みました: On Tue, Apr 14, 2020 at 02:45:29PM +0000, Nasato Goto wrote: > Hi all, > > I found a bit strange behavior of handling AGGREGATOR attributes in BIRD v2.0.7 route server. > When BIRD receives a route containing AGGREGATOR attribute from 4byte-AS capable peer, > it sends the route with strange AGGREGATOR to 4byte-AS incapable peer. > On the other hand, BIRD can sends AS4_AGGREGATOR attribute correctly. > > Here is a scenario what I did. > In the following simple topology, Peer1 and BIRD RS are 4byte-AS capable and Peer2 is 2byte-AS support only. > > Peer1 --(r1)--> BIRD RS --(r1’)--> Peer2 > > ... > > Then I tried a small patch, this fixed my problem. > But I’m not sure if it’s appropriate. Hi Thanks for the bugreport and the patch. It is fully appropriate, the second part is a clear omission on my side. Will merge it. Just wondering whether you noticed this issue during testing or during operation (so there are still some 2B-AS-only implementations in the wild. > ------------------------------------------ > diff --git a/nest/attrs.h b/nest/attrs.h > index 6fb0a8fa..e0399708 100644 > --- a/nest/attrs.h > +++ b/nest/attrs.h > @@ -111,7 +111,7 @@ static inline struct adata * > aggregator_to_old(struct linpool *pool, const struct adata *a) > { > struct adata *d = lp_alloc_adata(pool, 8); > - put_u32(d->data, 0xFFFF); > + put_u32(d->data, AS_TRANS); > memcpy(d->data + 4, a->data + 4, 4); > return d; > } > diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c > index b8921363..f2a4d5b2 100644 > --- a/proto/bgp/attrs.c > +++ b/proto/bgp/attrs.c > @@ -562,6 +562,7 @@ bgp_encode_aggregator(struct bgp_write_state *s, eattr *a, byte *buf, uint size) > /* Prepare 16-bit AGGREGATOR (from 32-bit one) in a temporary buffer */ > byte *dst = alloca(6); > len = aggregator_32to16(dst, data); > + data = dst; > } > > return bgp_put_attr(buf, size, BA_AGGREGATOR, a->flags, data, len); > ------------------------------------------ > > If further information is needed I will be happy to provide it. > > > Best regards, > > ----- > Nasato Goto > JPNAP / INTERNET MULTIFEED CO. > goto@mfeed.ad.jp > ----- -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On Wed, Apr 15, 2020 at 01:59:22AM +0000, Nasato Goto wrote:
Hi Ondrej,
Thank you! Could you tell me will it be merged on next release, v2.0.8?
Yes, i just committed it.
Just wondering whether you noticed this issue during testing or during operation (so there are still some 2B-AS-only implementations in the wild. I found this through our test suite. We faced with this kind of issue in production few years ago. Currently, we still have only few customers who don't support 4byte-AS.
Best regards,
-- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Thanks! ----- Nasato Goto JPNAP / INTERNET MULTIFEED CO. goto@mfeed.ad.jp ----- 2020/04/15 11:07 に、"Ondrej Zajicek" <santiago@crfreenet.org> を書き込みました: On Wed, Apr 15, 2020 at 01:59:22AM +0000, Nasato Goto wrote: > Hi Ondrej, > > Thank you! > Could you tell me will it be merged on next release, v2.0.8? Yes, i just committed it. > > Just wondering whether you noticed this issue during testing or during > > operation (so there are still some 2B-AS-only implementations in the > > wild. > I found this through our test suite. We faced with this kind of issue in production few years ago. > Currently, we still have only few customers who don't support 4byte-AS. > > Best regards, -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
participants (2)
-
Nasato Goto -
Ondrej Zajicek