On 7/26/24 3:30 PM, Amery Hung wrote:
The pre/post is mainly to initialize and cleanup the "struct bpf_sched_data"
before/after calling the bpf prog.
For the pre (init), there is a ".gen_prologue(...., const struct bpf_prog
*prog)" in the "bpf_verifier_ops". Take a look at the tc_cls_act_prologue().
It calls a BPF_FUNC_skb_pull_data helper. It potentially can call a kfunc
bpf_qdisc_watchdog_cancel. However, the gen_prologue is invoked too late in the
verifier for kfunc calling now. This will need some thoughts and works.
For the post (destroy,reset), there is no "gen_epilogue" now. If
bpf_qdisc_watchdog_schedule() is not allowed to be called in the ".reset" and
".destroy" bpf prog. I think it can be changed to pre also? There is a ".filter"
function in the "struct btf_kfunc_id_set" during the kfunc register.
I can see how that would work. The ability to add prologue, epilogue
to struct_ops operators is one thing on my wish list.
Meanwhile, I am not sure whether that should be written in the kernel
or rewritten by the verifier. An argument for keeping it in the kernel
is that the prologue or epilogue can get quite complex and involves
many kernel structures not exposed to the bpf program (pre-defined ops
in Qdisc_ops in v8).
Can the v8 pre-defined ops be called as a kfunc? The qdisc_watchdog_cancel/init
in v9 could be a kfunc and called by pro/epilogue.
For checking and/or resetting skb->dev, it should be simple enough without
kfunc. e.g. when reusing the skb->rbnode in the future followup effort.
[ Unrelated to qdisc. bpf_tcp_ca can also use help to ensure the cwnd is sane. ]
Maybe we can keep the current approach in the initial version as they
are not in the fast path, and then move to (gen_prologue,
gen_epilogue) once the plumbing is done?
Sure. It can be improved when things are ready.
I am trying some ideas on how to do gen_epilogue and will share when I get some
tests working.
