Skip to content

bpf: nodeport: don't track L2 addr for connection to local backend (v2) #39640

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

Merged
merged 1 commit into from
May 20, 2025

Conversation

julianwiedmann
Copy link
Member

This re-introduces the optimization from
#24324.

The reasoning back then was

Replies by a local backend either get their L2 resolution by the stack,
or (when bpf_lxc uses bpf-host-routing) by bpf_redirect_neigh().
But fib_redirect_*() will never fall through to the L2 neigh cache to get
the client's MAC address.

But this was later reverted in #34303 because we had missed one case - older kernels which don't have fib_redirect_neigh(), and where Cilium thus might end up looking for the client's MAC in the neighbor map.

Now that we have bumped the minimum kernel requirement to v5.10 and require fib_redirect_neigh() to be available, let's bring this change back.

This re-introduces the optimization from
cilium#24324.

The reasoning back then was
> Replies by a local backend either get their L2 resolution by the stack,
> or (when bpf_lxc uses bpf-host-routing) by bpf_redirect_neigh().
> But fib_redirect_*() will never fall through to the L2 neigh cache to get
> the client's MAC address.

But this was later reverted in cilium#34303
because we had missed one case - older kernels which don't have
fib_redirect_neigh(), and where Cilium thus might end up looking for the
client's MAC in the neighbor map.

Now that we have bumped the minimum kernel requirement to v5.10 and
*require* fib_redirect_neigh() to be available, let's bring this change
back.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann julianwiedmann requested a review from a team as a code owner May 20, 2025 14:38
@julianwiedmann julianwiedmann added area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. area/loadbalancing Impacts load-balancing and Kubernetes service implementations labels May 20, 2025
@julianwiedmann julianwiedmann enabled auto-merge May 20, 2025 14:38
@julianwiedmann
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 20, 2025
@julianwiedmann julianwiedmann added this pull request to the merge queue May 20, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 20, 2025
@julianwiedmann julianwiedmann added this pull request to the merge queue May 20, 2025
Merged via the queue into cilium:main with commit 0ab817e May 20, 2025
73 checks passed
@julianwiedmann julianwiedmann deleted the 1.18-bpf-local-backend branch May 20, 2025 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/loadbalancing Impacts load-balancing and Kubernetes service implementations ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants