On Tue, Jul 14, 2020 at 4:11 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Wed, Jul 15, 2020 at 12:19:03AM +0200, Toke Høiland-Jørgensen wrote: > > Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> 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. How did you come to this conclusion? Inability to figure out what's wrong when using BPF is at the top of complaints from many users, together with hard to understand logs from verifier. > That is not an excuse to bloat api that will be > useful to two people. What do you mean specifically by bloat in API? I could understand how it would bloat the API if we were to add log-related fields into every part of bpf_attr, one for each type of commands. But that's exactly what I advocate to not do. But having even a slight improvement of error reporting, beyond current -EINVAL, -E2BIG, etc, would improve experience immensely for *all* BPF users. > > 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.
