tests: Check PIM Register/-Stop handling in pim_igmp_vrf topotest

[gromit1811]

Check whether PIM Register messages are generated towards the RP and answered by Register-Stop from the RP. To force generation of PIM Register, move the DR role from the RP on R11/12 to R1 (otherwise, with DR and RP being the same routers, no Register messages would be needed). The RPs need to generate Register-Stop because there are no other group members besides the ones on the path whether the traffic is received from on the RPs.

Useful as a regression test for #18216 even though the situation there was slightly different: In that case, the RP was the VRF router and its Register-Stop messages ended up the in the wrong VRF. In our case, the DRs are the VRF routers and the Register messages would end up in the wrong VRF. But our check works for both cases: If we don't receive Register-Stop messages, either the Register or the Register-Stop messages got lost and will trigger an assertion failure.

Note: The last commit is the actual change to check PIM Register. The other commits are just minor cleanup/bugfix changes to the original topotest.

[Jafaral]

style suggestions:
https://github.com/FRRouting/frr/pull/18329/checks?check_run_id=38321233157

[gromit1811]

Converted to draft while working on IPv6 support

[gromit1811]

Added IPv6 support. Open issues, to be fixed before removing "draft" status:

• Lots of duplicated code, refactor
• Rebase to current master
• Register/Register-Stop check sometimes fails with IPv6. Looks like a pim6d issue rather than a test case bug. Analyze
• We need explicit vrf red and vrf blue statements in r1/pim*d.conf to avoid the following assertion failure: 2025/03/10 17:02:12 PIM: lib/routing_nb_config.c:51: routing_control_plane_protocols_control_plane_protocol_create(): assertion (vrf) failed. An assertion failure is never the correct way to handle config errors, check what the actual problem is.

[Jafaral]

Thank you @gromit1811

[Jafaral]

Some minor comments. Sorry, for the incremental review, finding time here and there!

[Jafaral]

new format suggestion as well.

[Jafaral]

@gromit1811 if you rebase , you no longer need to do the funny vrf business. Fixed on master.

[gromit1811]

Rebased to get the vrf fix.

Added yet another test case to check interface/router removal. PIM interface removal seems to work properly, but completely disabling PIM (e.g. causing the pimreg interface to disappear) doesn't. Also, "no router pim vrf XXX" doesn't actually remove the PIM section from config, "show run" afterwards still shows it.

I guess that's not expected behavior, right? I'll open an issue for that unless somebody tells me it's OK like this.

[Jafaral]

Rebased to get the vrf fix.

Added yet another test case to check interface/router removal. PIM interface removal seems to work properly, but completely disabling PIM (e.g. causing the pimreg interface to disappear) doesn't. Also, "no router pim vrf XXX" doesn't actually remove the PIM section from config, "show run" afterwards still shows it.

I guess that's not expected behavior, right? I'll open an issue for that unless somebody tells me it's OK like this.

Yeah, that should probably be fixed.

[github-actions]

This PR is stale because it has been open 180 days with no activity. Comment or remove the autoclose label in order to avoid having this PR closed.

[gromit1811]

No autoclose please, #18445 is still open, so this should remain as well

[gromit1811]

Rebased, updated to cope with new OSPFv3 JSON route format, dropped empty VRF config to work around assertion failure (no longer needed with current master), removed new cleanup test cases (will open a separate PR so we can finally merge this one).
This now passes with latest master thanks to f41e730.

[donaldsharp]

Once this passes CI I will get this in.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR significantly enhances the pim_igmp_vrf topotest by adding IPv6 support, implementing PIM Register/-Stop validation, and consolidating configuration files. The changes serve as a regression test for PR #18216 which fixed VRF routing issues with PIM Register messages.

Key Changes:

