Re: [PATCH bpf-next v4 3/8] bpf: Update descriptions for helpers bpf_get_func_arg[_cnt]()

[Yonghong Song <yhs@fb.com>]

On 9/2/22 12:56 AM, Jiri Olsa wrote:
> On Wed, Aug 31, 2022 at 08:26:57AM -0700, Yonghong Song wrote:
>> Now instead of the number of arguments, the number of registers
>> holding argument values are stored in trampoline. Update
>> the description of bpf_get_func_arg[_cnt]() helpers. Previous
>> programs without struct arguments should continue to work
>> as usual.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   include/uapi/linux/bpf.h       | 9 +++++----
>>   tools/include/uapi/linux/bpf.h | 9 +++++----
>>   2 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 962960a98835..f9f43343ef93 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -5079,12 +5079,12 @@ union bpf_attr {
>>    *
>>    * long bpf_get_func_arg(void *ctx, u32 n, u64 *value)
>>    *	Description
>> - *		Get **n**-th argument (zero based) of the traced function (for tracing programs)
>> + *		Get **n**-th argument register (zero based) of the traced function (for tracing programs)
> 
> I'm bit worried about the confusion between args/regs we create with
> this, but I don't any have better idea how to solve this
> 
> keeping extra stack values for nr_args and nr_regs and have new helpers
> to get reg values.. but then bpf_get_func_arg still does not return the
> full argument value.. also given that the struct args should be rare,
> I guess it's fine ;-)

You are right. Since an argument may takes two u64 space, the helper
bpf_get_func_arg() might not return the *whole* argument any more.
But it still returns a func arg, maybe just a partial func arg.
So the 'n' here has a semantic change, instead of 'n'th argument,
it becomes 'n'th argument register. To create new helpers probably
not needed, so that is why I made the description change to the
existing helper which maintains backward compatability too.

> 
> jirka
> 
>>    *		returned in **value**.
>>    *
>>    *	Return
>>    *		0 on success.
>> - *		**-EINVAL** if n >= arguments count of traced function.
>> + *		**-EINVAL** if n >= argument register count of traced function.
>>    *
>>    * long bpf_get_func_ret(void *ctx, u64 *value)
>>    *	Description
>> @@ -5097,10 +5097,11 @@ union bpf_attr {
>>    *
>>    * long bpf_get_func_arg_cnt(void *ctx)
>>    *	Description
>> - *		Get number of arguments of the traced function (for tracing programs).
>> + *		Get number of registers of the traced function (for tracing programs) where
>> + *		function arguments are stored in these registers.
>>    *
>>    *	Return
>> - *		The number of arguments of the traced function.
>> + *		The number of argument registers of the traced function.
>>    *
>>    * int bpf_get_retval(void)
>>    *	Description
[...]