Skip to content

pimd,pim6d: implement PIM join filtering #19299

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

pimd,pim6d: implement PIM join filtering #19299

wants to merge 6 commits into from

Conversation

rzalamena
Copy link
Member

This PR is a continuation of #18955 .

The previous PR implemented the route-map infrastructure code in PIM and its first use for filtering IGMP / MLD joins, now in this continuation we further build on that code to filter PIM joins (the one that happens between PIM routers).

@rzalamena
lib: fix route map multicast interface output
6990a45 
Multicast interface doesn't belong to any IP type and the commands don't
have the IP specification either.

Signed-off-by: Rafael Zalamena <[email protected]>
@rzalamena
pimd,pim6d,yang: support filtering PIM joins
74b9436 
New configuration for selecting a route map to filter PIM joins.

Signed-off-by: Rafael Zalamena <[email protected]>
@nabahr
Copy link
Contributor

nabahr commented Jul 30, 2025

How does this differ from ip multicast boundary oil WORD and ip multicast boundary ACCESS-LIST?
Those two use a prefix list or an access list and this uses a route map. So if the intent is to just gain the ability to use route maps for filtering as well then can we build this together to use the same cli and filtering function calls? Right now the boundary check uses pim_is_group_filtered.

@rzalamena
Copy link
Member Author

How does this differ from ip multicast boundary oil WORD and ip multicast boundary ACCESS-LIST? Those two use a prefix list or an access list and this uses a route map. So if the intent is to just gain the ability to use route maps for filtering as well then can we build this together to use the same cli and filtering function calls? Right now the boundary check uses pim_is_group_filtered.

This new command only filters PIM joins (between PIM routers), it doesn't affect IGMP or MLD. The other difference is that the join-filter is global (inside router pim configuration node) and not bound to a specific interface (though you can use match multicast-interface to limit to a specific interface).

@nabahr
Copy link
Contributor

nabahr commented Jul 30, 2025

How does this differ from ip multicast boundary oil WORD and ip multicast boundary ACCESS-LIST? Those two use a prefix list or an access list and this uses a route map. So if the intent is to just gain the ability to use route maps for filtering as well then can we build this together to use the same cli and filtering function calls? Right now the boundary check uses pim_is_group_filtered.

This new command only filters PIM joins (between PIM routers), it doesn't affect IGMP or MLD. The other difference is that the join-filter is global (inside router pim configuration node) and not bound to a specific interface (though you can use match multicast-interface to limit to a specific interface).

Seems like maybe the old commands could actually be deprecated then? I just don't like having multiple different filtering actions happening back to back. In pim_join.c, just a few lines above the new addition, there are calls to pim_is_group_filtered and I'd like to see how we can make that cleaner and easier to maintain.

rzalamena added 2 commits July 30, 2025 14:44
@rzalamena
doc: fix MLD route map documentation
3a40fe5 
Replace `IGMP` with `MLD`, this page is about IPv6 and not IPv4.

Signed-off-by: Rafael Zalamena <[email protected]>
@rzalamena
doc: document new PIM filtering commands
368a08d 
Document the command `join-filter route-map RMAP_NAME` for both PIM and
PIMv6.

Signed-off-by: Rafael Zalamena <[email protected]>
@donaldsharp
Copy link
Member

I'm not terribly concerned about having multiple layers here. BGP already allows this via route-maps and access-lists etc. As long as it is well documented in pim.rst I am not overly bothered by it.

@nabahr
Copy link
Contributor

nabahr commented Jul 30, 2025

I lean towards agreement as far as the CLI is concerned but on the internal implementation I think it'd be easier to maintain if the filtering was coordinated in one area.

@donaldsharp
Copy link
Member

@rzalamena and @nabahr do we need to discuss next tuesday?

@rzalamena
topotests: new test for PIM filtering
90529f3 
Implement tests for the new PIM join route-map filtering.

Signed-off-by: Rafael Zalamena <[email protected]>
@rzalamena
topotest: workaround PIMv6 convergence bug
0efdfaa 
If we get to the PIMv6 join test too fast then PIMv6 will not have
converged and the static joins will fail.

Sample of non working case:
```
r1# show ipv6 pim state
Codes: J -> Pim Join, I -> MLD Report, S -> Source, * -> Inherited from (*,G), V -> VxLAN, M -> Muted
 Active  Source             Group       RPT  IIF     OIL
 0       *                  ff05::100   y    <iif?>  pim6reg(I    )
                                                     r1-eth1(I    )
 0       *                  ff05::200   y    <iif?>  pim6reg(I    )
                                                     r1-eth1(I    )
 0       *                  ff05::300   y    <iif?>  pim6reg(I    )
                                                     r1-eth1(I    )
 0       *                  ff05::1000  y    <iif?>  pim6reg(I    )
                                                     r1-eth1(I    )
 0       *                  ff05::2000  y    <iif?>  pim6reg(I    )
                                                     r1-eth1(I    )
 0       2001:db8:101::100  ff35::100   y    <iif?>  r1-eth1(I    )
 0       2001:db8:101::200  ff35::100   y    <iif?>  r1-eth1(I    )
```

Sample of working case:
```
r1# show ipv6 pim state
Codes: J -> Pim Join, I -> MLD Report, S -> Source, * -> Inherited from (*,G), V -> VxLAN, M -> Muted
 Active  Source             Group       RPT  IIF      OIL
 1       *                  ff05::100   y    r1-eth0  pim6reg(I    )
                                                      r1-eth1(I    )
 1       *                  ff05::200   y    r1-eth0  pim6reg(I    )
                                                      r1-eth1(I    )
 1       *                  ff05::300   y    r1-eth0  pim6reg(I    )
                                                      r1-eth1(I    )
 1       *                  ff05::1000  y    r1-eth0  pim6reg(I    )
                                                      r1-eth1(I    )
 1       *                  ff05::2000  y    r1-eth0  pim6reg(I    )
                                                      r1-eth1(I    )
 1       2001:db8:101::100  ff35::100   n    r1-eth0  r1-eth1(I    )
 1       2001:db8:101::200  ff35::100   n    r1-eth0  r1-eth1(I    )
```

Signed-off-by: Rafael Zalamena <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bugfix documentation libfrr master pim rebase PR needs rebase size/XL yang
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rzalamena @nabahr @donaldsharp