On Wed, Dec 1, 2021 at 12:29 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Mon, Nov 29, 2021 at 05:29:40PM -0800, Hao Luo wrote:
> > There are some common properties shared between bpf reg, ret and arg
> > values. For instance, a value may be a NULL pointer, or a pointer to
> > a read-only memory. Previously, to express these properties, enumeration
> > was used. For example, in order to test whether a reg value can be NULL,
> > reg_type_may_be_null() simply enumerates all types that are possibly
> > NULL. The problem of this approach is that it's not scalable and causes
> > a lot of duplication. These properties can be combined, for example, a
> > type could be either MAYBE_NULL or RDONLY, or both.
> >
> > This patch series rewrites the layout of reg_type, arg_type and
> > ret_type, so that common properties can be extracted and represented as
> > composable flag. For example, one can write
> >
> > ARG_PTR_TO_MEM | PTR_MAYBE_NULL
> >
> > which is equivalent to the previous
> >
> > ARG_PTR_TO_MEM_OR_NULL
> >
> > The type ARG_PTR_TO_MEM are called "base types" in this patch. Base
> > types can be extended with flags. A flag occupies the higher bits while
> > base types sits in the lower bits.
> >
> > This patch in particular sets up a set of macro for this purpose. The
> > followed patches rewrites arg_types, ret_types and reg_types
> > respectively.
> >
> > Signed-off-by: Hao Luo <haoluo@xxxxxxxxxx>
> > ---
> > include/linux/bpf.h | 50 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index cc7a0c36e7df..b592b3f7d223 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -297,6 +297,37 @@ bool bpf_map_meta_equal(const struct bpf_map *meta0,
> >
> > extern const struct bpf_map_ops bpf_map_offload_ops;
> >
> > +/* bpf_type_flag contains a set of flags that are applicable to the values of
> > + * arg_type, ret_type and reg_type. For example, a pointer value may be null,
> > + * or a memory is read-only. We classify types into two categories: base types
> > + * and extended types. Extended types are base types combined with a type flag.
> > + *
> > + * Currently there are no more than 32 base types in arg_type, ret_type and
> > + * reg_types.
> > + */
> > +#define BPF_BASE_TYPE_BITS 8
> > +
> > +enum bpf_type_flag {
> > + /* PTR may be NULL. */
> > + PTR_MAYBE_NULL = BIT(0 + BPF_BASE_TYPE_BITS),
> > +
> > + __BPF_TYPE_LAST_FLAG = PTR_MAYBE_NULL,
> > +};
> > +
> > +#define BPF_BASE_TYPE_MASK GENMASK(BPF_BASE_TYPE_BITS, 0)
> > +
> > +/* Max number of base types. */
> > +#define BPF_BASE_TYPE_LIMIT (1UL << BPF_BASE_TYPE_BITS)
> > +
> > +/* Max number of all types. */
> > +#define BPF_TYPE_LIMIT (__BPF_TYPE_LAST_FLAG | (__BPF_TYPE_LAST_FLAG - 1))
> > +
> > +/* extract base type. */
> > +#define BPF_BASE_TYPE(x) ((x) & BPF_BASE_TYPE_MASK)
> > +
> > +/* extract flags from an extended type. */
> > +#define BPF_TYPE_FLAG(x) ((enum bpf_type_flag)((x) & ~BPF_BASE_TYPE_MASK))
>
> Overall I think it's really great.
> The only suggestion is to use:
> static inline u32 base_type(u32 x)
> {
> return x & BPF_BASE_TYPE_MASK;
> }
> and
> static inline u32 type_flag(u32 x) ..
>
> The capital letter macros are too loud.
>
> wdyt?
>
Sounds good to me. Will do in the next iteration.
