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.

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

bgpv1: Add GetRoutes method to Router interface and generic Path type #26803

Merged
merged 2 commits into from Jul 14, 2023

Conversation

rastislavs
Copy link
Contributor

@rastislavs rastislavs commented Jul 13, 2023

Adds GetRoutes method to the Router interface and generic Path and Route types, that can be used to retrieve existing routes from the RIB of underlying router.

This is the first step towards exposing the BGP routes via cilium API + CLI.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 13, 2023
@rastislavs rastislavs added the release-note/misc This PR makes changes that have no direct user impact. label Jul 13, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 13, 2023
@rastislavs rastislavs force-pushed the bgp-get-routes branch 2 times, most recently from 87f7bf3 to 8736d6d Compare July 13, 2023 10:07
@rastislavs rastislavs marked this pull request as ready for review July 13, 2023 10:12
@rastislavs rastislavs requested a review from a team as a code owner July 13, 2023 10:12
Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

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

Thanks for taking over this! Overall looks good except some minor things. Could you consider consolidating last two commits into first two? So that we will have a clean commit history.

pkg/bgpv1/gobgp/conversions_test.go Outdated Show resolved Hide resolved
Add a new generic Path type that which is an analogue of the GoBGP's
Path object, but only contains a minimal fields required for Cilium's
usecases. It consists of NLRI, Path Attributes and some status fields.

We are relying on the GoBGP's pkg/packet/bgp package which is a generic
BGP library (say net.IP package, but for BGP). We decided to reuse this
library as a part of our router-independent intermediate representation
because it's too painful for us to reinvent the BGP library by
ourselves.

This type will be used for implementing various BGP CPlane
functionalities like retrieving the route.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Co-authored-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
Add a new GetRoutes method to the Router interface. This method
retrieves routes from unerlying router's RIB. It supports retrieving
Loc-RIB as well as Adj-RIB-In/Out.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Co-authored-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
@rastislavs
Copy link
Contributor Author

@YutaroHayakawa I consolidated the commits

@rastislavs
Copy link
Contributor Author

/test

Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

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

Now LGTM 👍 Thanks!

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 14, 2023
@julianwiedmann julianwiedmann merged commit c0748ec into cilium:main Jul 14, 2023
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bgp ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants