On Wed, Jul 15, 2020 at 12:19:03AM +0200, Toke Høiland-Jørgensen wrote: > Andrii Nakryiko <andrii.nakry...@gmail.com> writes: > > >> However, assuming it *is* possible, my larger point was that we > >> shouldn't add just a 'logging struct', but rather a 'common options > >> struct' which can be extended further as needed. And if it is *not* > >> possible to add new arguments to a syscall like you're proposing, my > >> suggestion above would be a different way to achieve basically the same > >> (at the cost of having to specify the maximum reserved space in advance). > >> > > > > yeah-yeah, I agree, it's less a "logging attr", more of "common attr > > across all commands". > > Right, great. I think we are broadly in agreement with where we want to > go with this, actually :)
I really don't like 'common attr across all commands'. Both of you are talking as libbpf developers who occasionally need to add printk-s to the kernel. That is not an excuse to bloat api that will be useful to two people. The only reason log_buf sort-of make sense in raw_tp_open is because btf comparison is moved from prog_load into raw_tp_open. Miscompare of (prog_fd1, btf_id1) vs (prog_fd2, btf_id2) can be easily solved by libbpf with as nice and as human friendly message libbpf can do. I'm not convinced yet that it's a kernel job to print it nicely. It certainly can, but it's quite a bit different from two existing bpf commands where log_buf is used: PROG_LOAD and BTF_LOAD. In these two cases the kernel verifies the program and the BTF. raw_tp_open is different, since the kernel needs to compare that function signatures (prog_fd1, btf_id1) and (prog_fd2, btf_id2) are exactly the same. The kernel can indicate that with single specific errno and libbpf can print human friendly function signatures via btf_dump infra for humans to see. So I really don't see why log_buf is such a necessity for raw_tp_open.
