Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

gromit1811
Copy link
Contributor

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.

Using "router pim" blocks instead of "ip pim ..." statements. No functional
changes.

Signed-off-by: Martin Buck <[email protected]>
@frrbot frrbot bot added the tests Topotests, make check, etc label Mar 6, 2025
@Jafaral
Copy link
Member

Jafaral commented Mar 6, 2025

Shouldn't be required, but without this, the "router pim vrf ..." command
introduced in the previous commit crashes with an assertion failure:

2025/03/10 17:02:12 PIM: [G822R-SBMNH] config-from-file# router pim vrf blue
2025/03/10 17:02:12 PIM: lib/routing_nb_config.c:51: routing_control_plane_protocols_control_plane_protocol_create(): assertion (vrf) failed

To be analyzed/fixed later. Once fixed, this commit may be reverted.

Signed-off-by: Martin Buck <[email protected]>
…otest

Only documentation/formatting, no functional changes.

Signed-off-by: Martin Buck <[email protected]>
…test

We use VRF interfaces as loopback interfaces, but they're not real loopback
interfaces, so in contrast to lo, pimd will not automatically use passive
mode for them. So explicitly enable passive mode when adding them to PIM.

Doesn't change results of the topotest, but causes less clutter in captured
PCAP files.

Signed-off-by: Martin Buck <[email protected]>
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 FRRouting#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.

Signed-off-by: Martin Buck <[email protected]>
@gromit1811 gromit1811 force-pushed the bugfix/pim_vrf_topotest branch from c3a53d4 to cea2017 Compare March 11, 2025 10:23
@gromit1811 gromit1811 marked this pull request as draft March 11, 2025 10:24
@gromit1811
Copy link
Contributor Author

Converted to draft while working on IPv6 support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master size/M tests Topotests, make check, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants