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

wido · 2024-11-22T15:53:39Z

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.

codecov · 2024-11-22T16:02:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 19.80%. Comparing base (75748a5) to head (65f9514).
⚠️ Report is 130 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9964      +/-   ##
============================================
+ Coverage     19.65%   19.80%   +0.15%     
- Complexity    19792    20023     +231     
============================================
  Files          6368     6371       +3     
  Lines        575107   575954     +847     
  Branches      70370    70521     +151     
============================================
+ Hits         113016   114050    +1034     
+ Misses       449808   449469     -339     
- Partials      12283    12435     +152

Flag	Coverage Δ
uitests	`3.52% <ø> (+0.11%)`	⬆️
unittests	`21.07% <ø> (+0.15%)`	⬆️

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 · 2024-11-24T15:39:24Z

thanks @wido !

weizhouapache · 2024-11-24T15:39:31Z

@blueorangutan package

blueorangutan · 2024-11-24T15:40:04Z

@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 · 2024-11-24T16:47:43Z

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

weizhouapache · 2024-11-25T07:51:45Z

@blueorangutan test ubuntu24 kvm-ubuntu24

blueorangutan · 2024-11-25T07:54:04Z

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

blueorangutan · 2024-11-25T23:28:50Z

[SF] Trillian test result (tid-11794)
Environment: kvm-ubuntu24 (x2), Advanced Networking with Mgmt server u24
Total time taken: 54170 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9964-t11794-kvm-ubuntu24.zip
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 · 2024-11-30T07:57:10Z

@blueorangutan package

blueorangutan · 2024-11-30T07:58:03Z

@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 · 2024-11-30T08:44:57Z

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

weizhouapache

code lgtm

need testing

weizhouapache · 2026-07-01T10:05:44Z

@blueorangutan package

blueorangutan · 2026-07-01T10:08:04Z

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2026-07-01T11:02:41Z

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

weizhouapache · 2026-07-01T11:12:25Z

@blueorangutan test

blueorangutan · 2026-07-01T11:14:04Z

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

blueorangutan · 2026-07-02T03:03:46Z

[SF] Trillian test result (tid-16457)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 54106 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9964-t16457-kvm-ol8.zip
Smoke tests completed. 150 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestClusterDRS>:setup	`Error`	0.00	test_cluster_drs.py
test_list_system_vms_metrics_history	`Failure`	1.31	test_metrics_api.py

Copilot

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.

weizhouapache · 2026-07-13T12:11:33Z

without the change

root@r-269-VM:~# cat /etc/frr/frr.conf 
frr version 6.0
frr defaults traditional
hostname r-269-VM
service integrated-vtysh-config
ip nht resolve-via-default
router bgp 10010
 bgp router-id 10.200.0.12
 bgp default ipv6-unicast
 neighbor 10.200.0.1 remote-as 64999
 neighbor 10.200.0.1 password password3
 neighbor fc00:2024:9:7::1 remote-as 64999
 neighbor fc00:2024:9:7::1 password password3
 address-family ipv4 unicast
  network 192.168.120.0/28
 exit-address-family
 address-family ipv6 unicast
  network fd00:2026:2:19::/64
 exit-address-family
line vty
root@r-269-VM:~#

with the change

root@r-269-VM:~# cat /etc/frr/frr.conf 
frr defaults traditional
hostname r-269-VM
service integrated-vtysh-config
ip 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
ipv6 prefix-list default-v6 seq 1 permit ::/0
ip prefix-list local-v4 seq 1 permit 192.168.120.0/28
ipv6 prefix-list local-v6 seq 1 permit fd00:2026:2:19::/64
router bgp 10010
 bgp router-id 10.200.0.12
 bgp default ipv6-unicast
 neighbor 10.200.0.1 remote-as 64999
 neighbor {} route-map upstream-v4-in in
 neighbor {} route-map upstream-v4-out out
 neighbor 10.200.0.1 password password3
 neighbor fc00:2024:9:7::1 remote-as 64999
 neighbor {} route-map upstream-v6-in in
 neighbor {} route-map upstream-v6-out out
 neighbor fc00:2024:9:7::1 password password3
 address-family ipv4 unicast
  network 192.168.120.0/28
 exit-address-family
 address-family ipv6 unicast
  network fd00:2026:2:19::/64
 exit-address-family
route-map upstream-v4-in permit 10
  match ip address prefix-list default-v4
route-map upstream-v4-in deny 1000
  match ip address prefix-list all-v4
