diff mbox series

[bpf,v2] bpf: Add config for skipping BTF enum64s

Message ID 20220828233317.35464-1-yakoyoku@gmail.com (mailing list archive)
State Not Applicable
Delegated to: BPF
Headers show
Series [bpf,v2] bpf: Add config for skipping BTF enum64s | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 6 maintainers not CCed: john.fastabend@gmail.com song@kernel.org haoluo@google.com martin.lau@linux.dev kpsingh@kernel.org sdf@google.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-VM_Test-4 success Logs for llvm-toolchain
bpf/vmtest-bpf-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-VM_Test-2 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-3 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-1 success Logs for build for s390x with gcc
bpf/vmtest-bpf-VM_Test-7 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-8 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-16 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-17 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-10 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-11 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-13 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-14 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-6 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-VM_Test-12 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-VM_Test-15 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-PR success PR summary
bpf/vmtest-bpf-VM_Test-9 success Logs for test_progs on s390x with gcc

Commit Message

Martin Rodriguez Reboredo Aug. 28, 2022, 11:33 p.m. UTC
After the release of pahole 1.24 some people in the dwarves mailing list
notified issues related to building the kernel with the BTF_DEBUG_INFO
option toggled. They seem to be happenning due to the kernel and
resolve_btfids interpreting btf types erroneously. In the dwarves list
I've proposed a change to the scripts that I've written while testing
the Rust kernel, it simply passes the --skip_encoding_btf_enum64 to
pahole if it has version 1.24.

v1 -> v2:
- Switch to off by default and remove the config option.
- Send it to stable instead.

Signed-off-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
---
 scripts/pahole-flags.sh | 3 +++
 1 file changed, 3 insertions(+)

Comments

Daniel Borkmann Aug. 29, 2022, 3:57 p.m. UTC | #1
On 8/29/22 1:33 AM, Martin Rodriguez Reboredo wrote:
> After the release of pahole 1.24 some people in the dwarves mailing list
> notified issues related to building the kernel with the BTF_DEBUG_INFO
> option toggled. They seem to be happenning due to the kernel and
> resolve_btfids interpreting btf types erroneously. In the dwarves list
> I've proposed a change to the scripts that I've written while testing
> the Rust kernel, it simply passes the --skip_encoding_btf_enum64 to
> pahole if it has version 1.24.
> 
> v1 -> v2:
> - Switch to off by default and remove the config option.
> - Send it to stable instead.

This should only go to stable and not to bpf tree, right? To which stable versions?
For stable tree submissions you'd need to send this to stable@vger.kernel.org in Cc
and subject should say e.g. '[PATCH stable 5.4]' for 5.4 branch [0]. Either way, pls
clarify.

   [0] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/log/?h=linux-5.4.y

Thanks,
Daniel

> Signed-off-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> ---
>   scripts/pahole-flags.sh | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh
> index 0d99ef17e4a5..0a48fd86bc68 100755
> --- a/scripts/pahole-flags.sh
> +++ b/scripts/pahole-flags.sh
> @@ -19,5 +19,8 @@ fi
>   if [ "${pahole_ver}" -ge "122" ]; then
>   	extra_paholeopt="${extra_paholeopt} -j"
>   fi
> +if [ "${pahole_ver}" -ge "124" ]; then
> +	extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_enum64"
> +fi
>   
>   echo ${extra_paholeopt}
>
Jiri Olsa Sept. 2, 2022, 4:51 p.m. UTC | #2
On Sun, Aug 28, 2022 at 08:33:17PM -0300, Martin Rodriguez Reboredo wrote:
> After the release of pahole 1.24 some people in the dwarves mailing list
> notified issues related to building the kernel with the BTF_DEBUG_INFO
> option toggled. They seem to be happenning due to the kernel and
> resolve_btfids interpreting btf types erroneously. In the dwarves list
> I've proposed a change to the scripts that I've written while testing
> the Rust kernel, it simply passes the --skip_encoding_btf_enum64 to
> pahole if it has version 1.24.
> 
> v1 -> v2:
> - Switch to off by default and remove the config option.
> - Send it to stable instead.

hi,
we have change that needs to go to stable kernels but does not have the
equivalent fix in Linus tree

what would be the best way to submit it?

