On Fri, Dec 14, 2018 at 03:34:28PM -0800, Yonghong Song wrote:
> Commit 970289fc0a83 ("bpf: add bpffs pretty print for cgroup
> local storage maps") added bpffs pretty print for cgroup
> local storage maps. The commit worked for struct without kind_flag
> set.
>
> This patch refactored and made pretty print also work
> with kind_flag set for the struct.
>
> Signed-off-by: Yonghong Song <y...@fb.com>
> ---
> include/linux/btf.h | 5 ++++-
> kernel/bpf/btf.c | 37 ++++++++++++++++++++++++++++---------
> kernel/bpf/local_storage.c | 17 ++++-------------
> 3 files changed, 36 insertions(+), 23 deletions(-)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 58000d7e06e3..12502e25e767 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -7,6 +7,7 @@
> #include <linux/types.h>
>
> struct btf;
> +struct btf_member;
> struct btf_type;
> union bpf_attr;
>
> @@ -46,7 +47,9 @@ void btf_type_seq_show(const struct btf *btf, u32 type_id,
> void *obj,
> struct seq_file *m);
> int btf_get_fd_by_id(u32 id);
> u32 btf_id(const struct btf *btf);
> -bool btf_type_is_reg_int(const struct btf_type *t, u32 expected_size);
> +bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
> + const struct btf_member *m,
> + u32 expected_offset, u32 expected_size);
>
> #ifdef CONFIG_BPF_SYSCALL
> const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index ec3f80d3bef6..30028fa187df 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -546,22 +546,41 @@ static bool btf_type_int_is_regular(const struct
> btf_type *t)
> }
>
> /*
> - * Check that given type is a regular int and has the expected size.
> + * Check that given struct member is a regular int with expected
> + * offset and size.
> */
> -bool btf_type_is_reg_int(const struct btf_type *t, u32 expected_size)
> +bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
> + const struct btf_member *m,
> + u32 expected_offset, u32 expected_size)
> {
> - u8 nr_bits, nr_bytes;
> - u32 int_data;
> + const struct btf_type *t;
> + u32 id, int_data;
> + u8 nr_bits;
>
> - if (!btf_type_is_int(t))
> + id = m->type;
> + t = btf_type_id_size(btf, &id, NULL);
> + if (!t || !btf_type_is_int(t))
> return false;
>
> int_data = btf_type_int(t);
> nr_bits = BTF_INT_BITS(int_data);
> - nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> - if (BITS_PER_BYTE_MASKED(nr_bits) ||
> - BTF_INT_OFFSET(int_data) ||
> - nr_bytes != expected_size)
> + if (btf_type_kflag(s)) {
> + u32 bitfield_size = BTF_MEMBER_BITFIELD_SIZE(m->offset);
> + u32 bit_offset = BTF_MEMBER_BIT_OFFSET(m->offset);
> +
> + /* if kflag set, int should be a regular int and
> + * bit offset should be at byte boundary.
> + */
> + return !bitfield_size &&
Make sense. When kflag == true, by design, the member is a reg int only if
bitfield_size == 0. The BTF verifier has already ensured everything else.
Acked-by: Martin KaFai Lau <ka...@fb.com>
> + BITS_ROUNDUP_BYTES(bit_offset) == expected_offset &&
> + BITS_ROUNDUP_BYTES(nr_bits) == expected_size;
> + }
> +
> + if (BTF_INT_OFFSET(int_data) ||
> + BITS_PER_BYTE_MASKED(m->offset) ||
> + BITS_ROUNDUP_BYTES(m->offset) != expected_offset ||
> + BITS_PER_BYTE_MASKED(nr_bits) ||
> + BITS_ROUNDUP_BYTES(nr_bits) != expected_size)
> return false;
>
> return true;
> diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
> index 5eca03da0989..07a34ef562a0 100644
> --- a/kernel/bpf/local_storage.c
> +++ b/kernel/bpf/local_storage.c
> @@ -315,9 +315,8 @@ static int cgroup_storage_check_btf(const struct bpf_map
> *map,
> const struct btf_type *key_type,
> const struct btf_type *value_type)
> {
> - const struct btf_type *t;
> struct btf_member *m;
> - u32 id, size;
> + u32 offset, size;
>
> /* Key is expected to be of struct bpf_cgroup_storage_key type,
> * which is:
> @@ -338,25 +337,17 @@ static int cgroup_storage_check_btf(const struct
> bpf_map *map,
> * The first field must be a 64 bit integer at 0 offset.
> */
> m = (struct btf_member *)(key_type + 1);
> - if (m->offset)
> - return -EINVAL;
> - id = m->type;
> - t = btf_type_id_size(btf, &id, NULL);
> size = FIELD_SIZEOF(struct bpf_cgroup_storage_key, cgroup_inode_id);
> - if (!t || !btf_type_is_reg_int(t, size))
> + if (!btf_member_is_reg_int(btf, key_type, m, 0, size))
> return -EINVAL;
>
> /*
> * The second field must be a 32 bit integer at 64 bit offset.
> */
> m++;
> - if (m->offset != offsetof(struct bpf_cgroup_storage_key, attach_type) *
> - BITS_PER_BYTE)
> - return -EINVAL;
> - id = m->type;
> - t = btf_type_id_size(btf, &id, NULL);
> + offset = offsetof(struct bpf_cgroup_storage_key, attach_type);
> size = FIELD_SIZEOF(struct bpf_cgroup_storage_key, attach_type);
> - if (!t || !btf_type_is_reg_int(t, size))
> + if (!btf_member_is_reg_int(btf, key_type, m, offset, size))
> return -EINVAL;
>
> return 0;
> --
> 2.17.1
>
