-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Improve HAVE_FIB_IFINDEX probe #27642
Comments
The probe would roughly be as follows: create a new netns, add a route in there, assign a BPF prog to the interface in the netns which does a fib lookup, ensure the neighbor resolution of the fib lookup fails but the route lookup succeeds and finally test whether the fib_params->ifindex output is non-zero while the input was zero. |
This issue has been automatically marked as stale because it has not |
This issue has not seen any activity since it was marked stale. |
@borkmann Would there be a way to test this using prog_run that doesn't involve sending a packet? Our netns tooling isn't great at the moment, it still shells out to ip2 and I'd like to avoid that in new code. Reworking it first will take time and will be annoying to backport. Triggering a prog by sending a packet is going to be error-prone, and we'd need to write the retcode to a map instead of receiving it from prog_run. Maybe we can find something that works from the hostns? I understand we need to induce a failed fib lookup and a successful route lookup on the same ifindex? If we only need to read out fib_params->ifindex after the call, why do we need the route lookup? |
I would hope we can do without sending a packet. Just calling
We basically need to control that the routing succeeds, but the kernel can't provide a L2 resolution for the next-hop. Controlling that from hostns would be difficult I believe. |
Team, I've been looking into this issue as my first contrib. I think the probe is almost done here (still need to check ret value of lookup to make sure it fails). It's follows a similar approach to what Paul did in another probe. On this:
I think Let me know if my understanding is correct, or I am missing s/t. |
Ack, I think that's correct. The |
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
Previously, `HaveFibIfindex` probe checked the presence of commit d1c362e1dd68 in the Linux kernel indirectly, by verifying the presence of the redirect helpers (which were merged alongside). Since `d1c362e1dd68` has been backported to older kernels - but not helpers -, this commit improves `HaveFibIfindex` detection by running a live probe. Fixes: cilium#27642 Signed-off-by: Marc Suñé <marc.sune@isovalent.com>
#27622 lifted a limitation that requires Cilium-managed native devices (where
bpf_host
is attached) to have an ifindex <= MAX_UINT16.This limitation no longer applies on systems with a kernel that has the
HAVE_FIB_IFINDEX
feature. Currently this feature is only detected on kernels >= 5.10. But the relevant patch was actually backported to older kernels. It would be good to also detect the feature on those older kernels, to further reduce the impact of the MAX_UINT16 limitation.Quoting @borkmann
When this is fixed, we should also add
HAVE_FIB_IFINDEX
to the complexity test configs for the relevant kernels.The text was updated successfully, but these errors were encountered: