All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] bpftool: Query only cgroup-related attach types
@ 2024-06-07 10:21 Kenta Tada
  2024-06-07 10:50 ` Quentin Monnet
  0 siblings, 1 reply; 3+ messages in thread
From: Kenta Tada @ 2024-06-07 10:21 UTC (permalink / raw)
  To: bpf, qmo
  Cc: daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, Kenta Tada

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.

v1->v2:
  - used an array of cgroup attach types

Signed-off-by: Kenta Tada <tadakentaso@gmail.com>
---
 tools/bpf/bpftool/cgroup.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index af6898c0f388..afab728468bf 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -19,6 +19,38 @@
 
 #include "main.h"
 
+static const int cgroup_attach_types[] = {
+	BPF_CGROUP_INET_INGRESS,
+	BPF_CGROUP_INET_EGRESS,
+	BPF_CGROUP_INET_SOCK_CREATE,
+	BPF_CGROUP_INET_SOCK_RELEASE,
+	BPF_CGROUP_INET4_BIND,
+	BPF_CGROUP_INET6_BIND,
+	BPF_CGROUP_INET4_POST_BIND,
+	BPF_CGROUP_INET6_POST_BIND,
+	BPF_CGROUP_INET4_CONNECT,
+	BPF_CGROUP_INET6_CONNECT,
+	BPF_CGROUP_UNIX_CONNECT,
+	BPF_CGROUP_INET4_GETPEERNAME,
+	BPF_CGROUP_INET6_GETPEERNAME,
+	BPF_CGROUP_UNIX_GETPEERNAME,
+	BPF_CGROUP_INET4_GETSOCKNAME,
+	BPF_CGROUP_INET6_GETSOCKNAME,
+	BPF_CGROUP_UNIX_GETSOCKNAME,
+	BPF_CGROUP_UDP4_SENDMSG,
+	BPF_CGROUP_UDP6_SENDMSG,
+	BPF_CGROUP_UNIX_SENDMSG,
+	BPF_CGROUP_UDP4_RECVMSG,
+	BPF_CGROUP_UDP6_RECVMSG,
+	BPF_CGROUP_UNIX_RECVMSG,
+	BPF_CGROUP_SOCK_OPS,
+	BPF_CGROUP_DEVICE,
+	BPF_CGROUP_SYSCTL,
+	BPF_CGROUP_GETSOCKOPT,
+	BPF_CGROUP_SETSOCKOPT,
+	BPF_LSM_CGROUP
+};
+
 #define HELP_SPEC_ATTACH_FLAGS						\
 	"ATTACH_FLAGS := { multi | override }"
 
@@ -183,11 +215,11 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
 
 static int cgroup_has_attached_progs(int cgroup_fd)
 {
-	enum bpf_attach_type type;
+	unsigned int i = 0;
 	bool no_prog = true;
 
-	for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
-		int count = count_attached_bpf_progs(cgroup_fd, type);
+	for (i = 0; i < ARRAY_SIZE(cgroup_attach_types); i++) {
+		int count = count_attached_bpf_progs(cgroup_fd, cgroup_attach_types[i]);
 
 		if (count < 0 && errno != EINVAL)
 			return -1;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] bpftool: Query only cgroup-related attach types
  2024-06-07 10:21 [PATCH v2] bpftool: Query only cgroup-related attach types Kenta Tada
@ 2024-06-07 10:50 ` Quentin Monnet
  2024-06-07 10:57   ` Kenta Tada
  0 siblings, 1 reply; 3+ messages in thread
From: Quentin Monnet @ 2024-06-07 10:50 UTC (permalink / raw)
  To: Kenta Tada, bpf
  Cc: daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa

2024-06-07 11:21 UTC+0100 ~ Kenta Tada <tadakentaso@gmail.com>
> 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.
> 
> v1->v2:
>   - used an array of cgroup attach types
> 
> Signed-off-by: Kenta Tada <tadakentaso@gmail.com>
> ---
>  tools/bpf/bpftool/cgroup.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index af6898c0f388..afab728468bf 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -19,6 +19,38 @@
>  
>  #include "main.h"
>  
> +static const int cgroup_attach_types[] = {
> +	BPF_CGROUP_INET_INGRESS,
> +	BPF_CGROUP_INET_EGRESS,
> +	BPF_CGROUP_INET_SOCK_CREATE,
> +	BPF_CGROUP_INET_SOCK_RELEASE,
> +	BPF_CGROUP_INET4_BIND,
> +	BPF_CGROUP_INET6_BIND,
> +	BPF_CGROUP_INET4_POST_BIND,
> +	BPF_CGROUP_INET6_POST_BIND,
> +	BPF_CGROUP_INET4_CONNECT,
> +	BPF_CGROUP_INET6_CONNECT,
> +	BPF_CGROUP_UNIX_CONNECT,
> +	BPF_CGROUP_INET4_GETPEERNAME,
> +	BPF_CGROUP_INET6_GETPEERNAME,
> +	BPF_CGROUP_UNIX_GETPEERNAME,
> +	BPF_CGROUP_INET4_GETSOCKNAME,
> +	BPF_CGROUP_INET6_GETSOCKNAME,
> +	BPF_CGROUP_UNIX_GETSOCKNAME,
> +	BPF_CGROUP_UDP4_SENDMSG,
> +	BPF_CGROUP_UDP6_SENDMSG,
> +	BPF_CGROUP_UNIX_SENDMSG,
> +	BPF_CGROUP_UDP4_RECVMSG,
> +	BPF_CGROUP_UDP6_RECVMSG,
> +	BPF_CGROUP_UNIX_RECVMSG,
> +	BPF_CGROUP_SOCK_OPS,
> +	BPF_CGROUP_DEVICE,
> +	BPF_CGROUP_SYSCTL,
> +	BPF_CGROUP_GETSOCKOPT,
> +	BPF_CGROUP_SETSOCKOPT,
> +	BPF_LSM_CGROUP
> +};
> +
>  #define HELP_SPEC_ATTACH_FLAGS						\
>  	"ATTACH_FLAGS := { multi | override }"
>  
> @@ -183,11 +215,11 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
>  
>  static int cgroup_has_attached_progs(int cgroup_fd)
>  {
> -	enum bpf_attach_type type;
> +	unsigned int i = 0;
>  	bool no_prog = true;
>  
> -	for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
> -		int count = count_attached_bpf_progs(cgroup_fd, type);
> +	for (i = 0; i < ARRAY_SIZE(cgroup_attach_types); i++) {


Thanks, it looks better that way.


> +		int count = count_attached_bpf_progs(cgroup_fd, cgroup_attach_types[i]);
>  
>  		if (count < 0 && errno != EINVAL)


I think the "errno != EINVAL" exception was here to allow iterating over
unsupported attach types for the queries. Now that we only do supported
types, we can probably remove it and return if "(count < 0)".


>  			return -1;


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] bpftool: Query only cgroup-related attach types
  2024-06-07 10:50 ` Quentin Monnet
@ 2024-06-07 10:57   ` Kenta Tada
  0 siblings, 0 replies; 3+ messages in thread
From: Kenta Tada @ 2024-06-07 10:57 UTC (permalink / raw)
  To: Quentin Monnet, bpf
  Cc: daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa

On 2024/06/07 19:50, Quentin Monnet wrote:
> 2024-06-07 11:21 UTC+0100 ~ Kenta Tada <tadakentaso@gmail.com>
>> 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.
>>
>> v1->v2:
>>   - used an array of cgroup attach types
>>
>> Signed-off-by: Kenta Tada <tadakentaso@gmail.com>
>> ---
>>  tools/bpf/bpftool/cgroup.c | 38 +++++++++++++++++++++++++++++++++++---
>>  1 file changed, 35 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
>> index af6898c0f388..afab728468bf 100644
>> --- a/tools/bpf/bpftool/cgroup.c
>> +++ b/tools/bpf/bpftool/cgroup.c
>> @@ -19,6 +19,38 @@
>>  
>>  #include "main.h"
>>  
>> +static const int cgroup_attach_types[] = {
>> +	BPF_CGROUP_INET_INGRESS,
>> +	BPF_CGROUP_INET_EGRESS,
>> +	BPF_CGROUP_INET_SOCK_CREATE,
>> +	BPF_CGROUP_INET_SOCK_RELEASE,
>> +	BPF_CGROUP_INET4_BIND,
>> +	BPF_CGROUP_INET6_BIND,
>> +	BPF_CGROUP_INET4_POST_BIND,
>> +	BPF_CGROUP_INET6_POST_BIND,
>> +	BPF_CGROUP_INET4_CONNECT,
>> +	BPF_CGROUP_INET6_CONNECT,
>> +	BPF_CGROUP_UNIX_CONNECT,
>> +	BPF_CGROUP_INET4_GETPEERNAME,
>> +	BPF_CGROUP_INET6_GETPEERNAME,
>> +	BPF_CGROUP_UNIX_GETPEERNAME,
>> +	BPF_CGROUP_INET4_GETSOCKNAME,
>> +	BPF_CGROUP_INET6_GETSOCKNAME,
>> +	BPF_CGROUP_UNIX_GETSOCKNAME,
>> +	BPF_CGROUP_UDP4_SENDMSG,
>> +	BPF_CGROUP_UDP6_SENDMSG,
>> +	BPF_CGROUP_UNIX_SENDMSG,
>> +	BPF_CGROUP_UDP4_RECVMSG,
>> +	BPF_CGROUP_UDP6_RECVMSG,
>> +	BPF_CGROUP_UNIX_RECVMSG,
>> +	BPF_CGROUP_SOCK_OPS,
>> +	BPF_CGROUP_DEVICE,
>> +	BPF_CGROUP_SYSCTL,
>> +	BPF_CGROUP_GETSOCKOPT,
>> +	BPF_CGROUP_SETSOCKOPT,
>> +	BPF_LSM_CGROUP
>> +};
>> +
>>  #define HELP_SPEC_ATTACH_FLAGS						\
>>  	"ATTACH_FLAGS := { multi | override }"
>>  
>> @@ -183,11 +215,11 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
>>  
>>  static int cgroup_has_attached_progs(int cgroup_fd)
>>  {
>> -	enum bpf_attach_type type;
>> +	unsigned int i = 0;
>>  	bool no_prog = true;
>>  
>> -	for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
>> -		int count = count_attached_bpf_progs(cgroup_fd, type);
>> +	for (i = 0; i < ARRAY_SIZE(cgroup_attach_types); i++) {
> 
> 
> Thanks, it looks better that way.
> 
> 
>> +		int count = count_attached_bpf_progs(cgroup_fd, cgroup_attach_types[i]);
>>  
>>  		if (count < 0 && errno != EINVAL)
> 
> 
> I think the "errno != EINVAL" exception was here to allow iterating over
> unsupported attach types for the queries. Now that we only do supported
> types, we can probably remove it and return if "(count < 0)".

OK.
I'll fix it quickly.
Thanks!

> 
> 
>>  			return -1;
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-06-07 10:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-07 10:21 [PATCH v2] bpftool: Query only cgroup-related attach types Kenta Tada
2024-06-07 10:50 ` Quentin Monnet
2024-06-07 10:57   ` Kenta Tada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.