Re: [PATCH v2] samples/bpf: Add more instructions to build dependencies and, configuration in README.rst

[Anh Tuan Phan <tuananhlfc@gmail.com>]

On 7/9/23 22:21, Khalid Masum wrote:
> Hi,
> 
> On Sun, Jul 9, 2023 at 8:38 PM Anh Tuan Phan <tuananhlfc@gmail.com> wrote:
>>
>> Hi Stanislav,
>>
>> I have updated the Documentation according to your suggestion. Please
>> see it in the below patch. Thanks!
>>
>> On 7/7/23 23:57, Stanislav Fomichev wrote:
>>> On 07/07, Anh Tuan Phan wrote:
>>>>
>>>>
>>>> On 7/7/23 01:16, Stanislav Fomichev wrote:
>>>>> On 07/06, Anh Tuan Phan wrote:
>>>>>> Update the Documentation to mention that some samples require pahole
>>>>>> v1.16 and kernel built with CONFIG_DEBUG_INFO_BTF=y
>>>>>>
>>>>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>>>>>> ---
>>>>>>  samples/bpf/README.rst | 7 +++++++
>>>>>>  1 file changed, 7 insertions(+)
>>>>>>
>>>>>> diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst
>>>>>> index 57f93edd1957..631592b83d60 100644
>>>>>> --- a/samples/bpf/README.rst
>>>>>> +++ b/samples/bpf/README.rst
>>>>>> @@ -14,6 +14,9 @@ Compiling requires having installed:
>>>>>>  Note that LLVM's tool 'llc' must support target 'bpf', list version
>>>>>>  and supported targets with command: ``llc --version``
>>>>>>
>>>>>> +Some samples require pahole version 1.16 as a dependency. See
>>>>>> +https://docs.kernel.org/bpf/bpf_devel_QA.html for reference.
>>>>>> +
>>>>>
>>>>> Any reason no to add pahole 1.16 to this section above?
>>>>>> Compiling requires having installed:
>>>>>  * clang >= version 3.4.0
>>>>>  * llvm >= version 3.7.1
>>>>>  * pahole >= version 1.16
>>>>>
>>>>> Although clang 3.4 probably won't get you anywhere these days. The
>>>>> whole README seems a bit outdated :-)
>>>>>
>>>>
>>>> Put pahole requirement as your idea is better, thanks for suggestion.
>>>> Will update it and clang version as well. For clang version, I think I
>>>> can update min version as 11.0.0 (reference from
>>>> https://www.kernel.org/doc/html/next/process/changes.html). Do you see
>>>> any other potential outdated things in this document? I follow the above
>>>> steps and it help me compile the sample code successfully.
>>>
>>> Maybe we can reference that doc instead here? Otherwise that copy-pasted
>>> 11.0.0 will also get old. Just mention here that we need
>>> clang/llvm/pahole to compile the samples and for specific versions
>>> put a link to process/changes.rst
>>>
>>>>>>  Clean and configuration
>>>>>>  -----------------------
>>>>>>
>>>>>> @@ -28,6 +31,10 @@ Configure kernel, defconfig for instance::
>>>>>>
>>>>>>   make defconfig
>>>>>>
>>>>>> +Some samples require support for BPF Type Format (BTF). To enable it,
>>>>>> open the
>>>>>> +generated config file, or use menuconfig (by "make menuconfig") to
>>>>>> enable the
>>>>>> +following configs: CONFIG_BPF_SYSCALL and CONFIG_DEBUG_INFO_BTF.
>>>>>> +
>>>>>
>>>>> This is usually enabled by default, so why special case it here?
>>>>> Maybe, if you want some hints about the config, we should add
>>>>> a reference to tools/testing/selftests/bpf/config ?
>>>>>
>>>>
>>>> The config CONFIG_DEBUG_INFO_BTF is disabled for some distros at least
>>>> for mine. I ran "make defconfig" and it's not enabled by default so I
>>>> think it worth to mention it here to help novice get started. I'll
>>>> update it to reference to tools/testing/selftests/bpf/config .
>>>>
>>>>>>  Kernel headers
>>>>>>  --------------
>>>>>>
>>>>>> --
>>>>>> 2.34.1
>>
>> Signed-off-by: Anh Tuan Phan <tuananhlfc@gmail.com>
>> ---
>>
>> Change from the original patch:
>>
>> - Move pahole to the list installed requirements
>> - Remove minimal version and link the related doc
>> - Add a reference of kernel configuration
>>
>>  samples/bpf/README.rst | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst
>> index 57f93edd1957..e18500753ba5 100644
>> --- a/samples/bpf/README.rst
>> +++ b/samples/bpf/README.rst
>> @@ -8,9 +8,12 @@ Build dependencies
>>  ==================
>>
>>  Compiling requires having installed:
>> - * clang >= version 3.4.0
>> - * llvm >= version 3.7.1
>> + * clang
>> + * llvm
>> + * pahole
>>
>> +The minimal version of the above software is referenced in
>> +https://www.kernel.org/doc/html/next/process/changes.html.
> 
> I think it is better to not use docs from linux-next as it keeps changing
> too frequently. How about using the latest documentation's link instead? :)
> 
> https://www.kernel.org/doc/html/latest/process/changes.html
> 
> However, something to think about is: If future versions of clang, llvm etc
> do not support compiling our code as it is now, it may become misleading.
> 

Thanks, I'll update the documentation's link in the next version.

> 
>>  Note that LLVM's tool 'llc' must support target 'bpf', list version
>>  and supported targets with command: ``llc --version``
>>
>> @@ -24,7 +27,8 @@ after some changes (on demand)::
>>   make -C samples/bpf clean
>>   make clean
>>
>> -Configure kernel, defconfig for instance::
>> +Configure kernel, defconfig for instance
>> +(see "tools/testing/selftests/bpf/config" for a reference config)::
>>
>>   make defconfig
>>
>> --
> 
> thanks,
>   -- Khalid Masum
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees