| Message ID | 20191126232818.226454-1-sdf@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf,v2] bpf: support pre-2.25-binutils objcopy for vmlinux BTF | expand |
Stanislav Fomichev wrote: > If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set, > .BTF section of vmlinux is empty and kernel will prohibit > BPF loading and return "in-kernel BTF is malformed". > > --dump-section argument to binutils' objcopy was added in version 2.25. > When using pre-2.25 binutils, BTF generation silently fails. Convert > to --only-section which is present on pre-2.25 binutils. > > Documentation/process/changes.rst states that binutils 2.21+ > is supported, not sure those standards apply to BPF subsystem. > > v2: > * exit and print an error if gen_btf fails (John Fastabend) > > Cc: Andrii Nakryiko <andriin@fb.com> > Cc: John Fastabend <john.fastabend@gmail.com> > Fixes: 341dfcf8d78ea ("btf: expose BTF info through sysfs") > Signed-off-by: Stanislav Fomichev <sdf@google.com> > --- > scripts/link-vmlinux.sh | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index 06495379fcd8..2998ddb323e3 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -127,7 +127,8 @@ gen_btf() > cut -d, -f1 | cut -d' ' -f2) > bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > awk '{print $4}') > - ${OBJCOPY} --dump-section .BTF=.btf.vmlinux.bin ${1} 2>/dev/null > + ${OBJCOPY} --set-section-flags .BTF=alloc -O binary \ > + --only-section=.BTF ${1} .btf.vmlinux.bin 2>/dev/null > ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > --rename-section .data=.BTF .btf.vmlinux.bin ${2} > } > @@ -253,6 +254,10 @@ btf_vmlinux_bin_o="" > if [ -n "${CONFIG_DEBUG_INFO_BTF}" ]; then > if gen_btf .tmp_vmlinux.btf .btf.vmlinux.bin.o ; then > btf_vmlinux_bin_o=.btf.vmlinux.bin.o > + else > + echo >&2 "Failed to generate BTF for vmlinux" > + echo >&2 "Try to disable CONFIG_DEBUG_INFO_BTF" I think we should encourage upgrading binutils first? Maybe "binutils 2.25+ required for BTF please upgrade or disable CONFIG_DEBUG_INFO_BTF" otherwise I guess its going to be a bit mystical why it works in cases and not others to folks unfamiliar with the details. > + exit 1 > fi > fi > > -- > 2.24.0.432.g9d3f5f5b63-goog >
On 11/26, John Fastabend wrote: > Stanislav Fomichev wrote: > > If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set, > > .BTF section of vmlinux is empty and kernel will prohibit > > BPF loading and return "in-kernel BTF is malformed". > > > > --dump-section argument to binutils' objcopy was added in version 2.25. > > When using pre-2.25 binutils, BTF generation silently fails. Convert > > to --only-section which is present on pre-2.25 binutils. > > > > Documentation/process/changes.rst states that binutils 2.21+ > > is supported, not sure those standards apply to BPF subsystem. > > > > v2: > > * exit and print an error if gen_btf fails (John Fastabend) > > > > Cc: Andrii Nakryiko <andriin@fb.com> > > Cc: John Fastabend <john.fastabend@gmail.com> > > Fixes: 341dfcf8d78ea ("btf: expose BTF info through sysfs") > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > --- > > scripts/link-vmlinux.sh | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > > index 06495379fcd8..2998ddb323e3 100755 > > --- a/scripts/link-vmlinux.sh > > +++ b/scripts/link-vmlinux.sh > > @@ -127,7 +127,8 @@ gen_btf() > > cut -d, -f1 | cut -d' ' -f2) > > bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > > awk '{print $4}') > > - ${OBJCOPY} --dump-section .BTF=.btf.vmlinux.bin ${1} 2>/dev/null > > + ${OBJCOPY} --set-section-flags .BTF=alloc -O binary \ > > + --only-section=.BTF ${1} .btf.vmlinux.bin 2>/dev/null > > ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > > --rename-section .data=.BTF .btf.vmlinux.bin ${2} > > } > > @@ -253,6 +254,10 @@ btf_vmlinux_bin_o="" > > if [ -n "${CONFIG_DEBUG_INFO_BTF}" ]; then > > if gen_btf .tmp_vmlinux.btf .btf.vmlinux.bin.o ; then > > btf_vmlinux_bin_o=.btf.vmlinux.bin.o > > + else > > + echo >&2 "Failed to generate BTF for vmlinux" > > + echo >&2 "Try to disable CONFIG_DEBUG_INFO_BTF" > > I think we should encourage upgrading binutils first? Maybe > > "binutils 2.25+ required for BTF please upgrade or disable CONFIG_DEBUG_INFO_BTF" > > otherwise I guess its going to be a bit mystical why it works in > cases and not others to folks unfamiliar with the details. With the conversion from --dump-section to --only-section that I did in this patch, binutils 2.25+ is no longer a requirement. 2.21 (minimal version from Documentation/process/changes.rst) should work just fine.
On 11/26/19 3:45 PM, Stanislav Fomichev wrote: > On 11/26, John Fastabend wrote: >> Stanislav Fomichev wrote: >>> If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set, >>> .BTF section of vmlinux is empty and kernel will prohibit >>> BPF loading and return "in-kernel BTF is malformed". >>> >>> --dump-section argument to binutils' objcopy was added in version 2.25. >>> When using pre-2.25 binutils, BTF generation silently fails. Convert >>> to --only-section which is present on pre-2.25 binutils. >>> >>> Documentation/process/changes.rst states that binutils 2.21+ >>> is supported, not sure those standards apply to BPF subsystem. >>> >>> v2: >>> * exit and print an error if gen_btf fails (John Fastabend) >>> >>> Cc: Andrii Nakryiko <andriin@fb.com> >>> Cc: John Fastabend <john.fastabend@gmail.com> >>> Fixes: 341dfcf8d78ea ("btf: expose BTF info through sysfs") >>> Signed-off-by: Stanislav Fomichev <sdf@google.com> >>> --- >>> scripts/link-vmlinux.sh | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >>> index 06495379fcd8..2998ddb323e3 100755 >>> --- a/scripts/link-vmlinux.sh >>> +++ b/scripts/link-vmlinux.sh >>> @@ -127,7 +127,8 @@ gen_btf() >>> cut -d, -f1 | cut -d' ' -f2) >>> bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >>> awk '{print $4}') >>> - ${OBJCOPY} --dump-section .BTF=.btf.vmlinux.bin ${1} 2>/dev/null >>> + ${OBJCOPY} --set-section-flags .BTF=alloc -O binary \ >>> + --only-section=.BTF ${1} .btf.vmlinux.bin 2>/dev/null >>> ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >>> --rename-section .data=.BTF .btf.vmlinux.bin ${2} >>> } >>> @@ -253,6 +254,10 @@ btf_vmlinux_bin_o="" >>> if [ -n "${CONFIG_DEBUG_INFO_BTF}" ]; then >>> if gen_btf .tmp_vmlinux.btf .btf.vmlinux.bin.o ; then >>> btf_vmlinux_bin_o=.btf.vmlinux.bin.o >>> + else >>> + echo >&2 "Failed to generate BTF for vmlinux" >>> + echo >&2 "Try to disable CONFIG_DEBUG_INFO_BTF" >> >> I think we should encourage upgrading binutils first? Maybe >> >> "binutils 2.25+ required for BTF please upgrade or disable CONFIG_DEBUG_INFO_BTF" >> >> otherwise I guess its going to be a bit mystical why it works in >> cases and not others to folks unfamiliar with the details. > With the conversion from --dump-section to --only-section that I > did in this patch, binutils 2.25+ is no longer a requirement. > 2.21 (minimal version from Documentation/process/changes.rst) should work > just fine. Yeah, instead it's better to mention that pahole v1.13+ is required. >
On 11/26, Andrii Nakryiko wrote: > On 11/26/19 3:45 PM, Stanislav Fomichev wrote: > > On 11/26, John Fastabend wrote: > >> Stanislav Fomichev wrote: > >>> If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set, > >>> .BTF section of vmlinux is empty and kernel will prohibit > >>> BPF loading and return "in-kernel BTF is malformed". > >>> > >>> --dump-section argument to binutils' objcopy was added in version 2.25. > >>> When using pre-2.25 binutils, BTF generation silently fails. Convert > >>> to --only-section which is present on pre-2.25 binutils. > >>> > >>> Documentation/process/changes.rst states that binutils 2.21+ > >>> is supported, not sure those standards apply to BPF subsystem. > >>> > >>> v2: > >>> * exit and print an error if gen_btf fails (John Fastabend) > >>> > >>> Cc: Andrii Nakryiko <andriin@fb.com> > >>> Cc: John Fastabend <john.fastabend@gmail.com> > >>> Fixes: 341dfcf8d78ea ("btf: expose BTF info through sysfs") > >>> Signed-off-by: Stanislav Fomichev <sdf@google.com> > >>> --- > >>> scripts/link-vmlinux.sh | 7 ++++++- > >>> 1 file changed, 6 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > >>> index 06495379fcd8..2998ddb323e3 100755 > >>> --- a/scripts/link-vmlinux.sh > >>> +++ b/scripts/link-vmlinux.sh > >>> @@ -127,7 +127,8 @@ gen_btf() > >>> cut -d, -f1 | cut -d' ' -f2) > >>> bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ > >>> awk '{print $4}') > >>> - ${OBJCOPY} --dump-section .BTF=.btf.vmlinux.bin ${1} 2>/dev/null > >>> + ${OBJCOPY} --set-section-flags .BTF=alloc -O binary \ > >>> + --only-section=.BTF ${1} .btf.vmlinux.bin 2>/dev/null > >>> ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ > >>> --rename-section .data=.BTF .btf.vmlinux.bin ${2} > >>> } > >>> @@ -253,6 +254,10 @@ btf_vmlinux_bin_o="" > >>> if [ -n "${CONFIG_DEBUG_INFO_BTF}" ]; then > >>> if gen_btf .tmp_vmlinux.btf .btf.vmlinux.bin.o ; then > >>> btf_vmlinux_bin_o=.btf.vmlinux.bin.o > >>> + else > >>> + echo >&2 "Failed to generate BTF for vmlinux" > >>> + echo >&2 "Try to disable CONFIG_DEBUG_INFO_BTF" > >> > >> I think we should encourage upgrading binutils first? Maybe > >> > >> "binutils 2.25+ required for BTF please upgrade or disable CONFIG_DEBUG_INFO_BTF" > >> > >> otherwise I guess its going to be a bit mystical why it works in > >> cases and not others to folks unfamiliar with the details. > > With the conversion from --dump-section to --only-section that I > > did in this patch, binutils 2.25+ is no longer a requirement. > > 2.21 (minimal version from Documentation/process/changes.rst) should work > > just fine. > > Yeah, instead it's better to mention that pahole v1.13+ is required. We already have most of the messages about missing pahole or wrong version: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/scripts/link-vmlinux.sh#n111 https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/scripts/link-vmlinux.sh#n117 They are 'info' though, but it seems logical to drop -s from make arguments if someone wants to debug further.
On 11/26/19 4:04 PM, Stanislav Fomichev wrote: > On 11/26, Andrii Nakryiko wrote: >> On 11/26/19 3:45 PM, Stanislav Fomichev wrote: >>> On 11/26, John Fastabend wrote: >>>> Stanislav Fomichev wrote: >>>>> If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set, >>>>> .BTF section of vmlinux is empty and kernel will prohibit >>>>> BPF loading and return "in-kernel BTF is malformed". >>>>> >>>>> --dump-section argument to binutils' objcopy was added in version 2.25. >>>>> When using pre-2.25 binutils, BTF generation silently fails. Convert >>>>> to --only-section which is present on pre-2.25 binutils. >>>>> >>>>> Documentation/process/changes.rst states that binutils 2.21+ >>>>> is supported, not sure those standards apply to BPF subsystem. >>>>> >>>>> v2: >>>>> * exit and print an error if gen_btf fails (John Fastabend) >>>>> >>>>> Cc: Andrii Nakryiko <andriin@fb.com> >>>>> Cc: John Fastabend <john.fastabend@gmail.com> >>>>> Fixes: 341dfcf8d78ea ("btf: expose BTF info through sysfs") >>>>> Signed-off-by: Stanislav Fomichev <sdf@google.com> >>>>> --- >>>>> scripts/link-vmlinux.sh | 7 ++++++- >>>>> 1 file changed, 6 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh >>>>> index 06495379fcd8..2998ddb323e3 100755 >>>>> --- a/scripts/link-vmlinux.sh >>>>> +++ b/scripts/link-vmlinux.sh >>>>> @@ -127,7 +127,8 @@ gen_btf() >>>>> cut -d, -f1 | cut -d' ' -f2) >>>>> bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ >>>>> awk '{print $4}') >>>>> - ${OBJCOPY} --dump-section .BTF=.btf.vmlinux.bin ${1} 2>/dev/null >>>>> + ${OBJCOPY} --set-section-flags .BTF=alloc -O binary \ >>>>> + --only-section=.BTF ${1} .btf.vmlinux.bin 2>/dev/null >>>>> ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ >>>>> --rename-section .data=.BTF .btf.vmlinux.bin ${2} >>>>> } >>>>> @@ -253,6 +254,10 @@ btf_vmlinux_bin_o="" >>>>> if [ -n "${CONFIG_DEBUG_INFO_BTF}" ]; then >>>>> if gen_btf .tmp_vmlinux.btf .btf.vmlinux.bin.o ; then >>>>> btf_vmlinux_bin_o=.btf.vmlinux.bin.o >>>>> + else >>>>> + echo >&2 "Failed to generate BTF for vmlinux" >>>>> + echo >&2 "Try to disable CONFIG_DEBUG_INFO_BTF" >>>> >>>> I think we should encourage upgrading binutils first? Maybe >>>> >>>> "binutils 2.25+ required for BTF please upgrade or disable CONFIG_DEBUG_INFO_BTF" >>>> >>>> otherwise I guess its going to be a bit mystical why it works in >>>> cases and not others to folks unfamiliar with the details. >>> With the conversion from --dump-section to --only-section that I >>> did in this patch, binutils 2.25+ is no longer a requirement. >>> 2.21 (minimal version from Documentation/process/changes.rst) should work >>> just fine. >> >> Yeah, instead it's better to mention that pahole v1.13+ is required. > We already have most of the messages about missing pahole or wrong version: > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/scripts/link-vmlinux.sh#n111 > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/scripts/link-vmlinux.sh#n117 > > They are 'info' though, but it seems logical to drop -s from make arguments > if someone wants to debug further. > Fair enough. Please add back my Tested-by: tag. Acked-by: Andrii Nakryiko <andriin@fb.com>
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 06495379fcd8..2998ddb323e3 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -127,7 +127,8 @@ gen_btf() cut -d, -f1 | cut -d' ' -f2) bin_format=$(LANG=C ${OBJDUMP} -f ${1} | grep 'file format' | \ awk '{print $4}') - ${OBJCOPY} --dump-section .BTF=.btf.vmlinux.bin ${1} 2>/dev/null + ${OBJCOPY} --set-section-flags .BTF=alloc -O binary \ + --only-section=.BTF ${1} .btf.vmlinux.bin 2>/dev/null ${OBJCOPY} -I binary -O ${bin_format} -B ${bin_arch} \ --rename-section .data=.BTF .btf.vmlinux.bin ${2} } @@ -253,6 +254,10 @@ btf_vmlinux_bin_o="" if [ -n "${CONFIG_DEBUG_INFO_BTF}" ]; then if gen_btf .tmp_vmlinux.btf .btf.vmlinux.bin.o ; then btf_vmlinux_bin_o=.btf.vmlinux.bin.o + else + echo >&2 "Failed to generate BTF for vmlinux" + echo >&2 "Try to disable CONFIG_DEBUG_INFO_BTF" + exit 1 fi fi
If vmlinux BTF generation fails, but CONFIG_DEBUG_INFO_BTF is set, .BTF section of vmlinux is empty and kernel will prohibit BPF loading and return "in-kernel BTF is malformed". --dump-section argument to binutils' objcopy was added in version 2.25. When using pre-2.25 binutils, BTF generation silently fails. Convert to --only-section which is present on pre-2.25 binutils. Documentation/process/changes.rst states that binutils 2.21+ is supported, not sure those standards apply to BPF subsystem. v2: * exit and print an error if gen_btf fails (John Fastabend) Cc: Andrii Nakryiko <andriin@fb.com> Cc: John Fastabend <john.fastabend@gmail.com> Fixes: 341dfcf8d78ea ("btf: expose BTF info through sysfs") Signed-off-by: Stanislav Fomichev <sdf@google.com> --- scripts/link-vmlinux.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)