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: <3961c9ce-21d3-4a35-956c-5e1a6eb6031b@linux.dev>
Date: Wed, 8 Jan 2025 16:20:21 -0800
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Amery Hung <ameryhung@...il.com>
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 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.

> +	*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;
> +}
> +