Re: [PATCH 2/2] bpf: fix have_attach_type() detection

[Eugene Loh <eugene.loh@oracle.com>]

Reviewed-by: Eugene Loh <eugene.loh@oracle.com>

I'll try to run tests tonight.

In what order will patches be applied?  I vote for these two ahead of 
the 8-patch perf series.

dt_prov_fbt.c needs updated copyright year

Would it be possible/practical in the commit message to mention kernel 
commits or version numbers perhaps?  Not a big deal, perhaps.

On 3/17/25 14:41, Kris Van Hees wrote:
> There are kernel versions that support the BPF_TRACE_FENTRY attach type
> at program load, but do not support opening the attachment point (a
> kernel symbol by BTF id) as a raw tracepoint.  The cause is that the
> support for fentry as a raw tracepoint was initially only implemented
> on x86_64.
>
> We now test both program load *and* opening the raw tracepoint to know
> if BPF_TRACE_FENTRY as attach type is supported.
>
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
>   libdtrace/dt_bpf.c      | 29 ++++++++++++++++++++++-------
>   libdtrace/dt_prov_fbt.c |  2 ++
>   2 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index e50bb536..9ee32e8b 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -486,19 +486,34 @@ have_attach_type(enum bpf_prog_type ptype, enum bpf_attach_type atype,
>   				BPF_RETURN()
>   			};
>   	dtrace_difo_t	dp;
> -	int		fd;
> +	int		pfd, tfd = -1;
>   
>   	dp.dtdo_buf = insns;
>   	dp.dtdo_len = ARRAY_SIZE(insns);
>   
> -	fd = dt_bpf_prog_attach(ptype, atype, 0, btf_id, &dp, 0, NULL, 0);
> -	/* If the program loads, we can use the attach type. */
> -	if (fd > 0) {
> -		close(fd);
> -		return 1;
> -	}
> +	pfd = dt_bpf_prog_attach(ptype, atype, 0, btf_id, &dp, 0, NULL, 0);
> +	/* If the program load fails, we cannot iuse the attach type. */
> +	if (pfd < 0)
> +		goto fail;
>   
> +	/*
> +	 * If the program loads, we still need to verify that probe can be
> +	 * opened as a raw tracepoint.  Some kernels allow the program load
> +	 * but return -ENOTSUPP when you try to open the raw tracepoint.
> +	 */
> +	tfd = dt_bpf_raw_tracepoint_open(NULL, pfd);
> +	if (tfd < 0)
> +		goto fail;
> +
> +	close(tfd);
> +	close(pfd);
> +	return 1;
> +
> +fail:
>   	/* Failed -> attach type not available to us */
> +	if (pfd >= 0)
> +		close(pfd);
> +
>   	return 0;
>   }
>   
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 1489275a..00f9174c 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -74,6 +74,8 @@ dt_provimpl_t			dt_rawfbt;
>   static int populate(dtrace_hdl_t *dtp)
>   {
>   	dt_fbt = BPF_HAS(dtp, BPF_FEAT_FENTRY) ? dt_fbt_fprobe : dt_fbt_kprobe;
> +	dt_dprintf("fbt: Using %s implementation\n",
> +		   BPF_HAS(dtp, BPF_FEAT_FENTRY) ? "fentry/fexit" : "kprobe");
>   
>   	if (dt_provider_create(dtp, dt_fbt.name, &dt_fbt, &pattr,
>   			       NULL) == NULL ||