Re: [PATCH v6 06/26] perf trace: Copy sockaddr arguments by their length

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Wed, Sep 30, 2026 at 04:08:17PM -0700, Namhyung Kim wrote:
> On Wed, Sep 30, 2026 at 09:14:42PM +0200, Arnaldo Carvalho de Melo wrote:
> > On Mon, Sep 28, 2026 at 11:25:45AM -0700, Ian Rogers wrote:
> > > +++ b/tools/perf/builtin-trace.c
> > > @@ -4159,7 +4159,12 @@ static int trace__bpf_sys_enter_beauty_map(struct trace *trace, int e_machine, i
> > >  				continue;

> > >  			bt = sc->arg_fmt[i].type;
> > > -			beauty_array[i] = bt->size;
> > > +			/* Copy a sockaddr as a buffer sized by the next argument, e.g. addrlen. */
> > > +			if (strcmp(name, "sockaddr") == 0 && field->next &&
> > > +			    strstr(field->next->name, "len"))
> > > +				beauty_array[i] = -((i + 1) + 1);
> > > +			else
> > > +				beauty_array[i] = bt->size;

> > And it knows how many bytes to read by looking at socklen
> > (args->args[2]), i.e. not use the generic BPF handler that uses this
> > beauty_array, because knowing how many bytes to read in this case is
> > dynamic, varies with each syscall, according to one of its arguments :-\

> > What am I missing?
 
> I think Ian's patch update the beauty map which is used by
> augment_sys_enter() before tail-calling syscall-specific functions.
 
> It'd be great if we cover all syscalls in the BPF skeleton and switch
> to the beauty-map and discard the functions.

I was missing the convention that a negative size means read some
other argument with a cap, as Ian explained in his response.

So checking if a syscall arg is of type sockaddr (or if the name is
always sockaddr as Ian did above) and the next arg has name "len", then
we can set the beauty_array[index_of_sockaddr_arg] = -index_of_len_arg,
that extra + 1 looks odd, but must be part of the convention too.

Since we have it there already and we may not have BTF and BTF isn't yet
a hard requirement, we leave the fallbacks in place for the time being?

- Arnaldo