On Thu, May 4, 2023 at 11:54 PM Menglong Dong <menglong8.dong@xxxxxxxxx> wrote: > > On Fri, May 5, 2023 at 12:12 PM Andrii Nakryiko > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > On Thu, May 4, 2023 at 7:42 PM Menglong Dong <menglong8.dong@xxxxxxxxx> wrote: > > > > > > On Fri, May 5, 2023 at 12:53 AM Yonghong Song <yhs@xxxxxxxx> wrote: > > > > > > > > > > > > > > > > On 5/4/23 4:09 AM, Menglong Dong wrote: > > > > > Hello, > > > > > > > > > > I find that it's not supported yet to check if the bpf features are > > > > > supported by the target kernel in the BPF program, which makes > > > > > it hard to keep the BPF program compatible with different kernel > > > > > versions. > > > > > > > > > > For example, I want to use the helper bpf_jiffies64(), but I am not > > > > > sure if it is supported by the target, as my program can run in > > > > > kernel 5.4 or kernel 5.10. Therefore, I have to compile two versions > > > > > BPF elf and load one of them according to the current kernel version. > > > > > The part of BPF program can be this: > > > > > > > > > > #ifdef BPF_FEATS_JIFFIES64 > > > > > jiffies = bpf_jiffies64(); > > > > > #else > > > > > jiffies = 0; > > > > > #endif > > > > > > > > > > And I will generate xxx_no_jiffies.skel.h and xxx_jiffies.skel.h > > > > > with -DBPF_FEATS_JIFFIES64 or not. > > > > > > > > > > This method is too silly, as I have to compile 8(2*2*2) versions of > > > > > the BPF program if I am not sure if 3 bpf helpers are supported by the > > > > > target kernel. > > > > > > > > > > Therefore, I think it may be helpful if we can check if the helpers > > > > > are support like this: > > > > > > > > > > if (bpf_core_helper_exist(bpf_jiffies64)) > > > > > jiffies = bpf_jiffies64(); > > > > > else > > > > > jiffies = 0; > > > > > > > > > > And bpf_core_helper_exist() can be defined like this: > > > > > > > > > > #define bpf_core_helper_exist(helper) \ > > > > > __builtin_preserve_helper_info(helper, BPF_HELPER_EXISTS) > > > > > > > > > > Besides, in order to prevent the verifier from checking the helper > > > > > that is not supported, we need to remove the dead code in libbpf. > > > > > As the kernel already has the ability to remove dead and nop insn, > > > > > we can just make the dead insn to nop. > > > > > > > > > > Another option is to make the BPF program support "const value". > > > > > Such const values can be rewrite before load, the dead code can > > > > > be removed. For example: > > > > > > > > > > #define bpf_const_value __attribute__((preserve_const_value)) > > > > > > > > > > bpf_const_value bool is_bpf_jiffies64_supported = 0; > > > > > > > > > > if (is_bpf_jiffies64_supported) > > > > > jiffies = bpf_jiffies64(); > > > > > else > > > > > jiffies = 0; > > > > > > > > > > The 'is_bpf_jiffies64_supported' will be compiled to an imm, and > > > > > can be rewrite and relocated through libbpf by the user. Then, we > > > > > can make the dead insn 'nop'. > > > > > > > > A variant of the second approach should already work. > > > > You can do, > > > > > > > > volatile const is_bpf_jiffies64_supported; > > > > > > > > ... > > > > > > > > if (is_bpf_jiffies64_supported) > > > > you don't even have to use global variable to detect helper support, you can do: > > > > if (bpf_core_enum_value_exists(enum bpf_func_id, BPF_FUNC_jiffies64)) > > jiffies = bpf_jiffies64(); > > else > > jiffies = 0; > > Perfect! Now I don't even have to probe the helper support > in the user space. It maybe a good idea to introduce a: > > #define bpf_core_helper_exist(name) \ > bpf_core_enum_value_exists(enum bpf_func_id, BPF_FUNC_##name) > > to help the users who don't know this method. I'm hesitant because such a macro assumes that enum bpf_func_id (which presumably comes from user-supplied vmlinux.h) is up-to-date to contain all the interesting helpers. Such kind of assumptions make me question whether adding such macros as part of official APIs is the right choice. But using bpf_core_enum_value_exists() for checking helper support is explicitly called out ([0]) in the BPF CO-RE reference guide, so hopefully that makes it a bit more apparent to users. [0] https://nakryiko.com/posts/bpf-core-reference-guide/#bpf-core-enum-value-exists > > Thanks! > Menglong Dong > > > > > > > jiffies = bpf_jiffies64(); > > > > else > > > > jiffies = 0; > > > > > > > > > > > > After skeleton is openned but before prog load, you can do > > > > a probe into the kernel to find whether the helper is supported > > > > or not, and set is_bpf_jiffies64_supported accordingly. > > > > > > > > After loading the program, is_bpf_jiffies64_supported will be > > > > changed to 0/1, verifier will do dead code elimination properly. > > > > > > > > > > Great, that works! Thanks~ > > > > > > > > > > > > > What do you think? I'm not sure if these methods work and want > > > > > to get some advice before coding. > > > > > > > > > > Thanks! > > > > > Menglong Dong
