All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jackie Liu <liu.yun@linux.dev>
To: Jiri Olsa <olsajiri@gmail.com>
Cc: andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
	yhs@fb.com, bpf@vger.kernel.org, liuyun01@kylinos.cn
Subject: Re: [PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions
Date: Wed, 24 May 2023 09:19:48 +0800	[thread overview]
Message-ID: <eab45de6-f5cd-c500-e6b7-940540fa047a@linux.dev> (raw)
In-Reply-To: <f3b21f27-a284-a42c-8636-181e24c325fd@linux.dev>

Hi Jiri.

在 2023/5/24 09:03, Jackie Liu 写道:
> Hi Jiri.
> 
> 在 2023/5/24 00:17, Jiri Olsa 写道:
>> On Tue, May 23, 2023 at 09:25:47PM +0800, Jackie Liu wrote:
>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>
>>> When using regular expression matching with "kprobe multi", it scans all
>>> the functions under "/proc/kallsyms" that can be matched. However, 
>>> not all
>>> of them can be traced by kprobe.multi. If any one of the functions fails
>>> to be traced, it will result in the failure of all functions. The best
>>> approach is to filter out the functions that cannot be traced to ensure
>>> proper tracking of the functions.
>>>
>>> But, the addition of these checks will frequently probe whether a 
>>> function
>>> complies with "available_filter_functions" and ensure that it has not 
>>> been
>>> filtered by kprobe's blacklist. As a result, it may take a longer time
>>> during startup. The function implementation is referenced from BCC's
>>> "kprobe_exists()"
>>>
>>> Here is the test eBPF program [1].
>>> [1] 
>>> https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
>>>
>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>> ---
>>>   tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 47 insertions(+)
>>>
>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>> index ad1ec893b41b..6a201267fa08 100644
>>> --- a/tools/lib/bpf/libbpf.c
>>> +++ b/tools/lib/bpf/libbpf.c
>>> @@ -10421,6 +10421,50 @@ struct kprobe_multi_resolve {
>>>       size_t cnt;
>>>   };
>>> +static bool filter_available_function(const char *name)
>>> +{
>>> +    char addr_range[256];
>>> +    char sym_name[256];
>>> +    FILE *f;
>>> +    int ret;
>>> +
>>> +    f = fopen("/sys/kernel/debug/kprobes/blacklist", "r");
>>> +    if (!f)
>>> +        goto avail_filter;
>>> +
>>> +    while (true) {
>>> +        ret = fscanf(f, "%s %s%*[^\n]\n", addr_range, sym_name);
>>> +        if (ret == EOF && feof(f))
>>> +            break;
>>> +        if (ret != 2)
>>> +            break;
>>> +        if (!strcmp(name, sym_name)) {
>>> +            fclose(f);
>>> +            return false;
>>> +        }
>>> +    }
>>> +    fclose(f);
>>
>> so available_filter_functions already contains all traceable symbols
>> for kprobe_multi/fprobe
>>
>> kprobes/blacklist is kprobe specific and does not apply to fprobe,
>> is there a crash when attaching function from kprobes/blacklist ?
> 
> No, I haven't got crash before, Simply because BCC's kprobe_exists has
> implemented it so I added this, Yes, I also don't think 
> kprobes/blacklist will affect FPROBE, so I will remove it.
> 
>>
>>> +
>>> +avail_filter:
>>> +    f = 
>>> fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
>>> +    if (!f)
>>> +        return true;
>>> +
>>> +    while (true) {
>>> +        ret = fscanf(f, "%s%*[^\n]\n", sym_name);
>>> +        if (ret == EOF && feof(f))
>>> +            break;
>>> +        if (ret != 1)
>>> +            break;
>>> +        if (!strcmp(name, sym_name)) {
>>> +            fclose(f);
>>> +            return true;
>>> +        }
>>> +    }
>>> +    fclose(f);
>>> +    return false;
>>> +}
>>> +
>>>   static int
>>>   resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
>>>               const char *sym_name, void *ctx)
>>> @@ -10431,6 +10475,9 @@ resolve_kprobe_multi_cb(unsigned long long 
>>> sym_addr, char sym_type,
>>>       if (!glob_match(sym_name, res->pattern))
>>>           return 0;
>>> +    if (!filter_available_function(sym_name))
>>> +        return 0;
>>
>> I think it'd be better to parse available_filter_functions directly
>> for kprobe_multi instead of filtering out kallsyms entries
>>
>> we could add libbpf_available_filter_functions_parse function with
>> similar callback to go over available_filter_functions file
>>
> 
> Sure, if available_filter_functions not found, fallback to /proc/kallsyms.
> 

Um.

It is difficult to judge available_filter_functions directly, because we
not only need the function name, but also obtain its address and other
information, but we can indeed obtain the function set from
available_filter_functions first, and then obtain the function address
from /proc/kallsyms. which will be slightly faster than reading
available_filter_functions later, because if this function does not
exist in available_filter_functions, it will take a long time to read
the entire file.

Of course, it would be better if the kernel directly provided an
available_filter_functions -like file containing function address
information.

-- 
Jackie Liu

  reply	other threads:[~2023-05-24  1:19 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-23 13:25 [PATCH] libbpf: kprobe.multi: Filter with blacklist and available_filter_functions Jackie Liu
2023-05-23 16:17 ` Jiri Olsa
2023-05-23 18:22   ` Andrii Nakryiko
2023-05-24  7:03     ` Jiri Olsa
2023-05-24  1:03   ` Jackie Liu
2023-05-24  1:19     ` Jackie Liu [this message]
2023-05-24  6:47       ` Jiri Olsa
2023-05-24  7:06         ` Jackie Liu
2023-05-24  8:41         ` [PATCH v3] libbpf: kprobe.multi: Filter with available_filter_functions Jackie Liu
2023-05-25  8:44           ` Jiri Olsa
2023-05-25 10:27             ` [PATCH v4] " Jackie Liu
2023-05-25 20:43               ` Andrii Nakryiko
2023-05-26  1:38                 ` Jackie Liu
2023-05-26  8:58                   ` Jiri Olsa
2023-06-02 17:27                   ` Andrii Nakryiko
2023-06-07  6:01                     ` Jackie Liu
2023-06-07 22:37                       ` Andrii Nakryiko
2023-06-07 23:22                     ` Jiri Olsa
2023-06-08  0:00                       ` Andrii Nakryiko
2023-06-08  0:57                         ` Jackie Liu
2023-05-26  2:10                 ` [PATCH v5] " Jackie Liu
2023-05-26  9:53                   ` Jiri Olsa
2023-05-26 12:18                     ` Jackie Liu
2023-05-24  3:44   ` [PATCH v2] " Jackie Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eab45de6-f5cd-c500-e6b7-940540fa047a@linux.dev \
    --to=liu.yun@linux.dev \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=liuyun01@kylinos.cn \
    --cc=martin.lau@linux.dev \
    --cc=olsajiri@gmail.com \
    --cc=song@kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.