On 7/9/23 22:21, Khalid Masum wrote: > Hi, > > On Sun, Jul 9, 2023 at 8:38 PM Anh Tuan Phan <tuananhlfc@xxxxxxxxx> 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@xxxxxxxxx> >>>>>> --- >>>>>> 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@xxxxxxxxx> >> --- >> >> 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
