BPF List
 help / color / mirror / Atom feed
* bpf: add support to check kernel features in BPF program
@ 2023-05-04 11:09 Menglong Dong
  2023-05-04 16:53 ` Yonghong Song
  2023-05-05  7:51 ` zf
  0 siblings, 2 replies; 8+ messages in thread
From: Menglong Dong @ 2023-05-04 11:09 UTC (permalink / raw)
  To: Andrii Nakryiko, Alan Maguire; +Cc: bpf

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'.

What do you think? I'm not sure if these methods work and want
to get some advice before coding.

Thanks!
Menglong Dong

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: bpf: add support to check kernel features in BPF program
  2023-05-04 11:09 bpf: add support to check kernel features in BPF program Menglong Dong
@ 2023-05-04 16:53 ` Yonghong Song
  2023-05-05  2:42   ` Menglong Dong
  2023-05-05  7:51 ` zf
  1 sibling, 1 reply; 8+ messages in thread
From: Yonghong Song @ 2023-05-04 16:53 UTC (permalink / raw)
  To: Menglong Dong, Andrii Nakryiko, Alan Maguire; +Cc: bpf



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)
     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.

> 
> What do you think? I'm not sure if these methods work and want
> to get some advice before coding.
> 
> Thanks!
> Menglong Dong

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: bpf: add support to check kernel features in BPF program
  2023-05-04 16:53 ` Yonghong Song
@ 2023-05-05  2:42   ` Menglong Dong
  2023-05-05  4:12     ` Andrii Nakryiko
  0 siblings, 1 reply; 8+ messages in thread
From: Menglong Dong @ 2023-05-05  2:42 UTC (permalink / raw)
  To: Yonghong Song; +Cc: Andrii Nakryiko, Alan Maguire, bpf

On Fri, May 5, 2023 at 12:53 AM Yonghong Song <yhs@meta.com> 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)
>      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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: bpf: add support to check kernel features in BPF program
  2023-05-05  2:42   ` Menglong Dong
@ 2023-05-05  4:12     ` Andrii Nakryiko
  2023-05-05  6:54       ` Menglong Dong
  0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2023-05-05  4:12 UTC (permalink / raw)
  To: Menglong Dong; +Cc: Yonghong Song, Alan Maguire, bpf

On Thu, May 4, 2023 at 7:42 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> On Fri, May 5, 2023 at 12:53 AM Yonghong Song <yhs@meta.com> 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;

> >      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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: bpf: add support to check kernel features in BPF program
  2023-05-05  4:12     ` Andrii Nakryiko
@ 2023-05-05  6:54       ` Menglong Dong
  2023-05-05 16:58         ` Andrii Nakryiko
  0 siblings, 1 reply; 8+ messages in thread
From: Menglong Dong @ 2023-05-05  6:54 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yonghong Song, Alan Maguire, bpf

On Fri, May 5, 2023 at 12:12 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, May 4, 2023 at 7:42 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
> >
> > On Fri, May 5, 2023 at 12:53 AM Yonghong Song <yhs@meta.com> 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.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: bpf: add support to check kernel features in BPF program
  2023-05-04 11:09 bpf: add support to check kernel features in BPF program Menglong Dong
  2023-05-04 16:53 ` Yonghong Song
@ 2023-05-05  7:51 ` zf
  1 sibling, 0 replies; 8+ messages in thread
From: zf @ 2023-05-05  7:51 UTC (permalink / raw)
  To: Menglong Dong, Andrii Nakryiko, Alan Maguire; +Cc: bpf

在 2023/5/4 19:09, Menglong Dong 写道:
> 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'.
> 
> What do you think? I'm not sure if these methods work and want
> to get some advice before coding.
> 
> Thanks!
> Menglong Dong

Maybe you can use like this
bpf_core_enum_value_exists(enum bpf_func_id, BPF_FUNC_jiffies64);


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: bpf: add support to check kernel features in BPF program
  2023-05-05  6:54       ` Menglong Dong
@ 2023-05-05 16:58         ` Andrii Nakryiko
  2023-05-06  2:45           ` Menglong Dong
  0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2023-05-05 16:58 UTC (permalink / raw)
  To: Menglong Dong; +Cc: Yonghong Song, Alan Maguire, bpf

On Thu, May 4, 2023 at 11:54 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> On Fri, May 5, 2023 at 12:12 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, May 4, 2023 at 7:42 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
> > >
> > > On Fri, May 5, 2023 at 12:53 AM Yonghong Song <yhs@meta.com> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: bpf: add support to check kernel features in BPF program
  2023-05-05 16:58         ` Andrii Nakryiko
@ 2023-05-06  2:45           ` Menglong Dong
  0 siblings, 0 replies; 8+ messages in thread
From: Menglong Dong @ 2023-05-06  2:45 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: Yonghong Song, Alan Maguire, bpf

On Sat, May 6, 2023 at 12:58 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, May 4, 2023 at 11:54 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
> >
> > On Fri, May 5, 2023 at 12:12 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Thu, May 4, 2023 at 7:42 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
> > > >
> > > > On Fri, May 5, 2023 at 12:53 AM Yonghong Song <yhs@meta.com> 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.

You are right, it can confuse the users if the target helper
isn't present in the vmlinux.h.

>
> 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
>

This reference guide is awesome! I won't have these questions
if I have read it.

Thanks!
Menglong Dong

> >
> > 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-05-06  2:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-04 11:09 bpf: add support to check kernel features in BPF program Menglong Dong
2023-05-04 16:53 ` Yonghong Song
2023-05-05  2:42   ` Menglong Dong
2023-05-05  4:12     ` Andrii Nakryiko
2023-05-05  6:54       ` Menglong Dong
2023-05-05 16:58         ` Andrii Nakryiko
2023-05-06  2:45           ` Menglong Dong
2023-05-05  7:51 ` zf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox