Skip to content

Support wasm architecture #1552

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support wasm architecture #1552

wants to merge 1 commit into from

Conversation

alban
Copy link
Contributor

@alban alban commented Aug 26, 2024

cilium/ebpf currently does not compile to wasm with tinygo. This patch makes it compile.

I am working on a website where users can submit ebpf binaries (either in ELF format or as an Inspektor Gadget export) and the website will parse the ebpf binary client-side with wasm. The wasm code is written in Go, using cilium/ebpf and is compiled with tinygo.

My wasm code uses ebpf.LoadCollectionSpecFromReader() to display information about the ebpf binary. But it will not call ebpf.NewCollection() because the wasm/javascript environment in the browser cannot interact with the Linux kernel.


The website PoC looks like this:

image

Since cilium/ebpf is running in the browser, I can use Chrome DevTools to profile the performance:

Screenshot-Chrome-DevTools-cilium-ebpf

cilium/ebpf currently does not compile to wasm with tinygo. This patch
makes it compile.

I am working on a website where users can submit ebpf binaries (either
in ELF format or as an Inspektor Gadget export) and the website will
parse the ebpf binary client-side with wasm. The wasm code is written in
Go, using cilium/ebpf and is compiled with tinygo.

My wasm code uses ebpf.LoadCollectionSpecFromReader() to display
information about the ebpf binary. But it will not call
ebpf.NewCollection() because the wasm/javascript environment in the
browser cannot interact with the Linux kernel.

Signed-off-by: Alban Crequy <[email protected]>
@alban alban marked this pull request as ready for review August 26, 2024 16:27
@alban alban requested review from dylandreimerink and a team as code owners August 26, 2024 16:27
@alban
Copy link
Contributor Author

alban commented Aug 26, 2024

I cleaned up the code and marked the PR as ready.

Wasm is little endian and has a page size of 64 KiB according to its spec.

Copy link
Collaborator

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Hi Alban, very cool project! Note that we don't test wasm builds so this can regress at any time in the future. Feel free to add wasm builds to CI, and maybe create an issue to get (part of) the test suite to run on wasm.

For ebpf-on-windows, we're looking at the opposite: there are no CollectionSpecs on windows, so no ELF loader needed. (They ship bpf in signed winPE instead) Tests that load an ELF and put it into the kernel need to be skipped there. We'll need some generic mechanism to skip tests like we do for kernel versions etc.

@lmb
Copy link
Collaborator

lmb commented Oct 17, 2024

I blown away that this works, it's a very cool idea!

Note that we don't test wasm builds so this can regress at any time in the future. Feel free to add wasm builds to CI, and maybe create an issue to get (part of) the test suite to run on wasm.

Before going down this route: adding wasm to CI kind of implies that we will start maintaining a wasm "port". I personally don't have the capacity to do that. I really like this kind of work, but adding wasm support feels like a very steep price to pay to (essentially) get access to the elf reader.

Maybe a more maintainable solution would be to split the elf reader into a separate, cross-platform package? This is in line with the design goals we have for it. It would also mean moving a lot of code around unfortunately, and might get stuck in circular dependency hell.

@ti-mo
Copy link
Collaborator

ti-mo commented Oct 24, 2024

Before going down this route: adding wasm to CI kind of implies that we will start maintaining a wasm "port". I personally don't have the capacity to do that. I really like this kind of work, but adding wasm support feels like a very steep price to pay to (essentially) get access to the elf reader.

If it largely works as-is with a few lines of changes, is there a real downside? If we get some tests to run on tinygo (you just added the infra for it! 😉), that could be a good way of keeping the elf reader hermetic. (e.g. no touching /proc during ELF->CollectionSpec parsing, see recent example: 7f04cae, this would've broken Alban's use case as well)

This pales in comparison to, let's say, adding ebpf-for-windows support. 😉

Maybe a more maintainable solution would be to split the elf reader into a separate, cross-platform package? This is in line with the design goals we have for it.

