Bug? / Patch for BGP next hop issue with frr peers

[Ondrej Zajicek]

On Wed, Apr 22, 2020 at 10:53:12PM +0200, Sebastian Hahn wrote:
> 
> 
> > On 20. Apr 2020, at 10:06, Sebastian Hahn <bird_users at sebastianhahn.net> wrote:
> >> On 20. Apr 2020, at 04:02, Darren O'Connor <mellow.drifter at gmail.com> wrote:
> >> 
> >> Is this an ipv6 route? nh length 32 means both a global and link-local address is being set. RFC2545 section 3
> > 
> > Yes, this is indeed an ipv6 route. I am using many IPv6 routes with other peers, they do not hit the same code path. Thank you for the RFC pointer!
> Hi,
> 
> so I have an update here. My peer is now using an updated FRR, version 7.3-1~deb10u1 (installed from https://deb.frrouting.org), yet it still isn't working (in a normal bird 2.0.7, not using above patch). It seems to be FRR is advertising its link-local address twice as next-hop:
> 
>      /* GW_DIRECT -> single_hop -> p->neigh != NULL */
> -    if (ipa_nonzero(gw))
> +    if (ipa_nonzero(gw)) {
> +      log(L_ERR "looking for neigh %I ll %I", gw, ll);
>        nbr = neigh_find(&p->p, gw, NULL, 0);
> +    }
>      else if (ipa_nonzero(ll))
> 
> This small patch causes fe80::1 (my peer's link local address) to be logged for both gw and ll address. This seems like a problem with FRR/the peer's configuration to me, yes?
>
> Nevertheless, I wonder if bird's behaviour is correct here. It seems to
> me from reading the pointed out RFC that in the face of two nexthop
> addresses, bird should attempt the connection using the specified
> link-local address, not the global address (basicall, replacing the order
> of the if ... else if above), while passing on the global address to its
> peers. My first attempt at a patch seems pretty wrong to me from a look
> at the RFC.

Hi

Well, the RFC 2545 is a bit vague and AFAIK nobody standardized
link-local only sessions. Our position is that the first address is
always global (as that is necessary for next hop resolving) and the
second (optional) is link-local, therefore in cases where no global
address is available the proper format of next hop should be (:: ll).

Unfortunately, there are other implementations that use in such cases
(ll) or (ll ll), we should handle that in bgp_decode_next_hop_ip(), but
the second case is not handled there. Will send you a patch.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santiago at 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."