FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

virtual router: Add route-maps to BGP peers for Routed Mode by wido · Pull Request #9964 · apache/cloudstack · GitHub

  1. apache
  2. cloudstack

Repository navigation

virtual router: Add route-maps to BGP peers for Routed Mode - #9964

Merged
weizhouapache merged 2 commits into
apache:mainfrom
wido:frr-conf-virtual-router
Oct 7, 2026
ConversationCommits (2)ChecksFiles changed
Merged

virtual router: Add route-maps to BGP peers for Routed Mode#9964
weizhouapache merged 2 commits into
apache:mainfrom
wido:frr-conf-virtual-router

Conversation

wido commented Nov 22, 2024

Copy link
Copy Markdown
Contributor

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.

wido requested a review from weizhouapache November 22, 2024 15:53
boring-cyborg Bot added the Python Warning... Python code Ahead! label Nov 22, 2024

codecov Bot commented Nov 22, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 19.91%. Comparing base (7ed0cec) to head (0ad8689).

Additional details and impacted files 
@@            Coverage Diff            @@
##               main    #9964   +/-   ##
=========================================
  Coverage     19.91%   19.91%           
- Complexity    20198    20199    +1     
=========================================
  Files          6373     6373           
  Lines        577230   577230           
  Branches      70696    70696           
=========================================
+ Hits         114936   114952   +16     
+ Misses       449736   449712   -24     
- Partials      12558    12566    +8
Flag Coverage Δ
uitests 3.71% <ø> (ø)
unittests 21.18% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

weizhouapache commented Nov 24, 2024

Copy link
Copy Markdown
Member

thanks @wido !

weizhouapache commented Nov 24, 2024

Copy link
Copy Markdown
Member

@blueorangutan package

blueorangutan commented Nov 24, 2024

Copy link
Copy Markdown

@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.

blueorangutan commented Nov 24, 2024

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11599

weizhouapache commented Nov 25, 2024

Copy link
Copy Markdown
Member

@blueorangutan test ubuntu24 kvm-ubuntu24

blueorangutan commented Nov 25, 2024

Copy link
Copy Markdown

@weizhouapache a [SL] Trillian-Jenkins test job (ubuntu24 mgmt + kvm-ubuntu24) has been kicked to run smoke tests

blueorangutan commented Nov 25, 2024 •
edited
Loading

Copy link
Copy Markdown

[SF] Trillian test result (tid-11794)
Environment: kvm-ubuntu24 (x2), Advanced Networking with Mgmt server u24
Total time taken: 54170 seconds
Marvin logs: [archive could not be removed: not found]
Smoke tests completed. 139 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_oobm_background_powerstate_sync Failure 20.83 test_outofbandmanagement.py
test_oobm_enabledisable_across_clusterzones Error 38.04 test_outofbandmanagement.py
test_oobm_issue_power_cycle Error 19.81 test_outofbandmanagement.py
test_oobm_issue_power_off Error 19.77 test_outofbandmanagement.py
test_oobm_issue_power_on Error 19.80 test_outofbandmanagement.py
test_oobm_issue_power_reset Error 19.84 test_outofbandmanagement.py
test_oobm_issue_power_soft Error 19.80 test_outofbandmanagement.py
test_oobm_issue_power_status Error 18.79 test_outofbandmanagement.py
test_oobm_multiple_mgmt_server_ownership Failure 28.12 test_outofbandmanagement.py
test_oobm_zchange_password Error 7.45 test_outofbandmanagement.py
test_hostha_kvm_host_degraded Error 12.65 test_hostha_kvm.py
test_hostha_kvm_host_fencing Error 10.14 test_hostha_kvm.py
test_hostha_kvm_host_recovering Error 10.41 test_hostha_kvm.py

weizhouapache commented Nov 30, 2024

Copy link
Copy Markdown
Member

@blueorangutan package

blueorangutan commented Nov 30, 2024

Copy link
Copy Markdown

@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.

blueorangutan commented Nov 30, 2024

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11666

weizhouapache approved these changes Dec 3, 2024
View reviewed changes

weizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

code lgtm

need testing

