tests: add topotest for L3VNI in default VRF#22343
Conversation
Greptile SummaryThis PR adds a new topotest suite (
Confidence Score: 5/5This is an additive test-only change with no modifications to production FRR daemon code; it is safe to merge. All changed files are new test infrastructure (configs, Python test module, documentation). The test logic correctly uses tests/topotests/bgp_evpn_l3vni_default_vrf/test_bgp_evpn_l3vni_default_vrf.py — the debug JSON dumps and weak coexistence assertion are worth cleaning up before merge. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant C1 as Client c1 (192.168.1.x)
participant R1 as R1 (AS 65001) L3VNI 5000
participant Spine as Spine (AS 65000) EVPN RR
participant R2 as R2 (AS 65002) L3VNI 5000
participant R3 as R3 (AS 65003) No EVPN
Note over R1,R2: zebra vni 5000 triggers ZEBRA_L3VNI_ADD to bgpd
R1->>Spine: IPv4 unicast: 10.0.1.1/32 only (UNDERLAY_ONLY filter)
R1->>Spine: L2VPN EVPN RT-5 [5]:[192.168.1.0/24] RD 10.0.1.1:1
Spine->>R2: L2VPN EVPN RT-5 [5]:[192.168.1.0/24]
R2->>R2: Import to IPv4 unicast (distance 200, nexthop via vxlan5000)
R3->>Spine: IPv4 unicast: 10.0.3.3/32, 192.168.3.0/24
Spine->>R1: IPv4 unicast: 10.0.3.3/32, 192.168.3.0/24
Note over R1,R3: R3 NOT in L2VPN EVPN AF - no RT-5 routes - isolation enforced
C1->>R1: ping 192.168.3.10 - NO ROUTE (EVPN/BGP domain isolation)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant C1 as Client c1 (192.168.1.x)
participant R1 as R1 (AS 65001) L3VNI 5000
participant Spine as Spine (AS 65000) EVPN RR
participant R2 as R2 (AS 65002) L3VNI 5000
participant R3 as R3 (AS 65003) No EVPN
Note over R1,R2: zebra vni 5000 triggers ZEBRA_L3VNI_ADD to bgpd
R1->>Spine: IPv4 unicast: 10.0.1.1/32 only (UNDERLAY_ONLY filter)
R1->>Spine: L2VPN EVPN RT-5 [5]:[192.168.1.0/24] RD 10.0.1.1:1
Spine->>R2: L2VPN EVPN RT-5 [5]:[192.168.1.0/24]
R2->>R2: Import to IPv4 unicast (distance 200, nexthop via vxlan5000)
R3->>Spine: IPv4 unicast: 10.0.3.3/32, 192.168.3.0/24
Spine->>R1: IPv4 unicast: 10.0.3.3/32, 192.168.3.0/24
Note over R1,R3: R3 NOT in L2VPN EVPN AF - no RT-5 routes - isolation enforced
C1->>R1: ping 192.168.3.10 - NO ROUTE (EVPN/BGP domain isolation)
Reviews (8): Last reviewed commit: "tests: add comprehensive topotest for L3..." | Re-trigger Greptile |
|
Isn't there already support for setting the L3VNI for the default VRF via |
The existing vrf ... vni command (at global config) creates a Linux VRF and associates it with an L3VNI. This PR is different - it enables L3VNI on the default/global VRF (not a custom VRF) for pure Layer-3 EVPN deployments without creating additional VRFs. The new command is vrf DEFAULT-VRF vni inside [router bgp] context, specifically for L3-only EVPN use cases in the global routing table. |
41a7376 to
9a35472
Compare
|
@greptile-apps review |
This is inaccurate. FRR doesn't create Linux netdevs.
This is exactly what the existing
Not only that, but there is already a reserved keyword for the default VRF ( So it's not really clear to me what the new command or syntax does which can't already be accomplished through existing commands. If there's a bug or issue with the existing commands, I would advocate for reporting and fixing them rather than adding new alternatives that do the same thing. |
Hi, Do you have a sample config? I will be interested to see that. |
|
sure. either of these will apply the L3VNI to the default VRF: |
hold on.... will come back to you soon |
|
the vni you enter under |
@taspelund Thank you for the advice. I'm will close this PR or keep it to commit topotest covering the area. |
9a35472 to
b414f98
Compare
|
@greptile-apps review |
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
9bdf7a5 to
46e74ac
Compare
|
@greptile-apps review |
46e74ac to
3e1da40
Compare
|
@greptile-apps review |
3e1da40 to
38f791c
Compare
|
@greptile-apps review |
38f791c to
29fb544
Compare
|
@greptile-apps review |
| def _check_routes_restored(): | ||
| output = r1.vtysh_cmd("show bgp l2vpn evpn route type prefix json", isjson=True) | ||
| return len(output) > 0 |
There was a problem hiding this comment.
Vacuous route-restoration check after L3VNI re-add
len(output) > 0 does not reliably confirm actual routes have returned. FRR can return a metadata-only JSON response such as {"numPrefix": 0, "numPaths": 0} when there are no prefix routes, which evaluates to len == 2 > 0 and makes run_and_expect immediately return True without waiting for real routes to re-propagate. Compare this with _check_r1_rt5_advertisement (line 457) which explicitly guards against this with if not output or len(output) == 0 and then filters out "numPrefix" / "numPaths" metadata keys before looking for a valid RD. The same RD-based check should be applied here.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/topotests/bgp_evpn_l3vni_default_vrf/test_bgp_evpn_l3vni_default_vrf.py
Line: 903-905
Comment:
**Vacuous route-restoration check after L3VNI re-add**
`len(output) > 0` does not reliably confirm actual routes have returned. FRR can return a metadata-only JSON response such as `{"numPrefix": 0, "numPaths": 0}` when there are no prefix routes, which evaluates to `len == 2 > 0` and makes `run_and_expect` immediately return `True` without waiting for real routes to re-propagate. Compare this with `_check_r1_rt5_advertisement` (line 457) which explicitly guards against this with `if not output or len(output) == 0` and then filters out `"numPrefix"` / `"numPaths"` metadata keys before looking for a valid RD. The same RD-based check should be applied here.
How can I resolve this? If you propose a fix, please make it concise.29fb544 to
9508fce
Compare
|
Tick the box to add this pull request to the merge queue (same as
|
Add extensive functional test suite for BGP EVPN L3VNI configured in the default (global) VRF instead of custom VRFs. This validates the 'vrf DEFAULT-VRF vni <vni-id>' CLI command implementation. Test topology: - R1, R2: BGP EVPN routers with L3VNI 5000 in default VRF - R3: Traditional BGP router without EVPN - Spine: eBGP underlay + EVPN route reflector Test coverage (32 tests across 5 phases): Phase 1: Basic Configuration (8 tests) - BGP session establishment - L3VNI configuration and persistence - JSON output validation - Route-target configuration - EVPN address-family setup Phase 2: EVPN Route Advertisement (4 tests) - RT-5 routes with RFC-compliant RD format (router-id:vrf_id) - Validates RD is NOT "0:0" (which breaks multi-router deployments) - Route reception and propagation - EVPN and traditional BGP route coexistence Phase 3: Data Plane (11 tests) - BGP route installation - Zebra RIB validation - Kernel routing table verification - Bidirectional route exchange - Nexthop validation Phase 4: Connectivity (2 tests) - End-to-end ICMP connectivity via VXLAN tunnel - Validates complete data plane functionality Phase 5: Verification (7 tests) - VXLAN interface operational status - VRF isolation (no route leaking) - L3VNI/L2VNI conflict detection - L3VNI removal and re-addition - Invalid VNI range validation - Duplicate L3VNI configuration detection Key validations: - RFC-compliant RD format (10.0.1.1:1, NOT 0:0) - EVPN RT-5 and traditional BGP coexistence in default VRF - Proper VXLAN encapsulation with correct nexthops - Administrative distance 200 for EVPN-imported routes - Extended Community RT:65001:5000 on EVPN routes Signed-off-by: Patrice Brissette <pbrisset@cisco.com>
9508fce to
102b3a2
Compare
|
@greptile-apps review |
|
This PR is ready for final review |
Add comprehensive functional test suite validating L3VNI operation in the
default (global) VRF using zebra's existing 'vni ' command instead
of BGP configuration. This demonstrates that L3VNI functionality already
works in default VRF without requiring new BGP commands.
Default VRF Approach:
Test Topology:
Key Test Coverage (30 tests):
EVPN Isolation - Validates client networks advertised only via EVPN RT-5
Zebra L3VNI Detection - Validates zebra's VNI management
RFC-Compliant RD - Ensures proper multi-router deployments
Data Plane Validation
Configuration Management
Test Results:
Signed-off-by: Patrice Brissette pbrisset@cisco.com