the issue is that new 'pahole' will generate BTF data that are not supported
by older kernels, so we need to add --skip_encoding_btf_enum64 option to
stable kernel's scripts/pahole-flags.sh to generate proper BTF data

we got complains that after upgrading to latest pahole the stable kernel
compilation fails

thanks,
jirka


> 
> Signed-off-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> ---
>  scripts/pahole-flags.sh | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh
> index 0d99ef17e4a5..0a48fd86bc68 100755
> --- a/scripts/pahole-flags.sh
> +++ b/scripts/pahole-flags.sh
> @@ -19,5 +19,8 @@ fi
>  if [ "${pahole_ver}" -ge "122" ]; then
>  	extra_paholeopt="${extra_paholeopt} -j"
>  fi
> +if [ "${pahole_ver}" -ge "124" ]; then
> +	extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_enum64"
> +fi
>  
>  echo ${extra_paholeopt}
> -- 
> 2.37.2
>
Greg Kroah-Hartman Sept. 3, 2022, 5:26 a.m. UTC | #3
On Fri, Sep 02, 2022 at 06:51:00PM +0200, Jiri Olsa wrote:
> On Sun, Aug 28, 2022 at 08:33:17PM -0300, Martin Rodriguez Reboredo wrote:
> > After the release of pahole 1.24 some people in the dwarves mailing list
> > notified issues related to building the kernel with the BTF_DEBUG_INFO
> > option toggled. They seem to be happenning due to the kernel and
> > resolve_btfids interpreting btf types erroneously. In the dwarves list
> > I've proposed a change to the scripts that I've written while testing
> > the Rust kernel, it simply passes the --skip_encoding_btf_enum64 to
> > pahole if it has version 1.24.
> > 
> > v1 -> v2:
> > - Switch to off by default and remove the config option.
> > - Send it to stable instead.
> 
> hi,
> we have change that needs to go to stable kernels but does not have the
> equivalent fix in Linus tree

Why isn't it also relevant in Linus's tree?

> what would be the best way to submit it?

Submit it here and document the heck out of why this isn't in Linus's
tree, what changes instead fixed it there, and so on.  Look in the
archives for examples of how this is done, one recent one that I can
think of is here:
	https://lore.kernel.org/r/20220831191348.3388208-1-jannh@google.com

> the issue is that new 'pahole' will generate BTF data that are not supported
> by older kernels, so we need to add --skip_encoding_btf_enum64 option to
> stable kernel's scripts/pahole-flags.sh to generate proper BTF data
> 
> we got complains that after upgrading to latest pahole the stable kernel
> compilation fails

And what is happening in Linus's tree for this same issue?

thanks,

greg k-h
Arnaldo Carvalho de Melo Sept. 3, 2022, 2:13 p.m. UTC | #4
Em Sat, Sep 03, 2022 at 07:26:58AM +0200, Greg KH escreveu:
> On Fri, Sep 02, 2022 at 06:51:00PM +0200, Jiri Olsa wrote:
> > On Sun, Aug 28, 2022 at 08:33:17PM -0300, Martin Rodriguez Reboredo wrote:
> > > After the release of pahole 1.24 some people in the dwarves mailing list
> > > notified issues related to building the kernel with the BTF_DEBUG_INFO
> > > option toggled. They seem to be happenning due to the kernel and
> > > resolve_btfids interpreting btf types erroneously. In the dwarves list
> > > I've proposed a change to the scripts that I've written while testing
> > > the Rust kernel, it simply passes the --skip_encoding_btf_enum64 to
> > > pahole if it has version 1.24.
> > > 
> > > v1 -> v2:
> > > - Switch to off by default and remove the config option.
> > > - Send it to stable instead.
> > 
> > hi,
> > we have change that needs to go to stable kernels but does not have the
> > equivalent fix in Linus tree
> 
> Why isn't it also relevant in Linus's tree?

See below.
 
> > what would be the best way to submit it?
> 
> Submit it here and document the heck out of why this isn't in Linus's
> tree, what changes instead fixed it there, and so on.  Look in the
> archives for examples of how this is done, one recent one that I can
> think of is here:
> 	https://lore.kernel.org/r/20220831191348.3388208-1-jannh@google.com
> 
> > the issue is that new 'pahole' will generate BTF data that are not supported
> > by older kernels, so we need to add --skip_encoding_btf_enum64 option to
> > stable kernel's scripts/pahole-flags.sh to generate proper BTF data
> > 
> > we got complains that after upgrading to latest pahole the stable kernel
> > compilation fails
> 
> And what is happening in Linus's tree for this same issue?

