Re: [PATCH v3 bpf-next 4/7] libbpf: use static const fmt string in __bpf_printk

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Sat, Aug 28, 2021 at 12:40:17PM -0400, Dave Marchevsky wrote:
> On 8/28/21 1:20 AM, Dave Marchevsky wrote:   
> > The __bpf_printk convenience macro was using a 'char' fmt string holder
> > as it predates support for globals in libbpf. Move to more efficient
> > 'static const char', but provide a fallback to the old way via
> > BPF_NO_GLOBAL_DATA so users on old kernels can still use the macro.
> > 
> > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
> > ---
> >  tools/lib/bpf/bpf_helpers.h | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > index 5f087306cdfe..a1d5ec6f285c 100644
> > --- a/tools/lib/bpf/bpf_helpers.h
> > +++ b/tools/lib/bpf/bpf_helpers.h
> > @@ -216,10 +216,16 @@ enum libbpf_tristate {
> >  		     ___param, sizeof(___param));		\
> >  })
> >  
> > +#ifdef BPF_NO_GLOBAL_DATA
> > +#define BPF_PRINTK_FMT_TYPE char
> > +#else
> > +#define BPF_PRINTK_FMT_TYPE static const char
> 
> The reference_tracking prog test is failing as a result of this.
> Specifically, it fails to load bpf_sk_lookup_test0 prog, which 
> has a bpf_printk:
> 
>   47: (b4) w3 = 0
>   48: (18) r1 = 0x0
>   50: (b4) w2 = 7
>   51: (85) call bpf_trace_printk#6
>   R1 type=inv expected=fp, pkt, pkt_meta, map_key, map_value, mem, rdonly_buf, rdwr_buf
> 
> Setting BPF_NO_GLOBAL_DATA in the test results in a pass

hmm. that's odd. pls investigate.
Worst case we can just drop this patch for now.
The failing printk is this one, right?
bpf_printk("sk=%d\n", sk ? 1 : 0);
iirc we had an issue related to ?: operand being used as an argument
and llvm generating interesting code path with 'sk' and the later
if (sk) bpf_sk_release(sk);
would not be properly recognized by the verifier leading it to
believe that sk may not be released in some cases.
That printk was triggering such interesting llvm codegen.
See commit d844a71bff0f ("bpf: Selftests, add printk to test_sk_lookup_kern to encode null ptr check")