bgpd: fix ipv4-mapped ipv6 on non 6pe

[louis-6wind]

0325116 ("bgpd: fix 6vpe nexthop") enforced sending an IPv4-mapped IPv6 address as nexthop when the BGP session is on IPv4 and address family IPv6. This is not always correct. In some cases, sending an IPv6 global address is valid on an IPv4 session:

• MP-BGP is used and the sending router has an IPv4 and a global IPv6 address on the interface towards the peer.
• nexthop is set by a route-map
• router that is sending the UPDATE is a route-reflector

IPv4-mapped IPv6 nexthop should be limited to the 6PE /6vPE cases where the prefix is reachable via an IPv4 only MPLS backbone.

Do not replace the global IPv6 address when sending UPDATE if:

• the nexthop is set by a route-map
• a valid global IPv6 address is set on the interface towards the peer. The 6PE router is not supposed to have a global IPv6 address set on the interface. An IPv6 link-local may be set on it.
• the sending router is a route-reflector

Fixes: 0325116 ("bgpd: fix 6vpe nexthop")

[ton31337]

Isn't this the same technically as #15611?

[louis-6wind]

Isn't this the same technically as #15611?

I have just updated the commit.

In comparison with your fix, I do not send IPv4-mapped IPv6 nexhop if:

• route-reflector
• route-map nexthop unchanged
• interface has IPv4 and a global IPv6. If IPv4 only, IN6_IS_ADDR_UNSPECIFIED(mod_v6nhg) || IN6_IS_ADDR_LINKLOCAL(mod_v6nhg) is true. I assume a 6PE/6vPE has an IPv4-only interface towards the peer.

I will see what the CI is saying and will do some manual tests to confirm the patch is correct

[ton31337]

Actually, we should take care about RS (route-server) also. Can you check it?

[ton31337]

A more elegant solution would be adding a separate flag and avoiding lots of checks (as now). For instance, doing something in bpacket_attr_vec_arr_set_vec(), like BPKT_ATTRVEC_FLAGS_NO_IPV4_MAPPING or so. Thoughts?

[louis-6wind]

A more elegant solution would be adding a separate flag and avoiding lots of checks (as now). For instance, doing something in bpacket_attr_vec_arr_set_vec(), like BPKT_ATTRVEC_FLAGS_NO_IPV4_MAPPING or so. Thoughts?

I will try to do that

[bbs2web]

Please ignore my ignorance, but could we perhaps honour when 'attribute-unchanged next-hop' is set on a peer?

We have interoperability problems with different vendors in our network whereby there is no uniform way of stitching the layer 2 fabric together amongst those devices. We subsequently have layer 2 presented to FRR, where some of these could be route reflectors (these would work with the suggestion of not altering the next-hop when using route reflection or route servers, but also some that provide certain other functions. In those cases MBGP exchanges both IPv4 and IPv6 address families over a single session, the next-hop is the PE router that ingested the prefix via eBGP and the IPv6 next-hop is directly reachable from every device in the network. The key point being that the interface that talks IPv4 + MPLS does not hold the IPv6 global address, that's attached to a separate bridge interface, herewith an example:

zanjnb01-rr04# show interface brief
Interface       Status  VRF             Addresses
---------       ------  ---             ---------
br0             up      default         + 2001:db8:1:0:51:49:21:55/64    # IPv6 bridge
eth0            up      default
eth0.11         up      default         192.168.12.146/30    # Link to CR01
eth0.13         up      default         192.168.12.154/30    # Link to CR02
eth0.14         up      default         192.168.12.158/30    # Link to partner RR
eth0.15         up      default           # IPv6 over MPLS for non-l2vpn capable routers, connected to br0
lo              up      default         51.49.21.55/32
pim6reg         up      default

The other exceptions, listed above, rely on a route-maps setting the next-hop; we however wouldn't know what that should be as the ingress PE sets it's IPv6 global address as that before sending it to RRs which reflect it to other PEs.

Replacing the prefix's next hop with the route reflector's IPv4 encoded as IPv6 on iBGP route reflector sessions should IMHO be an option, to either disable or enable that behaviour, depending on the default.

[louis-6wind]

The only CI error is about a BGP table version difference. #15711 needs to be merged before to fix that

[ton31337]

Does it work if we set attribute-unchanged next-hop?

[riw777]

looks good, waiting on @ton31337 's comments