[DTrace-devel] [PATCH 04/04] rawtp: retrieve probe arguments from CTF data

[Eugene Loh]

On 11/22/23 10:49, Kris Van Hees via DTrace-devel wrote:

> We were determining the number of arguments using a trial-and-error
> approach using mimnimal BPF programs.  If there is CTF type data for

"using mimnimal" ->
"with minimal"

> the raw tracepoints (in the form of a function prototype), we use
> that because it also gives us the types of the arguuments.

s/arguuments/arguments/

> If no such CTF data is available, we fallback to the trial-and-error
> approach.

fallback ->
fall back

> diff --git a/libdtrace/dt_prov_rawtp.c b/libdtrace/dt_prov_rawtp.c
> @@ -157,8 +157,14 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>   	return 0;
>   }
>   
> -static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> -		      int *argcp, dt_argdesc_t **argvp)
> +/*
> + * If there is no btf_trace_* prototype available in CTF, we can still probe
> + * the number of available argument for a raw tracepoint by means of a trial
> + * and error loop to see what the highest argument index is that the BPF
> + * verifier allows us to load from.
> + */
> +static int probe_info_bpf(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> +			  int *argcp, dt_argdesc_t **argvp)
>   {
>   	int		argc, i;
>   	dt_argdesc_t	*argv = NULL;

We do not see it in this patch, but this function has the comment, "This 
is an unfortunate necessity."  That sentence should be struck.  What 
used to be a limitation is now that trick that makes probe_info_bpf() 
work.  The limitation has become a fortunate feature.

> @@ -205,10 +211,77 @@ static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
>   	}
>   
>   done:
> -        *argcp = argc;
> -        *argvp = argv;
> +	*argcp = argc;
> +	*argvp = argv;
> +
> +	return 0;
> +}
> +
> +static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
> +		      int *argcp, dt_argdesc_t **argvp)
> +{ ...
>   }

I guess the CTF stuff looks good, but I'm not real familiar with it and 
not 100% confident judging it.

> diff --git a/test/unittest/providers/rawtp/tst.lv-sched_process_fork.r b/test/unittest/providers/rawtp/tst.lv-sched_process_fork.r
> +	Argument Types
> +		args[0]: struct task_struct *
> +		args[1]: struct task_struct *

I'm not clear what position this patch is taking on testing.  With that 
.r file, I assume the trial-and-error fall-back method cannot pass.  So 
the test assumes that we do not fall back to trial and error.  So does 
the test really test this patch?