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

Would like to accept patches that add BTF/BPF support? #133

Open
inclyc opened this issue Dec 26, 2023 · 19 comments
Open

Would like to accept patches that add BTF/BPF support? #133

inclyc opened this issue Dec 26, 2023 · 19 comments

Comments

@inclyc
Copy link
Contributor

inclyc commented Dec 26, 2023

Recently I made patches for enable BTF & BPF in kernel config for eBPF programs. Would you like to accept such patches?

This basically changes the kernel config file, and it is not enabled by our upstream (asahi linux) by default, but enabled in NixOS.

Link: inclyc@18fe3ff

I can make the draft patch cleaner if the idea gets approved, thanks!

@zzywysm
Copy link
Contributor

zzywysm commented Dec 28, 2023

I added BPF support in #134

I don't think BTF is going to be useful for most users.

@inclyc
Copy link
Contributor Author

inclyc commented Dec 28, 2023

I don't think BTF is going to be useful for most users.

But most desktop distributions enable this option (e.g. Debian/Arch/NixOS (nixpkgs)) and I do need this feature. Why asahi - NixOS is special among these distributions / or our upstream NixOS ?

@zzywysm
Copy link
Contributor

zzywysm commented Dec 30, 2023

BTF is really great if you're developing BPF programs. BPF programs are intended to run within the kernel itself.

In other words, if you find BTF useful, you're likely to be a kernel developer, which means you probably know how to enable DEBUG_INFO_BTF within your NixOS kernel configuration.

By leaving it off, we avoid generating a bunch of debug crud that most folks will never use or care about.

@inclyc
Copy link
Contributor Author

inclyc commented Dec 30, 2023

In other words, if you find BTF useful, you're likely to be a kernel developer, which means you probably know how to enable DEBUG_INFO_BTF within your NixOS kernel configuration.

Yes, I have enabled this in my own fork. So I just kindly ask may I merge this setting into upstream.

By leaving it off, we avoid generating a bunch of debug crud that most folks will never use or care about.

Just FYI NixOS enabled this option by default. This is not a useless option but necessary for many BPF tools.

This is used by bpftrace and lots of new eBPF-based tooling to avoid a dependency
on LLVM on the host.

https://github.com/NixOS/nixpkgs/blob/98a0c372a314729fa4d811063ea2ad7f230b2f9b/pkgs/os-specific/linux/kernel/common-config.nix#L47

Did you mean that NixOS (in nixpkgs) is generating "a bunch of debug crud that most folks will never use or care about"?

never use or care about.

We introduced BTF in nixpkgs in this patch: NixOS/nixpkgs#127922

And quote:

BTF debug information is enabled on all major distributions: Fedora 31+,
RHEL 8.2+, Ubuntu 20.10, Debian 11 and ArchLinux all have enabled it.

I just cannot image that such "useless" feature is enabled by default in those major distros.

@zzywysm
Copy link
Contributor

zzywysm commented Dec 30, 2023

I just cannot image that such "useless" feature is enabled by default in those major distros.

Then you should take a closer look at some of the major distribution's kernel configurations. Holy mackerel do they enable a lot of stuff that will never ever be used. The Fedora Asahi Remix kernel config is an excellent example -- dozens if not hundreds of useless drivers!

https://copr-dist-git.fedorainfracloud.org/cgit/@asahi/kernel/kernel.git/tree/kernel-aarch64-16k-fedora.config?h=f39

This reality gets even more offensive when you learn about return-oriented progamming, and realize that this extra code is giving attackers more gadgets to play with.

https://en.wikipedia.org/wiki/Return-oriented_programming

@inclyc
Copy link
Contributor Author

inclyc commented Dec 30, 2023

Then you should take a closer look at some of the major distribution's kernel configurations. Holy mackerel do they enable a lot of stuff that will never ever be used. The Fedora Asahi Remix kernel config is an excellent example -- dozens if not hundreds of useless drivers!

I totally agree that these useless drivers should not be enabled in our (asahi-linux) kernel config, and thanks for you excellent patch to address this!

However my point is: BTF is not a "useless" feature, major BPF tools do utilize BTF. e.g.

bpftrace: https://github.com/iovisor/bpftrace/blob/1c5fac13d75ba250dfb3f51b3367571021867228/docs/reference_guide.md?plain=1#L3862
libbpf: https://github.com/andreasgerstmayr/bcc/blob/5f748492a3d2ae95fb6c2934b0251e1d5b92dac3/libbpf-tools/README.md?plain=1#L96

This reality gets even more offensive when you learn about return-oriented progamming, and realize that this extra code is giving attackers more gadgets to play with.

That's a good point! I suppose you are talking about those drivers, not BTF? In my humble opinion "debug info" does not affect compiled code at all.

@inclyc
Copy link
Contributor Author

inclyc commented Dec 30, 2023

Also kindly ping @tpwrules , WDYT?

@psanford
Copy link

The only thing that makes sense long term is to use the default nixpkgs aarch64 kernel config + asahi specific modules. That will give the most users the behavior that they expect (for drivers to be available without requiring a custom config).