weizhouapache self-assigned this Dec 3, 2024
sureshanaparti added this to the 4.21.0 milestone Jun 5, 2025
weizhouapache modified the milestones: 4.21.0, 4.22.0 Aug 29, 2025
harikrishna-patnala modified the milestones: 4.22.0, 4.22.1 Nov 12, 2025

wido commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

@wido the configuration looks ok, however. the VMs cannot reach the internet, even the default egress policy is Allow. I need more time to figure out what causes the issue. Would you mind moving this to next release (4.24.0) ?

Thanks for the test! Lets re-run the test and we can push this one for later.

In which env did you test it?

weizhouapache commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

@wido the configuration looks ok, however. the VMs cannot reach the internet, even the default egress policy is Allow. I need more time to figure out what causes the issue. Would you mind moving this to next release (4.24.0) ?

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
I tested on my testing environment with BGP

with the old ipv4 config

r-269-VM# show bgp summary

IPv4 Unicast Summary (VRF default):
BGP router identifier 10.200.0.12, local AS number 10010 vrf-id 0
BGP table version 3
RIB entries 5, using 960 bytes of memory
Peers 1, using 724 KiB of memory

Neighbor        V         AS   MsgRcvd   MsgSent   TblVer  InQ OutQ  Up/Down State/PfxRcd   PfxSnt Desc
10.200.0.1      4      64999         8         8        0    0    0 00:00:05            2        3 N/A

Total number of neighbors 1

with the new ipv4 config

r-269-VM# show bgp summary

IPv4 Unicast Summary (VRF default):
BGP router identifier 10.200.0.12, local AS number 10010 vrf-id 0
BGP table version 1
RIB entries 1, using 192 bytes of memory
Peers 1, using 724 KiB of memory

Neighbor        V         AS   MsgRcvd   MsgSent   TblVer  InQ OutQ  Up/Down State/PfxRcd   PfxSnt Desc
10.200.0.1      4      64999         5         3        0    0    0 00:00:07     (Policy) (Policy) N/A

Total number of neighbors 1
r-269-VM#

my bgp router (10.200.0.1) is also setup with frr. with the old config, show ip route has the line

B>* 192.168.120.0/28 [20/0] via 10.200.0.12, eth1, weight 1, 00:00:00

this is missing with the new configuration

nvazquez commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Hi @wido @weizhouapache is this PR still in progress or is it ready for testing?

wido commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

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.

DaanHoogland moved this from Backlog to Ready in CloudStack Testing Aug 31, 2026

wido commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

I updated this PR to add more tests and verify the FRR.conf looks as expected.

weizhouapache commented Sep 4, 2026

Copy link
Copy Markdown
Member

I updated this PR to add more tests and verify the FRR.conf looks as expected.

@wido
great, I will test it next week

weizhouapache commented Sep 7, 2026

Copy link
Copy Markdown
Member

@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

  • missing IPv6 next-hop tracking
  • route-map binds to the wrong address-family
diff --git a/systemvm/debian/opt/cloud/bin/cs/CsBgpPeers.py b/systemvm/debian/opt/cloud/bin/cs/CsBgpPeers.py
index d90d31cba5f..849685610fd 100755
--- a/systemvm/debian/opt/cloud/bin/cs/CsBgpPeers.py
+++ b/systemvm/debian/opt/cloud/bin/cs/CsBgpPeers.py
@@ -81,6 +81,7 @@ class CsBgpPeers(CsDataBag):
         self.frr_conf.add("hostname {}".format(CsHelper.get_hostname()))
         self.frr_conf.add("service integrated-vtysh-config")
         self.frr_conf.add("ip nht resolve-via-default")
+        self.frr_conf.add("ipv6 nht resolve-via-default")
         return
 
     def _access_list_set(self):
@@ -111,8 +112,6 @@ class CsBgpPeers(CsDataBag):
                 self.frr_conf.add(" bgp default ipv6-unicast")
             for ip4_peer in self.peers[as_number]['ip4_peers']:
                 self.frr_conf.add(" neighbor {} remote-as {}".format(ip4_peer['ip4_address'], ip4_peer['peer_as_number']))
