On 10/5/22 12:50 PM, Toke Høiland-Jørgensen wrote:
sdf@xxxxxxxxxx writes:
[...]
The series looks exciting, haven't had a chance to look deeply, will try to find some time this week. We've chatted briefly about priority during the talk, let's maybe discuss it here more? I, as a user, still really have no clue about what priority to use. We have this problem at tc, and we'll seemingly have the same problem here? I guess it's even more relevant in k8s because internally at G we can control the users. Is it worth at least trying to provide some default bands / guidance? For example, having SEC('tc/ingress') receive attach_priority=124 by default? Maybe we can even have something like 'tc/ingress_first' get attach_priority=1 and 'tc/ingress_last' with attach_priority=254? (the names are arbitrary, we can do something better) ingress_first/ingress_last can be used by some monitoring jobs. The rest can use default 124. If somebody really needs a custom priority, then they can manually use something around 124/2 if they need to trigger before the 'default' priority or 124+124/2 if they want to trigger after? Thoughts? Is it worth it? Do we care?I think we should care :) Having "better" defaults are probably a good idea (so not everything just ends up at priority 1 by default). However, I think ultimately the only robust solution is to make the priority override-able. Users are going to want to combine BPF programs in ways that their authors didn't anticipate, so the actual priority the programs run at should not be the sole choice of the program author. To use the example that Daniel presented at LPC: Running datadog and cilium at the same time broke cilium because datadog took over the prio-1 hook point. With the bpf_link API what would change is that (a) it would be obvious that something breaks (that is good), and (b) it would be datadog that breaks instead of cilium (because it can no longer just take over the hook, it'll get an error instead). However, (b) means that the user still hasn't gotten what they wanted: the ability to run datadog and cilium at the same time. To do this, they will need to be able to change the priorities of one or both applications.
(Just for the record :) it was an oversight on datadog agent part and it got fixed, somehow there was a corner-case race with device creation and bpf attachment which lead to this, but 100% it would make it obvious that something breaks which is already a good step forward - I just took this solely as a real-world example that these things /can/ happen and are /tricky/ to debug on top given the 'undefined' behavior resulting from this; this can happen to anyone in general ofc. Both sides (cilium, dd) are configurable to interoperate cleanly now through daemon config.)
I know cilium at least has a configuration option to change this somewhere, but I don't think relying on every BPF-using application to expose this (each in their own way) is a good solution. I think of priorities more like daemon startup at boot: this is system policy, decided by the equivalent of the init system (and in this analogy we are currently at the 'rc.d' stage of init system design, with the hook priorities). One way to resolve this is to have a central daemon that implements the policy and does all the program loading on behalf of the users. I think multiple such daemons exist already in more or less public and/or complete states. However, getting everyone to agree on one is also hard, so maybe the kernel needs to expose a mechanism for doing the actual overriding, and then whatever daemon people run can hook into that?
I think system policy but also user policy, kind of a mixed bag in the end. Just take the policy bpf app vs introspection bpf app as an example: a user might want to see either all traffic (thus before policy app), or just traffic that policy let through (thus after policy app).
Not sure what that mechanism would be? A(nother) BPF hook for overriding priority on load? An LSM hook that rewrites the system call? (can it already do that?) Something else?
Yeah, it could be a means to achieve that, some kind of policy agent which has awareness of the installed programs and their inter-dependencies resp. user intent where it then rewrites the prios dynamically.
Oh, and also, in the case of TC there's also the additional issue that execution only chains to the next program if the current one returns TC_ACT_UNSPEC; this should probably also be overridable somehow, for the same reasons...
Same category as above, yes. Thanks, Daniel
