On Wed, Jul 15, 2020 at 02:56:36PM +0200, Toke Høiland-Jørgensen wrote: > Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> writes: > > > 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. That is not an excuse to bloat api that will be > > useful to two people. > > What? No, this is about making error messages comprehensible to people > who *can't* just go around adding printks. "Guess the source of the > EINVAL" is a really bad user experience! > > > 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. > > So userspace is supposed to replicate all the checks done by the kernel > because we can't be bothered to add proper error messages? Really? That's not what I said. The kernel can report unique errno for miscompare and all nice messages can and _should be_ be printed by libbpf. On Wed, Jul 15, 2020, Andrii Nakryiko wrote: > > 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. Only the second part is true. All users are complaining about the verifier. No one is complaing that failed prog attach is somehow lacking string message. The users are also complaing about libbpf being too verbose. Yet you've refused to address the verbosity where it should be reduced and now refusing to add it where it's needed. It's libbpf job to explain users kernel errors. The same thing is happening with perf_event_open syscall. Every one who's trying to code it directly complaining about the kernel. But not a single user is complaing about perf syscall when they use libraries and tools. Same thing with bpf syscall. libbpf is the interface. It needs to clear and to the point. Right now it's not doing it well. elf dump is too verbose and unnecessary whereas in other places it says nothing informative where it could have printed human hint. libbpf's pr_perm_msg() hint is the only one where libbpf cares about its users. All other messages are useful to libbpf developers and not its users. The kernel verifier messages suck as well. They need to be improved. But this thread 'lets add strings everywhere and users will be happy' is completely missing the mark.