route-map upstream-v4-out permit 10
  match ip address prefix-list local-v4
route-map upstream-v4-out deny 1000
  match ip address prefix-list all-v4
route-map upstream-v6-in permit 10
  match ipv6 address prefix-list default-v6
route-map upstream-v6-in deny 1000
  match ipv6 address prefix-list all-v6
route-map upstream-v6-out permit 10
  match ipv6 address prefix-list local-v6
route-map upstream-v6-out deny 1000
  match ipv6 address prefix-list all-v6
line vty

weizhouapache · 2026-07-13T12:13:26Z

@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) ?

wido · 2026-07-14T19:53:04Z

@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 · 2026-07-16T08:48:58Z

@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 · 2026-08-12T16:23:38Z

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

wido · 2026-08-13T04:59:50Z

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.

wido · 2026-09-04T14:08:50Z

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

weizhouapache · 2026-09-04T14:45:41Z

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 · 2026-09-07T11:15:35Z

@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 · 2026-09-10T05:09:25Z

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

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 · 2026-09-10T05:25:21Z

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 added component:virtual-router component:networking labels Nov 22, 2024

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

alexandremattioli approved these changes Dec 3, 2024

View reviewed changes

weizhouapache approved these changes Dec 3, 2024

View reviewed changes

weizhouapache self-assigned this Dec 3, 2024

weizhouapache added the needs-testing label Dec 3, 2024

sureshanaparti added this to the 4.21.0 milestone Jun 5, 2025

sureshanaparti added this to Apache CloudStack 4.21.0 Jun 5, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.21.0 Jul 14, 2025

weizhouapache modified the milestones: 4.21.0, 4.22.0 Aug 29, 2025

sureshanaparti removed this from Apache CloudStack 4.21.0 Aug 29, 2025

sureshanaparti added this to Apache CloudStack 4.22.0 Aug 29, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.22.0 Aug 29, 2025

harikrishna-patnala modified the milestones: 4.22.0, 4.22.1 Nov 12, 2025

harikrishna-patnala removed this from Apache CloudStack 4.22.0 Nov 12, 2025

weizhouapache modified the milestones: 4.24.0, 4.23.0 Jul 1, 2026

weizhouapache requested a review from Copilot July 8, 2026 09:29

Copilot AI reviewed Jul 8, 2026

View reviewed changes

weizhouapache self-requested a review July 13, 2026 12:13

weizhouapache modified the milestones: 4.23.0, 4.24.0 Jul 14, 2026

wido force-pushed the frr-conf-virtual-router branch from 70b6cb4 to d2f1f04 Compare July 14, 2026 19:52

DaanHoogland added this to CloudStack Testing Aug 10, 2026

github-project-automation Bot moved this to Backlog in CloudStack Testing Aug 10, 2026

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

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

Uh oh!

Conversation

wido commented Nov 22, 2024

Uh oh!

codecov Bot commented Nov 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

weizhouapache commented Nov 24, 2024

Uh oh!

weizhouapache commented Nov 24, 2024

Uh oh!

blueorangutan commented Nov 24, 2024

Uh oh!

blueorangutan commented Nov 24, 2024

Uh oh!

weizhouapache commented Nov 25, 2024

Uh oh!

blueorangutan commented Nov 25, 2024

Uh oh!

blueorangutan commented Nov 25, 2024

Uh oh!

weizhouapache commented Nov 30, 2024

Uh oh!

blueorangutan commented Nov 30, 2024

Uh oh!

blueorangutan commented Nov 30, 2024

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Jul 1, 2026

Uh oh!

blueorangutan commented Jul 1, 2026

Uh oh!

blueorangutan commented Jul 1, 2026

Uh oh!

weizhouapache commented Jul 1, 2026

Uh oh!

blueorangutan commented Jul 1, 2026

Uh oh!

blueorangutan commented Jul 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Jul 13, 2026

Uh oh!

weizhouapache commented Jul 13, 2026

Uh oh!

wido commented Jul 14, 2026

Uh oh!

weizhouapache commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nvazquez commented Aug 12, 2026

Uh oh!

wido commented Aug 13, 2026

Uh oh!

wido commented Sep 4, 2026

Uh oh!

weizhouapache commented Sep 4, 2026

Uh oh!

weizhouapache commented Sep 7, 2026

Uh oh!

wido commented Sep 10, 2026

Uh oh!

wido commented Sep 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

codecov Bot commented Nov 22, 2024 •

edited

Loading

weizhouapache commented Jul 16, 2026 •

edited

Loading