pimd: add IGMP/MLD proxy route-map filtering#21906
Conversation
Greptile SummaryThis PR adds per-interface route-map filtering for the IGMP/MLD proxy feature, allowing operators to control which (S,G) groups are forwarded via proxy, along with a new
Confidence Score: 5/5Safe to merge; the new filter infrastructure is correctly wired into all relevant code paths and the previous config-persistence and write-ordering issues noted in earlier review rounds have been addressed. The core implementation is sound: gm_proxy_filter is properly initialized, registered in the global filter-ref list so route-map pointer updates propagate correctly, and finalized alongside gmp_filter. The interface/source_interface arguments are ordered consistently in every pim_filter_match call site. The config write was moved to the AF-generic pim_config_write path with proxy route-map emitted before proxy, fixing both the IPv6 persistence gap and the startup ordering race. The one behavioral nuance — that the filter gates prunes as well as joins, meaning stale proxy joins survive host leaves when a filter is tightened without cycling the proxy — is documented and only arises from a deliberate operator action without following the documented cycle step. pimd/pim_tib.c — the filter check on the prune path is worth a second look from a maintainability perspective. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[IGMP Report received on ifp] --> B{Is gmp_filter set?}
B -- Yes --> C[pim_filter_match\ngmp_filter, ifp, ifp]
B -- No --> D[Allow]
C -- PERMIT --> D
C -- DENY --> E[Drop/skip]
D --> F{Is proxy enabled\non any oif?}
F -- No --> G[Normal IGMP state update]
F -- Yes --> H[tib_sg_proxy_join_prune_check\nfor each oif with gm_proxy]
H --> I{gm_proxy_filter\nset on oif?}
I -- No --> J[pim_if_gm_join_add / del]
I -- Yes --> K[pim_filter_match\ngm_proxy_filter, oif, ifp\ninterface=oif, source_interface=ifp]
K -- PERMIT --> J
K -- DENY --> L[Skip join/prune]
M[ip igmp proxy\nconfig enable] --> N[pim_if_gm_proxy_init\noif = proxy output iface]
N --> O[For each ifp != oif\nFor each S,G in ifp group list]
O --> P{gm_proxy_filter set?}
P -- No --> Q[pim_if_gm_join_add]
P -- Yes --> R[pim_filter_match\ngm_proxy_filter, oif, ifp]
R -- PERMIT --> Q
R -- DENY --> S[Skip]
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
pimd/pim_tib.c:97-110
**Filter gates prunes as well as joins — stale proxy joins survive host leaves after a filter is tightened**
`pim_filter_match` is evaluated for both `join=true` and `join=false` paths. In steady-state with a stable filter this is correct: a group that was never joined also never needs to be pruned. However, if the proxy is running with no filter (all groups proxied), and an operator then adds a restrictive route-map *without* cycling the proxy, the subsequent host leave for a now-denied group hits `continue` here and the proxy join is silently skipped — leaving multicast state installed on the output interface even though no downstream host is requesting it. The stale join persists until the proxy is explicitly cycled.
Applying the filter only on the join leg (`if (join && !pim_filter_match(...)) continue;`) would allow prunes to always clean up existing state, regardless of whether the filter has changed. The current behavior requires an explicit cycle to take effect, which is documented but not enforced or warned about at the CLI.
Reviews (4): Last reviewed commit: "pimd: always populate source_interface f..." | Re-trigger Greptile |
|
@greptile review |
|
@greptile review |
57b5767 to
db15c77
Compare
|
@greptile review |
|
a) In be67c41 -> this will not be bisectable since it will fail to build.: |
| continue; | ||
|
|
||
| if (pim_ifp->gm_enable && pim_ifp->gm_proxy) { | ||
| struct prefix_sg pfx; |
There was a problem hiding this comment.
If I have an igmp join already and then we add a route-map to deny the join, and then we receive a prune, this will cause the prune to be dropped on the floor, thus leaving the igmp join, correct?
There was a problem hiding this comment.
Should we also have a debug log here to show the fact it was filtered? I think every other spot has it.
There was a problem hiding this comment.
I debated whether I should still allow prune always, I wasn't sure if it has any side effects on upstream-side, but I think it is safe.
There was a problem hiding this comment.
fixed the commit/compile issue. Once of my earlier fixups landed with the wrong signature for that commit. I also now only apply the filter on joins (exclude prune). the debug log is added too.
Add a new optional `proxy-route-map` leaf to the GMP interface address-family grouping, allowing operators to filter which (S,G) groups are forwarded via IGMP proxy. Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Introduce a new CLI command: ``` ip igmp proxy route-map RMAP_NAME no ip igmp proxy route-map ``` This wires the proxy-route-map YANG leaf into the northbound layer via lib_interface_gm_proxy_rmap_modify/destroy callbacks, stores the resolved route-map in a new gm_proxy_filter field, and emits the command in the running-config output. Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Add gm_proxy_filter (pim_filter_ref) to struct pim_interface, initialized and torn down alongside gmp_filter. Apply pim_filter_match() against gm_proxy_filter at both proxy join entry points: - pim_if_gm_proxy_init: filters groups replayed when proxy is enabled - tib_sg_proxy_join_prune_check: filters dynamic join/prune events as new IGMP reports arrive Only (S,G) groups permitted by the route-map are forwarded via proxy. Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Add a clicmd entry for the new 'ip igmp proxy route-map ROUTE-MAP' command, including a description of when filtering is evaluated, the supported match conditions, and a brief configuration example. Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Test that a route-map attached to an IGMP proxy interface correctly filters which (S,G) groups are forwarded: - Only groups permitted by the route-map appear after a proxy cycle - A join for a denied group does not appear dynamically - Removing the route-map and cycling proxy restores all groups Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Add a new identity 'multicast-source-interface' and the corresponding augment case, allowing route-map entries to match against the interface where the IGMP/MLD report was received (the proxy source interface). Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Extend pim_rmap_info with a source_interface field populated at proxy join time, and add a new route-map match condition: match multicast-source-interface IFNAME This matches the interface where the IGMP/MLD report was received (the upstream/listener-facing interface), distinct from the existing 'match multicast-interface' which matches the proxy output interface. Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Pass the IGMP/MLD report source interface to pim_filter_match in both proxy join paths: - pim_if_gm_proxy_init: pass the interface whose group list is being replayed (ifp) as source_interface - tib_sg_proxy_join_prune_check: pass oif (the interface that triggered the join/prune event) as source_interface Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Add documentation for the new 'match multicast-source-interface IFNAME' route-map condition in the 'ip igmp proxy route-map' command entry, explaining that it matches the interface where the IGMP/MLD report was received, and include a usage example. Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Verify that a proxy route-map using 'match multicast-source-interface' only forwards groups reported on the matched interface: - Apply a route-map matching r1-eth2; cycle proxy - Verify only 225.3.3.3/225.4.4.4 (static joins on r1-eth2) are proxied - Verify groups from r1-eth0 (225.2.2.2, 225.5.5.5, 225.7.7.7) are absent - Remove route-map; cycle proxy; verify all five groups return Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
Non-proxy callers passed NULL, making 'match multicast-source-interface' silently deny every (S,G) when used in a regular ip/ipv6 igmp/mld route-map. Pass the inbound interface in all callers; proxy callers keep their distinct listener/output values. Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
db15c77 to
67893ac
Compare
This PR introduces per-interface route-map filtering for the IGMP/MLD proxy feature, allowing operators to control which (S,G) groups are forwarded via proxy. It also adds a new route-map match condition for matching on the interface where the IGMP/MLD report was received.
New CLI
New route-map match condition
This matches the interface where the IGMP/MLD report was received (the upstream/listener-facing interface), and is distinct from the existing match multicast-interface, which matches the proxy output interface.