On 10/11/19 12:02 PM, Andrii Nakryiko wrote:
> On Wed, Oct 9, 2019 at 9:15 PM Alexei Starovoitov <ast@xxxxxxxxxx> wrote:
>>
>> /* type of values returned from helper functions */
>> @@ -235,11 +236,17 @@ struct bpf_func_proto {
>> bool gpl_only;
>> bool pkt_access;
>> enum bpf_return_type ret_type;
>> - enum bpf_arg_type arg1_type;
>> - enum bpf_arg_type arg2_type;
>> - enum bpf_arg_type arg3_type;
>> - enum bpf_arg_type arg4_type;
>> - enum bpf_arg_type arg5_type;
>> + union {
>> + struct {
>> + enum bpf_arg_type arg1_type;
>> + enum bpf_arg_type arg2_type;
>> + enum bpf_arg_type arg3_type;
>> + enum bpf_arg_type arg4_type;
>> + enum bpf_arg_type arg5_type;
>> + };
>> + enum bpf_arg_type arg_type[5];
>> + };
>> + u32 *btf_id; /* BTF ids of arguments */
>
> are you trying to save memory with this? otherwise not sure why it's
> not just `u32 btf_id[5]`? Even in that case it will save at most 12
> bytes (and I haven't even check alignment padding and stuff). So
> doesn't seem worth it?
Glad you asked :)
It cannot be "u32 btf_id[5];".
Guess why?
I think it's a cool trick.
I was happy when I finally figured out to solve it this way
after analyzing a bunch of ugly solutions.
>> + *
>> + * The value to write, of *size*, is passed through eBPF stack and
>> + * pointed by *data*.
>
> typo? pointed __to__ by *data*?
I'm not an grammar expert. That was a copy paste from existing comment.
>> + *
>> + * *ctx* is a pointer to in-kernel sutrct sk_buff.
>> + *
>> + * This helper is similar to **bpf_perf_event_output**\ () but
>> + * restricted to raw_tracepoint bpf programs.
>
> nit: with BTF type tracking enabled?
sure.
>> + for (i = 0; i < 5; i++) {
>> + if (fn->arg_type[i] == ARG_PTR_TO_BTF_ID) {
>> + if (!fn->btf_id[i])
>> + fn->btf_id[i] = btf_resolve_helper_id(&env->log, fn->func, 0);
>
> bug: 0 -> i :)
Nice catch.
Clearly I don't have a use case yet for 2nd arg being ptr_to_btf.
