| Message ID | 20210316011445.4179633-1-kafai@fb.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Support calling kernel function | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 5 maintainers not CCed: yhs@fb.com kpsingh@kernel.org andrii@kernel.org john.fastabend@gmail.com songliubraving@fb.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 86 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Tue, Mar 16, 2021 at 12:02 AM Martin KaFai Lau <kafai@fb.com> wrote: > > This patch records the extern sym relocs first before recording > subprog relocs. The later patch will have relocs for extern > kernel function call which is also using BPF_JMP | BPF_CALL. > It will be easier to handle the extern symbols first in > the later patch. > > Signed-off-by: Martin KaFai Lau <kafai@fb.com> > --- Looks good, just let's add that tiny helper for cleanliness and to match what we do for ldimm64 Acked-by: Andrii Nakryiko <andrii@kernel.org> > tools/lib/bpf/libbpf.c | 50 +++++++++++++++++++++--------------------- > 1 file changed, 25 insertions(+), 25 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 8f924aece736..0a60fcb2fba2 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -3416,31 +3416,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog, > > reloc_desc->processed = false; > > - /* sub-program call relocation */ > - if (insn->code == (BPF_JMP | BPF_CALL)) { > - if (insn->src_reg != BPF_PSEUDO_CALL) { > - pr_warn("prog '%s': incorrect bpf_call opcode\n", prog->name); > - return -LIBBPF_ERRNO__RELOC; > - } > - /* text_shndx can be 0, if no default "main" program exists */ > - if (!shdr_idx || shdr_idx != obj->efile.text_shndx) { > - sym_sec_name = elf_sec_name(obj, elf_sec_by_idx(obj, shdr_idx)); > - pr_warn("prog '%s': bad call relo against '%s' in section '%s'\n", > - prog->name, sym_name, sym_sec_name); > - return -LIBBPF_ERRNO__RELOC; > - } > - if (sym->st_value % BPF_INSN_SZ) { > - pr_warn("prog '%s': bad call relo against '%s' at offset %zu\n", > - prog->name, sym_name, (size_t)sym->st_value); > - return -LIBBPF_ERRNO__RELOC; > - } > - reloc_desc->type = RELO_CALL; > - reloc_desc->insn_idx = insn_idx; > - reloc_desc->sym_off = sym->st_value; > - return 0; > - } > - > - if (!is_ldimm64(insn)) { > + if (insn->code != (BPF_JMP | BPF_CALL) && !is_ldimm64(insn)) { > pr_warn("prog '%s': invalid relo against '%s' for insns[%d].code 0x%x\n", > prog->name, sym_name, insn_idx, insn->code); > return -LIBBPF_ERRNO__RELOC; > @@ -3469,6 +3445,30 @@ static int bpf_program__record_reloc(struct bpf_program *prog, > return 0; > } > > + /* sub-program call relocation */ > + if (insn->code == (BPF_JMP | BPF_CALL)) { can you please add is_call_insn() helper checking this, similarly to how we now have is_ldimm64() (should probably be is_ldimm64_insn() for consistency) > + if (insn->src_reg != BPF_PSEUDO_CALL) { > + pr_warn("prog '%s': incorrect bpf_call opcode\n", prog->name); > + return -LIBBPF_ERRNO__RELOC; > + } > + /* text_shndx can be 0, if no default "main" program exists */ > + if (!shdr_idx || shdr_idx != obj->efile.text_shndx) { > + sym_sec_name = elf_sec_name(obj, elf_sec_by_idx(obj, shdr_idx)); > + pr_warn("prog '%s': bad call relo against '%s' in section '%s'\n", > + prog->name, sym_name, sym_sec_name); > + return -LIBBPF_ERRNO__RELOC; > + } > + if (sym->st_value % BPF_INSN_SZ) { > + pr_warn("prog '%s': bad call relo against '%s' at offset %zu\n", > + prog->name, sym_name, (size_t)sym->st_value); > + return -LIBBPF_ERRNO__RELOC; > + } > + reloc_desc->type = RELO_CALL; > + reloc_desc->insn_idx = insn_idx; > + reloc_desc->sym_off = sym->st_value; > + return 0; > + } > + > if (!shdr_idx || shdr_idx >= SHN_LORESERVE) { > pr_warn("prog '%s': invalid relo against '%s' in special section 0x%x; forgot to initialize global var?..\n", > prog->name, sym_name, shdr_idx); > -- > 2.30.2 >
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 8f924aece736..0a60fcb2fba2 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -3416,31 +3416,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog, reloc_desc->processed = false; - /* sub-program call relocation */ - if (insn->code == (BPF_JMP | BPF_CALL)) { - if (insn->src_reg != BPF_PSEUDO_CALL) { - pr_warn("prog '%s': incorrect bpf_call opcode\n", prog->name); - return -LIBBPF_ERRNO__RELOC; - } - /* text_shndx can be 0, if no default "main" program exists */ - if (!shdr_idx || shdr_idx != obj->efile.text_shndx) { - sym_sec_name = elf_sec_name(obj, elf_sec_by_idx(obj, shdr_idx)); - pr_warn("prog '%s': bad call relo against '%s' in section '%s'\n", - prog->name, sym_name, sym_sec_name); - return -LIBBPF_ERRNO__RELOC; - } - if (sym->st_value % BPF_INSN_SZ) { - pr_warn("prog '%s': bad call relo against '%s' at offset %zu\n", - prog->name, sym_name, (size_t)sym->st_value); - return -LIBBPF_ERRNO__RELOC; - } - reloc_desc->type = RELO_CALL; - reloc_desc->insn_idx = insn_idx; - reloc_desc->sym_off = sym->st_value; - return 0; - } - - if (!is_ldimm64(insn)) { + if (insn->code != (BPF_JMP | BPF_CALL) && !is_ldimm64(insn)) { pr_warn("prog '%s': invalid relo against '%s' for insns[%d].code 0x%x\n", prog->name, sym_name, insn_idx, insn->code); return -LIBBPF_ERRNO__RELOC; @@ -3469,6 +3445,30 @@ static int bpf_program__record_reloc(struct bpf_program *prog, return 0; } + /* sub-program call relocation */ + if (insn->code == (BPF_JMP | BPF_CALL)) { + if (insn->src_reg != BPF_PSEUDO_CALL) { + pr_warn("prog '%s': incorrect bpf_call opcode\n", prog->name); + return -LIBBPF_ERRNO__RELOC; + } + /* text_shndx can be 0, if no default "main" program exists */ + if (!shdr_idx || shdr_idx != obj->efile.text_shndx) { + sym_sec_name = elf_sec_name(obj, elf_sec_by_idx(obj, shdr_idx)); + pr_warn("prog '%s': bad call relo against '%s' in section '%s'\n", + prog->name, sym_name, sym_sec_name); + return -LIBBPF_ERRNO__RELOC; + } + if (sym->st_value % BPF_INSN_SZ) { + pr_warn("prog '%s': bad call relo against '%s' at offset %zu\n", + prog->name, sym_name, (size_t)sym->st_value); + return -LIBBPF_ERRNO__RELOC; + } + reloc_desc->type = RELO_CALL; + reloc_desc->insn_idx = insn_idx; + reloc_desc->sym_off = sym->st_value; + return 0; + } + if (!shdr_idx || shdr_idx >= SHN_LORESERVE) { pr_warn("prog '%s': invalid relo against '%s' in special section 0x%x; forgot to initialize global var?..\n", prog->name, sym_name, shdr_idx);
This patch records the extern sym relocs first before recording subprog relocs. The later patch will have relocs for extern kernel function call which is also using BPF_JMP | BPF_CALL. It will be easier to handle the extern symbols first in the later patch. Signed-off-by: Martin KaFai Lau <kafai@fb.com> --- tools/lib/bpf/libbpf.c | 50 +++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 25 deletions(-)