Re: [PATCH v3 0/8] perf trace: Augment enum arguments with BTF

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Jun 24, 2024 at 07:06:26PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Jun 25, 2024 at 02:13:37AM +0800, Howard Chu wrote:
> > In this patch, BTF is used to turn enum value to the corresponding
> > enum variable name. There is only one system call that uses enum value
> > as its argument, that is `landlock_add_rule()`.
> > 
> > Enum arguments of non-syscall tracepoints can also be augmented, for
> > instance timer:hrtimer_start and timer:hrtimer_init's 'mode' argument.
> > 
> > Changes in v3:
> 
> Did a quick test, from a quick look you did the adjustments we agreed
> (if (val == 0 && !trace->show_zeros && !arg->show_zero && arg->strtoul
> != STUL_BTF_TYPE), etc), thanks!
> 
> And that is the way for collaboration we go on talking on the mailing
> list and sometimes writing code, making it available for review and
> adopting what we deem best at that point, rinse repeat.
> 
> Now I think it would be great if someone like Namhyung or Ian could try
> this last patch.
> 
> I have to comb thru it but, again, from a quick look and test, it seems
> great and probably ready for merging.

So I'm trying to test this but I got a build error like this:

tests/workloads/landlock.c: In function ‘landlock’:
tests/workloads/landlock.c:22:16: error: variable ‘net_port_attr’ has initializer but incomplete type
   22 |         struct landlock_net_port_attr net_port_attr = {
      |                ^~~~~~~~~~~~~~~~~~~~~~
tests/workloads/landlock.c:23:18: error: ‘struct landlock_net_port_attr’ has no member named ‘port’
   23 |                 .port = 19,
      |                  ^~~~
tests/workloads/landlock.c:23:25: error: excess elements in struct initializer [-Werror]
   23 |                 .port = 19,
      |                         ^~
tests/workloads/landlock.c:23:25: note: (near initialization for ‘net_port_attr’)
tests/workloads/landlock.c:24:18: error: ‘struct landlock_net_port_attr’ has no member named ‘allowed_access’
   24 |                 .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
      |                  ^~~~~~~~~~~~~~
tests/workloads/landlock.c:24:35: error: ‘LANDLOCK_ACCESS_NET_CONNECT_TCP’ undeclared (first use in this function); did you mean ‘LANDLOCK_ACCESS_FS_TRUNCATE’?
   24 |                 .allowed_access = LANDLOCK_ACCESS_NET_CONNECT_TCP,
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                   LANDLOCK_ACCESS_FS_TRUNCATE
tests/workloads/landlock.c:24:35: note: each undeclared identifier is reported only once for each function it appears in
tests/workloads/landlock.c:24:35: error: excess elements in struct initializer [-Werror]
tests/workloads/landlock.c:24:35: note: (near initialization for ‘net_port_attr’)
tests/workloads/landlock.c:22:39: error: storage size of ‘net_port_attr’ isn’t known
   22 |         struct landlock_net_port_attr net_port_attr = {
      |                                       ^~~~~~~~~~~~~
tests/workloads/landlock.c:30:45: error: ‘LANDLOCK_RULE_NET_PORT’ undeclared (first use in this function); did you mean ‘LANDLOCK_RULE_PATH_BENEATH’?
   30 |         syscall(__NR_landlock_add_rule, fd, LANDLOCK_RULE_NET_PORT,
      |                                             ^~~~~~~~~~~~~~~~~~~~~~
      |                                             LANDLOCK_RULE_PATH_BENEATH
tests/workloads/landlock.c:22:39: error: unused variable ‘net_port_attr’ [-Werror=unused-variable]
   22 |         struct landlock_net_port_attr net_port_attr = {
      |                                       ^~~~~~~~~~~~~

And I found it's reported before:
https://lore.kernel.org/linux-perf-users/Zn5aGnyjyCqAX+66@rli9-mobl/

It seems my system has an old copy of the linux/landlock.h and it
doesn't have the new struct and macro.  I think you can change it to
include what we have in the source tree.  Maybe like

#include "../../../../include/uapi/linux/landlock.h"

Probably the same for the syscall number by reading it from unistd.h in
the source tree.  Then we probably don't need to make it conditional as
we use the latest version.  But I'm not sure if it's ok to read the
syscall number from asm-generic header since it might be different on
other arch.

Thanks,
Namhyung