Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9964 +/- ##
============================================
+ Coverage 19.65% 19.89% +0.24%
- Complexity 19792 20138 +346
============================================
Files 6368 6371 +3
Lines 575107 576829 +1722
Branches 70370 70627 +257
============================================
+ Hits 113016 114754 +1738
+ Misses 449808 449530 -278
- Partials 12283 12545 +262
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
thanks @wido ! |
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11599 |
|
@blueorangutan test ubuntu24 kvm-ubuntu24 |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ubuntu24 mgmt + kvm-ubuntu24) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-11794)
|
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11666 |
|
[SF] Trillian test result (tid-16457)
|
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
without the change with the change |
|
@wido |
70b6cb4 to
d2f1f04
Compare
Thanks for the test! Lets re-run the test and we can push this one for later. In which env did you test it? |
@wido with the old ipv4 config with the new ipv4 config my bgp router (10.200.0.1) is also setup with frr. with the old config, this is missing with the new configuration |
|
Hi @wido @weizhouapache is this PR still in progress or is it ready for testing? |
I had Claude Fable take a look and it came back with a few small changes, nothing major. I also added tests. @weizhouapache In your tests, did you make sure the upstream router only sends 0.0.0.0/0 or ::/0 as routes? Anything else will be rejected by the VR. |
|
I updated this PR to add more tests and verify the FRR.conf looks as expected. |
@wido |
|
Tested this against a VR, IPv4 worked but ipv6 did not (same upstream router configured via FRR). I have validated the following changes, which fixes two issues
|
Thanks for testing! I just want to check: Your upstream is only sending a ::/0 route, right? Not something else? |
b7df9e5 to
65f9514
Compare
|
I updated the test to make sure we have a real frr.conf in systemvm/test which the Python code tests against. This should be easier to read and understand what the output should be. |
@wido thanks for the update. Please find my frr config below: |
It is best practice, and mandatory in newer version of FRR, that route-maps should be applied to BGP peers. This is to prevent that mistakes can propogate through a network and cause outages. This change changes the route-maps where the VR will only accept IPv4 and IPv4 default gateways (0.0.0.0/0 and ::/0) to be sent by the upstream router to the VR. The other way around this change makes sure that FRR will not allow announcing anything else than the locally defined subnets to the upstream BGP router.
65f9514 to
1d98c5c
Compare
Can you show me the routes you are advertising to the VR? Show bgp neighbor X advertised-routes |
@wido |
Does it work? Because the route-map should only allow for a default route ::/0 to be advertised to the VR. You are sending non default routes. That’s a use-case which you won’t see in most cases. Why aren’t you sending a default route? In the VR, you will see these routes are not accepted. |
It is best practice, and mandatory in newer version of FRR, that route-maps should be applied to BGP peers. This is to prevent that mistakes can propogate through a network and cause outages.
This change changes the route-maps where the VR will only accept IPv4 and IPv4 default gateways (0.0.0.0/0 and ::/0) to be sent by the upstream router to the VR.
The other way around this change makes sure that FRR will not allow announcing anything else than the locally defined subnets to the upstream BGP router.