datapath: Fix update to NodeAddress table and prioritize Node IP#33629
Merged
aanm merged 3 commits intocilium:mainfrom Jul 15, 2024
Merged
datapath: Fix update to NodeAddress table and prioritize Node IP#33629aanm merged 3 commits intocilium:mainfrom
aanm merged 3 commits intocilium:mainfrom
Conversation
dd13590 to
d5e1f5f
Compare
ec14b8c to
a1f83af
Compare
Contributor
Author
|
/test |
2566813 to
4c886f2
Compare
Contributor
Author
|
/test |
brb
requested changes
Jul 10, 2024
Member
brb
left a comment
There was a problem hiding this comment.
As discussed offline, let's split the first commit into two, and then update the commit msg to include more info on the first fix.
ae29acc to
2af1d78
Compare
Contributor
Author
Done! |
joamaki
commented
Jul 10, 2024
Member
|
/test |
2af1d78 to
a12e94d
Compare
Contributor
Author
|
/test |
The fallback addresses in Table[NodeAddress] were not properly updated when the address was removed. Fix this by clearing the addresses first when updating the fallbacks. Signed-off-by: Jussi Maki <jussi@isovalent.com>
When new node addresses were added after Cilium had started the
update to the internal table of node addresses accidentally removed
existing addresses due to comparing the NodeAddress instead of netip.Addr
which caused change in e.g. the NodePort flag to indicate that the address
should be removed.
As an example:
Before:
NodeAddress{Device: "eth0", Addr: "10.0.0.5", NodePort: true}
After (expected):
NodeAddress{Device: "eth0", Addr: "10.0.0.1", NodePort: true}
NodeAddress{Device: "eth0", Addr: "10.0.0.5", NodePort: false}
After (actual):
NodeAddress{Device: "eth0", Addr: "10.0.0.1", NodePort: true}
What happened was that the code was removing anything old that wasn't in
the new set of addresses, leading it to essentially this comparison:
NodeAddress{"eth0", "10.0.0.5", true} !=
NodeAddress{"eth0", "10.0.0.5", false}
The result of which was that the address "10.0.0.5" was updated and then
subsequently deleted.
Fix this by reworking the code to keep the old set of addresses as netip.Addr
and reducing that set down when inserting the new entries and then finally
cleaning up what remains.
The prefix match comment on the device name and the prefix length check was
removed as this issue had been fixed in StateDB.
As far as we can tell, the only production impact this could have had would
have been to users of v1.16 pre-releases or users running v1.15 with the
experimental runtime device detection enabled (only then would have changes
to Table[NodeAddress] mattered).
Fixes: cilium#33234
Signed-off-by: Jussi Maki <jussi@isovalent.com>
The K8s Node IP address should be preferred when choosing the NodePort and Primary (BPF masquerade) addresses. Signed-off-by: Jussi Maki <jussi@isovalent.com>
a12e94d to
899ff3a
Compare
Contributor
Author
|
/test |
3 tasks
usiegj00
added a commit
to buildio/cilium
that referenced
this pull request
Jan 25, 2026
…face When a network device has multiple IP addresses (both public and private), BPF masquerading was incorrectly selecting the Kubernetes Node IP even when it was a private address and a public address was available on the same interface. The issue was introduced in PR cilium#33629 which added K8s Node IP prioritization. The code was setting both ipv4PublicIndex and ipv4PrivateIndex to the K8s Node IP index, effectively forcing it to be selected regardless of public/private status. This broke the documented "prefer public over private" logic for Primary address selection used by BPF masquerading. The fix ensures that K8s Node IP prioritization only applies within its own category (public or private): - If K8s Node IP is public, it takes precedence over other public IPs - If K8s Node IP is private, it takes precedence over other private IPs - But public IPs still take precedence over private IPs for masquerading This restores the correct behavior where egress traffic is masqueraded using the public IP address when available, which is required for proper routing in environments with both public and private IPs on the same interface. Fixes: cilium#41866
2 tasks
usiegj00
added a commit
to buildio/cilium
that referenced
this pull request
Jan 25, 2026
…face When a network device has multiple IP addresses (both public and private), BPF masquerading was incorrectly selecting the Kubernetes Node IP even when it was a private address and a public address was available on the same interface. The issue was introduced in PR cilium#33629 which added K8s Node IP prioritization. The code was setting both ipv4PublicIndex and ipv4PrivateIndex to the K8s Node IP index, effectively forcing it to be selected regardless of public/private status. This broke the documented "prefer public over private" logic for Primary address selection used by BPF masquerading. The fix ensures that K8s Node IP prioritization only applies within its own category (public or private): - If K8s Node IP is public, it takes precedence over other public IPs - If K8s Node IP is private, it takes precedence over other private IPs - But public IPs still take precedence over private IPs for masquerading This restores the correct behavior where egress traffic is masqueraded using the public IP address when available, which is required for proper routing in environments with both public and private IPs on the same interface. Fixes: cilium#41866
usiegj00
added a commit
to buildio/cilium
that referenced
this pull request
Jan 25, 2026
…face When a network device has multiple IP addresses (both public and private), BPF masquerading was incorrectly selecting the Kubernetes Node IP even when it was a private address and a public address was available on the same interface. The issue was introduced in PR cilium#33629 which added K8s Node IP prioritization. The code was setting both ipv4PublicIndex and ipv4PrivateIndex to the K8s Node IP index, effectively forcing it to be selected regardless of public/private status. This broke the documented "prefer public over private" logic for Primary address selection used by BPF masquerading. The fix ensures that K8s Node IP prioritization only applies within its own category (public or private): - If K8s Node IP is public, it takes precedence over other public IPs - If K8s Node IP is private, it takes precedence over other private IPs - But public IPs still take precedence over private IPs for masquerading This restores the correct behavior where egress traffic is masqueraded using the public IP address when available, which is required for proper routing in environments with both public and private IPs on the same interface. Fixes: cilium#41866 Signed-off-by: Jonathan Siegel <248302+usiegj00@users.noreply.github.com>
usiegj00
added a commit
to buildio/cilium
that referenced
this pull request
Jan 25, 2026
…face When a network device has multiple IP addresses (both public and private), BPF masquerading was incorrectly selecting the Kubernetes Node IP even when it was a private address and a public address was available on the same interface. The issue was introduced in PR cilium#33629 which added K8s Node IP prioritization. The code was setting both ipv4PublicIndex and ipv4PrivateIndex to the K8s Node IP index, effectively forcing it to be selected regardless of public/private status. This broke the documented "prefer public over private" logic for Primary address selection used by BPF masquerading. The fix ensures that K8s Node IP prioritization only applies within its own category (public or private): - If K8s Node IP is public, it takes precedence over other public IPs - If K8s Node IP is private, it takes precedence over other private IPs - But public IPs still take precedence over private IPs for masquerading This restores the correct behavior where egress traffic is masqueraded using the public IP address when available, which is required for proper routing in environments with both public and private IPs on the same interface. Fixes: cilium#41866 Signed-off-by: Jonathan Siegel <248302+usiegj00@users.noreply.github.com>
pull bot
pushed a commit
to bhardwajRahul/cilium
that referenced
this pull request
Jan 29, 2026
…face When a network device has multiple IP addresses (both public and private), BPF masquerading was incorrectly selecting the Kubernetes Node IP even when it was a private address and a public address was available on the same interface. The issue was introduced in PR cilium#33629 which added K8s Node IP prioritization. The code was setting both ipv4PublicIndex and ipv4PrivateIndex to the K8s Node IP index, effectively forcing it to be selected regardless of public/private status. This broke the documented "prefer public over private" logic for Primary address selection used by BPF masquerading. The fix ensures that K8s Node IP prioritization only applies within its own category (public or private): - If K8s Node IP is public, it takes precedence over other public IPs - If K8s Node IP is private, it takes precedence over other private IPs - But public IPs still take precedence over private IPs for masquerading This restores the correct behavior where egress traffic is masqueraded using the public IP address when available, which is required for proper routing in environments with both public and private IPs on the same interface. Fixes: cilium#41866 Signed-off-by: Jonathan Siegel <248302+usiegj00@users.noreply.github.com>
rastislavs
pushed a commit
that referenced
this pull request
Feb 2, 2026
…face [ upstream commit a598dfd ] When a network device has multiple IP addresses (both public and private), BPF masquerading was incorrectly selecting the Kubernetes Node IP even when it was a private address and a public address was available on the same interface. The issue was introduced in PR #33629 which added K8s Node IP prioritization. The code was setting both ipv4PublicIndex and ipv4PrivateIndex to the K8s Node IP index, effectively forcing it to be selected regardless of public/private status. This broke the documented "prefer public over private" logic for Primary address selection used by BPF masquerading. The fix ensures that K8s Node IP prioritization only applies within its own category (public or private): - If K8s Node IP is public, it takes precedence over other public IPs - If K8s Node IP is private, it takes precedence over other private IPs - But public IPs still take precedence over private IPs for masquerading This restores the correct behavior where egress traffic is masqueraded using the public IP address when available, which is required for proper routing in environments with both public and private IPs on the same interface. Fixes: #41866 Signed-off-by: Jonathan Siegel <248302+usiegj00@users.noreply.github.com> Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 2, 2026
…face [ upstream commit a598dfd ] When a network device has multiple IP addresses (both public and private), BPF masquerading was incorrectly selecting the Kubernetes Node IP even when it was a private address and a public address was available on the same interface. The issue was introduced in PR #33629 which added K8s Node IP prioritization. The code was setting both ipv4PublicIndex and ipv4PrivateIndex to the K8s Node IP index, effectively forcing it to be selected regardless of public/private status. This broke the documented "prefer public over private" logic for Primary address selection used by BPF masquerading. The fix ensures that K8s Node IP prioritization only applies within its own category (public or private): - If K8s Node IP is public, it takes precedence over other public IPs - If K8s Node IP is private, it takes precedence over other private IPs - But public IPs still take precedence over private IPs for masquerading This restores the correct behavior where egress traffic is masqueraded using the public IP address when available, which is required for proper routing in environments with both public and private IPs on the same interface. Fixes: #41866 Signed-off-by: Jonathan Siegel <248302+usiegj00@users.noreply.github.com> Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When new node addresses were added after Cilium had started the update to the internal table of node addresses accidentally removed existing addresses due to wrong kind of comparison operation.
As part of fixing this issue a similar type of problem was found with update to the fallback addresses (used as backup when selecting BPF masquerading address). This commit also fixes this issue.
This only impacts users running v1.15 with the runtime device detection enabled.
The second commit adds in the prioritization of the K8s Node IP when selecting the NodeAddress. This was a semantic
difference between the old and new code and should have been retained.
Fixes: #33234