• IPv6 Support: Adds complete OSPFv3 and PIM6 testing alongside existing IPv4 tests, including MLD support
• PIM Register/-Stop Validation: Verifies that PIM Register messages are generated towards the RP and Register-Stop messages are received back by checking interface traffic statistics (registerTx and registerStopRx)
• DR Role Configuration: Sets R1 as Designated Router (DR priority 100) while RPs R11/R12 have lower priority (0), forcing Register message generation since DR and RP are now different routers
• Config Consolidation: Migrates from separate zebra.conf, ospfd.conf, pimd.conf files to unified frr.conf files for all routers
• Code Refactoring: Eliminates ~340 lines of duplicated code by merging IPv4/IPv6 and red/blue VRF test functions into parameterized helpers
• Test Reliability: Changes join/stream start order (join first, then stream after 1s delay) to improve test reproducibility and catch race conditions like pimd/pim6d: Sometimes doesn't send PIM Register towards RP #18445
• Link-Local Canonicalization: Implements proper handling of dynamic IPv6 link-local addresses in test expectations

Issues Found:

• One typo on line 406: "IPv&" instead of "IPv6" in the docstring

Confidence Score: 4/5

• Safe to merge after fixing the minor typo on line 406
• The PR is well-structured with systematic refactoring across 9 commits. The only issue is a trivial typo ("IPv&" vs "IPv6"). The logic for PIM Register/-Stop validation is sound, DR priority configuration is correct, and the test improvements enhance reliability. The code follows established patterns in the FRR test suite.
• Only tests/topotests/pim_igmp_vrf/test_pim_vrf.py needs attention to fix the typo on line 406

Important Files Changed

Filename	Overview
tests/topotests/pim_igmp_vrf/test_pim_vrf.py	Major refactoring adds IPv6 support, PIM Register/-Stop validation, unified config handling, and improves test reliability; contains one typo on line 406
tests/topotests/pim_igmp_vrf/r1/frr.conf	New unified FRR config consolidating zebra/ospf/pim configs with proper VRF, IPv4/IPv6, and DR priority settings
tests/topotests/pim_igmp_vrf/r11/frr.conf	New unified FRR config for RP router with lower DR priority (0) to force R1 as DR
tests/topotests/pim_igmp_vrf/r12/frr.conf	New unified FRR config for RP router with lower DR priority (0) to force R1 as DR

Sequence Diagram

sequenceDiagram
    participant H2 as H2 (Source)
    participant R1 as R1 (DR/VRF Router)
    participant R11 as R11/R12 (RP)
    participant H1 as H1 (Receiver)
    
    Note over H1,R1: Phase 1: Join Group
    H1->>R1: IGMP/MLD Join (239.100.0.1/ff18:100::1)
    R1->>R11: PIM Join (*,G) towards RP
    
    Note over H2,R11: Phase 2: Multicast Stream Start
    H2->>R1: Multicast data (239.100.0.1/ff18:100::1)
    Note over R1: R1 is DR (priority 100)<br/>RP has DR priority 0
    
    Note over R1,R11: Phase 3: PIM Register/Register-Stop
    R1->>R11: PIM Register (encapsulated MC data)
    Note over R11: RP has no downstream<br/>receivers besides path to H1
    R11->>R1: PIM Register-Stop
    
    Note over R1,R11: Test validates Register-Stop received
    Note over R1: Checks registerTx > 0<br/>and registerStopRx > 0
    
    R1->>H1: Multicast data forwarded
    Note over R1,H1: Data flows via SPT

Loading

[greptile-apps]

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[gromit1811]

Fixed minor docstring spelling issue found by greptile

[donaldsharp]

ci:rerun

[gromit1811]

@Jafaral This is finally ready for merging and CI seems to happy as well. I think I handled all your comments except for one I wasn't sure about and which you ACKed

[gromit1811]

Minor update, add back the PIM interface removal test. Should be passing with current master.
I completely dropped the "PIM router cleanup" tests now (also from my pipeline). They don't make sense and were based on my wrong understanding of what (no) router pim actually does.