-                self.frr_conf.add(" neighbor {} route-map upstream-v4-in in".format(ip4_peer['ip4_address']))
-                self.frr_conf.add(" neighbor {} route-map upstream-v4-out out".format(ip4_peer['ip4_address']))
                 if 'peer_password' in ip4_peer:
                     self.frr_conf.add(" neighbor {} password {}".format(ip4_peer['ip4_address'], ip4_peer['peer_password']))
                 if 'details' in ip4_peer:
@@ -120,8 +119,6 @@ class CsBgpPeers(CsDataBag):
                         self.frr_conf.add(" neighbor {} ebgp-multihop {}".format(ip4_peer['ip4_address'], ip4_peer['details']['EBGP_MultiHop']))
             for ip6_peer in self.peers[as_number]['ip6_peers']:
                 self.frr_conf.add(" neighbor {} remote-as {}".format(ip6_peer['ip6_address'], ip6_peer['peer_as_number']))
-                self.frr_conf.add(" neighbor {} route-map upstream-v6-in in".format(ip6_peer['ip6_address']))
-                self.frr_conf.add(" neighbor {} route-map upstream-v6-out out".format(ip6_peer['ip6_address']))
                 if 'peer_password' in ip6_peer:
                     self.frr_conf.add(" neighbor {} password {}".format(ip6_peer['ip6_address'], ip6_peer['peer_password']))
                 if 'details' in ip6_peer:
@@ -129,12 +126,18 @@ class CsBgpPeers(CsDataBag):
                         self.frr_conf.add(" neighbor {} ebgp-multihop {}".format(ip6_peer['ip6_address'], ip6_peer['details']['EBGP_MultiHop']))
             if self.peers[as_number]['ip4_peers']:
                 self.frr_conf.add(" address-family ipv4 unicast")
+                for ip4_peer in self.peers[as_number]['ip4_peers']:
+                    self.frr_conf.add("  neighbor {} route-map upstream-v4-in in".format(ip4_peer['ip4_address']))
+                    self.frr_conf.add("  neighbor {} route-map upstream-v4-out out".format(ip4_peer['ip4_address']))
                 ip4_cidrs = set({ip4_peer['guest_ip4_cidr'] for ip4_peer in self.peers[as_number]['ip4_peers']})
                 for ip4_cidr in ip4_cidrs:
                     self.frr_conf.add("  network {}".format(ip4_cidr))
                 self.frr_conf.add(" exit-address-family")
             if self.peers[as_number]['ip6_peers']:
                 self.frr_conf.add(" address-family ipv6 unicast")
+                for ip6_peer in self.peers[as_number]['ip6_peers']:
+                    self.frr_conf.add("  neighbor {} route-map upstream-v6-in in".format(ip6_peer['ip6_address']))
+                    self.frr_conf.add("  neighbor {} route-map upstream-v6-out out".format(ip6_peer['ip6_address']))
                 ip6_cidrs = set({ip6_peer['guest_ip6_cidr'] for ip6_peer in self.peers[as_number]['ip6_peers']})
                 for ip6_cidr in ip6_cidrs:
                     self.frr_conf.add("  network {}".format(ip6_cidr))
diff --git a/systemvm/test/TestCsBgpPeers.py b/systemvm/test/TestCsBgpPeers.py
index 2dddf7db0fd..64179575666 100644
--- a/systemvm/test/TestCsBgpPeers.py
+++ b/systemvm/test/TestCsBgpPeers.py
@@ -128,10 +128,15 @@ class TestCsBgpPeers(unittest.TestCase):
         self.assertIn("router bgp 64512", config)
         self.assertIn(" bgp router-id 100.64.0.10", config)
         self.assertIn(" neighbor 100.64.0.1 remote-as 64496", config)
-        self.assertIn(" neighbor 100.64.0.1 route-map upstream-v4-in in", config)
-        self.assertIn(" neighbor 100.64.0.1 route-map upstream-v4-out out", config)
+        self.assertIn("  neighbor 100.64.0.1 route-map upstream-v4-in in", config)
+        self.assertIn("  neighbor 100.64.0.1 route-map upstream-v4-out out", config)
         self.assertIn("  network 10.1.1.0/24", config)
         self.assertNotIn(" bgp default ipv6-unicast", config)
