bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jackie Liu <liu.yun@linux.dev>
To: Ratheesh Kannoth <rkannoth@marvell.com>,
	olsajiri@gmail.com, andrii@kernel.org
Cc: bpf@vger.kernel.org, liuyun01@kylinos.cn, martin.lau@linux.dev,
	song@kernel.org, yhs@fb.com
Subject: Re: [PATCH v7] libbpf: kprobe.multi: Filter with available_filter_functions
Date: Tue, 30 May 2023 14:07:54 +0800	[thread overview]
Message-ID: <aa14123d-6213-da9a-37f2-986b9670bb23@linux.dev> (raw)
In-Reply-To: <20230530010801.1558937-1-liu.yun@linux.dev>



在 2023/5/30 11:44, Ratheesh Kannoth 写道:
> From: Jackie Liu <liu.yun@linux.dev>
> 
>> +
>> +	if (!glob_match(sym_name, res->pattern))
>> +		return 0;
>> +
>> +	err = libbpf_ensure_mem((void **) &res->syms, &res->cap,
>> +				sizeof(const char *), res->cnt + 1);
>> +	if (err)
>> +		return err;
>> +
>> +	name = strdup(sym_name);
>> +	if (!name)
>> +		return -errno;
>> +
>> +	res->syms[res->cnt++] = name;
>> +	return 0;
>> +}
>> +
>> +typedef int (*available_kprobe_cb_t)(const char *sym_name, void *ctx);
>> +
>> +static int
>> +libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
>> +{
>> +	char sym_name[256];
>> +	FILE *f;
>> +	int ret, err = 0;
>> +	const char *available_path = tracefs_available_filter_functions();
> Dont we need to follow reverse x-mas tree ?
> 
>> +
>> +	f = fopen(available_path, "r");
>> +	if (!f) {
>> +		err = -errno;
>> +		pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
>> +			available_path);
>> +		return err;
>> +	}
>> +
>> +	while (true) {
>> +		ret = fscanf(f, "%255s%*[^\n]\n", sym_name);
>> +		if (ret == EOF && feof(f))
>> +			break;
> why fscanf() is not setting EOF. Why did you use feof() ?

The fscanf function returns EOF (End of File) when one of the following 
conditions is met:

End of file is reached: When fscanf reaches the end of the file being 
read, it returns EOF. This indicates that there are no more characters 
to read from the file.

Input error occurs: If fscanf encounters an error while reading input, 
such as a format mismatch or inability to read the expected data type, 
it returns EOF.

Stream error occurs: If an error occurs with the stream itself, such as 
an error in the underlying file system or I/O error, fscanf may return EOF.

-- 
Jackie Liu

> 
>> +		if (ret != 1) {
>> +			pr_warn("failed to read available kprobe entry: %d\n",
>> +				ret);
>> +			err = -EINVAL;
>> +			break;
>> +		}
>> +
>> +		err = cb(sym_name, ctx);
>> +		if (err)
>> +			break;
>> +	}
>> +
>> +	fclose(f);
>> +	return err;
>> +}
>> +
>> +static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res)
>> +{
>> +	while (res->syms && res->cnt)
>> +		free((char *)res->syms[--res->cnt]);
>> +
>> +	free(res->syms);
>> +	free(res->addrs);
> 
> it looks odd to do allocation in libbpf_xxx (libbpf_ensure_mem ) function and
> freeing in a static function.
> 
>> +
>> +	/* reset to zero, when fallback */
>> +	res->cap = 0;
>> +	res->cnt = 0;
>> +	res->syms = NULL;
>> +	res->addrs = NULL;
>> +}
>> +
>> struct bpf_link *
>> bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>> 				      const char *pattern,
>> @@ -10476,13 +10558,21 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>> 		return libbpf_err_ptr(-EINVAL);
>>
>> 	if (pattern) {
>> -		err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res);
>> -		if (err)
>> -			goto error;
>> +		err = libbpf_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
>> +						     &res);
>> +		if (err) {
>> +			/* fallback to kallsyms */
>> +			kprobe_multi_resolve_free(&res);
>> +			err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb,
>> +						    &res);
>> +			if (err)
>> +				goto error;
>> +		}
>> 		if (!res.cnt) {
>> 			err = -ENOENT;
>> 			goto error;
>> 		}
>> +		syms = res.syms;
>> 		addrs = res.addrs;
>> 		cnt = res.cnt;
>> 	}
>> @@ -10511,12 +10601,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>> 		goto error;
>> 	}
>> 	link->fd = link_fd;
>> -	free(res.addrs);
>> +	kprobe_multi_resolve_free(&res);
>> 	return link;
>>
>> error:
>> 	free(link);
>> -	free(res.addrs);
>> +	kprobe_multi_resolve_free(&res);
>> 	return libbpf_err_ptr(err);
>> }
>>
>> --
>> 2.25.1

      parent reply	other threads:[~2023-05-30  6:09 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-30  1:08 [PATCH v7] libbpf: kprobe.multi: Filter with available_filter_functions Jackie Liu
2023-05-30  3:44 ` Ratheesh Kannoth
2023-05-30  6:07 ` Jackie Liu [this message]

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=aa14123d-6213-da9a-37f2-986b9670bb23@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=rkannoth@marvell.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).