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

vrf: fix route filter to use output iface #927

Merged
merged 1 commit into from Jul 21, 2023

Conversation

sockmister
Copy link

Summary

The route filter currently filters based on the LinkIndex and Scope. The corresponding filter flag for LinkIndex should be netlink.RT_FILTER_OIF, and not netlink.RT_FILTER_IIF. This renders the filter ineffective, causing the wrong routes to be copied into the new VRF.

The netlink code here tells us which filter flags corresponds to which field.

Example

I have a k8s cluster that's using Multus to have multiple interfaces. Here's a concrete example demonstrating this issue:

  1. create a pod with 2 interfaces, the default pod CNI and an additional one, say, the bridge CNI
# ip -4 -br a
lo               UNKNOWN        127.0.0.1/8
eth0@if398       UP             10.244.2.151/24
net1@if399       UP             10.8.1.1/24
  1. on the second CNI, we also chain the VRF plugin
# ip vrf
Name              Table
-----------------------
gateway              1
  1. we see that the routes that are being added into the new VRF also has the routes that's being used for pod networking
# ip route
default via 10.244.2.1 dev eth0
10.244.0.0/16 via 10.244.2.1 dev eth0
10.244.2.0/24 dev eth0 proto kernel scope link src 10.244.2.151

# ip route list vrf gateway
default via 10.244.2.1 dev eth0
10.8.1.0/24 dev net1 proto kernel scope link src 10.8.1.1
10.244.0.0/16 via 10.244.2.1 dev eth0
  1. what we want to see instead:
# ip route list vrf gateway
default via 10.8.1.1 dev net1
10.8.1.0/24 dev net1 proto kernel scope link src 10.8.1.1

PR notes

  • I've added a test scenario roughly equivalent to the above description
  • I've changed checkRoutesOnVRF to filter only based on the table ID so we can check the LinkIndex of the routes that were copied to the new VRF

@squeed
Copy link
Member

squeed commented Jul 21, 2023

very good catch!

@squeed
Copy link
Member

squeed commented Jul 21, 2023

@sockmister can you please rebase to pick up a CI fix? Thanks!

current route filter uses RT_FILTER_IIF in conjunction with LinkIndex.
This combination is ignored by netlink, rendering the filter
ineffective

Signed-off-by: Poh Chiat Koh <poh@inter.link>
@sockmister
Copy link
Author

done!

@squeed squeed merged commit 9d9ec6e into containernetworking:main Jul 21, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants