On 8/21/20 9:44 AM, Andrii Nakryiko wrote:
On Thu, Aug 20, 2020 at 11:42 PM Yonghong Song <yhs@xxxxxx> wrote:
On 8/20/20 11:31 PM, Andrii Nakryiko wrote:
On Thu, Aug 20, 2020 at 3:50 PM Yonghong Song <yhs@xxxxxx> wrote:
This patch implemented bpf_link callback functions
show_fdinfo and fill_link_info to support link_query
interface.
The general interface for show_fdinfo and fill_link_info
will print/fill the target_name. Each targets can
register show_fdinfo and fill_link_info callbacks
to print/fill more target specific information.
For example, the below is a fdinfo result for a bpf
task iterator.
$ cat /proc/1749/fdinfo/7
pos: 0
flags: 02000000
mnt_id: 14
link_type: iter
link_id: 11
prog_tag: 990e1f8152f7e54f
prog_id: 59
target_name: task
Signed-off-by: Yonghong Song <yhs@xxxxxx>
---
include/linux/bpf.h | 6 ++++
include/uapi/linux/bpf.h | 7 ++++
kernel/bpf/bpf_iter.c | 58 ++++++++++++++++++++++++++++++++++
tools/include/uapi/linux/bpf.h | 7 ++++
4 files changed, 78 insertions(+)
[...]
+
+static int bpf_iter_link_fill_link_info(const struct bpf_link *link,
+ struct bpf_link_info *info)
+{
+ struct bpf_iter_link *iter_link =
+ container_of(link, struct bpf_iter_link, link);
+ char __user *ubuf = u64_to_user_ptr(info->iter.target_name);
+ bpf_iter_fill_link_info_t fill_link_info;
+ u32 ulen = info->iter.target_name_len;
+ const char *target_name;
+ u32 target_len;
+
+ if (ulen && !ubuf)
+ return -EINVAL;
+
+ target_name = iter_link->tinfo->reg_info->target;
+ target_len = strlen(target_name);
+ info->iter.target_name_len = target_len + 1;
+ if (!ubuf)
+ return 0;
this might return prematurely before fill_link_info() below gets a
chance to fill in some extra info?
The extra info filled by below fill_link_info is target specific
and we need a target name to ensure picking right union members.
So it is best to enforce a valid target name before filling
target dependent fields. See below, if there are any errors
for copy_to_user or enospc, we won't copy addition link info
either.
You are making an assumption that the caller doesn't know what time of
link it's requesting info for. That's not generally true. So I think
Based on my understanding, most users for bpf command
BPF_OBJ_GET_INFO_BY_FD is for tools, not the original application
which created the original link.
But I agree there are certain use cases where the caller has
much more knowledge about 'fd' than bpftool and they may just
want to get one particular piece of information.
we just shouldn't make unnecessary assumptions and provide as much
information on the first try. target_name should be treated as an
optional thing to request, that's all.
Okay, will do this.
+
+ if (ulen >= target_len + 1) {
+ if (copy_to_user(ubuf, target_name, target_len + 1))
+ return -EFAULT;
+ } else {
+ char zero = '\0';
+
+ if (copy_to_user(ubuf, target_name, ulen - 1))
+ return -EFAULT;
+ if (put_user(zero, ubuf + ulen - 1))
+ return -EFAULT;
+ return -ENOSPC;
+ }
+
+ fill_link_info = iter_link->tinfo->reg_info->fill_link_info;
+ if (fill_link_info)
+ return fill_link_info(&iter_link->aux, info);
+
+ return 0;
+}
+
[...]
