On Thu, Mar 13, 2025 at 12:03 PM Amery Hung <ameryhung@xxxxxxxxx> wrote:
>
> From: Amery Hung <amery.hung@xxxxxxxxxxxxx>
>
> Allow bpf qdisc programs to update Qdisc qstats directly with btf struct
> access.
>
> Signed-off-by: Amery Hung <amery.hung@xxxxxxxxxxxxx>
> ---
> net/sched/bpf_qdisc.c | 53 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/net/sched/bpf_qdisc.c b/net/sched/bpf_qdisc.c
> index edf01f3f1c2a..6ad3050275a4 100644
> --- a/net/sched/bpf_qdisc.c
> +++ b/net/sched/bpf_qdisc.c
> @@ -36,6 +36,7 @@ bpf_qdisc_get_func_proto(enum bpf_func_id func_id,
> }
> }
>
> +BTF_ID_LIST_SINGLE(bpf_qdisc_ids, struct, Qdisc)
> BTF_ID_LIST_SINGLE(bpf_sk_buff_ids, struct, sk_buff)
> BTF_ID_LIST_SINGLE(bpf_sk_buff_ptr_ids, struct, bpf_sk_buff_ptr)
>
> @@ -60,20 +61,37 @@ static bool bpf_qdisc_is_valid_access(int off, int size,
> return bpf_tracing_btf_ctx_access(off, size, type, prog, info);
> }
>
> -static int bpf_qdisc_btf_struct_access(struct bpf_verifier_log *log,
> - const struct bpf_reg_state *reg,
> - int off, int size)
> +static int bpf_qdisc_qdisc_access(struct bpf_verifier_log *log,
> + const struct bpf_reg_state *reg,
> + int off, int size)
Introducing this func in patch 3 and refactoring in patch 7 ?
pls avoid the churn.
squash it ?
if (off + size > end) check wouldn't need to be duplicated.
Can get the name of struct from btf for bpf_log() purpose.
> {
> - const struct btf_type *t, *skbt;
> size_t end;
>
> - skbt = btf_type_by_id(reg->btf, bpf_sk_buff_ids[0]);
> - t = btf_type_by_id(reg->btf, reg->btf_id);
> - if (t != skbt) {
> - bpf_log(log, "only read is supported\n");
> + switch (off) {
> + case offsetof(struct Qdisc, qstats) ... offsetofend(struct Qdisc, qstats) - 1:
> + end = offsetofend(struct Qdisc, qstats);
> + break;
> + default:
> + bpf_log(log, "no write support to Qdisc at off %d\n", off);
> + return -EACCES;
> + }
> +
> + if (off + size > end) {
> + bpf_log(log,
> + "write access at off %d with size %d beyond the member of Qdisc ended at %zu\n",
> + off, size, end);
> return -EACCES;
> }
>
> + return 0;
> +}
> +
> +static int bpf_qdisc_sk_buff_access(struct bpf_verifier_log *log,
> + const struct bpf_reg_state *reg,
> + int off, int size)
> +{
> + size_t end;
> +
> switch (off) {
> case offsetof(struct sk_buff, tstamp):
> end = offsetofend(struct sk_buff, tstamp);
> @@ -115,6 +133,25 @@ static int bpf_qdisc_btf_struct_access(struct bpf_verifier_log *log,
> return 0;
> }
>
> +static int bpf_qdisc_btf_struct_access(struct bpf_verifier_log *log,
> + const struct bpf_reg_state *reg,
> + int off, int size)
> +{
> + const struct btf_type *t, *skbt, *qdisct;
> +
> + skbt = btf_type_by_id(reg->btf, bpf_sk_buff_ids[0]);
> + qdisct = btf_type_by_id(reg->btf, bpf_qdisc_ids[0]);
> + t = btf_type_by_id(reg->btf, reg->btf_id);
> +
> + if (t == skbt)
> + return bpf_qdisc_sk_buff_access(log, reg, off, size);
> + else if (t == qdisct)
> + return bpf_qdisc_qdisc_access(log, reg, off, size);
> +
> + bpf_log(log, "only read is supported\n");
> + return -EACCES;
> +}
> +
> BTF_ID_LIST(bpf_qdisc_init_prologue_ids)
> BTF_ID(func, bpf_qdisc_init_prologue)
>
> --
> 2.47.1
>
