[v1.13] Always migrate cilium_calls_* during ELF load #28829
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
[ upstream commit 9420217 ] See code comments, as the change is subtle. This sets things up for subsequent commits. As a minor benefit, make the code slightly less unreadable. Signed-off-by: Timo Beckers <timo@isovalent.com>
[ upstream commit 94ec45a ] Note: this is obviously a hack. There's a long-standing issue with Cilium's loader infrastructure where program arrays like cilium_calls_* are reused if the map's properties in the ELF are compatible with the pinned version present on the system. This has been the case since the early iproute2 days, and ebpf-go has the same behaviour by default. This is unsound, but doesn't always cause issues in practice. However, as Cilium's eBPF code base grew, along with its contributor base, increasing code churn and the need to backport fixes, this method started falling apart. Pinned tail call maps are actively used for handling traffic, and should be considered fixed, as they are an integral part of the program's control flow. When another BPF object needs to be attached to a kernel hook, it's incorrect to replace entries in an existing program array used by a prior version of the BPF object, since logic in some tail calls may have been enabled or disabled in the new one, or may have been removed altogether. A prog array's contents cannot be replaced atomically, meaning that while the prog array is being repopulated by a new ELF, a packet may be processed by a chain of programs that are partially old and partially new. This is bad. This patch fixes the specific case of cilium_calls_*, since it's the only prog array across all ELFs that consistently acts as the central tail call map. The main motivation for this approach is backportability as far back as 1.12, since we're seeing increased connectivity interruptions (leading to test flakes) during up/downgrades and restarts in that branch. This is caused by other backports creating code churn, as well as the stark difference in the contents of tail calls between major Cilium versions. This code will be replaced by a 'commit' system that does away with the re- pinning behaviour and keeps new map fd's in memory; only committing them to bpffs when the necessary endpoint prog(s) were successfully replaced. Fixes: cilium#20691 Signed-off-by: Timo Beckers <timo@isovalent.com>
[ no upstream commit ] See previous commit. This one does the same but for the remaining BPF loads done from init.sh Signed-off-by: Timo Beckers <timo@isovalent.com>
maintainer-s-little-helper
bot
added
backport/1.13
|
/test-backport-1.13 |
margamanterola
added
the
dont-merge/waiting-for-user-feedback
|
Marking as "Don't merge", we'll let the changes bake in main for a few days to check that they are safe. |
ti-mo
removed
the
dont-merge/waiting-for-user-feedback
ti-mo
requested review from
a team and
dylandreimerink
and removed request for
a team
dylandreimerink
approved these changes
Nov 8, 2023
squeed
approved these changes
Nov 8, 2023
maintainer-s-little-helper
bot
added
the
ready-to-merge
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.
Manual backport of #28740.
Added a bit to init.sh since iproute2 is still used for loading some of the host datapath here.