Folks who want non-standard kernel options can then do that themselves using normal nix kernel configuration.

@zzywysm
Copy link
Contributor

zzywysm commented Jan 3, 2024

The only thing that makes sense long term is to use the default nixpkgs aarch64 kernel config + asahi specific modules. That will give the most users the behavior that they expect (for drivers to be available without requiring a custom config).

Folks who want non-standard kernel options can then do that themselves using normal nix kernel configuration.

When NixOS on Apple Silicon is officially supported by the NixOS/nixpkgs organization, then I agree with your logic 100%. I believe that @tpwrules has talked with them about that possibility, and I imagine they are likely waiting on the Asahi Linux code to get upstreamed into the vanilla Linux kernel. (There are still hundreds of commits that haven't been rebased and submitted for consideration by Hector and friends.). Perhaps this project can become part of the nixos-hardware project at that time.

https://github.com/NixOS/nixos-hardware
(Note the first line of the nixos-hardware README is "NixOS profiles to optimize settings for different hardware." Eeeeew, who would ever want that?! 🤣)

But this is a project that is specifically about NixOS on Apple Silicon. At this point I don't understand the concern about making every conceivable driver available -- we should endeavor to enable the drivers for hardware that folks might plug into their Mac, and make it easy for power users and developers to enable the options they need, e.g. DEBUG_INFO_BTF. I think this project is succeeding at that.

This project already ships a custom sound system, a custom OpenGL codebase, a custom bootloader stack, and a custom installation ISO image. And yet @psanford (and @tpwrules elsewhere) are very concerned that our kernel isn't standard enough. To which I respond: huh?!

For reference, I am attaching the default NixOS arm64 kernel configuration, which I obtained by creating a NixOS VM inside QEMU/KVM, injecting the IKCONFIG option into the kernel and rebuilding it, then after a reboot running zcat /proc/config.gz > hotmess.txt . Note that just the size of the config file itself is almost twice the size of the configs I suggest in my pull requests. It has support for every conceivable arm64 SoC that Linux itself supports, even though this project will never boot on those SoCs, nor is it intended to. The technical term for this is "bloat", and it makes @tpwrules' build process take longer, and it fills up your EFI System Partition really fast.

hotmess.txt

@psanford
Copy link

psanford commented Jan 3, 2024

Note that just the size of the config file itself is almost twice the size of the configs I suggest in my pull requests. It has support for every conceivable arm64 SoC that Linux itself supports, even though this project will never boot on those SoCs, nor is it intended to. The technical term for this is "bloat", and it makes @tpwrules' build process take longer, and it fills up your EFI System Partition really fast.

Not really. The reason we compile most things as kernel modules is so that we don't have to load them if they are not needed, but they are available on demand. Its also why we only include the required modules in the initrd. Looking at a stock nixos amd64 install, the kernel images and initrd files are roughly the same size as my nixos-apple-silicon install.

The main benefit to using the stock nixos kernel config with asahi specific changes applied is that it lowers the ongoing maintenance cost. We don't need to have constant discussions/debates about what should or should not be enabled.

If you want to run an optimized kernel, squeezing out those last few kb of kernel memory you can, and nixos makes that very easy to do. But for most folks the default kernel will give the expected experience of installing linux and having all the drivers available.

@zzywysm
Copy link
Contributor

zzywysm commented Jan 3, 2024

Not really. The reason we compile most things as kernel modules is so that we don't have to load them if they are not needed, but they are available on demand. Its also why we only include the required modules in the initrd. Looking at a stock nixos amd64 install, the kernel images and initrd files are roughly the same size as my nixos-apple-silicon install.

This still doesn't explain why it's desirable to use the NixOS default config at this time. I doubt that GitHub is thrilled about us running our builds on their infrastructure when a lot of the compiling happening is completely pointless. Or should we be wasting more electricity, not less?

The main benefit to using the stock nixos kernel config with asahi specific changes applied is that it lowers the ongoing maintenance cost. We don't need to have constant discussions/debates about what should or should not be enabled.

This particular complaint is not directed at you, @psanford, but I am making it in a reply to you. My bad.

You worry about maintenance cost. I have been offering my time to do maintenance of this project's kernel situation for months now. Often my pull requests are closed and not pulled. Currently I have two pull requests sitting for 5-7 days without any reply. If we are concerned about maintenance cost, why are y'all disrespecting free maintenance?

If you want to run an optimized kernel, squeezing out those last few kb of kernel memory you can, and nixos makes that very easy to do. But for most folks the default kernel will give the expected experience of installing linux and having all the drivers available.

In my experience spending days enabling and disabling config options, rebuilding kernels, and occasionally noticing very visible performance penalties, you can quite easily make wrong choices. Which choices has NixOS made that aren't very good? Do you know? Do you know how to find out? Do you really think that NixOS on arm64 gets anywhere the same tender love & care that amd64 receives?

@psanford
Copy link

psanford commented Jan 3, 2024

Currently I have two pull requests sitting for 5-7 days without any reply.

Reviewing PRs is a burden on maintainers. If you use the stock kernel config that is fewer ongoing PRs to review. It also fixes issues like this one where a default config option (enabling BPF/BTF) is present. That's the maintenance cost I'm talking about.

In my experience spending days enabling and disabling config options, rebuilding kernels, and occasionally noticing very visible performance penalties, you can quite easily make wrong choices. Which choices has NixOS made that aren't very good? Do you know? Do you know how to find out? Do you really think that NixOS on arm64 gets anywhere the same tender love & care that amd64 receives?

As a much younger person (2003-2004), I ran gentoo and spent way too much time "optimizing" my kernel and compiler settings. For all intents and purposes, that was a complete waste of time.

My professional career involves running linux systems at scale where small performance improvements can save millions of dollars. I in fact know quite a bit about systems optimization. The correct way to do performance optimization requires a rigorous methodology. You need to have well defined workloads with measurable performance characteristics. Any change you make for the sake of performance needs to be justified by demonstrating the actual measured change in those metrics.

None of your PRs have included any actual performance metrics or reproducible test cases. I can't tell from your PRs if they make performance better or worse because you have not measured anything.

It seems like you are really excited to contribute to this project. I don't want to discourage that, but I would like to nudge you in the direction of contributions that are more likely to be accepted. I think a PR for using the stock nixos kernel config is more likely to be accepted than your past kernel PRs (I don't speak for tpwrules so I might be wrong here).

