On Wed, Dec 2, 2020 at 10:18 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > I still think that adopting printk/vsnprintf for this instead of > reinventing the wheel > is more flexible and easier to maintain long term. > Almost the same layout can be done with vsnprintf > with exception of \0 char. > More meaningful names, etc. > See Documentation/core-api/printk-formats.rst I agree this would be nice. I finally got a bit of time to experiment with this and I noticed a few things: First of all, because helpers only have 5 arguments, if we use two for the output buffer and its size and two for the format string and its size, we are only left with one argument for a modifier. This is still enough for our usecase (where we'd only use "%ps" for example) but it does not strictly-speaking allow for the same layout that Andrii proposed. > If we force fmt to come from readonly map then bpf_trace_printk()-like > run-time check of fmt string can be moved into load time check > and performance won't suffer. Regarding this bit, I have the impression that this would not be possible, but maybe I'm missing something ? :) The iteration that bpf_trace_printk does over the format string argument is not only used for validation. It is also used to remember what extra operations need to be done based on the modifier types. For example, it remembers whether an arg should be interpreted as 32bits or 64bits. In the case of string printing, it also remembers whether it is a kernel-space or user-space pointer so that bpf_trace_copy_string can be called with the right arg. If we were to run the iteration over the format string in the verifier, how would you recommend that we "remember" the modifier type until the helper gets called ?
