Skip to content

bpf: lb: split up lb*_local() into backend selection / DNAT operation#42973

Merged
julianwiedmann merged 4 commits intomainfrom
pr/jwi/main/bpf-lb-split
Nov 26, 2025
Merged

bpf: lb: split up lb*_local() into backend selection / DNAT operation#42973
julianwiedmann merged 4 commits intomainfrom
pr/jwi/main/bpf-lb-split

Conversation

@julianwiedmann
Copy link
Copy Markdown
Member

@julianwiedmann julianwiedmann commented Nov 25, 2025

lb*_local() is shared amongst bpf_lxc and nodeport.h, and implements the
whole sequence of backend selection and DNAT for a service request.

But there's a bunch of code in it that actually only applies to *one* of
the callers. Let's make it easier to refactor by splitting this massive
function into two pieces - one that selects the backend, the other that
takes the backend as input and applies the DNAT. With room in-between to
run the caller-specific pieces (LRP + loopback for bpf_lxc, all sorts of
skip/punt cases for nodeport.h).

(Motivation is to flesh out which special-cases apply to which caller, and to isolate the LRP-specific handling to bpf_lxc. This will help with converting ENABLE_LOCAL_REDIRECT_POLICY to a load-time config).

When a tailcall fails, don't fall through into the subsequent error checks.
Doing so is brittle.

Signed-off-by: Julian Wiedmann <[email protected]>
Some of these error conditions only apply for lb4_local(), and not for
lb4_to_lb6(). Fine-tune the condition checks so that it looks a bit more
like the IPv6 path.

Signed-off-by: Julian Wiedmann <[email protected]>
We're updating the CT tuple so that it reflects the actual packet headers
after DNAT.

So if we exit *before* the rewrite happens, let's not update the tuple.

Signed-off-by: Julian Wiedmann <[email protected]>
@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 Nov 25, 2025
@julianwiedmann julianwiedmann changed the title Pr/jwi/main/bpf lb split bpf: lb: split up lb*_local() into backend selection / DNAT operation Nov 25, 2025
This function is shared amongst bpf_lxc and nodeport.h, and implements the
whole sequence of backend selection and DNAT for a service request.

But there's a bunch of code in it that actually only applies to *one* of
the callers. Let's make it easier to refactor by splitting this massive
function into two pieces - one that selects the backend, the other that
takes the backend as input and applies the DNAT. With room in-between to
run the caller-specific pieces (LRP + loopback for bpf_lxc, all sorts of
skip/punt cases for nodeport.h).

Signed-off-by: Julian Wiedmann <[email protected]>
@julianwiedmann julianwiedmann force-pushed the pr/jwi/main/bpf-lb-split branch from 9038af7 to 4723c05 Compare November 25, 2025 11:44
@julianwiedmann
Copy link
Copy Markdown
Member Author

/test

@julianwiedmann julianwiedmann marked this pull request as ready for review November 25, 2025 13:15
@julianwiedmann julianwiedmann requested a review from a team as a code owner November 25, 2025 13:15
@julianwiedmann julianwiedmann added this pull request to the merge queue Nov 25, 2025
@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 Nov 25, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 25, 2025
@julianwiedmann julianwiedmann added this pull request to the merge queue Nov 26, 2025
Merged via the queue into main with commit c171f22 Nov 26, 2025
370 of 372 checks passed
@julianwiedmann julianwiedmann deleted the pr/jwi/main/bpf-lb-split branch November 26, 2025 06:38
nezdolik pushed a commit to nezdolik/cilium that referenced this pull request Jan 14, 2026
- `go mod tidy && go mod vendor && go mod verify`
- `cd enterprise/hubble-timescape && go mod tidy && cd ../..`
- fixed minor conflicts in `bpf/bpf_lxc.c`, `bpf/bpf_overlay.c` and
  `bpf/lib/nodeport.h` so that both new OSS code and previous Enterprise
  includes are present
- fixed conflicts in `pkg/datapath/config/host_config.go`,
`pkg/datapath/config/lxc_config.go` and
`pkg/datapath/config/overlay_config.go`
- adapted `enterprise/pkg/maps/extepspolicy/table.go`,
  `enterprise/pkg/fqdnha/relay/namemanager.go` and
  `enterprise/pkg/maps/extepspolicy/writer_test.go` due to function
  signature changes in OSS
- `git cherry-pick -n 3d4abeb61b72d910c58ddb199b189c86c4eaf326
  71023768865b9085e6aa8991c553997e1cc6f9b8` to pick up patches from
  @rastislavs (+ manual added fix in
  `enterprise/pkg/bgpv1/manager/reconcilerv2/neighbor_test.go` based on
  patch changes)
- `make -C images update-builder-image update-runtime-image`
- `make -C Documentation update-cmdref`
- `./contrib/scripts/enterprise-testowners.sh`
- remove duplicate `Cleanup Disk space in runner` step in
`.github/workflows/cilium-cli.yaml`
- fix mindfulness issues by manually fixing stuff coming from the
  following PRs:
  - [cilium#42169](cilium#42169)
  - [cilium#42011](cilium#42011)
  - [cilium#42012](cilium#42012)
- `make generate-enterprise-apis`
~- adjusted `enterprise/pkg/ingresspolicy` after commit 2faed3a
  ("policy: fix selector policy leak and detachment issues") removed the
  implicit addition of the identity on lookup. Now the identity needs to
be added and removed in the identity manager.~ Split into separate PR
isovalent/cilium#9506 to ease review and
backporting.
- Set `clustermesh.config.enabled=true` in
  enterprise-clustermesh-overlapping-podcidr workflow following commit
  562ba2c ("clustermesh: set authMode to migration by default").
- Had to revert the following commits because they break the ILB CI
workflow. Thanks to @mhofstetter for bisecting! See discussion for more
details. Upstream fix and re-applying the changes is tracked in
isovalent/cilium#9511.
  - cilium#42986
    - 6781758
    - 3cfe7a1
    - a8fd4ed
    - 64e171e
  - cilium#42973
- c171f22 (with minor conflict
resolution)
    - 9530af5
    - not necessary to revert the last 2 commit of that PR
@cilium-release-bot cilium-release-bot bot moved this to Released in cilium v1.19.0 Feb 3, 2026
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

No open projects
Status: Released

Development

Successfully merging this pull request may close these issues.

3 participants