> On Oct 28, 2020, at 5:58 PM, Andrii Nakryiko <and...@kernel.org> wrote:
>
[...]
>
> BTF deduplication is not yet supported for split BTF and support for it will
> be added in separate patch.
>
> Signed-off-by: Andrii Nakryiko <and...@kernel.org>
Acked-by: Song Liu <songliubrav...@fb.com>
With a couple nits:
> ---
> tools/lib/bpf/btf.c | 205 ++++++++++++++++++++++++++++++---------
> tools/lib/bpf/btf.h | 8 ++
> tools/lib/bpf/libbpf.map | 9 ++
> 3 files changed, 175 insertions(+), 47 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index db9331fea672..20c64a8441a8 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -78,10 +78,32 @@ struct btf {
> void *types_data;
> size_t types_data_cap; /* used size stored in hdr->type_len */
>
> - /* type ID to `struct btf_type *` lookup index */
> + /* type ID to `struct btf_type *` lookup index
> + * type_offs[0] corresponds to the first non-VOID type:
> + * - for base BTF it's type [1];
> + * - for split BTF it's the first non-base BTF type.
> + */
> __u32 *type_offs;
> size_t type_offs_cap;
> + /* number of types in this BTF instance:
> + * - doesn't include special [0] void type;
> + * - for split BTF counts number of types added on top of base BTF.
> + */
> __u32 nr_types;
This is a little confusing. Maybe add a void type for every split BTF?
> + /* if not NULL, points to the base BTF on top of which the current
> + * split BTF is based
> + */
[...]
>
> @@ -252,12 +274,20 @@ static int btf_parse_str_sec(struct btf *btf)
> const char *start = btf->strs_data;
> const char *end = start + btf->hdr->str_len;
>
> - if (!hdr->str_len || hdr->str_len - 1 > BTF_MAX_STR_OFFSET ||
> - start[0] || end[-1]) {
> - pr_debug("Invalid BTF string section\n");
> - return -EINVAL;
> + if (btf->base_btf) {
> + if (hdr->str_len == 0)
> + return 0;
> + if (hdr->str_len - 1 > BTF_MAX_STR_OFFSET || end[-1]) {
> + pr_debug("Invalid BTF string section\n");
> + return -EINVAL;
> + }
> + } else {
> + if (!hdr->str_len || hdr->str_len - 1 > BTF_MAX_STR_OFFSET ||
> + start[0] || end[-1]) {
> + pr_debug("Invalid BTF string section\n");
> + return -EINVAL;
> + }
> }
> -
> return 0;
I found this function a little difficult to follow. Maybe rearrange it as
/* too long, or not \0 terminated */
if (hdr->str_len - 1 > BTF_MAX_STR_OFFSET || end[-1])
goto err_out;
/* for base btf, .... */
if (!btf->base_btf && (!hdr->str_len || start[0]))
goto err_out;
return 0;
err_out:
pr_debug("Invalid BTF string section\n");
return -EINVAL;
}
> }
>
> @@ -372,19 +402,9 @@ static int btf_parse_type_sec(struct btf *btf)
> struct btf_header *hdr = btf->hdr;
> void *next_type = btf->types_data;
> void *end_type = next_type + hdr->type_len;
> - int err, i = 0, type_size;
[...]