Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bgp: router id insufficient discriminator for route-refresh stale marking #241

Open
rcgoodfellow opened this issue May 2, 2024 · 1 comment · May be fixed by #398
Open

bgp: router id insufficient discriminator for route-refresh stale marking #241

rcgoodfellow opened this issue May 2, 2024 · 1 comment · May be fixed by #398
Labels
bgp Border Gateway Protocol Bug

Comments

@rcgoodfellow
Copy link
Collaborator

This is a follow up for this review comment

We need to mark routes as stale for route refresh based on session, not on router id, as we may have multiple sessions with a single router and thus single router id.
@rcgoodfellow rcgoodfellow added Bug bgp Border Gateway Protocol labels May 2, 2024
@rcgoodfellow rcgoodfellow mentioned this issue May 2, 2024
Merged
@rcgoodfellow rcgoodfellow changed the title bgp: stale route detection based on router id bgp: router id insufficient discriminator for route-refresh stale marking May 2, 2024
@taspelund taspelund mentioned this issue Oct 20, 2024
Merged
@taspelund
Copy link
Contributor

This discriminator should be something specific to the BGP session, since that should have a 1:1 mapping to a configured peer.
I did an initial code-dive to see what value might make sense to use in our BGP implementation, and it seems like Cnx: BgpConnection is the likely candidate. A tuple of local/remote sockaddrs might also be an option here, but that just seems like peeling one layer of abstraction off of the BgpConnection trait without an immediately obvious upside.

taspelund added a commit that referenced this issue Oct 25, 2024
@taspelund
Use socket addr pair to distinguish between BGP sessions
8e5d38f 
BGP-ID is insufficient to distinguish between BGP sessions, as it's
perfectly valid/reasonable to have multiple sessions to the same peer.
Or an operator can (incorrectly or otherwise) assign the same BGP-ID to
multiple routers.
This ensures that Session-A to Peer-A going down does not impact
Session-B to Peer-A.

Fixes: #241

Signed-off-by: Trey Aspelund <trey@oxidecomputer.com>
@taspelund taspelund linked a pull request Oct 25, 2024 that will close this issue
Open
taspelund added a commit that referenced this issue Nov 1, 2024
@taspelund
Use socket addr pair to distinguish between BGP sessions
a172be6 
BGP-ID is insufficient to distinguish between BGP sessions, as it's
perfectly valid/reasonable to have multiple sessions to the same peer.
Or an operator can (incorrectly or otherwise) assign the same BGP-ID to
multiple routers.
This ensures that Session-A to Peer-A going down does not impact
Session-B to Peer-A.

Fixes: #241

Signed-off-by: Trey Aspelund <trey@oxidecomputer.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bgp Border Gateway Protocol Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Stop using Router-ID to distinguish between BGP sessions oxidecomputer/maghemite
2 participants
@rcgoodfellow @taspelund