Skip to content

Conversation

naveenachyuta
Copy link
Contributor

@naveenachyuta naveenachyuta commented May 8, 2025

add auto-discovery of bgp peers using default gateway by fetching default routes from statedb

Fixes: #37315

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

This PR adds a new feature called bgp auto-discovery to discover bgp peers dynamically and create bgp sessions with it.
Auto-discovery can use multiple modes to discovery the peers. This change will include default-gateway mode to discovery peers.
Proposal: https://github.com/cilium/design-cfps/blob/main/cilium/CFP-37315-bgp-auto-discovery.md

When a cilium user uses default-gateway mode for auto-discovery for a specific address family, cilium will fetch the routes from route table of statedb and link data from device table of statedb.
It will filter default routes for the given address family and check if the link through which the default route is reachable is UP or not (it will use operational status of the link from the device table for up/down). If its down, we will ignore it.
Finally, it will use the default gateway of the default route with the lowest priority.

What will happen if the default route's attributes like priority or the link status has changed?
cilium will track of route event changes using a separate go-routine. If there is any change in the link or default route, route update will be triggered and we will run bgp reconciliation to find peers again.

Steps for testing this change with multi-homing:-

  1. create kind cluster
    sudo make kind-bgpv2-dg-multi-homing

  2. build cilium
    sudo KIND_CLUSTER_NAME=bgpv2-cplane-dev-mh make kind-image

  3. Install cilium into the kind cluster
    sudo cilium install --chart-directory install/kubernetes/cilium -f contrib/containerlab/bgpv2/auto-discovery/default-gateway/multi-homing/values.yaml --set image.override="localhost:5000/cilium/cilium-dev:local" --set image.pullPolicy=Never --set operator.image.override="localhost:5000/cilium/operator-generic:local" --set operator.image.pullPolicy=Never

  4. apply bgp policy (contrib/containerlab/bgpv2/auto-discovery/default-gateway/bgp.yaml)
    sudo make kind-bgpv2-dg-multi-homing-apply-bgp

Fixes: #37315

BGP: Add peer auto-discovery using default-gateway mode

@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 May 8, 2025
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label May 8, 2025
@naveenachyuta naveenachyuta marked this pull request as ready for review May 8, 2025 04:20
@naveenachyuta naveenachyuta requested review from a team as code owners May 8, 2025 04:20
@naveenachyuta naveenachyuta requested review from aanm and rastislavs May 8, 2025 04:20
Copy link
Contributor

@rastislavs rastislavs 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 the PR! I have some minor requests for changes.

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.

Made an initial review. This time, I focused on the core logic changes and didn't review the lab part much.

Please break this single large commit down into smaller commits.

  • k8s resource change
  • Core logic change
  • service lab
  • multi-homing lab

@naveenachyuta naveenachyuta force-pushed the add_autodiscovery_defaultgateway branch from 8a3bc26 to 319ae70 Compare May 9, 2025 23:07
@naveenachyuta
Copy link
Contributor Author

Made an initial review. This time, I focused on the core logic changes and didn't review the lab part much.

Please break this single large commit down into smaller commits.

  • k8s resource change
  • Core logic change
  • service lab
  • multi-homing lab

This is fixed. i broke it into smaller commits.

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.

Made a deeper look at the labs. There are some nits. Overall looks good to me.

Once you addressed the comments, please consolidate the commits to four parts as I mentioned before. We prefer clean history over the incremental change log.

@naveenachyuta naveenachyuta force-pushed the add_autodiscovery_defaultgateway branch from 305dfa8 to d3ab6c4 Compare May 16, 2025 05:47
@rastislavs
Copy link
Contributor

/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 looks good to me. Thanks!

@naveenachyuta naveenachyuta force-pushed the add_autodiscovery_defaultgateway branch from 45dbc1b to 430081c Compare May 20, 2025 06:04
Fixes: cilium#37315 (add bgp auto discovery field with default gateway mode)
Signed-off-by: Naveen Achyuta <naveen.achyuta22@gmail.com>
Fixes: cilium#37315 (add autodiscovery of bgp peers with default gateway mode)
Signed-off-by: Naveen Achyuta <naveen.achyuta22@gmail.com>
Fixes: cilium#37315 (add service lab for testing bgp autodiscovery with default gateway mode)
Signed-off-by: Naveen Achyuta <naveen.achyuta22@gmail.com>
Fixes: cilium#37315 (add mutlihoming lab for testing bgp autodiscovery with default gateway mode and update Makefile to run it)
Signed-off-by: Naveen Achyuta <naveen.achyuta22@gmail.com>
Fixes: 37315 (update bgpv1 tests with route and device table)
Signed-off-by: Naveen Achyuta <naveen.achyuta22@gmail.com>
@naveenachyuta naveenachyuta force-pushed the add_autodiscovery_defaultgateway branch from 430081c to 3c41939 Compare May 20, 2025 06:20
@rastislavs
Copy link
Contributor

/test

@rastislavs rastislavs added area/bgp Impacts the Border Gateway Protocol feature. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels May 20, 2025
@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 May 20, 2025
Copy link
Contributor

@rastislavs rastislavs left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks for your contribution!

@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 May 20, 2025
@julianwiedmann julianwiedmann added this pull request to the merge queue May 20, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 20, 2025
@julianwiedmann julianwiedmann added this pull request to the merge queue May 20, 2025
Merged via the queue into cilium:main with commit 088f292 May 20, 2025
67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bgp Impacts the Border Gateway Protocol feature. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CFP: Auto Peer Discovery for Cilium BGP
5 participants