+        # route-map must be applied inside the address-family block, not at
+        # router-bgp level, otherwise FRR silently binds it to IPv4 unicast
+        # regardless of the neighbor's actual AFI/SAFI.
+        self.assertNotIn(" neighbor 100.64.0.1 route-map upstream-v4-in in", config)
+        self.assertNotIn(" neighbor 100.64.0.1 route-map upstream-v4-out out", config)
 
     def test_process_peers_ip6(self):
         self.csbgppeers._process_dbag_item(self._peer(ip6_address='2001:db8::1',
@@ -141,9 +146,15 @@ class TestCsBgpPeers(unittest.TestCase):
         config = self._frr_conf()
         self.assertIn(" bgp default ipv6-unicast", config)
         self.assertIn(" neighbor 2001:db8::1 remote-as 64496", config)
-        self.assertIn(" neighbor 2001:db8::1 route-map upstream-v6-in in", config)
-        self.assertIn(" neighbor 2001:db8::1 route-map upstream-v6-out out", config)
+        self.assertIn("  neighbor 2001:db8::1 route-map upstream-v6-in in", config)
+        self.assertIn("  neighbor 2001:db8::1 route-map upstream-v6-out out", config)
         self.assertIn("  network 2001:db8:100::/64", config)
+        # Same guard as above, for the IPv6 address-family: FRR treats a
+        # bare "neighbor X route-map Y in/out" (outside an address-family
+        # block) as applying to IPv4 unicast, leaving IPv6 unicast with no
+        # policy at all (FRR then discards all updates by default).
+        self.assertNotIn(" neighbor 2001:db8::1 route-map upstream-v6-in in", config)
+        self.assertNotIn(" neighbor 2001:db8::1 route-map upstream-v6-out out", config)
 
     def test_process_peers_password_and_multihop(self):
         self.csbgppeers._process_dbag_item(self._peer(ip4_address='100.64.0.1',
@@ -197,6 +208,7 @@ class TestCsBgpPeers(unittest.TestCase):
             "hostname r-1001-VM",
             "service integrated-vtysh-config",
             "ip nht resolve-via-default",
+            "ipv6 nht resolve-via-default",
             "ip prefix-list all-v4 seq 1 permit any",
             "ip prefix-list default-v4 seq 1 permit 0.0.0.0/0",
             "ipv6 prefix-list all-v6 seq 1 permit any",
@@ -207,15 +219,15 @@ class TestCsBgpPeers(unittest.TestCase):
             " bgp router-id 100.64.0.10",
             " bgp default ipv6-unicast",
             " neighbor 100.64.0.1 remote-as 64496",
-            " neighbor 100.64.0.1 route-map upstream-v4-in in",
-            " neighbor 100.64.0.1 route-map upstream-v4-out out",
             " neighbor 2001:db8::1 remote-as 64496",
-            " neighbor 2001:db8::1 route-map upstream-v6-in in",
-            " neighbor 2001:db8::1 route-map upstream-v6-out out",
             " address-family ipv4 unicast",
+            "  neighbor 100.64.0.1 route-map upstream-v4-in in",
+            "  neighbor 100.64.0.1 route-map upstream-v4-out out",
             "  network 10.1.1.0/24",
             " exit-address-family",
             " address-family ipv6 unicast",
+            "  neighbor 2001:db8::1 route-map upstream-v6-in in",
+            "  neighbor 2001:db8::1 route-map upstream-v6-out out",
             "  network 2001:db8:100::/64",
             " exit-address-family",
             "route-map upstream-v4-in permit 10",

wido commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

@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

* missing IPv6 next-hop tracking

* route-map binds to the wrong address-family

diff --git a/systemvm/debian/opt/cloud/bin/cs/CsBgpPeers.py b/systemvm/debian/opt/cloud/bin/cs/CsBgpPeers.py
index d90d31cba5f..849685610fd 100755
--- a/systemvm/debian/opt/cloud/bin/cs/CsBgpPeers.py
+++ b/systemvm/debian/opt/cloud/bin/cs/CsBgpPeers.py
@@ -81,6 +81,7 @@ class CsBgpPeers(CsDataBag):
         self.frr_conf.add("hostname {}".format(CsHelper.get_hostname()))
         self.frr_conf.add("service integrated-vtysh-config")
         self.frr_conf.add("ip nht resolve-via-default")
+        self.frr_conf.add("ipv6 nht resolve-via-default")
         return
 
     def _access_list_set(self):
@@ -111,8 +112,6 @@ class CsBgpPeers(CsDataBag):
                 self.frr_conf.add(" bgp default ipv6-unicast")
             for ip4_peer in self.peers[as_number]['ip4_peers']:
                 self.frr_conf.add(" neighbor {} remote-as {}".format(ip4_peer['ip4_address'], ip4_peer['peer_as_number']))
-                self.frr_conf.add(" neighbor {} route-map upstream-v4-in in".format(ip4_peer['ip4_address']))
-                self.frr_conf.add(" neighbor {} route-map upstream-v4-out out".format(ip4_peer['ip4_address']))
                 if 'peer_password' in ip4_peer:
                     self.frr_conf.add(" neighbor {} password {}".format(ip4_peer['ip4_address'], ip4_peer['peer_password']))
                 if 'details' in ip4_peer:
@@ -120,8 +119,6 @@ class CsBgpPeers(CsDataBag):
                         self.frr_conf.add(" neighbor {} ebgp-multihop {}".format(ip4_peer['ip4_address'], ip4_peer['details']['EBGP_MultiHop']))
             for ip6_peer in self.peers[as_number]['ip6_peers']:
                 self.frr_conf.add(" neighbor {} remote-as {}".format(ip6_peer['ip6_address'], ip6_peer['peer_as_number']))
-                self.frr_conf.add(" neighbor {} route-map upstream-v6-in in".format(ip6_peer['ip6_address']))
-                self.frr_conf.add(" neighbor {} route-map upstream-v6-out out".format(ip6_peer['ip6_address']))
                 if 'peer_password' in ip6_peer:
                     self.frr_conf.add(" neighbor {} password {}".format(ip6_peer['ip6_address'], ip6_peer['peer_password']))
                 if 'details' in ip6_peer:
@@ -129,12 +126,18 @@ class CsBgpPeers(CsDataBag):
                         self.frr_conf.add(" neighbor {} ebgp-multihop {}".format(ip6_peer['ip6_address'], ip6_peer['details']['EBGP_MultiHop']))
             if self.peers[as_number]['ip4_peers']:
                 self.frr_conf.add(" address-family ipv4 unicast")
+                for ip4_peer in self.peers[as_number]['ip4_peers']:
+                    self.frr_conf.add("  neighbor {} route-map upstream-v4-in in".format(ip4_peer['ip4_address']))
+                    self.frr_conf.add("  neighbor {} route-map upstream-v4-out out".format(ip4_peer['ip4_address']))
                 ip4_cidrs = set({ip4_peer['guest_ip4_cidr'] for ip4_peer in self.peers[as_number]['ip4_peers']})
                 for ip4_cidr in ip4_cidrs:
                     self.frr_conf.add("  network {}".format(ip4_cidr))
                 self.frr_conf.add(" exit-address-family")
             if self.peers[as_number]['ip6_peers']:
                 self.frr_conf.add(" address-family ipv6 unicast")
+                for ip6_peer in self.peers[as_number]['ip6_peers']:
+                    self.frr_conf.add("  neighbor {} route-map upstream-v6-in in".format(ip6_peer['ip6_address']))
+                    self.frr_conf.add("  neighbor {} route-map upstream-v6-out out".format(ip6_peer['ip6_address']))
                 ip6_cidrs = set({ip6_peer['guest_ip6_cidr'] for ip6_peer in self.peers[as_number]['ip6_peers']})
                 for ip6_cidr in ip6_cidrs:
                     self.frr_conf.add("  network {}".format(ip6_cidr))
diff --git a/systemvm/test/TestCsBgpPeers.py b/systemvm/test/TestCsBgpPeers.py
index 2dddf7db0fd..64179575666 100644
--- a/systemvm/test/TestCsBgpPeers.py
+++ b/systemvm/test/TestCsBgpPeers.py
@@ -128,10 +128,15 @@ class TestCsBgpPeers(unittest.TestCase):
         self.assertIn("router bgp 64512", config)
         self.assertIn(" bgp router-id 100.64.0.10", config)
         self.assertIn(" neighbor 100.64.0.1 remote-as 64496", config)
-        self.assertIn(" neighbor 100.64.0.1 route-map upstream-v4-in in", config)
-        self.assertIn(" neighbor 100.64.0.1 route-map upstream-v4-out out", config)
+        self.assertIn("  neighbor 100.64.0.1 route-map upstream-v4-in in", config)
+        self.assertIn("  neighbor 100.64.0.1 route-map upstream-v4-out out", config)
         self.assertIn("  network 10.1.1.0/24", config)
         self.assertNotIn(" bgp default ipv6-unicast", config)
+        # route-map must be applied inside the address-family block, not at
+        # router-bgp level, otherwise FRR silently binds it to IPv4 unicast
+        # regardless of the neighbor's actual AFI/SAFI.
+        self.assertNotIn(" neighbor 100.64.0.1 route-map upstream-v4-in in", config)
+        self.assertNotIn(" neighbor 100.64.0.1 route-map upstream-v4-out out", config)
 
     def test_process_peers_ip6(self):
         self.csbgppeers._process_dbag_item(self._peer(ip6_address='2001:db8::1',
@@ -141,9 +146,15 @@ class TestCsBgpPeers(unittest.TestCase):
         config = self._frr_conf()
         self.assertIn(" bgp default ipv6-unicast", config)
         self.assertIn(" neighbor 2001:db8::1 remote-as 64496", config)
-        self.assertIn(" neighbor 2001:db8::1 route-map upstream-v6-in in", config)
-        self.assertIn(" neighbor 2001:db8::1 route-map upstream-v6-out out", config)
+        self.assertIn("  neighbor 2001:db8::1 route-map upstream-v6-in in", config)
+        self.assertIn("  neighbor 2001:db8::1 route-map upstream-v6-out out", config)
         self.assertIn("  network 2001:db8:100::/64", config)
+        # Same guard as above, for the IPv6 address-family: FRR treats a
+        # bare "neighbor X route-map Y in/out" (outside an address-family
+        # block) as applying to IPv4 unicast, leaving IPv6 unicast with no
+        # policy at all (FRR then discards all updates by default).
+        self.assertNotIn(" neighbor 2001:db8::1 route-map upstream-v6-in in", config)
+        self.assertNotIn(" neighbor 2001:db8::1 route-map upstream-v6-out out", config)
 
     def test_process_peers_password_and_multihop(self):
         self.csbgppeers._process_dbag_item(self._peer(ip4_address='100.64.0.1',
@@ -197,6 +208,7 @@ class TestCsBgpPeers(unittest.TestCase):
             "hostname r-1001-VM",
             "service integrated-vtysh-config",
             "ip nht resolve-via-default",
+            "ipv6 nht resolve-via-default",
             "ip prefix-list all-v4 seq 1 permit any",
             "ip prefix-list default-v4 seq 1 permit 0.0.0.0/0",
             "ipv6 prefix-list all-v6 seq 1 permit any",
@@ -207,15 +219,15 @@ class TestCsBgpPeers(unittest.TestCase):
             " bgp router-id 100.64.0.10",
             " bgp default ipv6-unicast",
             " neighbor 100.64.0.1 remote-as 64496",
-            " neighbor 100.64.0.1 route-map upstream-v4-in in",
-            " neighbor 100.64.0.1 route-map upstream-v4-out out",
             " neighbor 2001:db8::1 remote-as 64496",
-            " neighbor 2001:db8::1 route-map upstream-v6-in in",
-            " neighbor 2001:db8::1 route-map upstream-v6-out out",
             " address-family ipv4 unicast",
+            "  neighbor 100.64.0.1 route-map upstream-v4-in in",
+            "  neighbor 100.64.0.1 route-map upstream-v4-out out",
             "  network 10.1.1.0/24",
             " exit-address-family",
             " address-family ipv6 unicast",
+            "  neighbor 2001:db8::1 route-map upstream-v6-in in",
+            "  neighbor 2001:db8::1 route-map upstream-v6-out out",
             "  network 2001:db8:100::/64",
             " exit-address-family",
             "route-map upstream-v4-in permit 10",

Thanks for testing! I just want to check: Your upstream is only sending a ::/0 route, right? Not something else?

wido force-pushed the frr-conf-virtual-router branch from b7df9e5 to 65f9514 Compare September 10, 2026 05:23

wido commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

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.

weizhouapache commented Sep 10, 2026

Copy link
Copy Markdown
Member

Thanks for testing! I just want to check: Your upstream is only sending a ::/0 route, right? Not something else?

@wido thanks for the update.

Please find my frr config below:

 address-family ipv6 unicast
  redistribute connected     <==== !!
  neighbor ACS-V6 activate
  neighbor ACS-V6 route-reflector-client
 exit-address-family

weizhouapache reviewed Sep 10, 2026
View reviewed changes
Comment thread systemvm/test/frr-bgppeers-ipv4-ipv6.conf Resolved
wido force-pushed the frr-conf-virtual-router branch from 65f9514 to 1d98c5c Compare September 11, 2026 07:39

wido commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for testing! I just want to check: Your upstream is only sending a ::/0 route, right? Not something else?

@wido thanks for the update.

Please find my frr config below:

 address-family ipv6 unicast
  redistribute connected     <==== !!
  neighbor ACS-V6 activate
  neighbor ACS-V6 route-reflector-client
 exit-address-family

Can you show me the routes you are advertising to the VR?

Show bgp neighbor X advertised-routes

weizhouapache self-requested a review September 11, 2026 07:59

weizhouapache commented Sep 11, 2026

Copy link
Copy Markdown
Member

Thanks for testing! I just want to check: Your upstream is only sending a ::/0 route, right? Not something else?

@wido thanks for the update.
Please find my frr config below:

 address-family ipv6 unicast
  redistribute connected     <==== !!
  neighbor ACS-V6 activate
  neighbor ACS-V6 route-reflector-client
 exit-address-family

Can you show me the routes you are advertising to the VR?

Show bgp neighbor X advertised-routes

@wido
BGP connection works with your latest changes

bgp-router# show bgp neighbor 10.200.0.200 advertised-routes
BGP table version is 44, local router ID is 10.200.0.1, vrf id 0
Default local pref 100, local AS 64999
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

   Network          Next Hop            Metric LocPrf Weight Path
*> fc00:2024:9:7::/64                          <====== eth1 of the bgp-router (IPv4: 10.200.0.1/24)
                    ::                       0         32768 ?
*> fd00:2026:9:1::/64                                      <====== guest network IPv6 cidr
                    ::                                     0 65000 i
*> fd01::/64        ::                       0         32768 ?                   <====== eth0 of the bgp-router   (default interface)

Total number of prefixes 3

wido commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for testing! I just want to check: Your upstream is only sending a ::/0 route, right? Not something else?

@wido thanks for the update.
Please find my frr config below:

 address-family ipv6 unicast
  redistribute connected     <==== !!
  neighbor ACS-V6 activate
  neighbor ACS-V6 route-reflector-client
 exit-address-family

Can you show me the routes you are advertising to the VR?
Show bgp neighbor X advertised-routes

@wido BGP connection works with your latest changes

bgp-router# show bgp neighbor 10.200.0.200 advertised-routes
BGP table version is 44, local router ID is 10.200.0.1, vrf id 0
Default local pref 100, local AS 64999
Status codes:  s suppressed, d damped, h history, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

   Network          Next Hop            Metric LocPrf Weight Path
*> fc00:2024:9:7::/64                          <====== eth1 of the bgp-router (IPv4: 10.200.0.1/24)
                    ::                       0         32768 ?
*> fd00:2026:9:1::/64                                      <====== guest network IPv6 cidr
                    ::                                     0 65000 i
*> fd01::/64        ::                       0         32768 ?                   <====== eth0 of the bgp-router   (default interface)

Total number of prefixes 3

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.

weizhouapache commented Sep 28, 2026

Copy link
Copy Markdown
Member

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.

@wido
my router is just for testing, I do not have real IPv6 subnets.

any more work on this PR ? I think it is ready for merge, right ?

wido commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

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.

@wido my router is just for testing, I do not have real IPv6 subnets.

any more work on this PR ? I think it is ready for merge, right ?

Got it! But yes, this is ready for merge. In all real environments only a default route (::/0) should be received by the VR.

virtual router: Add route-maps to BGP peers for Routed Mode
f24af4b 
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.
weizhouapache force-pushed the frr-conf-virtual-router branch from 1d98c5c to f24af4b Compare October 7, 2026 07:41
weizhouapache approved these changes Oct 7, 2026
View reviewed changes

weizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm
verified OK

let's wait GHA to finish and then merge

systemvm: ignore comment lines in BGP peer test configuration
0ad8689 
- Filter comment lines (starting with #) when comparing generated vs expected FRR config
- Apply filtering to both generated config and expected config file
- Allows keeping license headers in test fixtures while ignoring them in tests
- All 11 BGP peer tests pass
weizhouapache merged commit e6cd123 into apache:main Oct 7, 2026
43 of 45 checks passed
github-actions Bot mentioned this pull request Oct 11, 2026
Closed
ribafish added a commit to ribafish/apache-cloudstack that referenced this pull request Oct 12, 2026
Merge remote-tracking branch 'origin/main' into gk/develocityIntegration
fc7b58a 
* origin/main: (156 commits)
  Upgrade Netris SDK version to 1.1.0 since 24.0 (apache#14365)
  feature: instance boot group and readiness rules (apache#13907)
  dont show unathorized notification for prelogin (apache#14054)
  .github: Upgrade docker/login-action to fix ASF Allowlist failure (apache#14276)
  virtual router: Add route-maps to BGP peers for Routed Mode (apache#9964)
  Add pre-commit hook to validate dependabot.yml (apache#14107)
  Add the ASF Allowlist check workflow (apache#14113)
  Revert "VMware to KVM: cold (VDDK) and warm (CBT) migration into Ceph/RBD and Linstor, plus importVm adoption of existing RBD root volumes and Linstor root/data volumes (apache#13656)"
  VMware to KVM: cold (VDDK) and warm (CBT) migration into Ceph/RBD and Linstor, plus importVm adoption of existing RBD root volumes and Linstor root/data volumes (apache#13656)
  engine-schema: upgrade path for 24.0.0 (apache#14033)
  xenserver/xcpng: do not bypass secondary storage when copy volumes between pools (apache#13750)
  backport: Incremental NAS backup support for KVM (apache#13074) (apache#13877)
  storage: KVM - enable RBD/Ceph volume encryption support (apache#13556)
  backup: remove the powered-off precondition for all backup providers (apache#14094)
  Nas backup: Fix mount/unmount error handling and timeout in LibvirtRestoreBackupCommandWrapper (apache#14006)
  kvm: apply rbd_default_data_pool when creating volumes from templates on RBD (apache#13361)
  server: copy template details to vm instance details when import VM (apache#12793)
  marvin: use pycryptodome instead of PyCrypt (apache#13594)
  Packaging: accept normalized Marvin archive names (apache#13793)
  SharedFS: fix NPE when accessing systemvm templates for unsupported hypervisors (apache#13930)
  ...
MitchDrage mentioned this pull request Nov 12, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
weizhouapache weizhouapache approved these changes
+1 more reviewer
alexandremattioli alexandremattioli approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

weizhouapache

Labels

component:agent component:api component:build component:integration component:networking component:virtual-router Python Warning... Python code Ahead! status:needs-testing

Projects

CloudStack Testing
Status: Done

Milestone

24.0

Development

Successfully merging this pull request may close these issues.

10 participants

Back | FazBrowse Home | New Git URL