[DTrace-devel] [PATCH v2 dtrace 2/3] fbt: revert to kprobe impl if we cannot find BTF for fprobe function

[Kris Van Hees]

On Fri, Oct 11, 2024 at 11:28:09AM +0100, Alan Maguire wrote:
> When using fprobes, it is possible the to-be-probed function does not
> have a BTF representation.  If this occurs, revert to using the kprobe
> implementation.
> 
> Signed-off-by: Alan Maguire <alan.maguire at oracle.com>
> ---
>  libdtrace/dt_bpf.h      | 5 +++++
>  libdtrace/dt_prov_fbt.c | 8 +++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h
> index 5716d232..dc4eb64d 100644
> --- a/libdtrace/dt_bpf.h
> +++ b/libdtrace/dt_bpf.h
> @@ -32,6 +32,11 @@ extern "C" {
>  					(dtp)->dt_bpffeatures |= (feat); \
>  				} while (0)
>  
> +#define BPF_UNSET_FEATURE(dtp, feat) \
> +				do { \
> +					(dtp)->dt_bpffeatures ^= (feat); \
> +				} while (0)
> +
>  #define DT_CONST_PRID			1
>  #define DT_CONST_ARGC			2
>  #define DT_CONST_STBSZ			3
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 0ba0fb71..15ffa137 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -232,8 +232,14 @@ static int fprobe_probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
>  
>  	btf_id = dt_btf_lookup_name_kind(dtp, dmp->dm_btf, desc->fun,
>  					 BTF_KIND_FUNC);
> -	if (btf_id <= 0)
> +	if (btf_id <= 0) {
> +		if (btf_id == -ENOENT) {
> +			dt_dprintf("fbt: cannot use fprobes, reverting to kprobes\n");
> +			BPF_UNSET_FEATURE(dtp, BPF_FEAT_FENTRY);
> +			prp->prov->impl = &dt_fbt_kprobe;
> +		}

This will indicate that fprobes are not supported at all, which is not what we
want because some of the FBT probes being used moght best be served (or need to
be served) using fprobes.  We will need to be a bit more specific than this.

Let me think about this for a little (with code digging)...  I think that we
can do this but the granularity needs to be at the probe level.  I.e. in the
above code, you replace the implementation of *all* FBT probes and that won't
be ideal at all.  It might be the way we need to go (worst case scenario) if
there are some probes that require kprobe instead of fprobe, but I hope we can
avoid that.

>  		goto done;
> +	}
>  
>  	dt_tp_set_event_id(prp, btf_id);
>  
> -- 
> 2.43.5
>