Skip to content

bpf: nodeport: skip SNAT conflict handling for local destinations - #49232

Open
lexfrei wants to merge 2 commits into
cilium:mainfrom
lexfrei:pr/host-snat-conflict-local-dst
Open

bpf: nodeport: skip SNAT conflict handling for local destinations#49232
lexfrei wants to merge 2 commits into
cilium:mainfrom
lexfrei:pr/host-snat-conflict-local-dst

Conversation

@lexfrei

@lexfrei lexfrei commented Oct 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title, description and a Fixes: #XXX line if the commit addresses a particular GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer's Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Describe how you have tested this change in a live environment such as kind, if applicable.
  • Disclose use of machine learning models (including LLMs and other generative AI) in accordance with the Cilium AI Policy, and indicate the rating using AI Influence Level. Example: "This PR was prepared with AIL:N. I personally checked X."
  • Thanks for contributing!

Host traffic to a local endpoint never leaves the node, so it cannot collide with egress SNAT. The port reservation added for host traffic in 990f79b is not needed for it, and running the conflict path is also harmful: ctx_snat_done_set() overwrites MARK_MAGIC_HOST, and under CNI chaining, where host-to-local-pod traffic passes cil_to_netdev on the chaining interface, the destination endpoint then classifies the source as world.

The conflict check now returns false when the destination is found in the local endpoint map: nodeport_has_nat_conflict_ipv4() and _ipv6() return before the port reservation runs. The reservation guards against a host connection sharing a 4-tuple with a masqueraded connection. Such a collision needs both connections to leave the node toward the same destination, and traffic to a local endpoint does not. After the early return, snat_v4_needs_masquerade() punts the packet to the stack (the source is the host endpoint), which is what v1.19 did for this traffic. Without chaining, host traffic to a local pod does not reach to-netdev at all, so nothing changes there.

Testing: I carried this as a downstream patch on v1.20.2 in Cozystack (a CNCF project) and checked it on a three-node kube-ovn chained cluster. Host-to-local-pod SYNs arrive with identity host again (policy verdict remote ID host, action allow), a webhook backend behind an ingress world deny answers kube-apiserver on its own node, a readiness probe under a cluster-only policy passes, and traffic to pods on other nodes is unchanged. New tc_nodeport_snat_conflict_host_local_{ipv4,ipv6} BPF tests fail on current main and pass with the fix. Once this is merged and released, we will drop our downstream copy. This PR was prepared with AIL:3. I personally checked the datapath behavior on the live cluster and verified the test results both ways.

Scope: the first commit covers the conflict path above. The second commit covers the older port reservation in __snat_v4_needs_masquerade() and its IPv6 counterpart (source equal to the masquerade address), which produced the same misclassification with BPF masquerading enabled. The two paths are separate commits so they can be backported independently.

Fixes: #49208

Fix host traffic to a pod on the same node being classified as world under CNI chaining, which broke kubelet probes under default-deny ingress policies.

@lexfrei
lexfrei requested review from a team as code owners October 8, 2026 05:12
@lexfrei
lexfrei requested review from joamaki and jrife October 8, 2026 05:12
@maintainer-s-little-helper maintainer-s-little-helper Bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 8, 2026
@github-actions github-actions Bot added the kind/community-contribution This was a contribution made by a community member. label Oct 8, 2026
lexfrei added a commit to cozystack/cozystack that referenced this pull request Oct 8, 2026
@lexfrei
fix(cilium): keep the host mark on host traffic to a local pod
e358514 
Since v1.20.0 the bpf_host SNAT conflict check fires for any packet
carrying the host mark on every device bpf_host runs on, and the SNAT
path replaces that mark with SNAT_DONE. With kube-ovn chaining a host
connection to a pod on the same node leaves through ovn0, so the pod's
to-container program sees SNAT_DONE and classifies the source as world
instead of host. The implicit localhost allow then stops matching:
kubelet probes fail under any default-deny ingress policy that does not
admit world, and a world deny drops the API server calling a webhook on
its own node.

