Re: [PATCH bpf-next v1 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Hao Luo <haoluo@google.com>, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, KP Singh <kpsingh@kernel.org>,
	bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next v1 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL
Date: Thu, 9 Dec 2021 13:45:27 -0800	[thread overview]
Message-ID: <CAADnVQJzq5j2WecExdzwG+oc8iTP3pSm5OXsmF5cLQvPKHNKeA@mail.gmail.com> (raw)
In-Reply-To: <CAEf4BzZ1-5Tbq5MXkJ=8REFGRFs5aXnT0aGbaQkWYKVo4vuqcA@mail.gmail.com>

On Wed, Dec 8, 2021 at 12:04 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> > > > +static const char *reg_type_str(enum bpf_reg_type type)
> > > > +{
> > > > +       static const char * const str[] = {
> > > > +               [NOT_INIT]              = "?",
> > > > +               [SCALAR_VALUE]          = "inv",
> > > > +               [PTR_TO_CTX]            = "ctx",
> > > > +               [CONST_PTR_TO_MAP]      = "map_ptr",
> > > > +               [PTR_TO_MAP_VALUE]      = "map_value",
> > > > +               [PTR_TO_STACK]          = "fp",
> > > > +               [PTR_TO_PACKET]         = "pkt",
> > > > +               [PTR_TO_PACKET_META]    = "pkt_meta",
> > > > +               [PTR_TO_PACKET_END]     = "pkt_end",
> > > > +               [PTR_TO_FLOW_KEYS]      = "flow_keys",
> > > > +               [PTR_TO_SOCKET]         = "sock",
> > > > +               [PTR_TO_SOCK_COMMON]    = "sock_common",
> > > > +               [PTR_TO_TCP_SOCK]       = "tcp_sock",
> > > > +               [PTR_TO_TP_BUFFER]      = "tp_buffer",
> > > > +               [PTR_TO_XDP_SOCK]       = "xdp_sock",
> > > > +               [PTR_TO_BTF_ID]         = "ptr_",
> > > > +               [PTR_TO_PERCPU_BTF_ID]  = "percpu_ptr_",
> > > > +               [PTR_TO_MEM]            = "mem",
> > > > +               [PTR_TO_RDONLY_BUF]     = "rdonly_buf",
> > > > +               [PTR_TO_RDWR_BUF]       = "rdwr_buf",
> > > > +               [PTR_TO_FUNC]           = "func",
> > > > +               [PTR_TO_MAP_KEY]        = "map_key",
> > > > +       };
> > > > +
> > > > +       return str[base_type(type)];
> > >
> > > well... now we lose the "_or_null" part, that can be pretty important.
> > > Same will happen with RDONLY flag, right?
> > >
> > > What can we do about that?
> > >
> >
> > We can format the string into a global buffer and return the buffer to
> > the caller. A little overhead on string formatting.
>
> Can't use global buffer, because multiple BPF verifications can
> proceed at the same time.

I think reg_type_str() can still return a const char string
with flags.
It's not going to be a direct str[base_type(type)]; of course.
The str[] would be different.

If that doesn't work out we can try:
s/verbose(,"%s", reg_type_str[])
/verbose(, "%s%s", reg_type_str(), reg_type_flags_str())