| Message ID | 20211029135321.94065-2-houtao1@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | clean-up for BPF_LOG_KERNEL log level | expand |
On 10/29/21 3:53 PM, Hou Tao wrote: > An extra newline will output for bpf_log() with BPF_LOG_KERNEL level > as shown below: > > [ 52.095704] BPF:The function test_3 has 12 arguments. Too many. > [ 52.095704] > [ 52.096896] Error in parsing func ptr test_3 in struct bpf_dummy_ops > > Now all bpf_log() are ended by newline, but not all btf_verifier_log() > are ended by newline, so checking whether or not the log message > has the trailing newline and adding a newline if not. > > Also there is no need to calculate the left userspace buffer size > for kernel log output and to truncate the output by '\0' which > has already been done by vscnprintf(), so only do these for > userspace log output. > > Signed-off-by: Hou Tao <houtao1@huawei.com> > --- > kernel/bpf/verifier.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 3c8aa7df1773..22f0d2292c2c 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -299,13 +299,15 @@ void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt, > WARN_ONCE(n >= BPF_VERIFIER_TMP_LOG_SIZE - 1, > "verifier log line truncated - local buffer too short\n"); > > - n = min(log->len_total - log->len_used - 1, n); > - log->kbuf[n] = '\0'; > - > if (log->level == BPF_LOG_KERNEL) { > - pr_err("BPF:%s\n", log->kbuf); > + bool newline = n > 0 && log->kbuf[n - 1] == '\n'; > + > + pr_err("BPF:%s%s", log->kbuf, newline ? "" : "\n"); nit: Given you change this anyway, is there a reason not to go with "BPF: %s%s" instead? > return; > } > + > + n = min(log->len_total - log->len_used - 1, n); > + log->kbuf[n] = '\0'; > if (!copy_to_user(log->ubuf + log->len_used, log->kbuf, n + 1)) > log->len_used += n; > else >
Hi, On 11/2/2021 6:01 AM, Daniel Borkmann wrote: > On 10/29/21 3:53 PM, Hou Tao wrote: >> An extra newline will output for bpf_log() with BPF_LOG_KERNEL level >> as shown below: >> >> [ 52.095704] BPF:The function test_3 has 12 arguments. Too many. >> [ 52.095704] >> [ 52.096896] Error in parsing func ptr test_3 in struct bpf_dummy_ops >> >> if (log->level == BPF_LOG_KERNEL) { >> - pr_err("BPF:%s\n", log->kbuf); >> + bool newline = n > 0 && log->kbuf[n - 1] == '\n'; >> + >> + pr_err("BPF:%s%s", log->kbuf, newline ? "" : "\n"); > > nit: Given you change this anyway, is there a reason not to go with "BPF: > %s%s" instead? > My bad, and will do it in v3.
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 3c8aa7df1773..22f0d2292c2c 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -299,13 +299,15 @@ void bpf_verifier_vlog(struct bpf_verifier_log *log, const char *fmt, WARN_ONCE(n >= BPF_VERIFIER_TMP_LOG_SIZE - 1, "verifier log line truncated - local buffer too short\n"); - n = min(log->len_total - log->len_used - 1, n); - log->kbuf[n] = '\0'; - if (log->level == BPF_LOG_KERNEL) { - pr_err("BPF:%s\n", log->kbuf); + bool newline = n > 0 && log->kbuf[n - 1] == '\n'; + + pr_err("BPF:%s%s", log->kbuf, newline ? "" : "\n"); return; } + + n = min(log->len_total - log->len_used - 1, n); + log->kbuf[n] = '\0'; if (!copy_to_user(log->ubuf + log->len_used, log->kbuf, n + 1)) log->len_used += n; else
An extra newline will output for bpf_log() with BPF_LOG_KERNEL level as shown below: [ 52.095704] BPF:The function test_3 has 12 arguments. Too many. [ 52.095704] [ 52.096896] Error in parsing func ptr test_3 in struct bpf_dummy_ops Now all bpf_log() are ended by newline, but not all btf_verifier_log() are ended by newline, so checking whether or not the log message has the trailing newline and adding a newline if not. Also there is no need to calculate the left userspace buffer size for kernel log output and to truncate the output by '\0' which has already been done by vscnprintf(), so only do these for userspace log output. Signed-off-by: Hou Tao <houtao1@huawei.com> --- kernel/bpf/verifier.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)