cilium: fib lookup consolidation #23884
Merged
Conversation
This file contains 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
borkmann
added
sig/datapath
borkmann
force-pushed
the
pr/nat46x64-batch2
branch
from
7134301
to
17a3535
Compare
|
/test |
|
(All green except net-next and external workloads (https://github.com/cilium/cilium/actions/runs/4223077357)) |
borkmann
force-pushed
the
pr/nat46x64-batch2
branch
5 times, most recently
from
c908c37
to
446aa87
Compare
|
/test |
borkmann
added
the
release-note/major
borkmann
force-pushed
the
pr/nat46x64-batch2
branch
3 times, most recently
from
b3d63f8
to
31a4262
Compare
|
/test |
borkmann
force-pushed
the
pr/nat46x64-batch2
branch
from
b180b2e
to
007210a
Compare
|
(CI was all green before, squashed some of the commits. No code changes.) |
borkmann
added
the
ready-to-merge
2 tasks
1 task
|
@borkmann if this is major, can you please add a |
borkmann
removed
backport/author
maintainer-s-little-helper
bot
removed
ready-to-merge
|
@borkmann can you update the release note labels? This is currently marked as both major and misc. It's fixing bugs, so it seems like it's a bugfix? |
This was referenced Jun 22, 2023
borkmann
added a commit
that referenced
this pull request
Aug 22, 2023
The limitation exists mainly on old kernels where the fib lookup helper does not populate the outgoing ifindex. Only for this case we rely on the CT lookup stored ifindex which back then was added as a 16bit field due to limited padding space available. Nowadays this can be lifted after the big rework in #23884. We've seen users with high netdevice churn run into this limitation where the agent bails out. Apart from fixing the bleed, this can be further refined by not relying on the asm.FnRedirectPeer helper presence but by actually doing a runtime BPF program probe so that stable kernels can even be covered. Fixes: #16260 Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
borkmann
added a commit
that referenced
this pull request
Aug 22, 2023
The limitation exists mainly on old kernels where the fib lookup helper does not populate the outgoing ifindex. Only for this case we rely on the CT lookup stored ifindex which back then was added as a 16bit field due to limited padding space available. Nowadays this can be lifted after the big rework in #23884. We've seen users with high netdevice churn run into this limitation where the agent bails out. Apart from fixing the bleed, this can be further refined by not relying on the asm.FnRedirectPeer helper presence but by actually doing a runtime BPF program probe so that stable kernels can even be covered. Fixes: #16260 Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
derailed
pushed a commit
to derailed/cilium
that referenced
this pull request
Aug 23, 2023
The limitation exists mainly on old kernels where the fib lookup helper does not populate the outgoing ifindex. Only for this case we rely on the CT lookup stored ifindex which back then was added as a 16bit field due to limited padding space available. Nowadays this can be lifted after the big rework in cilium#23884. We've seen users with high netdevice churn run into this limitation where the agent bails out. Apart from fixing the bleed, this can be further refined by not relying on the asm.FnRedirectPeer helper presence but by actually doing a runtime BPF program probe so that stable kernels can even be covered. Fixes: cilium#16260 Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
borkmann
added a commit
that referenced
this pull request
Sep 1, 2023
[ upstream commit bd8b4d0 ] [ manual conflict resolution in kube_proxy_replacement.go and nodeport.h due to difference from upstream, the latter mainly in locations where we ifdef ct_state.ifindex ] The limitation exists mainly on old kernels where the fib lookup helper does not populate the outgoing ifindex. Only for this case we rely on the CT lookup stored ifindex which back then was added as a 16bit field due to limited padding space available. Nowadays this can be lifted after the big rework in #23884. We've seen users with high netdevice churn run into this limitation where the agent bails out. Apart from fixing the bleed, this can be further refined by not relying on the asm.FnRedirectPeer helper presence but by actually doing a runtime BPF program probe so that stable kernels can even be covered. Fixes: #16260 Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
borkmann
added a commit
that referenced
this pull request
Sep 1, 2023
[ upstream commit bd8b4d0 ] [ manual conflict resolution in kube_proxy_replacement.go and nodeport.h due to difference from upstream, the latter mainly in locations where we ifdef ct_state.ifindex ] The limitation exists mainly on old kernels where the fib lookup helper does not populate the outgoing ifindex. Only for this case we rely on the CT lookup stored ifindex which back then was added as a 16bit field due to limited padding space available. Nowadays this can be lifted after the big rework in #23884. We've seen users with high netdevice churn run into this limitation where the agent bails out. Apart from fixing the bleed, this can be further refined by not relying on the asm.FnRedirectPeer helper presence but by actually doing a runtime BPF program probe so that stable kernels can even be covered. Fixes: #16260 Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
ldelossa
pushed a commit
to ldelossa/cilium
that referenced
this pull request
Sep 27, 2023
[ upstream commit bd8b4d0 ] [ manual conflict resolution in kube_proxy_replacement.go and nodeport.h due to difference from upstream, the latter mainly in locations where we ifdef ct_state.ifindex ] The limitation exists mainly on old kernels where the fib lookup helper does not populate the outgoing ifindex. Only for this case we rely on the CT lookup stored ifindex which back then was added as a 16bit field due to limited padding space available. Nowadays this can be lifted after the big rework in cilium#23884. We've seen users with high netdevice churn run into this limitation where the agent bails out. Apart from fixing the bleed, this can be further refined by not relying on the asm.FnRedirectPeer helper presence but by actually doing a runtime BPF program probe so that stable kernels can even be covered. Fixes: cilium#16260 Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This series reworks and consolidates all fib lookups in our BPF code to utilize common functionality.
For tc BPF, this means that redirect_neigh code can be used whenever neighbors cannot be resolved. XDP utilizes the BPF neighbor map as we cannot do this ad-hoc in this layer.
This affects KPR, standalone LB, NAT46x64 GW, DSR code.
Fixes: #22782
Fixes: #22800