Re: [PATCH] samples/bpf: Support both enter and exit kprobes in helper

[Lior Ribak <liorribak@gmail.com>]

On 8/15/20 11:02 PM, Yonghong Song wrote:
>On 8/15/20 12:57 PM, Lior Ribak wrote:
>> Currently, in bpf_load.c, the function write_kprobe_events sets
>> the function name to probe as the probe name.
>> Even though it's valid to set one kprobe on enter and another on exit,
>> bpf_load.c won't handle it, and will return an error 'File exists'.
>> 
>> Add a prefix to the event name to indicate if it's on enter or exit,
>> so both an enter and an exit kprobes can be attached.

>
>Only bpf_load.c change and no users here. So do you imply that
>use use this piece code in your own environment some how? But in
>that case, not sure whether this patch is necessary or not.
>
>The change here is for legacy bpf_load which may go away in the future.
>I understand this is for debugfs based kprobe_events where current
>libbpf does not support. But if possible, you should upgrade to use
>fd-based kprobe which is introduced roughly in/around 4.17 and libbpf 
>has proper support.

I used the samples and it's toolchain to write my own bpf, so i wrote this
patch to save the trouble to the next one who will try to register 2 
kprobes on the same function.
I still suggest to apply the patch because it solves a bug.

As I see it, all the wrappers around ebpf are duplicated everywhere, and
fd-based krpobe is already wrapped in the bcc project, so if you suggest
to switch and use it, maybe it's better to import some of the code from 
bcc instead

>
>> 
>> Signed-off-by: Lior Ribak <liorribak@gmail.com>
>> ---
>>   samples/bpf/bpf_load.c | 20 +++++++++++++-------
>>   1 file changed, 13 insertions(+), 7 deletions(-)
>> 
>> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
>> index c5ad528f046e..69102358e91a 100644
>> --- a/samples/bpf/bpf_load.c
>> +++ b/samples/bpf/bpf_load.c
>> @@ -184,18 +184,24 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
>>   
>>   #ifdef __x86_64__
>>           if (strncmp(event, "sys_", 4) == 0) {
>> -         snprintf(buf, sizeof(buf), "%c:__x64_%s __x64_%s",
>> -             is_kprobe ? 'p' : 'r', event, event);
>> +         if (is_kprobe)
>> +             event_prefix = "__x64_enter_";
>> +         else
>> +             event_prefix = "__x64_exit_";
>> +         snprintf(buf, sizeof(buf), "%c:%s%s __x64_%s",
>> +             is_kprobe ? 'p' : 'r', event_prefix, event, event);
>>               err = write_kprobe_events(buf);
>> -         if (err >= 0) {
>> +         if (err >= 0)
>>                   need_normal_check = false;
>> -             event_prefix = "__x64_";
>> -         }
>>           }
>>   #endif
>>           if (need_normal_check) {
>> -         snprintf(buf, sizeof(buf), "%c:%s %s",
>> -             is_kprobe ? 'p' : 'r', event, event);
>> +         if (is_kprobe)
>> +             event_prefix = "enter_";
>> +         else
>> +             event_prefix = "exit_";
>> +         snprintf(buf, sizeof(buf), "%c:%s%s %s",
>> +             is_kprobe ? 'p' : 'r', event_prefix, event, event);
>>               err = write_kprobe_events(buf);
>>               if (err < 0) {
>>                   printf("failed to create kprobe '%s' error '%s'\n",
>>