On Wed, Oct 16, 2024 at 9:13 PM Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> wrote: > > 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. Thanks for the explanation. I will take Martin's advice and use the CGROUP_SOCK_OPS static key. So I don't have to take efforts to implement the inc/dec/enable/disable the static key Thanks, Jason
