Skip to content

Levon/make bgp dispatch listen configurable#654

Open
internet-diglett wants to merge 7 commits intomainfrom
levon/make-bgp-dispatch-listen-configurable
Open

Levon/make bgp dispatch listen configurable#654
internet-diglett wants to merge 7 commits intomainfrom
levon/make-bgp-dispatch-listen-configurable

Conversation

@internet-diglett
Copy link
Copy Markdown
Contributor

No description provided.

We'd like the ability to create a topology of BGP routers in
our test and dev tooling. To do so with the current omicron-dev
and testing contexts, we need the peering to occur over localhost,
which means we need to be able to configure unique localhost
socketaddrs for each instance.
The omicron dev and test contexts provide :0 as the listen
port and rely on the log messages to output the actual port number
that the daemon bound to.
Copy link
Copy Markdown
Contributor

@taspelund taspelund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. If you wouldn't mind using the preexisting logging macros that wrap slog, that would be great. Otherwise, LGTM!

@taspelund taspelund self-requested a review March 9, 2026 18:09
In testing environments we commonly communicate over local addresses.
Previously, maghemite relied on the OS to select the source ip address
for peering requests. This can create problems in testing because the
host will pick the same source address as the the target peer address,
causing the peer to reject the bgp peering session. By allowing explicit
configuration of the source address we resolve this issue.
@internet-diglett internet-diglett marked this pull request as ready for review April 1, 2026 21:04
@taspelund
Copy link
Copy Markdown
Contributor

taspelund commented Apr 2, 2026

Overall this looks good to me. The only thing I have some concern about is the lack of validation that the address-family of the Source IP matches the address-family of the peer's IP (matching the variant inside PeerId::Ip or v6 for PeerId::Interface). I imagine that logic could live in helpers::add_neighbor and should be a quick and easy check.

Once that's done and the PR has been rebased, I think we're good to go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants