| Message ID | 20180518125039.6500-2-sandipan@linux.vnet.ibm.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: enhancements for multi-function programs | expand |
On 05/18/2018 02:50 PM, Sandipan Das wrote: > The imm field of a bpf instruction is a signed 32-bit integer. > For JIT bpf-to-bpf function calls, it stores the offset of the > start address of the callee's JITed image from __bpf_call_base. > > For some architectures, such as powerpc64, this offset may be > as large as 64 bits and cannot be accomodated in the imm field > without truncation. > > We resolve this by: > > [1] Additionally using the auxillary data of each function to > keep a list of start addresses of the JITed images for all > functions determined by the verifier. > > [2] Retaining the subprog id inside the off field of the call > instructions and using it to index into the list mentioned > above and lookup the callee's address. > > To make sure that the existing JIT compilers continue to work > without requiring changes, we keep the imm field as it is. > > Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com> > --- > kernel/bpf/verifier.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index a9e4b1372da6..6c56cce9c4e3 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -5383,11 +5383,24 @@ static int jit_subprogs(struct bpf_verifier_env *env) > insn->src_reg != BPF_PSEUDO_CALL) > continue; > subprog = insn->off; > - insn->off = 0; > insn->imm = (u64 (*)(u64, u64, u64, u64, u64)) > func[subprog]->bpf_func - > __bpf_call_base; > } > + > + /* we use the aux data to keep a list of the start addresses > + * of the JITed images for each function in the program > + * > + * for some architectures, such as powerpc64, the imm field > + * might not be large enough to hold the offset of the start > + * address of the callee's JITed image from __bpf_call_base > + * > + * in such cases, we can lookup the start address of a callee > + * by using its subprog id, available from the off field of > + * the call instruction, as an index for this list > + */ > + func[i]->aux->func = func; > + func[i]->aux->func_cnt = env->subprog_cnt + 1; The target tree you have here is infact bpf, since in bpf-next there was a cleanup where the + 1 is removed. Just for the record that we need to keep this in mind for bpf into bpf-next merge since this would otherwise subtly break. > } > for (i = 0; i < env->subprog_cnt; i++) { > old_bpf_func = func[i]->bpf_func; >
On 05/18/2018 08:45 PM, Daniel Borkmann wrote: > On 05/18/2018 02:50 PM, Sandipan Das wrote: >> The imm field of a bpf instruction is a signed 32-bit integer. >> For JIT bpf-to-bpf function calls, it stores the offset of the >> start address of the callee's JITed image from __bpf_call_base. >> >> For some architectures, such as powerpc64, this offset may be >> as large as 64 bits and cannot be accomodated in the imm field >> without truncation. >> >> We resolve this by: >> >> [1] Additionally using the auxillary data of each function to >> keep a list of start addresses of the JITed images for all >> functions determined by the verifier. >> >> [2] Retaining the subprog id inside the off field of the call >> instructions and using it to index into the list mentioned >> above and lookup the callee's address. >> >> To make sure that the existing JIT compilers continue to work >> without requiring changes, we keep the imm field as it is. >> >> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com> >> --- >> kernel/bpf/verifier.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index a9e4b1372da6..6c56cce9c4e3 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -5383,11 +5383,24 @@ static int jit_subprogs(struct bpf_verifier_env *env) >> insn->src_reg != BPF_PSEUDO_CALL) >> continue; >> subprog = insn->off; >> - insn->off = 0; >> insn->imm = (u64 (*)(u64, u64, u64, u64, u64)) >> func[subprog]->bpf_func - >> __bpf_call_base; >> } >> + >> + /* we use the aux data to keep a list of the start addresses >> + * of the JITed images for each function in the program >> + * >> + * for some architectures, such as powerpc64, the imm field >> + * might not be large enough to hold the offset of the start >> + * address of the callee's JITed image from __bpf_call_base >> + * >> + * in such cases, we can lookup the start address of a callee >> + * by using its subprog id, available from the off field of >> + * the call instruction, as an index for this list >> + */ >> + func[i]->aux->func = func; >> + func[i]->aux->func_cnt = env->subprog_cnt + 1; > > The target tree you have here is infact bpf, since in bpf-next there was a > cleanup where the + 1 is removed. Just for the record that we need to keep > this in mind for bpf into bpf-next merge since this would otherwise subtly > break. > Sorry about the wrong tag. This series is indeed based off bpf-next. - Sandipan >> } >> for (i = 0; i < env->subprog_cnt; i++) { >> old_bpf_func = func[i]->bpf_func; >> > >
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index a9e4b1372da6..6c56cce9c4e3 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5383,11 +5383,24 @@ static int jit_subprogs(struct bpf_verifier_env *env) insn->src_reg != BPF_PSEUDO_CALL) continue; subprog = insn->off; - insn->off = 0; insn->imm = (u64 (*)(u64, u64, u64, u64, u64)) func[subprog]->bpf_func - __bpf_call_base; } + + /* we use the aux data to keep a list of the start addresses + * of the JITed images for each function in the program + * + * for some architectures, such as powerpc64, the imm field + * might not be large enough to hold the offset of the start + * address of the callee's JITed image from __bpf_call_base + * + * in such cases, we can lookup the start address of a callee + * by using its subprog id, available from the off field of + * the call instruction, as an index for this list + */ + func[i]->aux->func = func; + func[i]->aux->func_cnt = env->subprog_cnt + 1; } for (i = 0; i < env->subprog_cnt; i++) { old_bpf_func = func[i]->bpf_func;
The imm field of a bpf instruction is a signed 32-bit integer. For JIT bpf-to-bpf function calls, it stores the offset of the start address of the callee's JITed image from __bpf_call_base. For some architectures, such as powerpc64, this offset may be as large as 64 bits and cannot be accomodated in the imm field without truncation. We resolve this by: [1] Additionally using the auxillary data of each function to keep a list of start addresses of the JITed images for all functions determined by the verifier. [2] Retaining the subprog id inside the off field of the call instructions and using it to index into the list mentioned above and lookup the callee's address. To make sure that the existing JIT compilers continue to work without requiring changes, we keep the imm field as it is. Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com> --- kernel/bpf/verifier.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)