-
Notifications
You must be signed in to change notification settings - Fork 79
Remove Rust/Aya-based toolchain #869
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
Conversation
Could you run some end-to-end testing with Parca Demo (or some other test program you trust) to double-check that everything works fine? 😄 |
686f313
to
de53aba
Compare
I have already done that. We do another run. After see everything ok with actions. |
I'll check the failures soon. |
1d28fee
to
9f91aeb
Compare
The CodeQL action is failing, but seems we can remove the LLVM bitcode generation (see below), and just generate the BPF object code
|
|
||
.PHONY: clean | ||
clean: | ||
cargo clean | ||
rm -f $(OUT_BPF) |
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.
nit: What do you think about also adding - rm -rf target/
so when people rebase on this branch the targets/ directory that might have been created by the previous build is removed, too?
-xc \ | ||
-nostdinc \ | ||
-target bpf \ | ||
-O2 -emit-llvm -c -g $< -o $(@:.o=.ll) |
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.
We can do this in one step without requiring the external linker binary
something like:
-O2 -emit-llvm -c -g $< -o $(@:.o=.ll) | |
-O2 -c -g $< -o $(@:.o) |
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.
Have we already used any branches in the agent? or any other projects?
-nostdinc \ | ||
-target bpf \ | ||
-O2 -emit-llvm -c -g $< -o $(@:.o=.ll) | ||
$(CMD_LLC) -march=bpf -filetype=obj -o $@ $(@:.o=.ll) |
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.
And we wouldn't need llc
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.
LGTM!
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
9f91aeb
to
72b552d
Compare
Signed-off-by: Kemal Akkoyun kakkoyun@gmail.com
This PR reverts the changes that introduced in #377.
We are basically removing the Rust, Aya-based toolchain for the time being to be able to deliver our goals quicker. Aya has some rough edges such as aya-rs/aya#351, we were missing certain APIs to be able to write complex eBPF programs (programs to solves issues such as #768).
This being said, we will still continue to spent our innovation tokens on Rust and Aya for the further. The team still wants to pursue the path when the conditions are ripe.