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

[Alan Maguire]

On 11/10/2024 15:23, Kris Van Hees wrote:
> 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.

Yeah, if we can do something more fine-grained than this, that would be
great. Thanks!

Alan

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