Re: [PATCH v2 bpf-next 4/5] selftests/bpf: Add tests for bpf_cgroup_read_xattr

["Jose E. Marchesi" <jose.marchesi@oracle.com>]

> On Fri, 2025-06-20 at 11:11 -0700, Alexei Starovoitov wrote:
>> On Thu, Jun 19, 2025 at 3:02 PM Song Liu <song@kernel.org> wrote:
>> > +       bpf_dynptr_from_mem(xattr_value, sizeof(xattr_value), 0, &value_ptr);
>> 
>> https://github.com/kernel-patches/bpf/actions/runs/15767046528/job/44445539248
>> 
>> progs/cgroup_read_xattr.c:19:9: error: ‘bpf_dynptr_from_mem’ is static
>> but used in inline function ‘read_xattr’ which is not static [-Werror]
>> 19 | bpf_dynptr_from_mem(value, sizeof(value), 0, &value_ptr);
>> > ^~~~~~~~~~~~~~~~~~~
>> 
>> 
>> Jose,
>> 
>> Could you please help us understand this gcc-bpf error ?
>> What does it mean?
>
> Not Jose, but was curious.
> Some googling lead to the following C99 wording [1]:
>
>   > An inline definition of a function with external linkage shall not
>   > contain a definition of a modifiable object with static storage
>   > duration, and shall not contain a reference to an identifier with
>   > internal linkage
>
> [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>     6.7.4 Function specifiers, paragraph 3
>
> The helper is defined as `static`:
>
>   static long (* const bpf_dynptr_from_mem)(...) = (void *) 197;
>
> While `read_xattr` has external linkage:
>
>   __always_inline void read_xattr(struct cgroup *cgroup)
>   {
> 	...
> 	bpf_dynptr_from_mem(value, sizeof(value), 0, &value_ptr);
> 	...
>   }
>
> I think that declaring `read_xattr` as `static` should help with gcc.

Yes I agree that is the issue.  It is a restriction introduced by C99.
I wasn't aware of it so I had to look it up.

If you need read_xattr to be accessible from other compilation units and
inlinable, you may declare it as static inline and put it in a header
file.  It will then use the copies of the helper pointers in the other
compilation units.

Alternatively, with GCC you could try to add a direct declaration for
the helper function to progs/cgroup_read_xattr.c, that doesnt need to
use any static intermediate pointer:

  #if COMPILING_WITH_GCC
  long _bpf_dynptr_from_mem (...) __attribute__ ((kernel_helper (197)));
  #define bpf_dynptr_from_mem _bpf_dynptr_from_mem
  #endif