Hey Daniel,
On Wed, Aug 23, 2023, at 9:08 AM, Daniel Xu wrote:
> This patchset changes the behavior of TC and XDP hooks during attachment
> such that any BPF_MAP_TYPE_PROG_ARRAY that the prog uses has an extra
> uref taken.
>
> The goal behind this change is to try and prevent confusion for the
> majority of use cases. The current behavior where when the last uref is
> dropped the prog array map is emptied is quite confusing. Confusing
> enough for there to be multiple references to it in ebpf-go [0][1].
>
> Completely solving the problem is difficult. As stated in c9da161c6517
> ("bpf: fix clearing on persistent program array maps"), it is
> difficult-to-impossible to walk the full dependency graph b/c it is too
> dynamic.
>
> However in practice, I've found that all progs in a tailcall chain
> share the same prog array map. Knowing that, if we take a uref on any
> used prog array map when the program is attached, we can simplify the
> majority use case and make it more ergonomic.
>
> I'll be the first to admit this is not a very clean solution. It does
> not fully solve the problem. Nor does it make overall logic any simpler.
> But I do think it makes a pretty big usability hole slightly smaller.
>
> I've done some basic testing using a repro program [3] I wrote to debug
> the original issue that eventually led me to this patchset. If we wanna
> move forward with this approach, I'll resend with selftests.
>
> [0]:
> https://github.com/cilium/ebpf/blob/01ebd4c1e2b9f8b3dd4fd2382aa1092c3c9bfc9d/doc.go#L22-L24
> [1]:
> https://github.com/cilium/ebpf/blob/d1a52333f2c0fed085f8d742a5a3c164795d8492/collection.go#L320-L321
> [2]: https://github.com/danobi/tc_tailcall_repro
I recently remembered about this again. Was suggested I poke you in case you're interested.
I looked again and I think this is kinda a neat hack. I probably won't have time to pick this back
up either way.
