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

Probe kernel bpf2bpf support #657

Merged
merged 3 commits into from May 10, 2022
Merged

Probe kernel bpf2bpf support #657

merged 3 commits into from May 10, 2022

Conversation

ti-mo
Copy link
Collaborator

@ti-mo ti-mo commented May 4, 2022

The adjustments performed by fixupAndValidate() don't rely on CO-RE being
applied first. Execute it earlier in the chain to potentially avoid loading
BTF and executing CO-RE when we could bail out instead.

Also avoids having to touch test cases due to changes in a subsequent commit.
When trying to load a program containing bpf2bpf calls on a kernel that
doesn't support it, we bubble up the verifier error to the caller:

  program xdp_prog: load program: invalid argument: unreachable insn 28

This is clear, but doesn't allow skipping tests when loading e.g. loader.c
on older kernels due to it containing bpf2bpf calls.

Implement a feature probe for bpf2bpf, return an unsupported error when
trying to load a program with multiple subprogs on an older kernel.

Update the test suite to skip unsupported features instead of hardcoding
a skip on 4.16 and earlier.
  • Provide clear(er) and earlier error feedback about the kernel's bpf2bpf support
  • Allow test skips when loading a bpf2bpf prog on a kernel that doesn't support it
  • Remove hardcoded feature gates for bpf2bpf on kernel 4.16

prog.go Outdated Show resolved Hide resolved
syscalls.go Show resolved Hide resolved
linker.go Outdated Show resolved Hide resolved
linker.go Outdated Show resolved Hide resolved
@ti-mo ti-mo force-pushed the tb/bpf2bpf-probe branch 2 times, most recently from 1e0c614 to 162f314 Compare May 6, 2022 12:39
prog.go Outdated Show resolved Hide resolved
@lmb
Copy link
Collaborator

lmb commented May 6, 2022

Thanks, checking after loading is nicer!

ti-mo added 3 commits May 9, 2022 17:07
Whitespace-trimming a C string using bytes.Trim is ineffective without
first trimming the NUL byte(s).

[ ... 108 101 32 105 110 115 110 32 50 56 10 0 0 0 ... ]

Don't add \x00 to the cutset for performance reasons. Truncate on the first
NUL like unix.ByteSliceToString does, then trim the remaining bytes.

Signed-off-by: Timo Beckers <timo@isovalent.com>
When trying to load a program containing bpf2bpf calls on a kernel that
doesn't support it, we bubble up the verifier error to the caller:

  program xdp_prog: load program: invalid argument: unreachable insn 28

This is clear, but doesn't allow skipping tests when loading e.g. loader.c
on older kernels due to it containing bpf2bpf calls.

Implement a feature probe for bpf2bpf, return an unsupported error when
trying to load a program with multiple subprogs on an older kernel.

Update the test suite to skip unsupported features instead of hardcoding
a skip on 4.16 and earlier.

Signed-off-by: Timo Beckers <timo@isovalent.com>
If a program has function references, return ErrNotSupported if program
creation fails and we know the kernel doesn't support bpf2bpf calls, or if the
user is unprivileged.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo requested a review from lmb May 9, 2022 15:08
@ti-mo
Copy link
Collaborator Author

ti-mo commented May 9, 2022

@lmb I've found a bug introduced in 9b5a039 and made some changes to ErrorWithLog. PTAL 🙏

I've also added hasReferences() and looked at err instead of logErr.

Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for looking at ErrorWithLog as well

@ti-mo ti-mo merged commit 314c773 into cilium:master May 10, 2022
@ti-mo ti-mo deleted the tb/bpf2bpf-probe branch May 10, 2022 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants