| Message ID | 20210812153011.983006-2-sdf@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [bpf-next,v2,1/2] bpf: Allow bpf_get_netns_cookie in BPF_PROG_TYPE_CGROUP_SOCKOPT | expand |
On Thu, Aug 12, 2021 at 08:30:10AM -0700, Stanislav Fomichev wrote: > This is similar to existing BPF_PROG_TYPE_CGROUP_SOCK > and BPF_PROG_TYPE_CGROUP_SOCK_ADDR. > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > --- > kernel/bpf/cgroup.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c > index b567ca46555c..ca5af8852260 100644 > --- a/kernel/bpf/cgroup.c > +++ b/kernel/bpf/cgroup.c > @@ -1846,11 +1846,30 @@ const struct bpf_verifier_ops cg_sysctl_verifier_ops = { > const struct bpf_prog_ops cg_sysctl_prog_ops = { > }; > > +#ifdef CONFIG_NET > +BPF_CALL_1(bpf_get_netns_cookie_sockopt, struct bpf_sockopt_kern *, ctx) > +{ > + struct sock *sk = ctx ? ctx->sk : NULL; > + const struct net *net = sk ? sock_net(sk) : &init_net; A nit. ctx->sk can not be NULL here, so it only depends on ctx is NULL or not. If I read it correctly, would it be less convoluted to directly test ctx and use ctx->sk here, like: const struct net *net = ctx ? sock_net(ctx->sk) : &init_net; and the previous "struct sock *sk = ctx ? ctx->sk : NULL;" statement can also be removed. > + > + return net->net_cookie; > +} > +
On 08/13, Martin KaFai Lau wrote: > On Thu, Aug 12, 2021 at 08:30:10AM -0700, Stanislav Fomichev wrote: > > This is similar to existing BPF_PROG_TYPE_CGROUP_SOCK > > and BPF_PROG_TYPE_CGROUP_SOCK_ADDR. > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > --- > > kernel/bpf/cgroup.c | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c > > index b567ca46555c..ca5af8852260 100644 > > --- a/kernel/bpf/cgroup.c > > +++ b/kernel/bpf/cgroup.c > > @@ -1846,11 +1846,30 @@ const struct bpf_verifier_ops > cg_sysctl_verifier_ops = { > > const struct bpf_prog_ops cg_sysctl_prog_ops = { > > }; > > > > +#ifdef CONFIG_NET > > +BPF_CALL_1(bpf_get_netns_cookie_sockopt, struct bpf_sockopt_kern *, > ctx) > > +{ > > + struct sock *sk = ctx ? ctx->sk : NULL; > > + const struct net *net = sk ? sock_net(sk) : &init_net; > A nit. > ctx->sk can not be NULL here, so it only depends on ctx is NULL or not. > If I read it correctly, would it be less convoluted to directly test ctx > and use ctx->sk here, like: > const struct net *net = ctx ? sock_net(ctx->sk) : &init_net; > and the previous "struct sock *sk = ctx ? ctx->sk : NULL;" statement > can also be removed. Agreed, makes sense. Let me also add bpf_get_netns_cookie to some existing BPF prog to make sure it's executed. That ctx.c isn't really running the prog..
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index b567ca46555c..ca5af8852260 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -1846,11 +1846,30 @@ const struct bpf_verifier_ops cg_sysctl_verifier_ops = { const struct bpf_prog_ops cg_sysctl_prog_ops = { }; +#ifdef CONFIG_NET +BPF_CALL_1(bpf_get_netns_cookie_sockopt, struct bpf_sockopt_kern *, ctx) +{ + struct sock *sk = ctx ? ctx->sk : NULL; + const struct net *net = sk ? sock_net(sk) : &init_net; + + return net->net_cookie; +} + +static const struct bpf_func_proto bpf_get_netns_cookie_sockopt_proto = { + .func = bpf_get_netns_cookie_sockopt, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_CTX_OR_NULL, +}; +#endif + static const struct bpf_func_proto * cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) { switch (func_id) { #ifdef CONFIG_NET + case BPF_FUNC_get_netns_cookie: + return &bpf_get_netns_cookie_sockopt_proto; case BPF_FUNC_sk_storage_get: return &bpf_sk_storage_get_proto; case BPF_FUNC_sk_storage_delete:
This is similar to existing BPF_PROG_TYPE_CGROUP_SOCK and BPF_PROG_TYPE_CGROUP_SOCK_ADDR. Signed-off-by: Stanislav Fomichev <sdf@google.com> --- kernel/bpf/cgroup.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)