[PATCH bpf-next] docs/bpf: Update documentation for BTF_KIND_FUNC

[PATCH bpf-next] docs/bpf: Update documentation for BTF_KIND_FUNC

[Indu Bhagat]

The vlen bits in the BTF type of kind BTF_KIND_FUNC are used to convey the
linkage information for functions.

Signed-off-by: Indu Bhagat <indu.bhagat@oracle.com>
---
 Documentation/bpf/btf.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
index f49aeef62d0c..b3a9d5ac882c 100644
--- a/Documentation/bpf/btf.rst
+++ b/Documentation/bpf/btf.rst
@@ -369,7 +369,7 @@ No additional type data follow ``btf_type``.
   * ``name_off``: offset to a valid C identifier
   * ``info.kind_flag``: 0
   * ``info.kind``: BTF_KIND_FUNC
-  * ``info.vlen``: 0
+  * ``info.vlen``: linkage information (static=0, global=1)
   * ``type``: a BTF_KIND_FUNC_PROTO type
 
 No additional type data follow ``btf_type``.
-- 
2.31.1

Re: [PATCH bpf-next] docs/bpf: Update documentation for BTF_KIND_FUNC

[sdf]

On 07/13, Indu Bhagat wrote:
> The vlen bits in the BTF type of kind BTF_KIND_FUNC are used to convey the
> linkage information for functions.

> Signed-off-by: Indu Bhagat <indu.bhagat@oracle.com>

Reviewed-by: Stanislav Fomichev <sdf@google.com>

Judging by:

static inline u16 btf_func_linkage(const struct btf_type *t)
{
	return BTF_INFO_VLEN(t->info);
}

> ---
>   Documentation/bpf/btf.rst | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
> index f49aeef62d0c..b3a9d5ac882c 100644
> --- a/Documentation/bpf/btf.rst
> +++ b/Documentation/bpf/btf.rst
> @@ -369,7 +369,7 @@ No additional type data follow ``btf_type``.
>     * ``name_off``: offset to a valid C identifier
>     * ``info.kind_flag``: 0
>     * ``info.kind``: BTF_KIND_FUNC
> -  * ``info.vlen``: 0
> +  * ``info.vlen``: linkage information (static=0, global=1)
>     * ``type``: a BTF_KIND_FUNC_PROTO type

>   No additional type data follow ``btf_type``.
> --
> 2.31.1

Re: [PATCH bpf-next] docs/bpf: Update documentation for BTF_KIND_FUNC

[Andrii Nakryiko]

On Wed, Jul 13, 2022 at 3:37 PM Indu Bhagat <indu.bhagat@oracle.com> wrote:
>
> The vlen bits in the BTF type of kind BTF_KIND_FUNC are used to convey the
> linkage information for functions.
>
> Signed-off-by: Indu Bhagat <indu.bhagat@oracle.com>
> ---
>  Documentation/bpf/btf.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
> index f49aeef62d0c..b3a9d5ac882c 100644
> --- a/Documentation/bpf/btf.rst
> +++ b/Documentation/bpf/btf.rst
> @@ -369,7 +369,7 @@ No additional type data follow ``btf_type``.
>    * ``name_off``: offset to a valid C identifier
>    * ``info.kind_flag``: 0
>    * ``info.kind``: BTF_KIND_FUNC
> -  * ``info.vlen``: 0
> +  * ``info.vlen``: linkage information (static=0, global=1)

there is also extern=2, but I think we should just refer to enum
btf_func_linkage, defined in UAPI (include/uapi/linux/btf.h) ?

>    * ``type``: a BTF_KIND_FUNC_PROTO type
>
>  No additional type data follow ``btf_type``.
> --
> 2.31.1
>

Re: [PATCH bpf-next] docs/bpf: Update documentation for BTF_KIND_FUNC

[Yonghong Song]

