-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
docs: Add documentation for BPF targets #135107
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @GuillaumeGomez. Use |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to add the page to the SUMMARY.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for signing up as maintainer contact by the way, this is definitely one of our... odder... targets and it will probably benefit a lot from having someone to talk to about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting this going!
e323a85
to
9397c4d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird I can't resolve comments :(
This comment has been minimized.
This comment has been minimized.
9397c4d
to
9a6636d
Compare
This comment has been minimized.
This comment has been minimized.
BPF targets require a Rust toolchain with the `rust-src` component and | ||
[bpf-linker][bpf-linker]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is best to full stop here. The current phrasing seems easy to misinterpret as "bpf-linker is part of the Rust toolchain". Notably, we do ship custom linkers for many targets!
BPF targets require a Rust toolchain with the `rust-src` component and | |
[bpf-linker][bpf-linker]. | |
BPF targets require a Rust toolchain with the `rust-src` component. | |
In addition, you must install the [bpf-linker]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a side note, I would love to see bpf-linker eventually being included in the Rust toolchain. I'm happy to help with that. I suppose it would be one of requirements of updating BPF targets to Tier 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably!
Also uh, probably would need them to support more than 5 arguments in Rust functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who is "them"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tamird The BPF targets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Do I understand you correctly that such support would be a requirement for promotion to Tier 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be... difficult for me to imagine that we would accept a tier 2 target that cannot accept a dozen arguments to its Rust functions, yes. Generally, for tier 2 targets, we want them to feel like fairly "normal" targets, even if they don't have first class support (yet?).
Note how I am saying "Rust functions" very specifically: if you want to impose some peculiar constraint on the extern "C"
ABI, that's less likely to be a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But what would be the point of supporting more than 5 args if the eBPF spec mandates max 5 and the vms don't support more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be possible to handle more than 5 arguments by passing the rest on the stack, right? That is what regular targets do too when they run out of argument registers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default response to upstreaming changes - even a lot less invasive than changing the calling convention - so far has been "if clang and the kernel don't need it, it doesn't belong to the target". Then there's the issue that the verifier would have to be made aware too, or Fn(u8, u8, u8, u8, u8)
and Fn(u8, u8, u8, u8, u8, u8)
would look like the same type to it. But nothing in the kernel uses more than 5 args, so the chance of getting that merged seems very slim too.
Technically yes this is a very easy thing to fix (and in fact at work we have a LLVM fork that already does all this and more), but socially it's much harder (and at work we've given up on upstreaming).
BTF, which can be enabled by adding `-C link-arg=--btf` to `RUSTFLAGS`. With | ||
that feature enabled, [bpf-linker][bpf-linker] does not only link different | ||
crates/modules, but also performs a necessary sanitization of debug info, which | ||
is required to produce valid [BTF][btf] acceptable by the Linux kernel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accepted by what, exactly? You mean a specific component of the kernel, right? Does the eBPF verifier also check debuginfo? Or is it that the debugger requires a specific debuginfo format?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK the BPF dynamic linker uses BTF to do things like checking for compatibility of the BPF module with the type layouts of the kernel itself. And through special relocations it can fill in the offsets of fields for kernel types such that the BPF module can run on multiple kernel versions even if the layout of types used by the BPF module changes across those kernel versions. The latter almost certainly will require explicit rustc support to get wired up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@workingjubilee the kernel validates the BTF info as part of calling bpf_program_load
. Producing valid BTF right now means... generating whatever clang generates for C code. Non-C symbols in type names are not allowed (eg <
in Foo<u8>
), and there are some other annoying constraints.
@bjorn3 we discussed implementing BTF relocs on zulip a couple of years ago. Long term, you're correct it requires explicit rustc support if only because what debuginfo rustc generates with -C debuginfo
is unspecified, so going from DI => relocs is fragile. In the meantime we think we can probably hack it up in bpf-linker tho.
|
||
BPF bytecode needs to be executed on a BPF virtual machine, like the one | ||
provided by the Linux kernel or one of the user-space implementations like | ||
[rbpf][rbpf]. None of them support running Rust testsuite. One of the reasons |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this referring to our UI testsuite, or is it referring to #[test]
functions, or both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To both.
Currently the best way of unit testing the code used in BPF programs is | ||
extracting it to a separate crate, which can be built and ran on a host target, | ||
and using it as a dependency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh...
wouldn't #[cfg_attr(not(target_arch = "bpfel"), test)]
work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#[cfg(all(not(target_arch = "bpf"), test))]
works. I updated the paragraph accordingly.
[uprobes][uprobe] allow dynamic function tracing and lookup into registers. | ||
Registers and calling convenctions are different across architectures. Due to | ||
`char` in C being signed or unsigned on different architectures, the signatures | ||
of [BPF helpers][bpf-helpers] and [kfuncs][kfunc] also end up different. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't Linux compile with -funsigned-char
nowadays?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From relatively recently right? 6.1 or 6.2 can't remember. But we need to support older kernels.
But I agree that it seems a detail I wouldn't single out char
here.
9a6636d
to
28cf691
Compare
This comment has been minimized.
This comment has been minimized.
expected to handle errors in a recoverable manner. Therefore most BPF programs | ||
written in Rust use the following no-op panic handler implementation: | ||
|
||
```rust |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this has to be one of these in order for CI to stop complaining:
```rust | |
```rust,ignore |
```rust | |
```rust | |
#![no_std] |
28cf691
to
00ddeac
Compare
This comment has been minimized.
This comment has been minimized.
00ddeac
to
33873b5
Compare
Some changes occurred in src/doc/rustc/src/platform-support cc @Noratrieb |
The job Click to see the possible cause of the failure (guessed by this bot)
|
Support clobber_abi in BPF inline assembly This supports [`clobber_abi`](https://doc.rust-lang.org/nightly/reference/inline-assembly.html#abi-clobbers) which is one of the requirements of stabilization mentioned in the tracking Issue for `asm_experimental_arch` (rust-lang#93335). Refs: [Section 1.1 "Registers and calling convention" in BPF ABI Recommended Conventions and Guidelines v1.0](https://github.com/torvalds/linux/blob/v6.13/Documentation/bpf/standardization/abi.rst#11registers-and-calling-convention) > R0 - R5 are scratch registers and BPF programs needs to spill/fill them if necessary across calls. cc `@alessandrod` `@dave-tucker` `@tamird` `@vadorovsky` (target maintainers mentioned in platform support document which will be added by rust-lang#135107) r? `@Amanieu` `@rustbot` label +O-eBPF +A-inline-assembly
Rollup merge of rust-lang#136194 - taiki-e:bpf-clobber-abi, r=amanieu Support clobber_abi in BPF inline assembly This supports [`clobber_abi`](https://doc.rust-lang.org/nightly/reference/inline-assembly.html#abi-clobbers) which is one of the requirements of stabilization mentioned in the tracking Issue for `asm_experimental_arch` (rust-lang#93335). Refs: [Section 1.1 "Registers and calling convention" in BPF ABI Recommended Conventions and Guidelines v1.0](https://github.com/torvalds/linux/blob/v6.13/Documentation/bpf/standardization/abi.rst#11registers-and-calling-convention) > R0 - R5 are scratch registers and BPF programs needs to spill/fill them if necessary across calls. cc `@alessandrod` `@dave-tucker` `@tamird` `@vadorovsky` (target maintainers mentioned in platform support document which will be added by rust-lang#135107) r? `@Amanieu` `@rustbot` label +O-eBPF +A-inline-assembly
Support clobber_abi in BPF inline assembly This supports [`clobber_abi`](https://doc.rust-lang.org/nightly/reference/inline-assembly.html#abi-clobbers) which is one of the requirements of stabilization mentioned in the tracking Issue for `asm_experimental_arch` (#93335). Refs: [Section 1.1 "Registers and calling convention" in BPF ABI Recommended Conventions and Guidelines v1.0](https://github.com/torvalds/linux/blob/v6.13/Documentation/bpf/standardization/abi.rst#11registers-and-calling-convention) > R0 - R5 are scratch registers and BPF programs needs to spill/fill them if necessary across calls. cc `@alessandrod` `@dave-tucker` `@tamird` `@vadorovsky` (target maintainers mentioned in platform support document which will be added by rust-lang/rust#135107) r? `@Amanieu` `@rustbot` label +O-eBPF +A-inline-assembly
Support clobber_abi in BPF inline assembly This supports [`clobber_abi`](https://doc.rust-lang.org/nightly/reference/inline-assembly.html#abi-clobbers) which is one of the requirements of stabilization mentioned in the tracking Issue for `asm_experimental_arch` (#93335). Refs: [Section 1.1 "Registers and calling convention" in BPF ABI Recommended Conventions and Guidelines v1.0](https://github.com/torvalds/linux/blob/v6.13/Documentation/bpf/standardization/abi.rst#11registers-and-calling-convention) > R0 - R5 are scratch registers and BPF programs needs to spill/fill them if necessary across calls. cc `@alessandrod` `@dave-tucker` `@tamird` `@vadorovsky` (target maintainers mentioned in platform support document which will be added by rust-lang/rust#135107) r? `@Amanieu` `@rustbot` label +O-eBPF +A-inline-assembly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vadorovsky looks like this needs minor updates to land.
built from C code with [clang][clang]. It can be done using a `rustc-link-lib` | ||
instruction in `build.rs`. Example: | ||
|
||
```rust |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this wants a no_run?
---- /checkout/src/doc/rustc/src/platform-support/bpf-unknown-none.md - Tier__3::C_code (line 127) stdout ----
Test executable failed (exit status: 101).
stderr:
thread 'main' panicked at /checkout/src/doc/rustc/src/platform-support/bpf-unknown-none.md:5:35:
called `Result::unwrap()` on an `Err` value: NotPresent
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
expected to handle errors in a recoverable manner. Therefore most BPF programs | ||
written in Rust use the following no-op panic handler implementation: | ||
|
||
```rust,ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error: tidy error: /checkout/src/doc/rustc/src/platform-support/bpf-unknown-none.md:74: unexplained "```ignore" doctest; try one:
* make the test actually pass, by adding necessary imports and declarations, or
* use "```text", if the code is not Rust code, or
* use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or
* use "```should_panic", if the code is expected to fail at run time, or
* use "```no_run", if the code should type-check but not necessary linkable/runnable, or
* explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided.
Therefore, unit tests need to run on the host system. That requirement can be | ||
enforced by the following conditional check: | ||
|
||
```rust,ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error: tidy error: /checkout/src/doc/rustc/src/platform-support/bpf-unknown-none.md:95: unexplained "```ignore" doctest; try one:
* make the test actually pass, by adding necessary imports and declarations, or
* use "```text", if the code is not Rust code, or
* use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or
* use "```should_panic", if the code is expected to fail at run time, or
* use "```no_run", if the code should type-check but not necessary linkable/runnable, or
* explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided.
|
||
```toml | ||
[build] | ||
target = ["bpfel-unknown-none"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fails building the docs at stage2:
Documenting stage2 library {alloc, core, panic_abort, panic_unwind, proc_macro, std, sysroot, test, unwind} in HTML format (aarch64-apple-darwin -> bpfel-unknown-none)
The errors being, among others:
error[E0277]: the trait bound `System: core::alloc::GlobalAlloc` is not satisfied
--> std/src/alloc.rs:220:43
|
220 | unsafe { GlobalAlloc::dealloc(self, ptr.as_ptr(), layout) }
| -------------------- ^^^^ the trait `core::alloc::GlobalAlloc` is not implemented for `System`
| |
| required by a bound introduced by this call
error[E0277]: the trait bound `System: core::alloc::GlobalAlloc` is not satisfied
--> std/src/alloc.rs:270:52
|
270 | let raw_ptr = GlobalAlloc::realloc(self, ptr.as_ptr(), old_layout, new_size);
| -------------------- ^^^^ the trait `core::alloc::GlobalAlloc` is not implemented for `System`
| |
| required by a bound introduced by this call
error[E0599]: no method named `fetch_add` found for struct `AtomicUsize` in the current scope
--> std/src/panicking.rs:395:47
|
395 | let global_count = GLOBAL_PANIC_COUNT.fetch_add(1, Ordering::Relaxed);
| ^^^^^^^^^ method not found in `AtomicUsize`
error[E0599]: no method named `fetch_sub` found for struct `AtomicUsize` in the current scope
--> std/src/panicking.rs:419:28
|
419 | GLOBAL_PANIC_COUNT.fetch_sub(1, Ordering::Relaxed);
| ^^^^^^^^^ method not found in `AtomicUsize`
error[E0599]: no method named `fetch_or` found for struct `AtomicUsize` in the current scope
--> std/src/panicking.rs:427:28
|
427 | GLOBAL_PANIC_COUNT.fetch_or(ALWAYS_ABORT_FLAG, Ordering::Relaxed);
| ^^^^^^^^ method not found in `AtomicUsize
Could you also document how to prevent building docs for std and alloc here, which are not supported by the bpf targets? (without disabling docs entirely through --disable-docs
, and without dropping other targets from the same rustc that do successfully build std docs)
No description provided.