Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.2] cgroups: ebpf: use link.Anchor to check for BPF_F_REPLACE support #4551

Merged
merged 3 commits into from
Dec 7, 2024

Conversation

cyphar
Copy link
Member

@cyphar cyphar commented Dec 7, 2024

Backport of #4548. (Draft until merged.)


In v0.13.0, cilium/ebpf stopped supporting setting BPF_F_REPLACE as an
explicit flag and instead requires us to use link.Anchor to specify
where the program should be attached.

Commit 216175a ("Upgrade Cilium's eBPF library version to 0.16")
did update this correctly for the actual attaching logic, but when
checking for kernel support we still passed BPF_F_REPLACE. This would
result in a generic error being returned, which our feature-support
checking logic would treat as being an error the indicates that
BPF_F_REPLACE is supported, resulting in a regression on pre-5.6
kernels.

It turns out that our debug logging saying that this unexpected error
was happening was being output as a result of this change, but nobody
noticed...

Fixes: 216175a ("Upgrade Cilium's eBPF library version to 0.16")
Fixes #3008
Signed-off-by: Aleksa Sarai cyphar@cyphar.com

@cyphar cyphar added the backport/1.2-pr A backport PR to release-1.2 label Dec 7, 2024
@cyphar cyphar added this to the 1.2.3 milestone Dec 7, 2024
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah marked this pull request as ready for review December 7, 2024 12:18
(This is a cherry-pick of dea0e04.)

In v0.13.0, cilium/ebpf stopped supporting setting BPF_F_REPLACE as an
explicit flag and instead requires us to use link.Anchor to specify
where the program should be attached.

Commit 216175a ("Upgrade Cilium's eBPF library version to 0.16")
did update this correctly for the actual attaching logic, but when
checking for kernel support we still passed BPF_F_REPLACE. This would
result in a generic error being returned, which our feature-support
checking logic would treat as being an error the indicates that
BPF_F_REPLACE *is* supported, resulting in a regression on pre-5.6
kernels.

It turns out that our debug logging saying that this unexpected error
was happening was being output as a result of this change, but nobody
noticed...

Fixes: 216175a ("Upgrade Cilium's eBPF library version to 0.16")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(This is a cherry-pick of dea0e04.)

It is possible for LinkAttachProgram to return ErrNotSupported if
program attachment is not supported at all (which doesn't matter in this
case), but it seems possible that upstream will start returning
ErrNotSupported for BPF_F_REPLACE at some point so it's best to make
sure we don't cause additional regressions here.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
(This is a cherry-pick of c0044c7.)

If we get an unexpected error here, it is probably because of a library
or kernel change that could cause our detection logic to be invalid. As
a result, these warnings should be louder so users have a chance to tell
us about them sooner (or so we might notice them before doing a release,
as happened with the 1.2.0 regression).

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
@lifubang lifubang merged commit fceb5ef into opencontainers:release-1.2 Dec 7, 2024
40 checks passed
@cyphar cyphar deleted the 1.2-ebf-enotsup branch December 7, 2024 17:34
@lifubang lifubang mentioned this pull request Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.2-pr A backport PR to release-1.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants