Add RecursionMisses to access ProgramInfo recursion misses counter

[olsajiri]

Adding RecursionMisses to access ProgramInfo recursion misses counter.

It does not depend on whether collection of statistics is enabled, the value is always counted.

[lmb]

Which kernel did this appear on though? The boolean is meant to indicate whether the kernel exposes this flag in general, not whether stats were enabled (IIRC).

[olsajiri]

it got in v5.11 9ed9e9ba2337 bpf: Count the number of times recursion was prevented

I'm actually not sure how is the availability detected, both Runtime and RunCount
return values if pi.stats != nil which seems to be the case always AFAICS

I can keep the same checks as for the Runtime and RunCount

[ti-mo]

I'm actually not sure how is the availability detected, both Runtime and RunCount
return values if pi.stats != nil which seems to be the case always AFAICS

Can't remember who contributed it, but iirc they were planning to add a probe, and the bool was added to not have to break API later. Looks like the probe was never implemented, and looking back it hardly seems worth it to load a prog, call prog_run and verify if the run counter increases just so we can return that bool correctly.

In the case of RecursionMisses, that one feels unrealistic to try to reproduce reliably at all. I'd just drop the bool and add a comment about it being 5.11+.

[lmb]

That breaks when using newProgramInfoFromProc. So unless we deprecate that we should just stick with the established pattern of returning a bool.

[olsajiri]

ok, keeping the bool then, thanks

[ti-mo]

That breaks when using newProgramInfoFromProc. So unless we deprecate that we should just stick with the established pattern of returning a bool.

@lmb Could you elaborate on what breaks exactly?

[lmb]

pi.stats is nil and therefore the line panics.

[ti-mo]

Thanks! Left some feedback. Let's just drop the bool, fix up the comment and call it a day.

[ti-mo]

I'm actually not sure how is the availability detected, both Runtime and RunCount
return values if pi.stats != nil which seems to be the case always AFAICS

Can't remember who contributed it, but iirc they were planning to add a probe, and the bool was added to not have to break API later. Looks like the probe was never implemented, and looking back it hardly seems worth it to load a prog, call prog_run and verify if the run counter increases just so we can return that bool correctly.

In the case of RecursionMisses, that one feels unrealistic to try to reproduce reliably at all. I'd just drop the bool and add a comment about it being 5.11+.

[lmb]

Pushed a fixup, PTAL. Looking at the upstream commit it's not clear to me where the idea that this is related to program statistics comes from? Seems easier to me to just treat these things as completely independent.

Just returning pi.stats.recursionMisses, true doesn't work due to newProgramInfoFromProc. Reworked to just use a separate field which works around this problem.

[olsajiri]

hm, it's per program stats collected under same struct in kernel together with run count and run time

struct bpf_prog_stats {
        u64_stats_t cnt;
        u64_stats_t nsecs;
        u64_stats_t misses;
        struct u64_stats_sync syncp;
} __aligned(2 * sizeof(u64));

it's incremented through bpf_prog_inc_misses_counter call any time there's recursion detected on link and we can't run the program

it makes sense to me to place it under programStats

[lmb]

I can see your point, moving it out of the struct doesn't really add value. I've changed it back, fixed the panic and added a comment.

[mmat11]

Suggested change

	// Total number of timer the programm was NOT called.
	// Total number of times the program was NOT called.

[lmb]

CI failures are due to unrelated upstream changes.

Stale