On 7/13/22 11:00 PM, Andrii Nakryiko wrote:
> On Wed, Jul 13, 2022 at 3:37 PM Indu Bhagat <indu.bhagat@oracle.com> wrote:
>>
>> The vlen bits in the BTF type of kind BTF_KIND_FUNC are used to convey the
>> linkage information for functions.
>>
>> Signed-off-by: Indu Bhagat <indu.bhagat@oracle.com>
>> ---
>>   Documentation/bpf/btf.rst | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
>> index f49aeef62d0c..b3a9d5ac882c 100644
>> --- a/Documentation/bpf/btf.rst
>> +++ b/Documentation/bpf/btf.rst
>> @@ -369,7 +369,7 @@ No additional type data follow ``btf_type``.
>>     * ``name_off``: offset to a valid C identifier
>>     * ``info.kind_flag``: 0
>>     * ``info.kind``: BTF_KIND_FUNC
>> -  * ``info.vlen``: 0
>> +  * ``info.vlen``: linkage information (static=0, global=1)
> 
> there is also extern=2, but I think we should just refer to enum
> btf_func_linkage, defined in UAPI (include/uapi/linux/btf.h) ?

Currently kernel rejects extern=2. In kernel btf.c, we have

         if (btf_type_vlen(t) > BTF_FUNC_GLOBAL) {
                 btf_verifier_log_type(env, t, "Invalid func linkage");
                 return -EINVAL;
         }

and extern=2 will cause btf loading failure.

The BTF_FUNC_EXTERN is generated when you call an extern *global*
function. I suspect that during static linking, all these
extern globals should become true global/static functions and
not extern func's any more so kernel is okay.

So looks like it is worthwhile to mention that BTF_KIND_FUNC
supports all three modes as specified in enum btf_func_linkage.
But only static/global is supported in the kernel.

> 
>>     * ``type``: a BTF_KIND_FUNC_PROTO type
>>
>>   No additional type data follow ``btf_type``.
>> --
>> 2.31.1
>>

Re: [PATCH bpf-next] docs/bpf: Update documentation for BTF_KIND_FUNC

[Andrii Nakryiko]

On Wed, Jul 13, 2022 at 11:42 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 7/13/22 11:00 PM, Andrii Nakryiko wrote:
> > On Wed, Jul 13, 2022 at 3:37 PM Indu Bhagat <indu.bhagat@oracle.com> wrote:
> >>
> >> The vlen bits in the BTF type of kind BTF_KIND_FUNC are used to convey the
> >> linkage information for functions.
> >>
> >> Signed-off-by: Indu Bhagat <indu.bhagat@oracle.com>
> >> ---
> >>   Documentation/bpf/btf.rst | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
> >> index f49aeef62d0c..b3a9d5ac882c 100644
> >> --- a/Documentation/bpf/btf.rst
> >> +++ b/Documentation/bpf/btf.rst
> >> @@ -369,7 +369,7 @@ No additional type data follow ``btf_type``.
> >>     * ``name_off``: offset to a valid C identifier
> >>     * ``info.kind_flag``: 0
> >>     * ``info.kind``: BTF_KIND_FUNC
> >> -  * ``info.vlen``: 0
> >> +  * ``info.vlen``: linkage information (static=0, global=1)
> >
> > there is also extern=2, but I think we should just refer to enum
> > btf_func_linkage, defined in UAPI (include/uapi/linux/btf.h) ?
>
> Currently kernel rejects extern=2. In kernel btf.c, we have
>
>          if (btf_type_vlen(t) > BTF_FUNC_GLOBAL) {
>                  btf_verifier_log_type(env, t, "Invalid func linkage");
>                  return -EINVAL;
>          }
>
> and extern=2 will cause btf loading failure.
>
> The BTF_FUNC_EXTERN is generated when you call an extern *global*
> function. I suspect that during static linking, all these
> extern globals should become true global/static functions and
> not extern func's any more so kernel is okay.
>
> So looks like it is worthwhile to mention that BTF_KIND_FUNC
> supports all three modes as specified in enum btf_func_linkage.
> But only static/global is supported in the kernel.
>

Exactly. This is specification of BTF format (for clang and gcc to
agree upon, for example), while kernel BTF verifier puts extra
conditions on top of BTF format itself (as required by C and kernel
semantics). So it makes sense to document all possibilities and extra
kernel checks.

> >
> >>     * ``type``: a BTF_KIND_FUNC_PROTO type
> >>
> >>   No additional type data follow ``btf_type``.
> >> --
> >> 2.31.1
> >>