Re: [RFC bpf-next 09/12] kbuild: Header guards for types from include/uapi/*.h in kernel BTF

[Yonghong Song <yhs@meta.com>]

On 10/27/22 5:00 PM, Eduard Zingerman wrote:
> On Thu, 2022-10-27 at 15:44 -0700, Yonghong Song wrote:
>>
>> On 10/27/22 11:55 AM, Yonghong Song wrote:
>>>
>>>
>>> On 10/27/22 11:43 AM, Yonghong Song wrote:
>>>>
>>>>
>>>> On 10/25/22 3:27 PM, Eduard Zingerman wrote:
>>>>> Use pahole --header_guards_db flag to enable encoding of header guard
>>>>> information in kernel BTF. The actual correspondence between header
>>>>> file and guard string is computed by the scripts/infer_header_guards.pl.
>>>>>
>>>>> The encoded header guard information could be used to restore the
>>>>> original guards in the vmlinux.h, e.g.:
>>>>>
>>>>>       include/uapi/linux/tcp.h:
>>>>>
>>>>>         #ifndef _UAPI_LINUX_TCP_H
>>>>>         #define _UAPI_LINUX_TCP_H
>>>>>         ...
>>>>>         union tcp_word_hdr {
>>>>>           struct tcphdr hdr;
>>>>>           __be32        words[5];
>>>>>         };
>>>>>         ...
>>>>>         #endif /* _UAPI_LINUX_TCP_H */
>>>>>
>>>>>       vmlinux.h:
>>>>>
>>>>>         ...
>>>>>         #ifndef _UAPI_LINUX_TCP_H
>>>>>
>>>>>         union tcp_word_hdr {
>>>>>           struct tcphdr hdr;
>>>>>           __be32 words[5];
>>>>>         };
>>>>>
>>>>>         #endif /* _UAPI_LINUX_TCP_H */
>>>>>         ...
>>>>>
>>>>> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
>>>>> ---
>>>>>    scripts/link-vmlinux.sh | 13 ++++++++++++-
>>>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
>>>>> index 918470d768e9..f57f621eda1f 100755
>>>>> --- a/scripts/link-vmlinux.sh
>>>>> +++ b/scripts/link-vmlinux.sh
>>>>> @@ -110,6 +110,7 @@ vmlinux_link()
>>>>>    gen_btf()
>>>>>    {
>>>>>        local pahole_ver
>>>>> +    local extra_flags
>>>>>        if ! [ -x "$(command -v ${PAHOLE})" ]; then
>>>>>            echo >&2 "BTF: ${1}: pahole (${PAHOLE}) is not available"
>>>>> @@ -122,10 +123,20 @@ gen_btf()
>>>>>            return 1
>>>>>        fi
>>>>> +    if [ "${pahole_ver}" -ge "124" ]; then
>>>>> +        scripts/infer_header_guards.pl \
>>>>
>>>> We should have full path like
>>>>       ${srctree}/scripts/infer_header_guards.pl
>>>> so it can work if build directory is different from source directory.
>>>
>>> handling arguments for infer_header_guards.pl should also take
>>> care of full file path.
>>>
>>> + /home/yhs/work/bpf-next/scripts/infer_header_guards.pl include/uapi
>>> include/generated/uapi arch/x86/include/uapi
>>> arch/x86/include/generated/uapi
>>> + return 1
>>
>> Also, please pay attention to bpf selftest result. I see quite a
>> few selftest failures with this patch set.
> 
> Hi Yonghong,
> 
> Could you please copy-paste some of the error reports? I just re-run
> the selftests locally and have test_maps, test_verifier, test_progs
> and test_progs-no_alu32 passing.

Sorry about the noise. It is my fault. My default build is out of
source tree with KBUILD_OUTPUT=<path>. Since the current patch set
won't work with it, so I build a in-tree one for vmlinux but forgot
to adjust selftest build which still has KBUILD_OUTPUT=<path> and
it caused some selftest failures. Consistently doing in-tree build
for vmlinux and selftest results in the same Success/Failure rate
with and without this patch set.

> 
> Thanks,
> Eduard
> 
>>
>>>>
>>>>> +            include/uapi \
>>>>> +            include/generated/uapi \
>>>>> +            arch/${SRCARCH}/include/uapi \
>>>>> +            arch/${SRCARCH}/include/generated/uapi \
>>>>> +            > .btf.uapi_header_guards || return 1;
>>>>> +        extra_flags="--header_guards_db .btf.uapi_header_guards"
>>>>> +    fi
>>>>> +
>>>>>        vmlinux_link ${1}
>>>>>        info "BTF" ${2}
>>>>> -    LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1}
>>>>> +    LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS}
>>>>> ${extra_flags} ${1}
>>>>>        # Create ${2} which contains just .BTF section but no symbols. Add
>>>>>        # SHF_ALLOC because .BTF will be part of the vmlinux image.
>>>>> --strip-all
>