Carry a verbatim backport of cilium/cilium#49232, which skips the
conflict check when the destination is a local endpoint: such traffic
never leaves the node and cannot collide with pod egress SNAT. Keeping
the hunks identical to upstream lets the patch be dropped as soon as a
shipped cilium release contains the fix. Without chaining, host traffic
to a local pod does not pass to-netdev and nothing changes.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
@lexfrei lexfrei mentioned this pull request Oct 8, 2026
Merged
1 of 12 tasks
@lexfrei
lexfrei force-pushed the pr/host-snat-conflict-local-dst branch from e046227 to 4ba3e28 Compare October 8, 2026 08:35
lexfrei added a commit to cozystack/cozystack that referenced this pull request Oct 8, 2026
@lexfrei
fix(cilium): keep the host mark on host traffic to a local pod
9205f93 
Since v1.20.0 the bpf_host SNAT conflict check fires for any packet
carrying the host mark on every device bpf_host runs on, and the SNAT
path replaces that mark with SNAT_DONE. With kube-ovn chaining a host
connection to a pod on the same node leaves through ovn0, so the pod's
to-container program sees SNAT_DONE and classifies the source as world
instead of host. The implicit localhost allow then stops matching:
kubelet probes fail under any default-deny ingress policy that does not
admit world, and a world deny drops the API server calling a webhook on
its own node.

Carry a verbatim backport of cilium/cilium#49232, which skips the
conflict check when the destination is a local endpoint: such traffic
never leaves the node and cannot collide with pod egress SNAT. Keeping
the hunks identical to upstream lets the patch be dropped as soon as a
shipped cilium release contains the fix. Without chaining, host traffic
to a local pod does not pass to-netdev and nothing changes.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
lexfrei added a commit to cozystack/cozystack that referenced this pull request Oct 8, 2026
@lexfrei
fix(cilium): keep the host mark on host traffic to a local pod
24ec358 
Since v1.20.0 the bpf_host SNAT conflict check fires for any packet
carrying the host mark on every device bpf_host runs on, and the SNAT
path replaces that mark with SNAT_DONE. With kube-ovn chaining a host
connection to a pod on the same node leaves through ovn0, so the pod's
to-container program sees SNAT_DONE and classifies the source as world
instead of host. The implicit localhost allow then stops matching:
kubelet probes fail under any default-deny ingress policy that does not
admit world, and a world deny drops the API server calling a webhook on
its own node.

Carry a verbatim backport of cilium/cilium#49232, which skips the
conflict check when the destination is a local endpoint: such traffic
never leaves the node and cannot collide with pod egress SNAT. Keeping
the hunks identical to upstream lets the patch be dropped as soon as a
shipped cilium release contains the fix. Without chaining, host traffic
to a local pod does not pass to-netdev and nothing changes.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
lexfrei added a commit to cozystack/cozystack that referenced this pull request Oct 8, 2026
@lexfrei
fix(cilium): keep the host mark on host traffic to a local pod
fef7e3f 
Since v1.20.0 the bpf_host SNAT conflict check fires for any packet
carrying the host mark on every device bpf_host runs on, and the SNAT
path replaces that mark with SNAT_DONE. With kube-ovn chaining a host
connection to a pod on the same node leaves through ovn0, so the pod's
to-container program sees SNAT_DONE and classifies the source as world
instead of host. The implicit localhost allow then stops matching:
kubelet probes fail under any default-deny ingress policy that does not
admit world, and a world deny drops the API server calling a webhook on
its own node.

Carry a verbatim backport of cilium/cilium#49232, which skips the
conflict check when the destination is a local endpoint: such traffic
never leaves the node and cannot collide with pod egress SNAT. Keeping
the hunks identical to upstream lets the patch be dropped as soon as a
shipped cilium release contains the fix. Without chaining, host traffic
to a local pod does not pass to-netdev and nothing changes.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
lexfrei added a commit to cozystack/cozystack that referenced this pull request Oct 8, 2026
@lexfrei
fix(cilium): keep the host mark on host traffic to a local pod
16664ad 
Since v1.20.0 the bpf_host SNAT conflict check fires for any packet
carrying the host mark on every device bpf_host runs on, and the SNAT
path replaces that mark with SNAT_DONE. With kube-ovn chaining a host
connection to a pod on the same node leaves through ovn0, so the pod's
to-container program sees SNAT_DONE and classifies the source as world
instead of host. The implicit localhost allow then stops matching:
kubelet probes fail under any default-deny ingress policy that does not
admit world, and a world deny drops the API server calling a webhook on
its own node.

