weizhouapache commented on PR #9964:
URL: https://github.com/apache/cloudstack/pull/9964#issuecomment-5569798660
@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",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
