[DTrace-devel] [PATCH 10/15] Change condition to bypass trampoline generation

[Nick Alcock]

On 23 Feb 2023, Kris Van Hees told this:

> On Thu, Feb 23, 2023 at 01:03:52PM +0000, Nick Alcock wrote:
>> On 23 Feb 2023, Kris Van Hees via DTrace-devel verbalised:
>> > -		if (prp->prov->impl->trampoline == NULL)
>> > +		if (prp->prov->impl->prog_type == BPF_PROG_TYPE_UNSPEC)
>> >  			continue;
>> 
>> This was there for a reason: if something doesn't have a trampoline we
>> don't want to do anything that relies on it. You're replacing this with
>> the implicit assumption that only UNSPEC-type providers will ever not
>> have a trampoline, and that's not guaranteed, it's just something that
>> happens to be true right now.
>
> It is actually the other way around.  We started with the notion that providers
> always need a trampoline and that was true because all programs were attached
> as BPF programs to real probes.
>
> Then we added a group of probes that use underlying probes to do the real work,
> but the overlying and under;ying probes had the same DTrace context and so we
> ended up with providers that do not need a trampoline.  And so that ended up
> introducing the change to only call the trampoline function when there actually
> is one.  But in retrospect that was a bit of a shortsighted idea, because...
>
> ... we now have another case where regular trampoline generation should not be
> taking place because we are not generating a program that will be attached to
> a real probe but we do need to generate a trampoline to set up the specific
> DTrace context for the dependent probe.

Ah!

> The common thing here is that both groups of providers are not actually
> representing real probes, which ccan be specified by setting the prog_type to
> be BPF_PROG_TYPE_UNSPEC.  Therefore, this patch uses that as an explicit way
> to indicate that program load should not generate a trampoline for this probe.

Ah right, so this *is* the "no trampoline generation please, we are
calling the trampoline from somewhere else" flag I was wondering about.
The name BPF_PROG_TYPE_UNSPEC didn't make that very clear to me.

> We need to be able to support providers that *have* a trampoline function *and*
> do not want it called during program load.  This does that.
>
> I'll update the commit message explaining this.
>
> r-b OK with that explanation?

Absolutely.