From 20664936afc7f952a51a19ab26daece5105bbee5 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 12 Feb 2025 12:56:49 +0100 Subject: [PATCH 1/6] bgpd: fix default instance name when un-hiding When unconfiguring a default BGP instance with VPN SAFI configurations, the default BGP structure remains but enters a hidden state. Upon reconfiguration, the instance name incorrectly appears as "VIEW ?" instead of "VRF default". And the name_pretty pointer The name_pretty pointer is replaced by another one with the incorrect name. This also leads to a memory leak as the previous pointer is not properly freed. Do not rewrite the instance name. Fixes: 4d0e7a49cf ("bgpd: VRF-Lite fix default bgp delete") Signed-off-by: Louis Scalbert --- bgpd/bgpd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index e3911de28a..7b60c4fc17 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3561,7 +3561,7 @@ peer_init: /* printable name we can use in debug messages */ if (inst_type == BGP_INSTANCE_TYPE_DEFAULT && !hidden) { bgp->name_pretty = XSTRDUP(MTYPE_BGP_NAME, "VRF default"); - } else { + } else if (!hidden) { const char *n; int len; From a2c1956efa399c6770605712d0b53e5f24979acc Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 12 Feb 2025 13:09:37 +0100 Subject: [PATCH 2/6] bgpd: fix process_queue when un-hiding bgp_process_queue_init() is not called in bgp_create() when leaving the BGP instance hidden state because of the following goto: > if (hidden) { > bgp = bgp_old; > goto peer_init; > } Upon reconfiguration of the default instance, the prefixes are never set into a meta queue by mq_add_handler(). They are never processed for zebra RIB installation and announcements of update/withdraw. Do not delete the BGP process_queue when hiding. Fixes: 4d0e7a49cf ("bgpd: VRF-Lite fix default bgp delete") Signed-off-by: Louis Scalbert --- bgpd/bgpd.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 7b60c4fc17..edda82b63c 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -4257,12 +4257,11 @@ int bgp_delete(struct bgp *bgp) bgp_set_evpn(bgp_get_default()); } - if (bgp->process_queue) - work_queue_free_and_null(&bgp->process_queue); - - if (!IS_BGP_INSTANCE_HIDDEN(bgp)) + if (!IS_BGP_INSTANCE_HIDDEN(bgp)) { + if (bgp->process_queue) + work_queue_free_and_null(&bgp->process_queue); bgp_unlock(bgp); /* initial reference */ - else { + } else { for (afi = AFI_IP; afi < AFI_MAX; afi++) { enum vpn_policy_direction dir; From d9d74d33bca0e23a2cd4723fbc4709dc83fd1332 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 14 Feb 2025 18:01:00 +0100 Subject: [PATCH 3/6] Revert "bgpd: fix bgp vrf instance creation from implicit" This reverts commit 2ff08af78e315c69795417d150cd23649f68c655. The fix is obviously wrong. Link: 2ff08af78e315c69795417d150cd23649f68c655 Signed-off-by: Louis Scalbert --- bgpd/bgpd.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index edda82b63c..8a3ae538dc 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3410,6 +3410,17 @@ static struct bgp *bgp_create(as_t *as, const char *name, } bgp = XCALLOC(MTYPE_BGP, sizeof(struct bgp)); + bgp->as = *as; + if (as_pretty) + bgp->as_pretty = XSTRDUP(MTYPE_BGP_NAME, as_pretty); + else + bgp->as_pretty = XSTRDUP(MTYPE_BGP_NAME, asn_asn2asplain(*as)); + + if (asnotation != ASNOTATION_UNDEFINED) { + bgp->asnotation = asnotation; + SET_FLAG(bgp->config, BGP_CONFIG_ASNOTATION); + } else + asn_str2asn_notation(bgp->as_pretty, NULL, &bgp->asnotation); if (BGP_DEBUG(zebra, ZEBRA)) { if (inst_type == BGP_INSTANCE_TYPE_DEFAULT) @@ -3453,18 +3464,6 @@ static struct bgp *bgp_create(as_t *as, const char *name, bgp->peer = list_new(); peer_init: - bgp->as = *as; - if (as_pretty) - bgp->as_pretty = XSTRDUP(MTYPE_BGP_NAME, as_pretty); - else - bgp->as_pretty = XSTRDUP(MTYPE_BGP_NAME, asn_asn2asplain(*as)); - - if (asnotation != ASNOTATION_UNDEFINED) { - bgp->asnotation = asnotation; - SET_FLAG(bgp->config, BGP_CONFIG_ASNOTATION); - } else - asn_str2asn_notation(bgp->as_pretty, NULL, &bgp->asnotation); - bgp->peer->cmp = (int (*)(void *, void *))peer_cmp; bgp->peerhash = hash_create(peer_hash_key_make, peer_hash_same, "BGP Peer Hash"); From 8e04277fff9597412198da123250e1951bfc580d Mon Sep 17 00:00:00 2001 From: Alexander Skorichenko Date: Sat, 1 Feb 2025 01:52:17 +0100 Subject: [PATCH 4/6] bgpd: update AS value of a hidden bgp instance 'import vrf VRF' could define a hidden bgp instance with the default AS_UNSPECIFIED (i.e. = 1) value. When a router bgp AS vrf VRF gets configured later on, replace this AS_UNSPECIFIED setting with a requested value. Fixes: 9680831518 ("bgpd: fix as_pretty mem leaks when un-hiding") Signed-off-by: Alexander Skorichenko Signed-off-by: Louis Scalbert --- bgpd/bgpd.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 8a3ae538dc..d90875b78c 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3404,13 +3404,15 @@ static struct bgp *bgp_create(as_t *as, const char *name, afi_t afi; safi_t safi; - if (hidden) { + if (hidden) bgp = bgp_old; - goto peer_init; - } + else + bgp = XCALLOC(MTYPE_BGP, sizeof(struct bgp)); - bgp = XCALLOC(MTYPE_BGP, sizeof(struct bgp)); bgp->as = *as; + + if (bgp->as_pretty) + XFREE(MTYPE_BGP_NAME, bgp->as_pretty); if (as_pretty) bgp->as_pretty = XSTRDUP(MTYPE_BGP_NAME, as_pretty); else @@ -3422,6 +3424,9 @@ static struct bgp *bgp_create(as_t *as, const char *name, } else asn_str2asn_notation(bgp->as_pretty, NULL, &bgp->asnotation); + if (hidden) + goto peer_init; + if (BGP_DEBUG(zebra, ZEBRA)) { if (inst_type == BGP_INSTANCE_TYPE_DEFAULT) zlog_debug("Creating Default VRF, AS %s", From bb79a6562ffd520329246920597f83ca32e9a468 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 14 Feb 2025 14:07:40 +0100 Subject: [PATCH 5/6] tests: add bgp_l3vpn_hidden topotest Test that leaving the hidden BGP instance state is working. Signed-off-by: Louis Scalbert --- tests/topotests/bgp_l3vpn_hidden/__init__.py | 0 tests/topotests/bgp_l3vpn_hidden/ce1/frr.conf | 52 ++++ .../ce1/show_bgp_ipv4_unicast.json | 24 ++ .../ce1/show_bgp_ipv4_unicast_step1.json | 1 + .../ce1/show_bgp_ipv6_unicast.json | 25 ++ .../ce1/show_bgp_ipv6_unicast_step1.json | 1 + .../ce1/show_bgp_summary.json | 24 ++ tests/topotests/bgp_l3vpn_hidden/pe1/frr.conf | 107 +++++++ .../pe1/show_bgp_ipv4_vpn.json | 29 ++ .../pe1/show_bgp_ipv6_vpn.json | 29 ++ .../pe1/show_bgp_summary.json | 46 +++ tests/topotests/bgp_l3vpn_hidden/rr1/frr.conf | 79 +++++ .../rr1/show_bgp_ipv4_vpn.json | 28 ++ .../rr1/show_bgp_ipv6_vpn.json | 28 ++ .../rr1/show_bgp_summary.json | 24 ++ .../bgp_l3vpn_hidden/test_bgp_l3vpn_hidden.py | 289 ++++++++++++++++++ 16 files changed, 786 insertions(+) create mode 100644 tests/topotests/bgp_l3vpn_hidden/__init__.py create mode 100644 tests/topotests/bgp_l3vpn_hidden/ce1/frr.conf create mode 100644 tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv4_unicast.json create mode 120000 tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv4_unicast_step1.json create mode 100644 tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv6_unicast.json create mode 120000 tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv6_unicast_step1.json create mode 100644 tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_summary.json create mode 100644 tests/topotests/bgp_l3vpn_hidden/pe1/frr.conf create mode 100644 tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_ipv4_vpn.json create mode 100644 tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_ipv6_vpn.json create mode 100644 tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_summary.json create mode 100644 tests/topotests/bgp_l3vpn_hidden/rr1/frr.conf create mode 100644 tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_ipv4_vpn.json create mode 100644 tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_ipv6_vpn.json create mode 100644 tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_summary.json create mode 100644 tests/topotests/bgp_l3vpn_hidden/test_bgp_l3vpn_hidden.py diff --git a/tests/topotests/bgp_l3vpn_hidden/__init__.py b/tests/topotests/bgp_l3vpn_hidden/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_l3vpn_hidden/ce1/frr.conf b/tests/topotests/bgp_l3vpn_hidden/ce1/frr.conf new file mode 100644 index 0000000000..95d84d5e6b --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/ce1/frr.conf @@ -0,0 +1,52 @@ +debug bgp neighbor-events +! +ip prefix-list PLIST-LAN seq 10 permit 172.20.0.0/16 le 24 +! +ipv6 prefix-list PLIST-LAN6 seq 10 permit 2001:db8::/32 le 64 +! +route-map RMAP-LAN permit 10 + match ip address prefix-list PLIST-LAN +exit +! +route-map RMAP-LAN6 permit 10 + match ipv6 address prefix-list PLIST-LAN6 +exit +! +interface eth-lan + ip address 172.20.1.1/24 + ipv6 address 2001:db8:1::ff/64 +exit +! +interface eth-pe1 + ip address 172.16.1.254/24 + ipv6 address 3fff:1::ff/64 +exit +! +router bgp 65501 + bgp router-id 172.16.1.254 + no bgp ebgp-requires-policy + bgp bestpath compare-routerid + neighbor 172.16.1.1 remote-as external + neighbor 172.16.1.1 bfd profile BGP + neighbor 3fff:1::1 remote-as external + neighbor 3fff:1::1 bfd profile BGP + ! + address-family ipv4 unicast + redistribute connected route-map RMAP-LAN + neighbor 172.16.1.1 next-hop-self + no neighbor 3fff:1::1 activate + exit-address-family +! + address-family ipv6 unicast + redistribute connected route-map RMAP-LAN6 + neighbor 3fff:1::1 activate + neighbor 3fff:1::1 next-hop-self + exit-address-family +exit +bfd + profile BGP + transmit-interval 2000 + receive-interval 2000 + exit + ! +exit diff --git a/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv4_unicast.json b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv4_unicast.json new file mode 100644 index 0000000000..e9741a2fbd --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv4_unicast.json @@ -0,0 +1,24 @@ +{ + "vrfName": "default", + "routerId": "172.16.1.254", + "localAS": 65501, + "routes": { + "172.20.1.0/24": [ + { + "valid": true, + "bestpath": true, + "peerId": "(unspec)", + "path": "", + "nexthops": [ + { + "ip": "0.0.0.0", + "hostname": "ce1", + "used": true + } + ] + } + ] + }, + "totalRoutes": 1, + "totalPaths": 1 +} diff --git a/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv4_unicast_step1.json b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv4_unicast_step1.json new file mode 120000 index 0000000000..0d02eacc65 --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv4_unicast_step1.json @@ -0,0 +1 @@ +show_bgp_ipv4_unicast.json \ No newline at end of file diff --git a/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv6_unicast.json b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv6_unicast.json new file mode 100644 index 0000000000..1120d30edd --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv6_unicast.json @@ -0,0 +1,25 @@ +{ + "vrfName": "default", + "routerId": "172.16.1.254", + "localAS": 65501, + "routes": { + "2001:db8:1::/64": [ + { + "valid": true, + "bestpath": true, + "peerId": "(unspec)", + "path": "", + "nexthops": [ + { + "ip": "::", + "hostname": "ce1", + "scope": "global", + "used": true + } + ] + } + ] + }, + "totalRoutes": 1, + "totalPaths": 1 +} diff --git a/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv6_unicast_step1.json b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv6_unicast_step1.json new file mode 120000 index 0000000000..94f8f5bba9 --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_ipv6_unicast_step1.json @@ -0,0 +1 @@ +show_bgp_ipv6_unicast.json \ No newline at end of file diff --git a/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_summary.json b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_summary.json new file mode 100644 index 0000000000..5a0699ed9b --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/ce1/show_bgp_summary.json @@ -0,0 +1,24 @@ +{ + "default": { + "ipv4Unicast": { + "routerId": "172.16.1.254", + "peers": { + "172.16.1.1": { + "hostname": "pe1", + "state": "Established" + } + }, + "totalPeers": 1 + }, + "ipv6Unicast": { + "routerId": "172.16.1.254", + "peers": { + "3fff:1::1": { + "hostname": "pe1", + "state": "Established" + } + }, + "totalPeers": 1 + } + } +} diff --git a/tests/topotests/bgp_l3vpn_hidden/pe1/frr.conf b/tests/topotests/bgp_l3vpn_hidden/pe1/frr.conf new file mode 100644 index 0000000000..95a7262b32 --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/pe1/frr.conf @@ -0,0 +1,107 @@ +mpls label dynamic-block 1000 1048575 +! +interface lo + ip address 192.168.0.1/32 + ipv6 address 3fff::192:168:0:1/128 +! +interface eth-rr1 + ip address 10.0.1.1/24 +! +interface eth-ce1 + ip address 172.16.1.1/24 + ipv6 address 3fff:1::1/64 +! +router bgp 65000 + bgp router-id 192.168.0.1 + no bgp ebgp-requires-policy + no bgp default ipv4-unicast + neighbor 192.168.0.101 remote-as 65000 + neighbor 192.168.0.101 bfd profile BGP + neighbor 192.168.0.101 update-source 192.168.0.1 + neighbor 3fff::192:168:0:101 remote-as 65000 + neighbor 3fff::192:168:0:101 bfd profile BGP + neighbor 3fff::192:168:0:101 update-source 3fff::192:168:0:1 + +! + address-family ipv4 unicast + no neighbor 192.168.0.101 activate + exit-address-family +! + address-family ipv4 vpn + neighbor 192.168.0.101 activate + neighbor 192.168.0.101 soft-reconfiguration inbound + exit-address-family +! + address-family ipv6 vpn + neighbor 3fff::192:168:0:101 activate + neighbor 3fff::192:168:0:101 soft-reconfiguration inbound + exit-address-family +! +router bgp 65000 vrf RED + bgp router-id 192.168.0.1 + no bgp ebgp-requires-policy + bgp bestpath compare-routerid + neighbor 172.16.1.254 remote-as external + neighbor 172.16.1.254 bfd profile BGP + neighbor 3fff:1::ff remote-as external + neighbor 3fff:1::ff bfd profile BGP + ! + address-family ipv4 unicast + label vpn export 100 + rd vpn export 65000:100 + rt vpn both 65000:100 + export vpn + import vpn + neighbor 172.16.1.254 next-hop-self + no neighbor 3fff:1::ff activate + exit-address-family +! + address-family ipv6 unicast + label vpn export 200 + rd vpn export 65000:100 + rt vpn both 65000:100 + export vpn + import vpn + neighbor 3fff:1::ff activate + neighbor 3fff:1::ff next-hop-self + exit-address-family +exit +! +interface lo + ip router isis 1 + isis hello-interval 2 + ipv6 router isis 1 +! +interface eth-rr1 + ip router isis 1 + isis hello-interval 2 + ipv6 router isis 1 +! +router isis 1 + lsp-gen-interval 2 + net 10.0000.0000.0000.0000.0000.0000.0000.0000.0001.00 + metric-style wide + exit +! +mpls ldp + router-id 192.168.0.1 + ! + address-family ipv4 + discovery transport-address 192.168.0.1 + ! + interface eth-rr1 + ! + address-family ipv6 + discovery transport-address 3fff::192:168:0:1 + ! + interface eth-rr1 + ! + ! +! +bfd + profile BGP + transmit-interval 2000 + receive-interval 2000 + exit + ! +exit diff --git a/tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_ipv4_vpn.json b/tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_ipv4_vpn.json new file mode 100644 index 0000000000..d21dd89291 --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_ipv4_vpn.json @@ -0,0 +1,29 @@ +{ + "vrfName": "default", + "routerId": "192.168.0.1", + "localAS": 65000, + "routes": { + "routeDistinguishers": { + "65000:100": { + "172.20.1.0/24": [ + { + "valid": true, + "bestpath": true, + "peerId": "(unspec)", + "path": "65501", + "nhVrfName": "RED", + "nexthops": [ + { + "ip": "172.16.1.254", + "hostname": "pe1", + "used": true + } + ] + } + ] + } + } + }, + "totalRoutes": 1, + "totalPaths": 1 +} diff --git a/tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_ipv6_vpn.json b/tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_ipv6_vpn.json new file mode 100644 index 0000000000..b42656f44f --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_ipv6_vpn.json @@ -0,0 +1,29 @@ +{ + "vrfName": "default", + "routerId": "192.168.0.1", + "localAS": 65000, + "routes": { + "routeDistinguishers": { + "65000:100": { + "2001:db8:1::/64": [ + { + "valid": true, + "bestpath": true, + "peerId": "(unspec)", + "path": "65501", + "nhVrfName": "RED", + "nexthops": [ + { + "ip": "3fff:1::ff", + "hostname": "pe1", + "used": true + } + ] + } + ] + } + } + }, + "totalRoutes": 1, + "totalPaths": 1 +} diff --git a/tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_summary.json b/tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_summary.json new file mode 100644 index 0000000000..b414d2e4ae --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/pe1/show_bgp_summary.json @@ -0,0 +1,46 @@ +{ + "default": { + "ipv4Vpn": { + "routerId": "192.168.0.1", + "peers": { + "192.168.0.101": { + "hostname": "rr1", + "state": "Established" + } + }, + "totalPeers": 1 + }, + "ipv6Vpn": { + "routerId": "192.168.0.1", + "peers": { + "3fff::192:168:0:101": { + "hostname": "rr1", + "state": "Established" + } + }, + "totalPeers": 1 + } + }, + "RED": { + "ipv4Unicast": { + "routerId": "192.168.0.1", + "peers": { + "172.16.1.254": { + "hostname": "ce1", + "state": "Established" + } + }, + "totalPeers": 1 + }, + "ipv6Unicast": { + "routerId": "192.168.0.1", + "peers": { + "3fff:1::ff": { + "hostname": "ce1", + "state": "Established" + } + }, + "totalPeers": 1 + } + } +} diff --git a/tests/topotests/bgp_l3vpn_hidden/rr1/frr.conf b/tests/topotests/bgp_l3vpn_hidden/rr1/frr.conf new file mode 100644 index 0000000000..b25a7a336b --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/rr1/frr.conf @@ -0,0 +1,79 @@ +mpls label dynamic-block 1000 1048575 +! +interface lo + ip address 192.168.0.101/32 + ipv6 address 3fff::192:168:0:101/128 +! +interface eth-pe1 + ip address 10.0.1.101/24 +! +router bgp 65000 + bgp router-id 192.168.0.101 + bgp cluster-id 192.168.0.101 + bgp log-neighbor-changes + no bgp default ipv4-unicast + neighbor PE peer-group + neighbor PE remote-as 65000 + neighbor PE bfd profile BGP + neighbor PE update-source 192.168.0.101 + neighbor PE6 peer-group + neighbor PE6 remote-as 65000 + neighbor PE6 bfd profile BGP + neighbor PE6 update-source 3fff::192:168:0:101 + neighbor 192.168.0.1 peer-group PE + neighbor 3fff::192:168:0:1 peer-group PE6 +! + address-family ipv4 unicast + no neighbor PE activate + exit-address-family +! + address-family ipv4 vpn + neighbor PE activate + neighbor PE route-reflector-client + neighbor PE soft-reconfiguration inbound + exit-address-family +! + address-family ipv6 vpn + neighbor PE6 activate + neighbor PE6 route-reflector-client + neighbor PE6 soft-reconfiguration inbound + exit-address-family +! +! +interface lo + ip router isis 1 + isis hello-interval 2 + ipv6 router isis 1 +! +interface eth-pe1 + ip router isis 1 + isis hello-interval 2 + ipv6 router isis 1 +! +! +router isis 1 + lsp-gen-interval 2 + net 10.0000.0000.0000.0000.0000.0000.0000.0000.0100.00 + metric-style wide + exit +! +mpls ldp + router-id 192.168.0.101 + ! + address-family ipv4 + discovery transport-address 192.168.0.101 + ! + interface eth-pe1 + ! + address-family ipv6 + discovery transport-address 3fff::192:168:0:101 + ! + interface eth-pe1 +! +bfd + profile BGP + transmit-interval 2000 + receive-interval 2000 + exit + ! +exit diff --git a/tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_ipv4_vpn.json b/tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_ipv4_vpn.json new file mode 100644 index 0000000000..3fd451640c --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_ipv4_vpn.json @@ -0,0 +1,28 @@ +{ + "vrfName": "default", + "routerId": "192.168.0.101", + "localAS": 65000, + "routes": { + "routeDistinguishers": { + "65000:100": { + "172.20.1.0/24": [ + { + "valid": true, + "bestpath": true, + "peerId": "192.168.0.1", + "path": "65501", + "nexthops": [ + { + "ip": "192.168.0.1", + "hostname": "pe1", + "used": true + } + ] + } + ] + } + } + }, + "totalRoutes": 1, + "totalPaths": 1 +} diff --git a/tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_ipv6_vpn.json b/tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_ipv6_vpn.json new file mode 100644 index 0000000000..c3ecd71c46 --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_ipv6_vpn.json @@ -0,0 +1,28 @@ +{ + "vrfName": "default", + "routerId": "192.168.0.101", + "localAS": 65000, + "routes": { + "routeDistinguishers": { + "65000:100": { + "2001:db8:1::/64": [ + { + "valid": true, + "bestpath": true, + "peerId": "3fff::192:168:0:1", + "path": "65501", + "nexthops": [ + { + "ip": "3fff::192:168:0:1", + "hostname": "pe1", + "used": true + } + ] + } + ] + } + } + }, + "totalRoutes": 1, + "totalPaths": 1 +} diff --git a/tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_summary.json b/tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_summary.json new file mode 100644 index 0000000000..8d6c15526e --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/rr1/show_bgp_summary.json @@ -0,0 +1,24 @@ +{ + "default": { + "ipv4Vpn": { + "routerId": "192.168.0.101", + "peers": { + "192.168.0.1": { + "hostname": "pe1", + "state": "Established" + } + }, + "totalPeers": 1 + }, + "ipv6Vpn": { + "routerId": "192.168.0.101", + "peers": { + "3fff::192:168:0:1": { + "hostname": "pe1", + "state": "Established" + } + }, + "totalPeers": 1 + } + } +} diff --git a/tests/topotests/bgp_l3vpn_hidden/test_bgp_l3vpn_hidden.py b/tests/topotests/bgp_l3vpn_hidden/test_bgp_l3vpn_hidden.py new file mode 100644 index 0000000000..5ed0bc642d --- /dev/null +++ b/tests/topotests/bgp_l3vpn_hidden/test_bgp_l3vpn_hidden.py @@ -0,0 +1,289 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + + +""" +Test BGP hidden +See https://github.com/FRRouting/frr/commit/4d0e7a49cf8d4311a485281fa50bbff6ee8ca6cc +""" + +import os +import sys +import re +import json +import pytest +import functools + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +from lib import topotest +from lib.topolog import logger +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.common_config import step + + +pytestmark = [pytest.mark.bgpd, pytest.mark.bfdd, pytest.mark.isisd, pytest.mark.ldpd] + + +def build_topo(tgen): + """ + +---+ +---+ +---+ + |ce1|---|pe1|---|rr1| + +---+ +---+ +---+ +""" + + def connect_routers(tgen, left, right): + for rname in [left, right]: + if rname not in tgen.routers().keys(): + tgen.add_router(rname) + + switch = tgen.add_switch("s-{}-{}".format(left, right)) + switch.add_link(tgen.gears[left], nodeif="eth-{}".format(right)) + switch.add_link(tgen.gears[right], nodeif="eth-{}".format(left)) + if "ce" not in right and "ce" not in left: + tgen.gears[left].cmd(f"sysctl net.mpls.conf.eth-{right}.input=1") + tgen.gears[right].cmd(f"sysctl net.mpls.conf.eth-{left}.input=1") + + def connect_switchs(tgen, rname, switch): + if rname not in tgen.routers().keys(): + tgen.add_router(rname) + + switch.add_link(tgen.gears[rname], nodeif="eth-{}".format(switch.name)) + + def connect_lan(tgen, rname): + if rname not in tgen.routers().keys(): + tgen.add_router(rname) + + # Extra LAN interfaces. Not used for communication with hosts, just to + # hold an address we use to inject routes + switch = tgen.add_switch("s-{}".format(rname)) + switch.add_link(tgen.gears[rname], nodeif="eth-lan") + + # directly connected without switch routers + connect_routers(tgen, "rr1", "pe1") + connect_routers(tgen, "pe1", "ce1") + connect_lan(tgen, "ce1") + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + pe1 = tgen.gears["pe1"] + pe1.cmd( + f""" +ip link add RED type vrf table 100 +ip link set RED up +ip link set eth-ce1 master RED +""" + ) + + router_list = tgen.routers() + + for _, (rname, router) in enumerate(router_list.items(), 1): + router.load_frr_config( + os.path.join(CWD, "{}/frr.conf".format(rname)), + [ + (TopoRouter.RD_ZEBRA, None), + (TopoRouter.RD_MGMTD, None), + (TopoRouter.RD_BFD, None), + (TopoRouter.RD_LDP, None), + (TopoRouter.RD_ISIS, None), + (TopoRouter.RD_BGP, None), + ], + ) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def check_bgp_convergence(step=None): + """ + out was generated using + +FRRGIT=/path/git/frr + +vtysh -c 'show bgp vrf all summary json' | jq . | egrep -v 'ersion|idType|connections|peerState|pfx|outq|inq|msg|As|rib|Count|Memory|Uptime|vrf|\"as|failedPeers|displayedPeers|dynamicPeers' | awk '/ "bestPath": {/ {c=3; next} c-- > 0 {next} 1' | sed -E 's|"totalPeers": (.+),|"totalPeers": \1|g;s|"Established",|"Established"|g' | jq . >$FRRGIT/tests/topotests/bgp_l3vpn_hidden/$HOSTNAME/show_bgp_summary.json + +vtysh -c 'show bgp ipv4 vpn json' | jq . | egrep -v 'selectionReason|pathFrom|prefix|locPrf|ersion|weight|origin|vrfId|afi|defaultLocPrf|network|nhVrfId|announceNexthopSelf|metric|multipath|linkLocalOnly|length' | jq . >$FRRGIT/tests/topotests/bgp_l3vpn_hidden/$HOSTNAME/show_bgp_ipv4_vpn_step1.json +vtysh -c 'show bgp ipv6 vpn json' | jq . | egrep -v 'selectionReason|pathFrom|prefix|locPrf|ersion|weight|origin|vrfId|afi|defaultLocPrf|network|fe80|nhVrfId|announceNexthopSelf|metric|multipath|linkLocalOnly|length' | jq . >$FRRGIT/tests/topotests/bgp_l3vpn_hidden/$HOSTNAME/show_bgp_ipv6_vpn_step1.json + +vtysh -c 'show bgp ipv4 unicast json' | jq . | egrep -v 'selectionReason|pathFrom|prefix|locPrf|ersion|weight|origin|vrfId|afi|defaultLocPrf|network|nhVrfId|announceNexthopSelf|metric|multipath|linkLocalOnly|length' | jq . >$FRRGIT/tests/topotests/bgp_l3vpn_hidden/$HOSTNAME/show_bgp_ipv4_unicast.json +vtysh -c 'show bgp ipv6 unicast json' | jq . | egrep -v 'selectionReason|pathFrom|prefix|locPrf|ersion|weight|origin|vrfId|afi|defaultLocPrf|network|fe80|nhVrfId|announceNexthopSelf|metric|multipath|linkLocalOnly|length' | jq . >$FRRGIT/tests/topotests/bgp_l3vpn_hidden/$HOSTNAME/show_bgp_ipv6_unicast.json + """ + tgen = get_topogen() + + logger.info("waiting for bgp convergence") + + step_suffix = f"_step{step}" if step else "" + + if not step: + logger.info("Check BGP summary") + for rname, router in tgen.routers().items(): + reffile = os.path.join(CWD, f"{rname}/show_bgp_summary.json") + expected = json.loads(open(reffile).read()) + cmd = "show bgp vrf all summary json" + test_func = functools.partial( + topotest.router_json_cmp, router, cmd, expected + ) + _, res = topotest.run_and_expect(test_func, None, count=60, wait=1) + assertmsg = f"BGP did not converge. Error on {rname} {cmd}" + assert res is None, assertmsg + + logger.info("Check BGP IPv4/6 unicast/VPN table") + for rname, router in tgen.routers().items(): + for ipv in [4, 6]: + logger.info(f"Check BGP IPv4/6 unicast/VPN table: {rname} IPv{ipv}") + safi = "unicast" if "ce" in rname else "vpn" + reffile = os.path.join( + CWD, f"{rname}/show_bgp_ipv{ipv}_{safi}{step_suffix}.json" + ) + expected = json.loads(open(reffile).read()) + exact = not expected # exact match if json is void (ie. {}) + cmd = f"show bgp ipv{ipv} {safi} json" + test_func = functools.partial( + topotest.router_json_cmp, + router, + cmd, + expected, + exact=exact, + ) + _, res = topotest.run_and_expect(test_func, None, count=120, wait=1) + assertmsg = f"BGP did not converge. Error on {rname} {cmd}" + assert res is None, assertmsg + + +def configure_bgp(vrf=None, router_name="all", activate=False): + tgen = get_topogen() + + vrf_suffix = f" vrf {vrf}" if vrf else "" + as_pattern = re.compile(rf"^router bgp (\d+){vrf_suffix}$") + + for rname, router in tgen.routers().items(): + if router_name != "all" and router_name != rname: + continue + + if "ce" in rname: + continue + + as_number = "" + cmds = [] + router_bgp = False + with open(os.path.join(CWD, f"{rname}/frr.conf"), "r") as f: + for line in f: + line = line.strip() + if "router bgp" in line: + match = as_pattern.match(line) + if match: + as_number = match.group(1) + router_bgp = True + continue + if router_bgp: + # If we already hit "router bgp " once, + # and see another "router bgp" line, break. + break + if not router_bgp: + # Only capture lines after we've matched "router bgp" + continue + cmds.append(line) + + cfg = "configure terminal\n" + if activate: + cfg += f"router bgp {as_number}{vrf_suffix}\n" + for cmd in cmds: + cfg += f"{cmd}\n" + else: + cfg += f"no router bgp {as_number}{vrf_suffix}\n" + + router.vtysh_cmd(cfg) + + +def test_bgp_convergence(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + check_bgp_convergence() + + +def test_bgp_l3vpn_hidden_step1(): + """ + Remove pe1 router bgp blocks + The Default BGP instance becomes hidden + """ + + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + for vrf in ["RED", None]: + configure_bgp(router_name="pe1", vrf=vrf, activate=False) + + check_bgp_convergence(step=1) + + +def test_bgp_l3vpn_hidden_step2(): + """ + Restore pe1 router bgp blocks + """ + + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + for vrf in [None, "RED"]: + configure_bgp(router_name="pe1", vrf=vrf, activate=True) + + # identical to the intitial step + check_bgp_convergence(step=None) + + + +def test_bgp_l3vpn_hidden_step3(): + """ + Remove pe1 router bgp blocks + The Default BGP instance becomes hidden + """ + + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + for vrf in ["RED", None]: + configure_bgp(router_name="pe1", vrf=vrf, activate=False) + + # identical to the intitial step 1 + check_bgp_convergence(step=1) + + +def test_bgp_l3vpn_hidden_step4(): + """ + Restore pe1 router bgp blocks + Reconfigure VRF block first + """ + + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + for vrf in [None, "RED"]: + configure_bgp(router_name="pe1", vrf=vrf, activate=True) + + # identical to the intitial step + check_bgp_convergence(step=None) + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) From 85c5598bb95aa2eb17e8f617965affa7de627c69 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 14 Feb 2025 11:58:24 +0100 Subject: [PATCH 6/6] tests: check as number in show run Creates the default VRF instance after the other VRF instances. The default VRF instance is created in hidden state. Check that AS number in show run is correctly written. Signed-off-by: Louis Scalbert --- .../bgp_vrf_route_leak_basic/r1/bgpd.conf | 14 +++++++------- .../test_bgp-vrf-route-leak-basic.py | 10 ++++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf b/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf index 397f7938d2..8363e2bc99 100644 --- a/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf +++ b/tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf @@ -1,12 +1,5 @@ hostname r1 -router bgp 99 - no bgp ebgp-requires-policy - address-family ipv4 unicast - redistribute connected - import vrf DONNA - ! -! router bgp 99 vrf DONNA no bgp ebgp-requires-policy address-family ipv4 unicast @@ -31,3 +24,10 @@ router bgp 99 vrf ZITA network 172.16.101.0/24 ! ! +router bgp 99 + no bgp ebgp-requires-policy + address-family ipv4 unicast + redistribute connected + import vrf DONNA + ! +! diff --git a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py index 6d4b436bcc..61c62cafa6 100644 --- a/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py +++ b/tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py @@ -64,6 +64,16 @@ def teardown_module(mod): tgen.stop_topology() +def test_router_bgp_as_pretty(): + tgen = get_topogen() + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + output = tgen.gears["r1"].vtysh_cmd("show run") + assert "router bgp 99\n" in output, "router bgp 99 not found in show run" + + def test_vrf_route_leak_donna(): logger.info("Ensure that routes are leaked back and forth") tgen = get_topogen()