netdev - Re: [PATCH bpf-next v2 08/14] bpf: net_sched: Add a qdisc watchdog timer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMB2axNoh1uDLC4KubHjKSfRBfNWnG_tbuPK6BWzx4VA6Hx0tQ@mail.gmail.com>
Date: Thu, 9 Jan 2025 07:00:00 -0800
From: Amery Hung <ameryhung@...il.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, daniel@...earbox.net, 
	andrii@...nel.org, alexei.starovoitov@...il.com, martin.lau@...nel.org, 
	sinquersw@...il.com, toke@...hat.com, jhs@...atatu.com, jiri@...nulli.us, 
	stfomichev@...il.com, ekarani.silvestre@....ufcg.edu.br, 
	yangpeihao@...u.edu.cn, xiyou.wangcong@...il.com, yepeilin.cs@...il.com, 
	amery.hung@...edance.com
Subject: Re: [PATCH bpf-next v2 08/14] bpf: net_sched: Add a qdisc watchdog timer

On Wed, Jan 8, 2025 at 4:20 PM Martin KaFai Lau <martin.lau@...ux.dev> 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;
> > +}
> > +