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 PIC HLD #1493

Open
wants to merge 49 commits into
base: master
Choose a base branch
from
Open

BGP PIC HLD #1493

wants to merge 49 commits into from

Conversation

eddieruan-alibaba
Copy link
Contributor

@eddieruan-alibaba eddieruan-alibaba commented Oct 9, 2023 •
edited
Loading

pguibert6WIND
pguibert6WIND reviewed Oct 13, 2023
View reviewed changes
doc/bgp_pic/bgp_pic.md Outdated
1. In the 'zebra' component:
- Introduce a new Next Hop Group (PIC-NHG) specifically for the FORWARDING function. This NHG will serve as the shareable NHG in hardware.
- When a BGP next hop becomes unavailable, zebra will first update the new FORWARDING-ONLY NHG before BGP convergence takes place.
- If changes occur in the IGP NHG and these changes do not affect the reachability of individual members within the BGP NHG, there is no need to update the BGP NHG.
Copy link

@pguibert6WIND pguibert6WIND Oct 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think you should specify what you mean by "these changes do not affect the reachability ...".
I have 2 use cases in mind:

  • UCMP - weighted extended community",
  • route-map configured that look for the IGP metric, will be ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me update the wording. I mean the case BGP NH reachability would not be changed when IGP NHG updates. zebra needs to check BGP NH's reachability and skip BGP NHG update if all members's reachability is not changed.

Currently, the update is reported back to BGP and trigger BGP update even BGP NH's reachability is unchanged.

Copy link

Choose a reason for hiding this comment

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

thanks Eddie

pguibert6WIND
pguibert6WIND reviewed Oct 13, 2023
View reviewed changes
Copy link

@pguibert6WIND pguibert6WIND left a comment

Choose a reason for hiding this comment

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

Some questions (to clarify)
more will come before next Tuesday (for frr meeting).
thanks.

Can you clarify the use case: is it for ebgp single hop and multi hop ?

@eddieruan-alibaba
Copy link
Contributor Author

we use eBGP for underlay peering and MP-BGP for overlay peering with remote PE.

@eddieruan-alibaba eddieruan-alibaba mentioned this pull request Oct 31, 2023
Open
@eddieruan-alibaba
Copy link
Contributor Author

Suggested by FRR folks, I raised FRRouting/frr#14703 for tracking the discussions in FRR community.

@eddieruan-alibaba
Copy link
Contributor Author

HLD's direct link is https://github.com/eddieruan-alibaba/SONiC/blob/eruan-pic/doc/bgp_pic/bgp_pic.md

@eddieruan-alibaba
Copy link
Contributor Author

The new location is moved to doc/pic/

https://github.com/eddieruan-alibaba/SONiC/blob/eruan-pic/doc/pic/bgp_pic_edge.md

@eddieruan-alibaba
Copy link
Contributor Author

This PR contains two HLD now

  1. PIC core, a.ka. recursive route handling HLD
  2. PIC edge

@zice312963205 zice312963205 mentioned this pull request Sep 20, 2024
Open
@zice312963205 zice312963205 mentioned this pull request Oct 31, 2024
Closed
@zhangyanzhao
Copy link
Collaborator

@eddieruan-alibaba can you please add the code PRs to this HLD PR? Thanks.

@eddieruan-alibaba
Copy link
Contributor Author

http://phoenixwing.com.cn/codes

@zhangyanzhao
Copy link
Collaborator

PRs are not merged, move to backlog

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

@pguibert6WIND pguibert6WIND pguibert6WIND left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
SONiC 202411 Release
Status: MovedToBacklog
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eddieruan-alibaba @zhangyanzhao @pguibert6WIND @zice312963205 @xingrenwai