-
Notifications
You must be signed in to change notification settings - Fork 197
Frr phase2: multiple backup nhs in nhg #5007
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
base: main
Are you sure you want to change the base?
Conversation
|
/fptest virtual |
Summary of ChangesHello @dracazacula, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request extends an existing FRR test case by adding a new IP address and its corresponding next-hop configuration. The changes are mostly straightforward, but one modification from t.Fatal to t.Errorf introduces a critical bug that could cause a panic. I've provided a suggestion to fix this. Additionally, I've pointed out the use of a new magic number and recommended using a named constant for better maintainability, in line with the project's style guide.
feature/gribi/otg_tests/encap_frr_with_reencap_vrf_test/encap_frr_with_reencap_vrf_test.go
Outdated
Show resolved
Hide resolved
feature/gribi/otg_tests/encap_frr_with_reencap_vrf_test/encap_frr_with_reencap_vrf_test.go
Show resolved
Hide resolved
Pull Request Test Coverage Report for Build 21287586741Details
💛 - Coveralls |
36051a7 to
45a5ab0
Compare
|
/fptest virtual |
4f73062 to
655033a
Compare
a0d333f to
d4cd97a
Compare
d4cd97a to
bd781ec
Compare
|
|
||
| - vRX | ||
|
|
||
| ## Canonical OC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was completely missing. In the interest of time, I will not spend a lot of time to build it.
FRR Phase 2 introduces re-encapsulation as the primary repair mechanism. Instead of simply decapsulating, devices implementing FRR Phase 2 can steer affected traffic into alternative, pre-established or dynamically determined TE tunnels, known as backup tunnels. This allows the traffic to remain within the TE domain, preserving pathing optimizations, congestion mitigation, and policy enforcement.
A key capability within FRR Phase 2 is the ability to re-encapsulate traffic not just into a single backup tunnel, but to load-balance across multiple backup tunnels. This multi-tunnel re-encapsulation is critical for:
Effective Load Balancing: During FRR, putting all repaired traffic to a single backup tunnel can overwhelm that path. Supporting multiple next-hops in the backup NHG allows for hardware-based load balancing (ECMP/WCMP) across the diverse repair tunnels, making far better use of available network capacity.