Carry a verbatim backport of cilium/cilium#49232, which skips the
conflict check when the destination is a local endpoint: such traffic
never leaves the node and cannot collide with pod egress SNAT. Keeping
the hunks identical to upstream lets the patch be dropped as soon as a
shipped cilium release contains the fix. Without chaining, host traffic
to a local pod does not pass to-netdev and nothing changes.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
lexfrei added a commit to cozystack/cozystack that referenced this pull request Oct 8, 2026
@lexfrei
fix(cilium): keep the host mark on host traffic to a local pod
1d0e6cb 
Since v1.20.0 the bpf_host SNAT conflict check fires for any packet
carrying the host mark on every device bpf_host runs on, and the SNAT
path replaces that mark with SNAT_DONE. With kube-ovn chaining a host
connection to a pod on the same node leaves through ovn0, so the pod's
to-container program sees SNAT_DONE and classifies the source as world
instead of host. The implicit localhost allow then stops matching:
kubelet probes fail under any default-deny ingress policy that does not
admit world, and a world deny drops the API server calling a webhook on
its own node.

Carry a verbatim backport of cilium/cilium#49232, which skips the
conflict check when the destination is a local endpoint: such traffic
never leaves the node and cannot collide with pod egress SNAT. Keeping
the hunks identical to upstream lets the patch be dropped as soon as a
shipped cilium release contains the fix. Without chaining, host traffic
to a local pod does not pass to-netdev and nothing changes.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
lexfrei added a commit to cozystack/cozystack that referenced this pull request Oct 8, 2026
@lexfrei
fix(cilium): keep the host mark on host traffic to a local pod
8c8f252 
Since v1.20.0 the bpf_host SNAT conflict check fires for any packet
carrying the host mark on every device bpf_host runs on, and the SNAT
path replaces that mark with SNAT_DONE. With kube-ovn chaining a host
connection to a pod on the same node leaves through ovn0, so the pod's
to-container program sees SNAT_DONE and classifies the source as world
instead of host. The implicit localhost allow then stops matching:
kubelet probes fail under any default-deny ingress policy that does not
admit world, and a world deny drops the API server calling a webhook on
its own node.

Carry a verbatim backport of cilium/cilium#49232, which skips the
conflict check when the destination is a local endpoint: such traffic
never leaves the node and cannot collide with pod egress SNAT. Keeping
the hunks identical to upstream lets the patch be dropped as soon as a
shipped cilium release contains the fix. Without chaining, host traffic
to a local pod does not pass to-netdev and nothing changes.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
lexfrei added a commit to cozystack/cozystack that referenced this pull request Oct 9, 2026
@lexfrei
docs(cilium): say the SNAT backport is dropped by hand
3238d79 
The patch header cited a commit of cilium/cilium#49232, an open PR
whose branch is force-pushed, so the commit is reachable from no ref.
Cite the PR alone.

It also read as if the build would flag the patch once upstream
carries the fix. It does not: a fix that lands in another place leaves
this patch applying cleanly as a second early return, so the drop is a
manual step.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
lexfrei added a commit to cozystack/cozystack that referenced this pull request Oct 9, 2026
@lexfrei
docs(cilium): say the SNAT backport is dropped by hand
e1319b5 
The patch header cited a commit of cilium/cilium#49232, an open PR
whose branch is force-pushed, so the commit is reachable from no ref.
Cite the PR alone.

It also read as if the build would flag the patch once upstream
carries the fix. It does not: a fix that lands in another place leaves
this patch applying cleanly as a second early return, so the drop is a
manual step.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
@lexfrei lexfrei mentioned this pull request Oct 10, 2026
Open
2 of 3 tasks
joamaki
joamaki reviewed Oct 12, 2026
View reviewed changes
Comment thread bpf/lib/nodeport_egress.h
* and with CNI chaining the destination endpoint would then see
* the source as world.
*/
if (__lookup_ip4_endpoint(ip4->daddr))

