Hello, Alan On 2022/3/31 5:27 PM, Alan Maguire wrote: > On Wed, 30 Mar 2022, Hengqi Chen wrote: > >> >> >> On 2022/3/30 10:50 AM, Andrii Nakryiko wrote: >>> On Tue, Mar 29, 2022 at 7:30 PM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote: >>>> >>>> Hello, Andrii >>>> >>>> On 2022/3/30 7:18 AM, Andrii Nakryiko wrote: >>>>> On Sat, Mar 26, 2022 at 7:43 AM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote: >>>>>> >>>>>> On some old kernels, kprobe auto-attach may fail when attach to symbols >>>>>> like udp_send_skb.isra.52 . This is because the kernel has kprobe PMU >>>>>> but don't allow attach to a symbol with '.' ([0]). Add a new option to >>>>>> bpf_kprobe_opts to allow using the legacy kprobe attach directly. >>>>>> This way, users can use bpf_program__attach_kprobe_opts in a dedicated >>>>>> custom sec handler to handle such case. >>>>>> >>>>>> [0]: https://github.com/torvalds/linux/blob/v4.18/kernel/trace/trace_kprobe.c#L340-L343 >>>>>> >>>>>> Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx> >>>>>> --- >>>>> >>>>> It's sad, but it makes sense. But, let's have a selftests that >>>>> validates uses legacy option explicitly (e.g., in >>>>> prog_tests/attach_probe.c). Also, let's fix this limitation in the >>>> >>>> OK, will add a selftest to exercise the new option. >>>> >>>>> kernel? It makes no sense to limit attaching to a proper kallsym >>>>> symbol. >>>> >>>> This limitation is lifted in newer kernel. Kernel v5.4 don't have this issue. >>> >>> Oh, ok. So how about another plan of attack then: if kprobe target >>> function has '.' *and* we are on the kernel that doesn't support that, >>> switch to legacy kprobe automatically? No need for a new option, >>> libbpf handles this transparently. >>> >> >> That's better, and also eliminate the need for custom SEC() handler. >> >>> Still need a test for kprobe with '.' in it, though not sure how >>> reliable that will be... We can use kallsyms cache to check if >>> expected xxx.isra.0 (or whatever) is present, and if not - skip >>> subtest? >>> >> >> Not sure how to do that. Even if such symbol exists, how to reliably >> trigger it is another problem. >> > > could we add a function to bpf testmod that is easily triggered > and likely to be .isra-ed maybe? > > Experimenting, the following function becomes .isra-ed at when > compiled with -fipa-sra: > > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > b/tools/testing/selftests/bpf/bpf > index e585e1c..bb51e21 100644 > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > @@ -88,6 +88,17 @@ __weak noinline struct file *bpf_testmod_return_ptr(int > arg) > } > } > > +struct testisra { > + int val1; > + int val2; > + int val3; > +}; > + > +static noinline void bpf_testmod_test_isra(struct testisra *t, int val1, > int val2) > +{ > + t->val3 = val1 + val2; > +} > + > noinline ssize_t > bpf_testmod_test_read(struct file *file, struct kobject *kobj, > struct bin_attribute *bin_attr, > @@ -98,8 +109,14 @@ __weak noinline struct file > *bpf_testmod_return_ptr(int arg) > .off = off, > .len = len, > }; > + struct testisra t = { > + .val1 = off, > + .val2 = len > + }; > int i = 1; > > + bpf_testmod_test_isra(&t, t.val1, t.val2); > + > while (bpf_testmod_return_ptr(i)) > i++; > > > Tested on gcc 9; possibly different results on different versions.. > > Alan Thanks for the pointer, will give it a try. Hengqi
