Skip to content

l7lb: fix bypassing ingress policies for local backends#44693

Merged
julianwiedmann merged 3 commits intomainfrom
pr/smagnani96/sa-fix
Mar 13, 2026
Merged

l7lb: fix bypassing ingress policies for local backends#44693
julianwiedmann merged 3 commits intomainfrom
pr/smagnani96/sa-fix

Conversation

@smagnani96
Copy link
Copy Markdown
Contributor

In commit d1d8e7a ("datapath: Add support for re-entering LXC egress path after L7 LB"), we enabled support for re-entering the LXC egress path after a packet is processed by a L7 LB. This allows us to correctly apply egress policies to packets sent by a L7 LB to local backends.

When ENDPOINT_ROUTES are disabled, the pod policies are checked from the tail call in bpf_host.

With ENDPOINT_ROUTES enabled, the packet is handled directly in the local backend pod cil_to_container path, completely skipping ingress policies. This is because the packet is already in the backend pod ingress path, and returning CTX_ACT_OK would bypass policies. To fix this, we need to hairpin the packet back to its ingress path (the ctx->mark is already cleared at the beginning of the codepath).

This would allow us to correctly apply ingress policies on the local backend ingress path, restoring the expected behavior. For remote backends, it already works as expected, so we don't need adjustments.

Fixes: d1d8e7a ("datapath: Add support for re-entering LXC egress path after L7 LB")

@smagnani96 smagnani96 self-assigned this Mar 9, 2026
@smagnani96 smagnani96 added kind/bug This is a bug in the Cilium logic. area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. labels Mar 9, 2026
@smagnani96 smagnani96 force-pushed the pr/smagnani96/sa-fix branch 2 times, most recently from 476148d to b9aafb6 Compare March 10, 2026 14:04
@julianwiedmann julianwiedmann added affects/v1.16 This issue affects v1.16 branch needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch needs-backport/1.19 This PR / issue needs backporting to the v1.19 branch labels Mar 11, 2026
@smagnani96 smagnani96 force-pushed the pr/smagnani96/sa-fix branch from b9aafb6 to 1c0423f Compare March 11, 2026 16:39
@smagnani96 smagnani96 marked this pull request as ready for review March 11, 2026 22:28
@smagnani96 smagnani96 requested a review from a team as a code owner March 11, 2026 22:28
@julianwiedmann julianwiedmann self-requested a review March 12, 2026 07:39
@smagnani96 smagnani96 removed the request for review from harsimran-pabla March 12, 2026 10:47
@julianwiedmann julianwiedmann added the dont-merge/preview-only Only for preview or testing, don't merge it. label Mar 12, 2026
Copy link
Copy Markdown
Member

@julianwiedmann julianwiedmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you! Waiting for the pending tests ...

@smagnani96 smagnani96 force-pushed the pr/smagnani96/sa-fix branch from 1c0423f to 77aba33 Compare March 13, 2026 11:18
@smagnani96
Copy link
Copy Markdown
Contributor Author

I pushed two tests:

  1. the case we reach the tail call from bpf_host (ENABLE_ROUTING): make sure we don't re-circulate the packet back, and we return to stack.
  2. the case we reach tail call from bpf_lxc (ENABLE_ROUTING not defined): make sure we re-circulate back to the same iface Egress. We need to hit the cil_to_container once again, to check ingress policies for the local backend.

Copy link
Copy Markdown
Member

@julianwiedmann julianwiedmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor nit-picks on the naming in the test, so that it's less confusing for others. Sorry :)

The idea is that we're in the path from envoy to the backend. So the destination IP/port should be named BACKEND_*. And then we're tail-calling back into the client's egress path, so let's use CLIENT_EP_ID.

This is to clarify the naming and distinguish with the new tests we are
going to introduce in subsequent commits.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
This commits adds IPv4/IPv6 tests for the case in which L7LB is enabled
and opens a connection on behalf of a pod destined to a local backend.
In case per-endpoint routes are not enabled, the packet will be handled
in the cil_from_host program, will tail call to the egress policy program,
and then return to stack.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
In commit d1d8e7a ("datapath: Add support for re-entering LXC egress path after L7 LB"),
we enabled support for re-entering the LXC egress path after a packet is
processed by a L7 LB. This allows us to correctly apply egress policies to
packets sent by a L7 LB to local backends.

Without per-endpoint routes, the pod policies are checked from the tail
call in bpf_host. We reach this code from cilium_host (ctx->ifindex).

With per-endpoint routes, the packet is handled directly in the
local backend pod `cil_to_container` path, completely skipping ingress
policies. Returning CTX_ACT_OK at this point would bypass policies.
To fix this, we need to hairpin the packet back to cil_to_container
(the ctx->mark is already cleared at the beginning of the codepath).
This would allow us to correctly apply ingress policies on the local
backend ingress path, restoring the expected behavior.

Fixes: d1d8e7a ("datapath: Add support for re-entering LXC egress path after L7 LB")

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
@smagnani96 smagnani96 force-pushed the pr/smagnani96/sa-fix branch from 77aba33 to a48ed5d Compare March 13, 2026 14:29
@julianwiedmann julianwiedmann removed the dont-merge/preview-only Only for preview or testing, don't merge it. label Mar 13, 2026
@smagnani96
Copy link
Copy Markdown
Contributor Author

/test

@smagnani96 smagnani96 added the backport/author The backport will be carried out by the author of the PR. label Mar 13, 2026
@julianwiedmann julianwiedmann added this pull request to the merge queue Mar 13, 2026
@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 Mar 13, 2026
Merged via the queue into main with commit ea17ed8 Mar 13, 2026
492 checks passed
@julianwiedmann julianwiedmann deleted the pr/smagnani96/sa-fix branch March 13, 2026 17:01
@github-actions github-actions bot added backport-done/1.19 The backport for Cilium 1.19.x for this PR is done. backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. labels Mar 16, 2026
@smagnani96 smagnani96 removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch needs-backport/1.19 This PR / issue needs backporting to the v1.19 branch labels Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

affects/v1.16 This issue affects v1.16 branch area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. backport/author The backport will be carried out by the author of the PR. backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. backport-done/1.19 The backport for Cilium 1.19.x for this PR is done. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants