On 03-Mär 15:03, Andrii Nakryiko wrote: > On Tue, Mar 3, 2020 at 2:24 PM KP Singh <kpsingh@xxxxxxxxxxxx> wrote: > > > > On 03-Mär 14:12, Andrii Nakryiko wrote: > > > On Tue, Mar 3, 2020 at 6:13 AM KP Singh <kpsingh@xxxxxxxxxxxx> wrote: > > > > > > > > From: KP Singh <kpsingh@xxxxxxxxxx> > > > > > > > > As we need to introduce a third type of attachment for trampolines, the > > > > flattened signature of arch_prepare_bpf_trampoline gets even more > > > > complicated. > > > > > > > > Refactor the prog and count argument to arch_prepare_bpf_trampoline to > > > > use bpf_tramp_progs to simplify the addition and accounting for new > > > > attachment types. > > > > > > > > Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx> > > > > --- > > > > arch/x86/net/bpf_jit_comp.c | 31 +++++++++--------- > > > > include/linux/bpf.h | 13 ++++++-- > > > > kernel/bpf/bpf_struct_ops.c | 13 +++++++- > > > > kernel/bpf/trampoline.c | 63 +++++++++++++++++++++---------------- > > > > 4 files changed, 75 insertions(+), 45 deletions(-) > > > > > > > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > > > > index 9ba08e9abc09..15c7d28bc05c 100644 > > > > --- a/arch/x86/net/bpf_jit_comp.c > > > > +++ b/arch/x86/net/bpf_jit_comp.c > > > > @@ -1362,12 +1362,12 @@ static void restore_regs(const struct btf_func_model *m, u8 **prog, int nr_args, > > > > } > > > > > > > > static int invoke_bpf(const struct btf_func_model *m, u8 **pprog, > > > > - struct bpf_prog **progs, int prog_cnt, int stack_size) > > > > + struct bpf_tramp_progs *tp, int stack_size) > > > > > > nit: it's `tp` here, but `tprogs` in arch_prepare_bpf_trampoline. It's > > > minor, but would be nice to stick to consistent naming. > > > > I did this to ~distinguish~ that rather than being an array of > > tprogs it's a pointer to one of its members e.g. > > &tprogs[BPF_TRAMP_FEXIT]). > > > > I change it if you feel this is not a valuable disntinction. > > I think it's important distinction, but naming doesn't really help > with it... Not sure how you can make it more clear, though. I would prefer to keep the naming distinction. Hope that's okay with you. > > [...] > > > > > count. Am I missing something :) > > Ok, so it's setting entry 0 in bpf_tramp_progs->progs array, right? > Wouldn't it be less mind-bending and confusing written this way: > > tprogs[BPF_TRAMP_FENTRY].progs[0] = prog; Definitely much cleaner/less mind bending :) Updated. Thanks! - KP > > ? > > Syntax you used treats fixed-length progs array as a pointer, which is > valid C, but not the best C either. > [...] > > > > > [...]