In any case, I've said everything I care to on this topic so I'm going to leave this conversation here. I hope you are able to find productive ways to contribute to the project. Cheers!

@zzywysm
Copy link
Contributor

zzywysm commented Jan 4, 2024

That's a good point! I suppose you are talking about those drivers, not BTF? In my humble opinion "debug info" does not affect compiled code at all.

From the kernel page on the NixOS wiki (which I swear I didn't put there):

Troubleshooting
Build fails
Too high ram usage

turn off `DEBUG_INFO_BTF`

(Source: https://nixos.wiki/wiki/Kernel )

Clearly this is something everyone wants on their Mac which has 8GB RAM in the most common configuration.

@inclyc
Copy link
Contributor Author

inclyc commented Jan 5, 2024

This reality gets even more offensive when you learn about return-oriented progamming, and realize that this extra code is giving attackers more gadgets to play with.

That's a good point! I suppose you are talking about those drivers, not BTF? In my humble opinion "debug info" does not affect compiled code at all.

Clearly this is something everyone wants on their Mac which has 8GB RAM in the most common configuration.

Your previous discussion was about security issues. What I want to point out is that adding debugging information does not cause any CVEs you mentioned. The link you provided, as per my understanding, seems to only reduce memory consumption during "compile time" instead of "compiled code".

Clearly this is something everyone wants on their Mac which has 8GB RAM in the most common configuration.

Regarding reducing memory consumption during the compile time, I think @psanford has already had a detailed & professional discussion with you. I also kindly suggest you considering his opinions.

My professional career involves running linux systems at scale where small performance improvements can save millions of dollars. I in fact know quite a bit about systems optimization. The correct way to do performance optimization requires a rigorous methodology. You need to have well defined workloads with measurable performance characteristics. Any change you make for the sake of performance needs to be justified by demonstrating the actual measured change in those metrics.

@zzywysm
Copy link
Contributor

zzywysm commented Jan 5, 2024

Your previous discussion was about security issues. What I want to point out is that adding debugging information does not cause any CVEs you mentioned. The link you provided, as per my understanding, seems to only reduce memory consumption during "compile time" instead of "compiled code".

  1. I hesitate to share this next piece of information with you, because you may find it novel and shocking, but an idea can be bad for multiple reasons. If you tell me you're visiting the top of a volcano on vacation, and I warn that you might fall in, there is still a possibility the volcano will erupt.
  2. From tpwrules' own mouth in the installation guide:
    Building the image requires downloading of a large amount of data and compilation of a number of packages, including the kernel.

@inclyc
Copy link
Contributor Author

inclyc commented Jan 5, 2024

@zzywysm
Copy link
Contributor

zzywysm commented Jan 5, 2024

IIRC systemd's next version depends on BTF. Just for a note.

https://github.com/systemd/systemd/pull/26826/files#diff-30d8f6be6320feeacf686be94f48c70869b52630e01ea625f0f15adc0d57c3e4R1856

If by "depends on BTF" you mean "systemd will try to use BTF as a fallback if it can't find a vmlinux image using its primary preferred method", then yes, you are correct.

Or in other words: read the large comment above the line you linked to.

@rowanG077
Copy link
Contributor

rowanG077 commented Jan 5, 2024

Please desalinate the comments here. This is just a discussion on whether to include some features. No reason to move this discussion in such a hostile direction.

@yu-re-ka
Copy link
Contributor

yu-re-ka commented Feb 16, 2024

The only thing that makes sense long term is to use the default nixpkgs aarch64 kernel config + asahi specific modules. That will give the most users the behavior that they expect (for drivers to be available without requiring a custom config).

If you are on nixos-unstable, you can try my branch https://github.com/yu-re-ka/nixos-m1/tree/minimize-patches which does exactly this.

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

No branches or pull requests

5 participants