search

foniod / redbpf

Rust library for building and running BPF/eBPF modules
Apache License 2.0
1.71k stars 136 forks source link

sk_lookup loading support #216

Closed raftario closed 3 years ago

raftario commented 3 years ago

This pull request is pretty simple and just adds support for loading sk_lookup programs to the redbpf crate. I'm opening it as a draft as

  1. Support hasn't been added to the rest of the crates 2. sk_lookup requires a relatively recent kernel version (5.9) and merging this would probably break things for many people

Point 1 could be addressed by simply building upon this to add support everywhere else (I'll probably do it at some point but I don't personally need it at the moment), while for point 2 it might be interesting to consider feature-gating different program types.

rhdxmr commented 3 years ago

Hello @raftario

This feature looks cool!

Thanks to this PR I got to know about sk lookup feature. I just read this article: https://lwn.net/Articles/819618/ (I don't understand all yet)

IMO it's okay to include new kernel features in RedBPF. Even though sk lookup feature exists in RedBPF, users who don't use this feature can continue to use RedBPF with the older Linux kernel. Because nothing would be executed unless they call the new functions.

If users have problems while they try calling the sk lookup functions with the older kernel, users should handle their kernel version issue. But brief note about the minimum kernel version for sk lookup would be helpful to users.

And you concerned that you implemented only the userspace part of sk lookup. Don't worry about this. Somebody or you can freely implement the half part for BPF programs later.

raftario commented 3 years ago

I'm not particularly familiar with the library so I was probably confused. I just assumed that since bpf-sys uses bindgen, and bpf.h in libbpf includes linux/bpf.h, the bindings for sk_lookup types would not be generated on older kernels since they aren't present in the kernel headers, and useing them from redbpf would fail to compile on those systems.

raftario commented 3 years ago

Would you be interested in merging this without support in the other crates ?

rhdxmr commented 3 years ago

@raftario Yes, I welcome your contribution. API for BPF programs can be implemented later. Before this PR get merged to main branch, we have to make sure userspace API clear.

I wonder how you used this API. Can you add some example as a doc comment in the SkLookup structure?

raftario commented 3 years ago

I'll add some documentation and convert to a proper PR then !