On 5/24/24 12:33 PM, Alexei Starovoitov wrote:
On Thu, May 23, 2024 at 11:25 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
+
+unsigned long time_next_delayed_flow = ~0ULL;
+unsigned long unthrottle_latency_ns = 0ULL;
+unsigned long ktime_cache = 0;
+unsigned long dequeue_now;
+unsigned int fq_qlen = 0;
I suspect some of these globals may be more natural if it is stored private to
an individual Qdisc instance. i.e. qdisc_priv(). e.g. in the sch_mq setup.
A high level idea is to allow the SEC(".struct_ops.link") to specify its own
Qdisc_ops.priv_size.
The bpf prog could use it as a simple u8 array memory area to write anything but
the verifier can't learn a lot from it. It will be more useful if it can work
like map_value(s) to the verifier such that the verifier can also see the
bpf_rb_root/bpf_list_head/bpf_spin_lock...etc.
Qdisc_ops.priv_size is too qdsic specific.
Instead of priv_size, may be something like a bpf_local_storage for Qdisc is
closer to how other kernel objects (sk/task/cgrp) are doing it now. Like
bpf_sk_storage that goes away with the sk. It needs a storage that goes away
with the Qdisc.
I was thinking using priv_size to work something like a bpf_local_storage
without the pointer array redirection by pre-defining all map_values it wants to
store in the Qdisc, so the total size of the pre-defined map_values will be the
priv_size. However, I haven't thought through how it should look like from
bpf_prog.c to the kernel. It is an optimization of the bpf_local_storage.
imo using globals here is fine. bpf prog can use hash map or arena
to store per-netdev or per-qdisc data.
The less custom things the better.
