ctlplane: set loose reverse path filtering on TAP representors#640
ctlplane: set loose reverse path filtering on TAP representors#640rjarry wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis change adds reverse path filtering configuration in two places. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rjarry
changed the title
|
|
||
| ip link set lo up | ||
| sysctl -qw net.ipv4.conf.all.rp_filter=1 | ||
| sysctl -qw net.ipv4.conf.default.rp_filter=1 |
There was a problem hiding this comment.
In that commit, should we remove the other rp_filter config ?
For example:
smoke/bgp6_srv6_frr_test.sh:ip netns exec bgp-peer sysctl -w net.ipv4.conf.all.rp_filter=0
There was a problem hiding this comment.
These settings are for the bgp-peer netns that does not run grout. Just linux stack. I can try removing them but I think they are needed because of the assymetric vxlan routing.
There was a problem hiding this comment.
The default value is zero on kernel, why always enabled it ?
There was a problem hiding this comment.
Setting it to 2 (loose) is the only way to override anything set by system admins.
When rp_filter=1 (strict mode) is enabled globally, the kernel drops packets arriving on TAP interfaces because the reverse path check finds the default route pointing to the TUN loopback rather than the TAP. Set rp_filter=2 (loose mode) on each TAP at creation time. The effective rp_filter mode is max(conf.all, conf.<iface>), so the per-interface value of 2 overrides a global strict setting. Loose mode only checks that the source address is reachable via some interface, which always passes since grout has a default route. Signed-off-by: Robin Jarry <rjarry@redhat.com> Reviewed-by: Christophe Fontaine <cfontain@redhat.com>
Set rp_filter=1 globally in the smoke test init so that all tests run with strict reverse path filtering. This exercises the per-TAP loose mode override set by grout on ctlplane interfaces. Signed-off-by: Robin Jarry <rjarry@redhat.com> Reviewed-by: Christophe Fontaine <cfontain@redhat.com>
| // delivered by grout are not dropped by rp_filter. The effective | ||
| // rp_filter mode is max(conf.all, conf.<iface>), so setting the | ||
| // per-interface value to 2 (loose) overrides a global strict setting. | ||
| snprintf(path, sizeof(path), "/proc/sys/net/ipv4/conf/%s/rp_filter", iface->name); |
There was a problem hiding this comment.
rp_filter is disable by default, why you want to force at loose for tap. -> NACK
There was a problem hiding this comment.
Setting it to 2 (loose) is the only way to override net.ipv4.conf.all=1 set by system admins.
There was a problem hiding this comment.
Just don't put loose when it's disabled in all.
There was a problem hiding this comment.
I cannot know if the admin will enable it later. Why do you care about setting it loose? It has no impact at all. It will just verify there is one route to reach the source address. Since grout always installs a default route via the VRF tun interface, the check passes all the time.
3 participants
When rp_filter=1 (strict mode) is enabled, the kernel does a reverse path FIB lookup on incoming packets to verify that the source address is reachable via the same interface. With only oif rules, this lookup falls through to the main/VRF table where the default route points to the TUN loopback, causing an interface mismatch and dropping the packet.
Add a matching iif rule for each TAP so that the rp_filter FIB lookup hits table 999 and finds the default route via the same TAP the packet arrived on. This does not affect actual routing of return packets which still go through the TUN loopback into the datapath as before.
Update the stale rule flush logic to also collect and clean up iif rules on restart.
Enable rp_filter in all smoke tests to exercise the added iif rules.
net.ipv4.conf.all.rp_filter=1andnet.ipv4.conf.default.rp_filter=1after bringing up loopback./proc/sys/net/ipv4/conf/<iface>/rp_filter=2(loose) so packets delivered by grout aren’t dropped when an overall strict setting is enabled; emitsWARNINGif the per-interface sysctl file can’t be opened.999: creates per-TAPip ruleentries that match the TAP viaFRA_OIFNAMEand route lookups through table999, installs adefault dev <tap>route in table999for both IPv4 and IPv6, and flushes stale state on restart by dumping and deleting both the table999rules and the table999default routes (to avoid orphaned rules/routes accumulating across crash/restart cycles).