On Wed, Jan 8, 2025 at 4:20 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 12/20/24 11:55 AM, Amery Hung wrote:
> > diff --git a/net/sched/bpf_qdisc.c b/net/sched/bpf_qdisc.c
> > index 1c92bfcc3847..bbe7aded6f24 100644
> > --- a/net/sched/bpf_qdisc.c
> > +++ b/net/sched/bpf_qdisc.c
> > @@ -8,6 +8,10 @@
> >
> > static struct bpf_struct_ops bpf_Qdisc_ops;
> >
> > +struct bpf_sched_data {
> > + struct qdisc_watchdog watchdog;
> > +};
> > +
> > struct bpf_sk_buff_ptr {
> > struct sk_buff *skb;
> > };
> > @@ -108,6 +112,46 @@ static int bpf_qdisc_btf_struct_access(struct bpf_verifier_log *log,
> > return 0;
> > }
> >
> > +BTF_ID_LIST(bpf_qdisc_init_prologue_ids)
> > +BTF_ID(func, bpf_qdisc_init_prologue)
> > +
> > +static int bpf_qdisc_gen_prologue(struct bpf_insn *insn_buf, bool direct_write,
> > + const struct bpf_prog *prog)
> > +{
> > + struct bpf_insn *insn = insn_buf;
> > +
> > + if (strcmp(prog->aux->attach_func_name, "init"))
> > + return 0;
> > +
> > + *insn++ = BPF_MOV64_REG(BPF_REG_6, BPF_REG_1);
> > + *insn++ = BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 0);
> > + *insn++ = BPF_CALL_KFUNC(0, bpf_qdisc_init_prologue_ids[0]);
>
> I was wondering if patch 7 is needed if BPF_EMIT_CALL() and BPF_CALL_1() were
> used, so it looks more like a bpf helper instead of kfunc. I tried but failed at
> the "fn = env->ops->get_func_proto(insn->imm, env->prog);" in do_misc_fixups().
> I think the change in patch 7 is simple enough instead of getting
> get_func_proto() works for this case.
>
Yeah. Making BPF_EMIT_CALL + BPF_CALL_xxx work in
gen_prologue/epilogue doesn't seem trivial to me at least when
compared with BPF_CALL_KFUNC.
> > + *insn++ = BPF_MOV64_REG(BPF_REG_1, BPF_REG_6);
> > + *insn++ = prog->insnsi[0];
> > +
> > + return insn - insn_buf;
> > +}
> > +
> > +BTF_ID_LIST(bpf_qdisc_reset_destroy_epilogue_ids)
> > +BTF_ID(func, bpf_qdisc_reset_destroy_epilogue)
> > +
> > +static int bpf_qdisc_gen_epilogue(struct bpf_insn *insn_buf, const struct bpf_prog *prog,
> > + s16 ctx_stack_off)
> > +{
> > + struct bpf_insn *insn = insn_buf;
> > +
> > + if (strcmp(prog->aux->attach_func_name, "reset") &&
> > + strcmp(prog->aux->attach_func_name, "destroy"))
> > + return 0;
> > +
> > + *insn++ = BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_FP, ctx_stack_off);
> > + *insn++ = BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 0);
> > + *insn++ = BPF_CALL_KFUNC(0, bpf_qdisc_reset_destroy_epilogue_ids[0]);
> > + *insn++ = BPF_EXIT_INSN();
> > +
> > + return insn - insn_buf;
> > +}
> > +
