Re: [PATCH bpf-next 1/5] libbpf: update BTF reloc support to latest Clang format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/14/19 4:49 PM, Andrii Nakryiko wrote:
> BTF offset reloc was generalized in recent Clang into field relocation,
> capturing extra u32 field, specifying what aspect of captured field
> needs to be relocated. This changes .BTF.ext's record size for this
> relocation from 12 bytes to 16 bytes. Given these format changes
> happened in Clang before official released version, it's ok to not
> support outdated 12-byte record size w/o breaking ABI.
> 
> Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
...> -/* The minimum bpf_offset_reloc checked by the loader
> +/* bpf_field_reloc_kind encodes which aspect of captured field has to be
> + * adjusted by relocations. Currently supported values are:
> + *   - BPF_FI_BYTE_OFFSET: field offset (in bytes);
> + *   - BPF_FI_EXISTS: field existence (1 if field exists, 0 - otherwise);
> + */
> +enum bpf_field_reloc_kind {
> +	BPF_FRK_BYTE_OFFSET = 0,
> +	BPF_FRK_EXISTS = 2,
> +};

Comment above doesn't match the enum.
Also in patch 4 :
+enum bpf_field_info_kind {
+	BPF_FI_BYTE_OFFSET = 0,	  /* field byte offset */
+	BPF_FI_EXISTS = 2,	  /* whether field exists in target kernel */
+};

Do you expect that .btf.ext encoding to be different from
argument passed into __builtin_field_info() ?
May be better to design the interface that it always matches
and keep single enum for both?
I don't like either FRK or FI abbreviations.
May be
BPF_FIELD_BYTE_OFFSET
BPF_FIELD_EXISTS ?

These constants would need to be exposed in bpf_core_read.h and
will become uapi as soon as we release next libbpf at
the end of this bpf-next cycle. At that point llvm side would have
to stick to these constants as well.
It would have to understand them as arguments and use the same in .btf.ext.
Does it make sense?





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux