[PATCH bpf-next] bpf: allow CGROUP_SKB programs to use bpf_get_current_cgroup_id() helper

[PATCH bpf-next] bpf: allow CGROUP_SKB programs to use bpf_get_current_cgroup_id() helper

[Roman Gushchin]

Currently bpf_get_current_cgroup_id() is not supported for
CGROUP_SKB programs. An attempt to load such a program generates an
error like this:
    libbpf:
    0: (b7) r6 = 0
    ...
    8: (63) *(u32 *)(r10 -28) = r6
    9: (85) call bpf_get_current_cgroup_id#80
    unknown func bpf_get_current_cgroup_id#80

There are no particular reasons for denying it,
and we have some use cases where it might be useful.

So let's add it to the list of allowed helpers.

Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 net/core/filter.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 55bfc941d17a..19724bb1860d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5919,6 +5919,10 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_sk_storage_get_proto;
 	case BPF_FUNC_sk_storage_delete:
 		return &bpf_sk_storage_delete_proto;
+#ifdef CONFIG_CGROUPS
+	case BPF_FUNC_get_current_cgroup_id:
+		return &bpf_get_current_cgroup_id_proto;
+#endif
 #ifdef CONFIG_INET
 	case BPF_FUNC_tcp_sock:
 		return &bpf_tcp_sock_proto;
-- 
2.20.1

Re: [PATCH bpf-next] bpf: allow CGROUP_SKB programs to use bpf_get_current_cgroup_id() helper

[Roman Gushchin]

On Thu, Jun 06, 2019 at 11:59:11AM -0700, Roman Gushchin wrote:
> Currently bpf_get_current_cgroup_id() is not supported for
> CGROUP_SKB programs. An attempt to load such a program generates an
> error like this:
>     libbpf:
>     0: (b7) r6 = 0
>     ...
>     8: (63) *(u32 *)(r10 -28) = r6
>     9: (85) call bpf_get_current_cgroup_id#80
>     unknown func bpf_get_current_cgroup_id#80
> 
> There are no particular reasons for denying it,
> and we have some use cases where it might be useful.

Ah, sorry, it's not so simple, as we probably need to take
the cgroup pointer from the socket, not from current.

So the implementation of the helper should be different
for this type of programs.

So I wonder if it's better to introduce a new helper
bpf_get_sock_cgroup_id()?

What do you think?

Thanks!

Re: [PATCH bpf-next] bpf: allow CGROUP_SKB programs to use bpf_get_current_cgroup_id() helper

[Daniel Borkmann]

On 06/06/2019 09:08 PM, Roman Gushchin wrote:
> On Thu, Jun 06, 2019 at 11:59:11AM -0700, Roman Gushchin wrote:
>> Currently bpf_get_current_cgroup_id() is not supported for
>> CGROUP_SKB programs. An attempt to load such a program generates an
>> error like this:
>>     libbpf:
>>     0: (b7) r6 = 0
>>     ...
>>     8: (63) *(u32 *)(r10 -28) = r6
>>     9: (85) call bpf_get_current_cgroup_id#80
>>     unknown func bpf_get_current_cgroup_id#80
>>
>> There are no particular reasons for denying it,
>> and we have some use cases where it might be useful.
> 
> Ah, sorry, it's not so simple, as we probably need to take
> the cgroup pointer from the socket, not from current.
> 
> So the implementation of the helper should be different
> for this type of programs.
> 
> So I wonder if it's better to introduce a new helper
> bpf_get_sock_cgroup_id()?
> 
> What do you think?

We do have bpf_skb_cgroup_id(), did you give that a try?

Thanks,
Daniel

Re: [PATCH bpf-next] bpf: allow CGROUP_SKB programs to use bpf_get_current_cgroup_id() helper

[Roman Gushchin]

On Thu, Jun 06, 2019 at 09:39:35PM +0200, Daniel Borkmann wrote:
> On 06/06/2019 09:08 PM, Roman Gushchin wrote:
> > On Thu, Jun 06, 2019 at 11:59:11AM -0700, Roman Gushchin wrote:
> >> Currently bpf_get_current_cgroup_id() is not supported for
> >> CGROUP_SKB programs. An attempt to load such a program generates an
> >> error like this:
> >>     libbpf:
> >>     0: (b7) r6 = 0
> >>     ...
> >>     8: (63) *(u32 *)(r10 -28) = r6
> >>     9: (85) call bpf_get_current_cgroup_id#80
> >>     unknown func bpf_get_current_cgroup_id#80
> >>
> >> There are no particular reasons for denying it,
> >> and we have some use cases where it might be useful.
> > 
> > Ah, sorry, it's not so simple, as we probably need to take
> > the cgroup pointer from the socket, not from current.
> > 
> > So the implementation of the helper should be different
> > for this type of programs.
> > 
> > So I wonder if it's better to introduce a new helper
> > bpf_get_sock_cgroup_id()?
> > 
> > What do you think?
> 
> We do have bpf_skb_cgroup_id(), did you give that a try?

It also isn't supported for CGROUP_SKB, but other than that looks
exactly what I need.

Thank you for pointing at it!

Re: [PATCH bpf-next] bpf: allow CGROUP_SKB programs to use bpf_get_current_cgroup_id() helper

[Daniel Borkmann]

On 06/06/2019 09:53 PM, Roman Gushchin wrote:
> On Thu, Jun 06, 2019 at 09:39:35PM +0200, Daniel Borkmann wrote:
>> On 06/06/2019 09:08 PM, Roman Gushchin wrote:
>>> On Thu, Jun 06, 2019 at 11:59:11AM -0700, Roman Gushchin wrote:
>>>> Currently bpf_get_current_cgroup_id() is not supported for
>>>> CGROUP_SKB programs. An attempt to load such a program generates an
>>>> error like this:
>>>>     libbpf:
>>>>     0: (b7) r6 = 0
>>>>     ...
>>>>     8: (63) *(u32 *)(r10 -28) = r6
>>>>     9: (85) call bpf_get_current_cgroup_id#80
>>>>     unknown func bpf_get_current_cgroup_id#80
>>>>
>>>> There are no particular reasons for denying it,
>>>> and we have some use cases where it might be useful.
>>>
>>> Ah, sorry, it's not so simple, as we probably need to take
>>> the cgroup pointer from the socket, not from current.
>>>
>>> So the implementation of the helper should be different
>>> for this type of programs.
>>>
>>> So I wonder if it's better to introduce a new helper
>>> bpf_get_sock_cgroup_id()?
>>>
>>> What do you think?
>>
>> We do have bpf_skb_cgroup_id(), did you give that a try?
> 
> It also isn't supported for CGROUP_SKB, but other than that looks
> exactly what I need.
> 
> Thank you for pointing at it!

Yes, the helper would need to be enabled there.

Cheers,
Daniel