Not sure what you mean in practice, but we could potentially move some ELF reader logic into platform-specific internal packages. If you mean moving CollectionSpec and friends into a subpackage, that ship has long sailed I think.

if !wanted.AssignableTo(typeInterface) {
return fmt.Errorf("%T does not satisfy Type interface", typ)
switch wanted {
case reflect.TypeOf((**Datasec)(nil)).Elem():
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a bummer I think. How would we prevent this list from ever expanding as the ELF reader changes?

@ti-mo ti-mo changed the title [RFC] New architecture: wasm Support wasm architecture Feb 25, 2025
@@ -27,7 +27,14 @@ var (
// invalidBPFObjNameChar returns true if char may not appear in
// a BPF object name.
func invalidBPFObjNameChar(char rune) bool {
dotAllowed := objNameAllowsDot() == nil
var dotAllowed bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

@alban This code is now gone, you can drop this change.

@ti-mo
Copy link
Collaborator

ti-mo commented Apr 2, 2025

Hi @alban, any progress here? Are the Getpagesize() changes still required?

@lmb
Copy link
Collaborator

lmb commented Apr 2, 2025

Getpagesize is fixed, the reflection changes are the only blocker. I'd be happy to swap our GOOS=darwin compile check in CI to wasm if we could somehow make that less painful.

@ti-mo
Copy link
Collaborator

ti-mo commented Apr 3, 2025

I'm a newbie, but this was all that's needed to make GOOS=js GOARCH=wasm go build ./... work:

diff --git docs/examples/variables/variables_bpfel.go docs/examples/variables/variables_bpfel.go
index f091b898..469e6f7b 100644
--- docs/examples/variables/variables_bpfel.go
+++ docs/examples/variables/variables_bpfel.go
@@ -1,5 +1,5 @@
 // Code generated by bpf2go; DO NOT EDIT.
-//go:build 386 || amd64 || arm || arm64 || loong64 || mips64le || mipsle || ppc64le || riscv64
+//go:build 386 || amd64 || arm || arm64 || loong64 || mips64le || mipsle || ppc64le || riscv64 || wasm

 package main

diff --git internal/endian_le.go internal/endian_le.go
index 6dcd916d..d833ea76 100644
--- internal/endian_le.go
+++ internal/endian_le.go
@@ -1,4 +1,4 @@
-//go:build 386 || amd64 || amd64p32 || arm || arm64 || loong64 || mipsle || mips64le || mips64p32le || ppc64le || riscv64
+//go:build 386 || amd64 || amd64p32 || arm || arm64 || loong64 || mipsle || mips64le || mips64p32le || ppc64le || riscv64 || wasm

 package internal

Following the official wasm guide (https://go.dev/wiki/WebAssembly), GOOS=js GOARCH=wasm go test . also runs after installing wasmbrowsertest, albeit with a bunch of failures due to things not being implemented in wasm.

Just curious, what's the benefit of using tinygo for wasm? Is it more mature than the official Go implementation?

@alban
Copy link
Contributor Author

alban commented Apr 3, 2025

I have not worked on this topic since opening the PR, and we will likely not need the wasm support in cilium/ebpf anymore, sorry. Feel free to either close this PR or take it over.

@KapilSareen is taking over the work on analysing ELF BPF programs using cilium/ebpf (inspektor-gadget/inspektor-gadget#4259) but that is done without using WASM.

I'm a newbie, but this was all that's needed to make GOOS=js GOARCH=wasm go build ./... work:
[changes in variables_bpfel.go and endian_le.go]

It seems reasonable to make a PR with just those 2 changes to make it work for the official Go implementation. And leave the tinygo-specific changes for later, if someone ever wants to revisit the tinygo support.

Just curious, what's the benefit of using tinygo for wasm? Is it more mature than the official Go implementation?

It was more mature before, but I am not sure it is still the case. It still produces smaller binaries. But the official Go implementation improved a lot. In Inspektor Gadget, we recently switched to the official Go implementation (inspektor-gadget/inspektor-gadget#4130) because it is faster than tinygo. But IG uses WASM for things unrelated to cilium/ebpf.

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.

4 participants