Date:   Sat, 11 Apr 2020 23:51:22 -0700
From:   Yonghong Song <yhs@...com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
CC:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [RFC PATCH bpf-next 05/16] bpf: create file or anonymous dumpers



On 4/11/20 4:11 PM, Alexei Starovoitov wrote:
> On Fri, Apr 10, 2020 at 04:47:36PM -0700, Yonghong Song wrote:
>>>
>>> Instead of special-casing dumper_name, can we require specifying full
>>> path, and then check whether it is in BPF FS vs BPFDUMP FS? If the
>>> latter, additionally check that it is in the right sub-directory
>>> matching its intended target type.
>>
>> We could. I just think specifying full path for bpfdump is not necessary
>> since it is a single user mount...
>>
>>>
>>> But honestly, just doing everything within BPF FS starts to seem
>>> cleaner at this point...
>>
>> bpffs is multi mount, which is not a perfect fit for bpfdump,
>> considering mounting inside namespace, etc, all dumpers are gone.
> 
> As Yonghong pointed out reusing bpffs for dumpers doesn't look possible
> from implementation perspective.
> Even if it was possible the files in such mix-and-match file system
> would be of different kinds with different semantics. I think that
> will lead to mediocre user experience when file 'foo' is cat-able
> with nice human output, but file 'bar' isn't cat-able at all because
> it's just a pinned map. imo having all dumpers in one fixed location
> in /sys/kernel/bpfdump makes it easy to discover for folks who might
> not even know what bpf is.
> For example when I'm trying to learn some new area of the kernel I might go
> poke around /proc and /sys directory looking for a file name that could be
> interesting to 'cat'. This is how I discovered /sys/kernel/slab/ :)
> I think keeping all dumpers in /sys/kernel/bpfdump/ will make them
> similarly discoverable.
> 
> re: f_dump flag...
> May be it's a sign that pinning is not the right name for such operation?
> If kernel cannot distinguish pinning dumper prog into bpffs as a vanilla
> pinning operation vs pinning into bpfdumpfs to make it cat-able then something
> isn't right about api. Either it needs to be a new bpf syscall command (like
> install_dumper_in_dumpfs) or reuse pinning command, but make libbpf specify the
> full path. From bpf prog point of view it may still specify only the final
> name, but libbpf can prepend the /sys/kernel/bpfdump/.../. May be there is a
> third option. Extra flag for pinning just doesn't look right. What if we do
> another specialized file system later? It would need yet another flag to pin
> there?

For the 2nd option,
    - user still just specifying the dumper name, and
    - bpftool will prepend /sys/kernel/bpfdump/...
this should work. In this case, the kernel API
to create bpf dumper will be
    BPF_OBJ_PIN with a file path
this is fine only with one following annoyance.
Suppose somehow:
    - bpfdump is mounted at /sys/kernel/bpfdump and somewhere else say
      /root/tmp/bpfdump/
      [
        I checked do_mount in namespace.c, and did not find a flag
        to prevent multi mounting, maybe I missed something. I will be
        glad if somebody knows and let me know.
      ]
    - user call BPF_OBJ_PIN to path /root/tmp/bpfdump/task/my_task.
    - But actually the file will also appear in
      /sys/kernel/bpfdump/task/my_task.
there is a little confusion here based on kernel API.
That is exactly why I supplied with only filename. Conceptually, it
will be clear that the dumper will appear in all mount points.

Maybe a new bpf subcommand is warranted.
maybe BPF_DUMPER_INSTALL?

Powered by blists - more mailing lists