Skip to content

Add route summarization to bgp advertistments services #36791

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
Closed
wants to merge 3 commits into from
Closed

Add route summarization to bgp advertistments services #36791

wants to merge 3 commits into from

Conversation

romanspb80
Copy link
Contributor

New summarization option under the CiliumLoadBalancerIPPool to summarize address

apiVersion: "cilium.io/v2alpha1"
kind: CiliumLoadBalancerIPPool
metadata:
name: "lab-pool"
spec:
summarize: true <------- new option to summarize any service to the block address when advertised
blocks:

cidr: "10.100.11.0/24"

Fixes: #31116

@romanspb80 romanspb80 requested review from a team as code owners December 25, 2024 14:27
@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 Dec 25, 2024
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Dec 25, 2024
@romanspb80 romanspb80 mentioned this pull request Dec 25, 2024
Closed
@romanspb80 romanspb80 force-pushed the add-route-summarization-to-bgp-advertistments-services branch 3 times, most recently from ef13b67 to b8fbbc7 Compare December 25, 2024 14:57
@pchaigno pchaigno added release-note/major This PR introduces major new functionality to Cilium. area/bgp Impacts the Border Gateway Protocol feature. labels Dec 25, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Dec 25, 2024
@romanspb80 romanspb80 force-pushed the add-route-summarization-to-bgp-advertistments-services branch from b8fbbc7 to 9ab1f5d Compare December 25, 2024 18:28
@romanspb80
Add bgp summarization feature for service advertisements (RFC 4632)
199b069 
New summarization option under the CiliumLoadBalancerIPPool to summarize address

apiVersion: "cilium.io/v2alpha1"
kind: CiliumLoadBalancerIPPool
metadata:
  name: "lab-pool"
spec:
  summarize: true <------- new option to summarize any service to the block address when advertised
  blocks:
  - cidr: "10.100.11.0/24"

Fixes: cilium#31116

Signed-off-by: Roman Ptitcyn <romanspb@yahoo.com>
@romanspb80
Add files after generate-k8s-api && make manifests
2423bf5 
New summarization option under the CiliumLoadBalancerIPPool to summarize address

Fixes: cilium#31116

Signed-off-by: Roman Ptitcyn <romanspb@yahoo.com>
@romanspb80
Add files after go mod tidy && go mod vendor
685f5be 
New summarization option under the CiliumLoadBalancerIPPool to summarize address

Fixes: cilium#31116

Signed-off-by: Roman Ptitcyn <romanspb@yahoo.com>
@romanspb80 romanspb80 force-pushed the add-route-summarization-to-bgp-advertistments-services branch from 9ab1f5d to 685f5be Compare December 27, 2024 23:59
@harsimran-pabla
Copy link
Contributor

Hi @romanspb80 Thanks for your contribution, I see that feature request issue regarding route-aggregation was raised recently. So, implementation is very timely.

I have put my thoughts in this comment, I think we need to come up with approach which addresses those concerns. I suggest putting this PR in draft for time being till we conclude on scope and design. You can add additional context in that issue as well.

@bpinske bpinske mentioned this pull request Jan 2, 2025
Closed
@romanspb80
Copy link
Contributor Author

Hi @romanspb80 Thanks for your contribution, I see that feature request issue regarding route-aggregation was raised recently. So, implementation is very timely.

I have put my thoughts in this comment, I think we need to come up with approach which addresses those concerns. I suggest putting this PR in draft for time being till we conclude on scope and design. You can add additional context in that issue as well.

Hi @harsimran-pabla . Thanks

@harsimran-pabla
Copy link
Contributor

Hi @romanspb80 please let me know if you plan on using same PR for the new implementation. I'd suggest closing this one and opening new PR with agreed approach. You can open a draft PR and tag sig-bgp for initial review before pulling in other reviewers.

@romanspb80
Copy link
Contributor Author

romanspb80 commented Jan 27, 2025 •
edited
Loading

Hi @romanspb80 please let me know if you plan on using same PR for the new implementation. I'd suggest closing this one and opening new PR with agreed approach. You can open a draft PR and tag sig-bgp for initial review before pulling in other reviewers.

Hi @harsimran-pabla . I've rised PR for BGP Control Plane version2 in accordance we discussed that case. I haven't closed this PR yet, because it might make sense to implement aggregation for the BGP Control Plane version1 as well, while it's still relevant.

@romanspb80 romanspb80 mentioned this pull request Jan 30, 2025
Merged
@joestringer
Copy link
Member

From the community meeting: It looks like #37275 is superseding this PR, so there is no need for this PR. Closing.

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

@ovidiutirla ovidiutirla Awaiting requested review from ovidiutirla ovidiutirla is a code owner automatically assigned from cilium/vendor

@youngnick youngnick Awaiting requested review from youngnick youngnick is a code owner automatically assigned from cilium/sig-k8s

@harsimran-pabla harsimran-pabla Awaiting requested review from harsimran-pabla harsimran-pabla is a code owner automatically assigned from cilium/sig-bgp

Assignees
No one assigned
Labels
area/bgp Impacts the Border Gateway Protocol feature. kind/community-contribution This was a contribution made by a community member. release-note/major This PR introduces major new functionality to Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CFP: Provide support to advertised routes as a summarized address when doing router advertistment
4 participants
@romanspb80 @harsimran-pabla @joestringer @pchaigno