On 2/14/25 8:45 AM, Amery Hung wrote:
Hi, I am splitting the bpf qdisc patchset into smaller landable sets and this is the first part. This patchset supports struct_ops operators that acquire kptrs through arguments and operators that return a kptr. A coming new struct_ops use case, bpf qdisc [0], has two operators that are not yet supported by current struct_ops infrastructure. Qdisc_ops::enqueue requires getting referenced skb kptr from the argument; Qdisc_ops::dequeue needs to return a referenced skb kptr. This patchset will allow bpf qdisc and other potential struct_ops implementers to do so. For struct_ops implementers: - To get a kptr from an argument, a struct_ops implementer needs to annotate the argument name in the stub function with "__ref" suffix. - The kptr return will automatically work as we now allow operators that return a struct pointer. - The verifier allows returning a null pointer. More control can be added later if there is a future struct_ops implementer only expecting valid pointers. For struct_ops users: - The referenced kptr acquired through the argument needs to be released or xchged into maps just like ones acquired via kfuncs. - To return a referenced kptr in struct_ops, 1) The type of the pointer must matches the return type 2) The pointer must comes from the kernel (not locally allocated), and 3) The pointer must be in its unmodified form
I only left some minor comments in the patches. A few other thoughts: I think https://lore.kernel.org/bpf/20250210174336.2024258-11-ameryhung@xxxxxxxxx/ should be a good addition also. A new subtest in the prog_tests/pro_epilogue.c should do for testing it. Another thing is disabling tail call in the bpf_qdisc_get_func_proto in https://lore.kernel.org/bpf/20250210174336.2024258-9-ameryhung@xxxxxxxxx/ I am wondering if it can be done in a generic way for all struct_ops in check_struct_ops_btf_id(). At that point, we should know all the "has_tail_call". I meant only for ops with __ref arg such that it won't break the existing struct_ops. I think both of them could be a followup.
