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

[Kris Van Hees <kris.van.hees@oracle.com>]

On Mon, Mar 17, 2025 at 04:16:01PM -0400, Eugene Loh wrote:
> Reviewed-by: Eugene Loh <eugene.loh@oracle.com>

Thanks.

> 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.

I really prefer the opposite order, mainly because the performance improvements
really help with speeding up the testsuite runs.

> dt_prov_fbt.c needs updated copyright year

The perf series handles that.

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

Yes, because it is a bit of work to hunt it down, and not really relevant.

> 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 ||