* [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK
@ 2020-07-29 0:31 Stanislav Fomichev
2020-07-29 0:31 ` [PATCH bpf-next 2/2] selftests/bpf: verify socket storage in cgroup/sock_{create,release} Stanislav Fomichev
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stanislav Fomichev @ 2020-07-29 0:31 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin KaFai Lau
This lets us use socket storage from the following hooks:
* BPF_CGROUP_INET_SOCK_CREATE
* BPF_CGROUP_INET_SOCK_RELEASE
* BPF_CGROUP_INET4_POST_BIND
* BPF_CGROUP_INET6_POST_BIND
Using existing 'bpf_sk_storage_get_proto' doesn't work because
second argument is ARG_PTR_TO_SOCKET. Even though
BPF_PROG_TYPE_CGROUP_SOCK hooks operate on 'struct bpf_sock',
the verifier still considers it as a PTR_TO_CTX.
That's why I'm adding another 'bpf_sk_storage_get_cg_sock_proto'
definition strictly for BPF_PROG_TYPE_CGROUP_SOCK which accepts
ARG_PTR_TO_CTX which is really 'struct sock' for this program type.
Cc: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
net/core/bpf_sk_storage.c | 10 ++++++++++
net/core/filter.c | 3 +++
2 files changed, 13 insertions(+)
diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index eafcd15e7dfd..d3377c90a291 100644
--- a/net/core/bpf_sk_storage.c
+++ b/net/core/bpf_sk_storage.c
@@ -944,6 +944,16 @@ const struct bpf_func_proto bpf_sk_storage_get_proto = {
.arg4_type = ARG_ANYTHING,
};
+const struct bpf_func_proto bpf_sk_storage_get_cg_sock_proto = {
+ .func = bpf_sk_storage_get,
+ .gpl_only = false,
+ .ret_type = RET_PTR_TO_MAP_VALUE_OR_NULL,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_PTR_TO_CTX, /* context is 'struct sock' */
+ .arg3_type = ARG_PTR_TO_MAP_VALUE_OR_NULL,
+ .arg4_type = ARG_ANYTHING,
+};
+
const struct bpf_func_proto bpf_sk_storage_delete_proto = {
.func = bpf_sk_storage_delete,
.gpl_only = false,
diff --git a/net/core/filter.c b/net/core/filter.c
index 29e3455122f7..7124f0fe6974 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6187,6 +6187,7 @@ bool bpf_helper_changes_pkt_data(void *func)
}
const struct bpf_func_proto bpf_event_output_data_proto __weak;
+const struct bpf_func_proto bpf_sk_storage_get_cg_sock_proto __weak;
static const struct bpf_func_proto *
sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
@@ -6219,6 +6220,8 @@ sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_get_cgroup_classid:
return &bpf_get_cgroup_classid_curr_proto;
#endif
+ case BPF_FUNC_sk_storage_get:
+ return &bpf_sk_storage_get_cg_sock_proto;
default:
return bpf_base_func_proto(func_id);
}
--
2.28.0.163.g6104cc2f0b6-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: verify socket storage in cgroup/sock_{create,release}
2020-07-29 0:31 [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK Stanislav Fomichev
@ 2020-07-29 0:31 ` Stanislav Fomichev
2020-07-29 6:12 ` Song Liu
2020-07-29 6:11 ` [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK Song Liu
2020-07-29 22:52 ` Daniel Borkmann
2 siblings, 1 reply; 6+ messages in thread
From: Stanislav Fomichev @ 2020-07-29 0:31 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Martin KaFai Lau
Augment udp_limit test to set and verify socket storage value.
That should be enough to exercise the changes from the previous
patch.
Cc: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/progs/udp_limit.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/udp_limit.c b/tools/testing/selftests/bpf/progs/udp_limit.c
index 8429b22525a7..165e3c2dd9a3 100644
--- a/tools/testing/selftests/bpf/progs/udp_limit.c
+++ b/tools/testing/selftests/bpf/progs/udp_limit.c
@@ -6,14 +6,28 @@
int invocations = 0, in_use = 0;
+struct {
+ __uint(type, BPF_MAP_TYPE_SK_STORAGE);
+ __uint(map_flags, BPF_F_NO_PREALLOC);
+ __type(key, int);
+ __type(value, int);
+} sk_map SEC(".maps");
+
SEC("cgroup/sock_create")
int sock(struct bpf_sock *ctx)
{
+ int *sk_storage;
__u32 key;
if (ctx->type != SOCK_DGRAM)
return 1;
+ sk_storage = bpf_sk_storage_get(&sk_map, ctx, 0,
+ BPF_SK_STORAGE_GET_F_CREATE);
+ if (!sk_storage)
+ return 0;
+ *sk_storage = 0xdeadbeef;
+
__sync_fetch_and_add(&invocations, 1);
if (in_use > 0) {
@@ -31,11 +45,16 @@ int sock(struct bpf_sock *ctx)
SEC("cgroup/sock_release")
int sock_release(struct bpf_sock *ctx)
{
+ int *sk_storage;
__u32 key;
if (ctx->type != SOCK_DGRAM)
return 1;
+ sk_storage = bpf_sk_storage_get(&sk_map, ctx, 0, 0);
+ if (!sk_storage || *sk_storage != 0xdeadbeef)
+ return 0;
+
__sync_fetch_and_add(&invocations, 1);
__sync_fetch_and_add(&in_use, -1);
return 1;
--
2.28.0.163.g6104cc2f0b6-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK
2020-07-29 0:31 [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK Stanislav Fomichev
2020-07-29 0:31 ` [PATCH bpf-next 2/2] selftests/bpf: verify socket storage in cgroup/sock_{create,release} Stanislav Fomichev
@ 2020-07-29 6:11 ` Song Liu
2020-07-29 22:52 ` Daniel Borkmann
2 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2020-07-29 6:11 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Networking, bpf, David S . Miller, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau
On Tue, Jul 28, 2020 at 5:31 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> This lets us use socket storage from the following hooks:
> * BPF_CGROUP_INET_SOCK_CREATE
> * BPF_CGROUP_INET_SOCK_RELEASE
> * BPF_CGROUP_INET4_POST_BIND
> * BPF_CGROUP_INET6_POST_BIND
>
> Using existing 'bpf_sk_storage_get_proto' doesn't work because
> second argument is ARG_PTR_TO_SOCKET. Even though
> BPF_PROG_TYPE_CGROUP_SOCK hooks operate on 'struct bpf_sock',
> the verifier still considers it as a PTR_TO_CTX.
> That's why I'm adding another 'bpf_sk_storage_get_cg_sock_proto'
> definition strictly for BPF_PROG_TYPE_CGROUP_SOCK which accepts
> ARG_PTR_TO_CTX which is really 'struct sock' for this program type.
>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: verify socket storage in cgroup/sock_{create,release}
2020-07-29 0:31 ` [PATCH bpf-next 2/2] selftests/bpf: verify socket storage in cgroup/sock_{create,release} Stanislav Fomichev
@ 2020-07-29 6:12 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2020-07-29 6:12 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Networking, bpf, David S . Miller, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau
On Tue, Jul 28, 2020 at 5:31 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> Augment udp_limit test to set and verify socket storage value.
> That should be enough to exercise the changes from the previous
> patch.
>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK
2020-07-29 0:31 [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK Stanislav Fomichev
2020-07-29 0:31 ` [PATCH bpf-next 2/2] selftests/bpf: verify socket storage in cgroup/sock_{create,release} Stanislav Fomichev
2020-07-29 6:11 ` [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK Song Liu
@ 2020-07-29 22:52 ` Daniel Borkmann
2020-07-29 23:17 ` sdf
2 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2020-07-29 22:52 UTC (permalink / raw)
To: Stanislav Fomichev, netdev, bpf; +Cc: davem, ast, Martin KaFai Lau
On 7/29/20 2:31 AM, Stanislav Fomichev wrote:
> This lets us use socket storage from the following hooks:
> * BPF_CGROUP_INET_SOCK_CREATE
> * BPF_CGROUP_INET_SOCK_RELEASE
> * BPF_CGROUP_INET4_POST_BIND
> * BPF_CGROUP_INET6_POST_BIND
>
> Using existing 'bpf_sk_storage_get_proto' doesn't work because
> second argument is ARG_PTR_TO_SOCKET. Even though
> BPF_PROG_TYPE_CGROUP_SOCK hooks operate on 'struct bpf_sock',
> the verifier still considers it as a PTR_TO_CTX.
> That's why I'm adding another 'bpf_sk_storage_get_cg_sock_proto'
> definition strictly for BPF_PROG_TYPE_CGROUP_SOCK which accepts
> ARG_PTR_TO_CTX which is really 'struct sock' for this program type.
>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Makes sense, both applied, thanks!
[...]
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 29e3455122f7..7124f0fe6974 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -6187,6 +6187,7 @@ bool bpf_helper_changes_pkt_data(void *func)
> }
>
> const struct bpf_func_proto bpf_event_output_data_proto __weak;
> +const struct bpf_func_proto bpf_sk_storage_get_cg_sock_proto __weak;
>
> static const struct bpf_func_proto *
> sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> @@ -6219,6 +6220,8 @@ sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> case BPF_FUNC_get_cgroup_classid:
> return &bpf_get_cgroup_classid_curr_proto;
> #endif
> + case BPF_FUNC_sk_storage_get:
> + return &bpf_sk_storage_get_cg_sock_proto;
Been wondering whether we need these for connect/sendmsg/etc hooks that operate
on sock_addr, but for those we have them already covered in sock_addr_func_proto()
therefore all good.
sock_addr_func_proto() also lists the BPF_FUNC_sk_storage_delete. Should we add
that one as well for sock_filter_func_proto()? Presumably create/release doesn't
make sense, but any use case for bind hook?
> default:
> return bpf_base_func_proto(func_id);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK
2020-07-29 22:52 ` Daniel Borkmann
@ 2020-07-29 23:17 ` sdf
0 siblings, 0 replies; 6+ messages in thread
From: sdf @ 2020-07-29 23:17 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, bpf, davem, ast, Martin KaFai Lau
On 07/30, Daniel Borkmann wrote:
> On 7/29/20 2:31 AM, Stanislav Fomichev wrote:
[..]
> sock_addr_func_proto() also lists the BPF_FUNC_sk_storage_delete. Should
> we add
> that one as well for sock_filter_func_proto()? Presumably create/release
> doesn't
> make sense, but any use case for bind hook?
Right, I didn't think delete makes sense for create/release, but maybe
it does for post_bind :-/
Let me put it on my list, I'll follow up!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-29 23:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-29 0:31 [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK Stanislav Fomichev
2020-07-29 0:31 ` [PATCH bpf-next 2/2] selftests/bpf: verify socket storage in cgroup/sock_{create,release} Stanislav Fomichev
2020-07-29 6:12 ` Song Liu
2020-07-29 6:11 ` [PATCH bpf-next 1/2] bpf: expose socket storage to BPF_PROG_TYPE_CGROUP_SOCK Song Liu
2020-07-29 22:52 ` Daniel Borkmann
2020-07-29 23:17 ` sdf
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.