Jakub Kicinski <jakub.kicin...@netronome.com> [Mon, 2019-06-24 14:51 -0700]: > On Mon, 24 Jun 2019 16:22:25 +0200, Daniel Borkmann wrote: > > On 06/22/2019 12:33 AM, Takshak Chahande wrote: > > > With different bpf attach_flags available to attach bpf programs specially > > > with BPF_F_ALLOW_OVERRIDE and BPF_F_ALLOW_MULTI, the list of effective > > > bpf-programs available to any sub-cgroups really needs to be available for > > > easy debugging. > > > > > > Using BPF_F_QUERY_EFFECTIVE flag, one can get the list of not only > > > attached > > > bpf-programs to a cgroup but also the inherited ones from parent cgroup. > > > > > > So "-e" option is introduced to use BPF_F_QUERY_EFFECTIVE query flag here > > > to > > > list all the effective bpf-programs available for execution at a specified > > > cgroup. > > > > > > Reused modified test program test_cgroup_attach from > > > tools/testing/selftests/bpf: > > > # ./test_cgroup_attach > > > > > > With old bpftool (without -e option): > > > > > > # bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/ > > > ID AttachType AttachFlags Name > > > 271 egress multi pkt_cntr_1 > > > 272 egress multi pkt_cntr_2 > > > > > > Attached new program pkt_cntr_4 in cg2 gives following: > > > > > > # bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2 > > > ID AttachType AttachFlags Name > > > 273 egress override pkt_cntr_4 > > > > > > And with new "-e" option it shows all effective programs for cg2: > > > > > > # bpftool -e cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2 > > > ID AttachType AttachFlags Name > > > 273 egress override pkt_cntr_4 > > > 271 egress override pkt_cntr_1 > > > 272 egress override pkt_cntr_2 > > > > > > Signed-off-by: Takshak Chahande <ctaks...@fb.com> > > > Acked-by: Andrey Ignatov <r...@fb.com> > > > > Applied, thanks! > > This is a cgroup-specific flag, right? It should be a parameter > to cgroup show, not a global flag. Can we please drop this patch > from the tree?
Hey Jakub, I had same thought about cgroup-specific flag while reviewing the patch, but then found out that all flags in bpftool are now global, no mater if they're sub-command-specific or not. For example, --mapcompat is used only in prog-subcommand, but the option is global; --bpffs is used in prog- and map-subcommands, but the option is global as well, etc (there are more examples). I agree that limiting the scope of an option is a good idea in the long term and it'd be great to rework all existing options to be available only for corresponding sub-commands, but I don't see how the new `-e` options is different from existing options and why it should be dropped. Or you were counfused by the example in the commit log? Since all options are global they can be specific anywhere on the command line, i.e. instead of: # bpftool -e cgroup show /path/to/cgroup it can be: # bpftool cgroup show -e /path/to/cgroup -- Andrey Ignatov
