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?