So, BTF_KIND_ENUM64 is a new BTF tag, one that is not accepted by older
kernels, but is accepted by the BPF verifier on Linus' tree.

Its about avoiding having a pahole command line with lots of
--enable-new-feature-foo for new stuff with the default producing the
most recent BTF spec.

One way to documenting it: if you update pahole, then please use
--skip_encoding_FOO for these new FOO features on kernels where those
aren't supported.

So this isn't a backport from a fix on Linus' tree, as both the older
pahole that doesn't encode BTF_KIND_ENUM64 and the new one, that encodes
it by default, work with Linus' tree.

Does this violates the stable@ rules?

- Arnaldo
Greg Kroah-Hartman Sept. 4, 2022, 6:18 a.m. UTC | #5
On Sat, Sep 03, 2022 at 11:13:45AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Sep 03, 2022 at 07:26:58AM +0200, Greg KH escreveu:
> > On Fri, Sep 02, 2022 at 06:51:00PM +0200, Jiri Olsa wrote:
> > > On Sun, Aug 28, 2022 at 08:33:17PM -0300, Martin Rodriguez Reboredo wrote:
> > > > After the release of pahole 1.24 some people in the dwarves mailing list
> > > > notified issues related to building the kernel with the BTF_DEBUG_INFO
> > > > option toggled. They seem to be happenning due to the kernel and
> > > > resolve_btfids interpreting btf types erroneously. In the dwarves list
> > > > I've proposed a change to the scripts that I've written while testing
> > > > the Rust kernel, it simply passes the --skip_encoding_btf_enum64 to
> > > > pahole if it has version 1.24.
> > > > 
> > > > v1 -> v2:
> > > > - Switch to off by default and remove the config option.
> > > > - Send it to stable instead.
> > > 
> > > hi,
> > > we have change that needs to go to stable kernels but does not have the
> > > equivalent fix in Linus tree
> > 
> > Why isn't it also relevant in Linus's tree?
> 
> See below.
>  
> > > what would be the best way to submit it?
> > 
> > Submit it here and document the heck out of why this isn't in Linus's
> > tree, what changes instead fixed it there, and so on.  Look in the
> > archives for examples of how this is done, one recent one that I can
> > think of is here:
> > 	https://lore.kernel.org/r/20220831191348.3388208-1-jannh@google.com
> > 
> > > the issue is that new 'pahole' will generate BTF data that are not supported
> > > by older kernels, so we need to add --skip_encoding_btf_enum64 option to
> > > stable kernel's scripts/pahole-flags.sh to generate proper BTF data
> > > 
> > > we got complains that after upgrading to latest pahole the stable kernel
> > > compilation fails
> > 
> > And what is happening in Linus's tree for this same issue?
> 
> So, BTF_KIND_ENUM64 is a new BTF tag, one that is not accepted by older
> kernels, but is accepted by the BPF verifier on Linus' tree.
> 
> Its about avoiding having a pahole command line with lots of
> --enable-new-feature-foo for new stuff with the default producing the
> most recent BTF spec.
> 
> One way to documenting it: if you update pahole, then please use
> --skip_encoding_FOO for these new FOO features on kernels where those
> aren't supported.
> 
> So this isn't a backport from a fix on Linus' tree, as both the older
> pahole that doesn't encode BTF_KIND_ENUM64 and the new one, that encodes
> it by default, work with Linus' tree.
> 
> Does this violates the stable@ rules?

Not really, if it fixes an issue for those kernels when using newer
tools, that's fine.  Just document it well like you did here.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh
index 0d99ef17e4a5..0a48fd86bc68 100755
--- a/scripts/pahole-flags.sh
+++ b/scripts/pahole-flags.sh
@@ -19,5 +19,8 @@  fi
 if [ "${pahole_ver}" -ge "122" ]; then
 	extra_paholeopt="${extra_paholeopt} -j"
 fi
+if [ "${pahole_ver}" -ge "124" ]; then
+	extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_enum64"
+fi
 
 echo ${extra_paholeopt}