BGP unnumbered

[rcgoodfellow]

This PR adds support for BGP unnumbered.

Depends on

• Add support for MP-BGP #584
• add support for v4 routes over v6 nexthops dendrite#181

The Main Event

Unnumbered peers are added as a new type of peer with their own set of API functions. When an unnumbered peer is added it goes into the pending state. The new peer request is handed over to an unnumbered peer manager where IPv6/NDP router advertisements (RA) are used to discover the address of the peer over the interface provided. When a router is discovered on the interface the pending peer then turns into a real peer. We use the IPv6 link-local address discovered in the RA and create a BGP session using that address. From here the extant BGP machinery works in the same way it currently does.

Neighbor discovery is a one-shot process when the unnumbered peer is created. Once we determine the peer address through an RA, that is what we use. If the peer address changes, the session needs to be deleted and recreated so discovery will happen again. No attempt is made to track neighbor state beyond initial session establishment. This may be revisited in a future release. One shot semantics has the benefit of being simple and stable. I don't believe we want to be overly sensitive to NDP RA dynamics as that would undercut the intent of things like BGP hold time and TCP connection tracking. Having a persistent monitor of NDP dynamics will need to be designed carefully.

Unnumbered support is designed to work in a point to point setting. We do not currently handle situations where there are multiple reachable routers reachable over a broadcast domain that the unnumbered interface is connected to. We simply attempt to establish a session with the first peer we find. This is how things have generally worked for unnumbered in my experience. We could develop something more robust, but this becomes a rather complex state machine where we have to try to establish sessions with every peer we discover. This could lead to doomed BGP sessions iterating through a state machine indefinitely.

Testing

This PR also adds a new testing substrate in the falcon-lab folder. BGP unnumbered requires multicast ICMPv6/NDP to work. This means we need a testing environment that routers connected over a broadcast domain, and some other unnumbered implementation to peer with to ensure we are not deluding ourselves with self-peering. Using illumos zones would have been nice here, but none of the BGP implementations I'm aware of on illumos besides mgd fully support unnumbered. FRR on the other hand has great support for unnumbered. So peering with a linux-based router puts us in falcon territory.

Another really important thing to test for unnumbered is mg-lower interaction. We need test coverage on nexthop interface mapping for data plane route installation. The interop test environment is currently upper-half only. Due to license nonsense, the interop topology machinery is also stuck in purgatory. So in this PR I decided to add a new falcon-based testing substrate that only uses unencumbered peers and the oxide nodes are softnpu vms so we have a full lower half with dendrite, P4 and the whole 9 yards. This also means the testing machinery can actually live in this repository which is a nice win. The best way to get acquainted with this new testing substrate is starting by reading through the first test.

[taspelund]

Are we wanting to add a config option for this in this PR? if we address it in a later PR, we may have to introduce a new dropshot API version to accommodate it

[rcgoodfellow]

This is still a TODO for this PR. We should talk live about what we want to do.

[rcgoodfellow]

Going to plumb a user accessible option act-as-a-default-ipv6-router that takes the lifetime as a parameter.

[rcgoodfellow]

This is now done.

[taspelund]

I don't think we can use this as a default in Open messages for all peers.
My understanding of RFC 8950 is that we should only advertise this capability if we intend to advertise IPv4 Unicast NLRI with IPv6 next-hops... which is not something we can assume all the time, especially for preexisting numbered IPv4 peers.

[rcgoodfellow]

I'm not immediately seeing how advertising this capability would be harmful even when not used, but happy to talk through the issues this could cause.

[taspelund]

I don't recall if RFC 8950 says that ENHE advertises an additional next-hop AFI for the session or if ENHE advertises the only next-hop AFI for the session, but my experience with other routing stacks is that they assume all routes will use the ENHE next-hop AFI when that cap has been negotiated.

My concern is that unconditionally advertising ENHE would mean that we're allowing the peer to dictate which next-hop AFI we should use, rather than explicitly disallowing it when we don't want it.
e.g.
We configure a numbered peer over v4 and want to continue using v4 nexthops. If we enable ENHE unconditionally, it's now up to the peer to dictate whether we should send v4 or v6 next-hops depending on whether they advertise ENHE.

[rcgoodfellow]

For this PR we'll advertise extended nexthop for unnumbered sessions only and revisit broader support later.

[rcgoodfellow]

This is now done.

[taspelund]

If you want help with the struct version updates, I'm happy to do so. I think it would be best to try and get this into the API when we release ENHE so it can be visible to consumers of the API (e.g. when they poll the neighbor status to see what capabilities have been exchanged).

[rcgoodfellow]

Yeah, this is still a TODO on this PR. Now that we have the API versioning machinery in place, I think we need to think a bit about having raw BGP messages in the API definitions. In the original message history APIs I had these data structures as opaque objects to avoid all the API pain as we fill in capabilities.

[taspelund]

Another couple items:

1. There aren't any cargo tests for any of the rust code. I'd really like to see some unit tests for the unnumbered manager, and possibly some integration tests (although, it understandably seems difficult to integrate unnumbered into bgp/src/test.rs)
2. I'd like to see some docs update for this. I've made an effort to get BGP documented (docs/bgp-architecture.md), so it would be nice to have the BGP-related changes added to that doc + to have the unnumbered manager documented too (perhaps in its own markdown file), even if it's just LLM-generated docs that have been quickly reviewed. This isn't a blocker, but a nice to have.

[taspelund]

Re-running proptest -- it failed w/ the tokio "error 0" signature

[rcgoodfellow]

This logic needs to be updated

maghemite/bgp/src/session.rs

Lines 7542 to 7561 in 365caf9

	RouteUpdate::V4(RouteUpdate4::Announce(nlri)) => {
	let nh4 = match self.derive_nexthop(Afi::Ipv4, pc)? {
	BgpNexthop::Ipv4(addr) => addr,
	_ => {
	return Err(Error::InvalidAddress(
	"IPv4 routes require IPv4 next-hop".into(),
	));
	}
	};
	let mut path_attributes = self.router.base_attributes();
	path_attributes.push(PathAttributeValue::NextHop(nh4).into());
	UpdateMessage {
	withdrawn: vec![],
	path_attributes,
	nlri,
	..Default::default()
	}
	}

[taspelund]

One other small item I noticed: .github/buildomat/jobs/falcon-lab.sh doesn't have execute permissions tracked by git.
Should be easy to add a commit after running git update-index --chmod=+x .github/buildomat/jobs/falcon-lab.sh so there's no chance of a permissions issue later.