Re: [PATCH] bpftool: Query only cgroup-related attach types

[Kenta Tada <tadakentaso@gmail.com>]

On 2024/05/30 5:22, Quentin Monnet wrote:
> On 29/05/2024 14:10, Kenta Tada wrote:
>> When CONFIG_NETKIT=y,
>> bpftool-cgroup shows error even if the cgroup's path is correct:
>>
>> $ bpftool cgroup tree /sys/fs/cgroup
>> CgroupPath
>> ID       AttachType      AttachFlags     Name
>> Error: can't query bpf programs attached to /sys/fs/cgroup: No such device or address
>>
>> From strace and kernel tracing, I found netkit returned ENXIO and this command failed.
>> I think this AttachType(BPF_NETKIT_PRIMARY) is not relevant to cgroup.
>>
>> bpftool-cgroup should query just only cgroup-related attach types.
>>
>> Signed-off-by: Kenta Tada <tadakentaso@gmail.com>
>> ---
>>  tools/bpf/bpftool/cgroup.c | 47 +++++++++++++++++++++++++++++++++-----
>>  1 file changed, 41 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
>> index af6898c0f388..bb2703aa4756 100644
>> --- a/tools/bpf/bpftool/cgroup.c
>> +++ b/tools/bpf/bpftool/cgroup.c
>> @@ -19,6 +19,39 @@
>>  
>>  #include "main.h"
>>  
>> +static const bool cgroup_attach_types[] = {
>> +	[BPF_CGROUP_INET_INGRESS] = true,
>> +	[BPF_CGROUP_INET_EGRESS] = true,
>> +	[BPF_CGROUP_INET_SOCK_CREATE] = true,
>> +	[BPF_CGROUP_INET_SOCK_RELEASE] = true,
>> +	[BPF_CGROUP_INET4_BIND] = true,
>> +	[BPF_CGROUP_INET6_BIND] = true,
>> +	[BPF_CGROUP_INET4_POST_BIND] = true,
>> +	[BPF_CGROUP_INET6_POST_BIND] = true,
>> +	[BPF_CGROUP_INET4_CONNECT] = true,
>> +	[BPF_CGROUP_INET6_CONNECT] = true,
>> +	[BPF_CGROUP_UNIX_CONNECT] = true,
>> +	[BPF_CGROUP_INET4_GETPEERNAME] = true,
>> +	[BPF_CGROUP_INET6_GETPEERNAME] = true,
>> +	[BPF_CGROUP_UNIX_GETPEERNAME] = true,
>> +	[BPF_CGROUP_INET4_GETSOCKNAME] = true,
>> +	[BPF_CGROUP_INET6_GETSOCKNAME] = true,
>> +	[BPF_CGROUP_UNIX_GETSOCKNAME] = true,
>> +	[BPF_CGROUP_UDP4_SENDMSG] = true,
>> +	[BPF_CGROUP_UDP6_SENDMSG] = true,
>> +	[BPF_CGROUP_UNIX_SENDMSG] = true,
>> +	[BPF_CGROUP_UDP4_RECVMSG] = true,
>> +	[BPF_CGROUP_UDP6_RECVMSG] = true,
>> +	[BPF_CGROUP_UNIX_RECVMSG] = true,
>> +	[BPF_CGROUP_SOCK_OPS] = true,
>> +	[BPF_CGROUP_DEVICE] = true,
>> +	[BPF_CGROUP_SYSCTL] = true,
>> +	[BPF_CGROUP_GETSOCKOPT] = true,
>> +	[BPF_CGROUP_SETSOCKOPT] = true,
>> +	[BPF_LSM_CGROUP] = true,
>> +	[__MAX_BPF_ATTACH_TYPE] = false,
>> +};
> 
> 
> Thanks for this!
> 
> I can't say I'm glad to see another version of the list of
> cgroup-related attach types (in addition to HELP_SPEC_ATTACH_TYPES and
> to the manual page). But the alternative would be to explicitly skip
> BPF_NETKIT_PRIMARY, which is not great, either. Too bad we don't have a
> way to check whether the type is cgroup-related in libbpf or from the
> bpf.h headers; but I don't think there's much interest to add it there,
> so we'll probably have the array. We should account for it in
> tools/testing/selftests/bpf/test_bpftool_synctypes.py, but I can do this
> as a follow-up if you don't feel like messing up with the Python script.

I think some bpf management tools require how to get cgroup-related attach types.
So I'm interested in adding the new API to check whether the type is cgroup-related in libbpf.

Thank you for the information about test_bpftool_synctypes.py.
BTW, I'm getting some syntax warnings when I use test_bpftool_synctypes.py in Python 3.12.
Python 3.12 changes the behavior of incorrect escape sequences.
To try test_bpftool_synctypes, I add r to the head and fix it in my local environment.

> 
> 
>> +
>>  #define HELP_SPEC_ATTACH_FLAGS						\
>>  	"ATTACH_FLAGS := { multi | override }"
>>  
>> @@ -187,14 +220,16 @@ static int cgroup_has_attached_progs(int cgroup_fd)
>>  	bool no_prog = true;
>>  
>>  	for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
>> -		int count = count_attached_bpf_progs(cgroup_fd, type);
>> +		if (cgroup_attach_types[type]) {
> 
> 
> Please change here:
> 
>                 int count;
> 
>                 if (!cgroup_attach_types[type])
>                         continue;
> 
> And no need to further indent the rest of the block.
> 
> 
>> +			int count = count_attached_bpf_progs(cgroup_fd, type);
>>  
>> -		if (count < 0 && errno != EINVAL)
>> -			return -1;
>> +			if (count < 0 && errno != EINVAL)
>> +				return -1;
>>  
>> -		if (count > 0) {
>> -			no_prog = false;
>> -			break;
>> +			if (count > 0) {
>> +				no_prog = false;
>> +				break;
>> +			}
>>  		}
>>  	}
>>  
>