@joamaki joamaki Oct 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LLM flagged that if ip4->saddr matches the nat_ipv4_masquerade address and we have BPF masquerading enabled this could cause MARK_MAGIC_HOST to be dropped. Could you look into if this is an issue?

@lexfrei lexfrei Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@joamaki Yes, it's an issue. With BPF masquerade on and the source equal to the masquerade address, __snat_v4_needs_masquerade() (and the v6 one) reserves the port regardless of the destination, and the SNAT-done mark then overwrites MARK_MAGIC_HOST the same way. Under chaining that is a second route to the same world classification. I added the same local-destination check to both masquerade functions as a separate commit, so it can be split out if you want a different backport scope. The snat-conflict tests now set the masquerade address, so the host-to-local cases cover this path too. They fail without the nat.h change.

@lexfrei
lexfrei force-pushed the pr/host-snat-conflict-local-dst branch from ea5718c to 08543fb Compare October 13, 2026 02:57
This was referenced Oct 26, 2026
Closed
Open
@parlakisik

parlakisik commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

@lexfrei should we set this pr as draft while you are working on that ?

@lexfrei
bpf: nodeport: skip SNAT conflict handling for local destinations
65a4036 
Host traffic to a local endpoint never leaves the node, so it cannot
collide with egress SNAT, and the port reservation added for host
traffic in 990f79b is not needed for it. Running the conflict path
for it is also harmful: ctx_snat_done_set() overwrites MARK_MAGIC_HOST,
and under CNI chaining, where host-to-local-pod traffic passes
cil_to_netdev on the chaining interface, the destination endpoint then
classifies the source as world.

Make nodeport_has_nat_conflict_ipv4() and _ipv6() return false when the
destination is found in the local endpoint map. The reservation guards
against a host connection sharing a 4-tuple with a masqueraded
connection, which requires both connections to leave the node toward
the same destination; traffic to a local endpoint does not. With BPF
masquerading disabled the packet is then punted to the stack, as before
990f79b. With masquerading enabled, the port reservation for
connections sourced from the masquerade address in
__snat_v4_needs_masquerade() still applies; that path predates the
regression and is left unchanged.

Cover the case with tc_nodeport_snat_conflict_host_local_{ipv4,ipv6}
tests, which pin both the skipped port reservation and the preserved
MARK_MAGIC_HOST.

Closes: cilium#49208
Fixes: 990f79b ("bpf,nodeport: generalize SNAT conflict detection")

AIL:3
Signed-off-by: Aleksei Sviridkin <[email protected]>
@lexfrei
bpf: masquerade: skip port reservation for local destinations
5aa8674 
With BPF masquerading enabled, __snat_v4_needs_masquerade() and its
IPv6 counterpart reserve the source port of any connection sourced
from the masquerade address, to prevent aliasing with masqueraded
connections. For a destination in the local endpoint map the
reservation cannot be needed: such traffic never leaves the node, and
no masqueraded connection can share its 4-tuple. It is still harmful,
though: the packet goes through snat_v*_nat() and ctx_snat_done_set(),
which overwrites MARK_MAGIC_HOST, and under CNI chaining the
destination endpoint then classifies the source as world. This path
predates the SNAT conflict generalization and produces the same
misclassification.

Return NAT_PUNT_TO_STACK for such packets instead. In
tc_nodeport_snat_conflict the masquerade address is now configured, so
the host-to-local-endpoint cases exercise both reservation paths; they
fail without this change.

AIL:3
Signed-off-by: Aleksei Sviridkin <[email protected]>
@lexfrei
lexfrei force-pushed the pr/host-snat-conflict-local-dst branch from 08543fb to 5aa8674 Compare November 11, 2026 17:16
@lexfrei

lexfrei commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

@parlakisik I don't consider this PR a WIP. It waited for review for a month, and the only thing that changed was a merge conflict with main. I rebased it and resolved the conflict. The focused BPF test passes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joamaki joamaki joamaki left review comments
@jrife jrife Awaiting requested review from jrife jrife is a code owner automatically assigned from cilium/sig-datapath

At least 0 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. kind/community-contribution This was a contribution made by a community member.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bpf: host traffic to a local pod is classified as world under CNI chaining since the SNAT conflict generalization

3 participants

@lexfrei @parlakisik @joamaki