Jason Xing wrote: > On Wed, Oct 16, 2024 at 2:31 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > > > On 10/15/24 6:04 PM, Jason Xing wrote: > > > To be honest, I considered how to disable the static key. Like you > > > said, I failed to find a good chance that I can accurately disable it. > > > > It at least needs to be disabled whenever that bpf prog got detached. > > > > > > > >> The bpf prog may be detached also. (IF) it ends up staying with the > > >> cgroup/sockops interface, it should depend on the existing static key in > > >> cgroup_bpf_enabled(CGROUP_SOCK_OPS) instead of adding another one. > > > > > Are you suggesting that we need to remove the current static key? In > > > the previous thread, the reason why Willem came up with this idea is, > > > I think, to avoid affect the non-bpf timestamping feature. > > > > Take a look at cgroup_bpf_enabled(CGROUP_SOCK_OPS). There is a static key. I am > > saying to use that existing key. afaict, the newly added bpf_tstamp_control key > > is mainly an optimization. Yes, cgroup_bpf_enabled(CGROUP_SOCK_OPS) is less > > granular but it has the needed accounting to disable whenever the bpf prog got > > detached, so better just reuse the cgroup_bpf_enabled(CGROUP_SOCK_OPS). > > Good suggestion. Good thing is that I don't need to figure out a > proper place to disable it any more. I can directly use > cgroup_bpf_enabled(CGROUP_SOCK_OPS) to test if the timestamp should be > printed with BPF program loaded. > > BTW, I found that we don't implement how to disable the ip4_min_ttl > static key. Sometimes, I'm confused whether we have to disable it at a > certain time. In this case it would be fine to not disable it at all. The crux is that it is disabled on the vast majority of machines not using the feature. If a socket uses the feature, adding the small cost of the branches on the rest of the system is fine. Disabling requires refcounting usage. Sometimes the complexity and cost of that outweights the benefit.
