Skip to content

Commit

Permalink
bpf: Try to avoid kzalloc in cgroup/{s,g}etsockopt
Browse files Browse the repository at this point in the history
When we attach a bpf program to cgroup/getsockopt any other getsockopt()
syscall starts incurring kzalloc/kfree cost.

Let add a small buffer on the stack and use it for small (majority)
{s,g}etsockopt values. The buffer is small enough to fit into
the cache line and cover the majority of simple options (most
of them are 4 byte ints).

It seems natural to do the same for setsockopt, but it's a bit more
involved when the BPF program modifies the data (where we have to
kmalloc). The assumption is that for the majority of setsockopt
calls (which are doing pure BPF options or apply policy) this
will bring some benefit as well.

Without this patch (we remove about 1% __kmalloc):
     3.38%     0.07%  tcp_mmap  [kernel.kallsyms]  [k] __cgroup_bpf_run_filter_getsockopt
            |
             --3.30%--__cgroup_bpf_run_filter_getsockopt
                       |
                        --0.81%--__kmalloc

Signed-off-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
Link: https://lore.kernel.org/bpf/20210115163501.805133-3-sdf@google.com
  • Loading branch information
fomichev authored and Alexei Starovoitov committed Jan 20, 2021
1 parent 9cacf81 commit 20f2505
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 7 deletions.
5 changes: 5 additions & 0 deletions include/linux/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -1298,6 +1298,11 @@ struct bpf_sysctl_kern {
u64 tmp_reg;
};

#define BPF_SOCKOPT_KERN_BUF_SIZE 32
struct bpf_sockopt_buf {
u8 data[BPF_SOCKOPT_KERN_BUF_SIZE];
};

struct bpf_sockopt_kern {
struct sock *sk;
u8 *optval;
Expand Down
52 changes: 45 additions & 7 deletions kernel/bpf/cgroup.c
Original file line number Diff line number Diff line change
Expand Up @@ -1298,7 +1298,8 @@ static bool __cgroup_bpf_prog_array_is_empty(struct cgroup *cgrp,
return empty;
}

static int sockopt_alloc_buf(struct bpf_sockopt_kern *ctx, int max_optlen)
static int sockopt_alloc_buf(struct bpf_sockopt_kern *ctx, int max_optlen,
struct bpf_sockopt_buf *buf)
{
if (unlikely(max_optlen < 0))
return -EINVAL;
Expand All @@ -1310,6 +1311,15 @@ static int sockopt_alloc_buf(struct bpf_sockopt_kern *ctx, int max_optlen)
max_optlen = PAGE_SIZE;
}

if (max_optlen <= sizeof(buf->data)) {
/* When the optval fits into BPF_SOCKOPT_KERN_BUF_SIZE
* bytes avoid the cost of kzalloc.
*/
ctx->optval = buf->data;
ctx->optval_end = ctx->optval + max_optlen;
return max_optlen;
}

ctx->optval = kzalloc(max_optlen, GFP_USER);
if (!ctx->optval)
return -ENOMEM;
Expand All @@ -1319,16 +1329,26 @@ static int sockopt_alloc_buf(struct bpf_sockopt_kern *ctx, int max_optlen)
return max_optlen;
}

static void sockopt_free_buf(struct bpf_sockopt_kern *ctx)
static void sockopt_free_buf(struct bpf_sockopt_kern *ctx,
struct bpf_sockopt_buf *buf)
{
if (ctx->optval == buf->data)
return;
kfree(ctx->optval);
}

static bool sockopt_buf_allocated(struct bpf_sockopt_kern *ctx,
struct bpf_sockopt_buf *buf)
{
return ctx->optval != buf->data;
}

int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
int *optname, char __user *optval,
int *optlen, char **kernel_optval)
{
struct cgroup *cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
struct bpf_sockopt_buf buf = {};
struct bpf_sockopt_kern ctx = {
.sk = sk,
.level = *level,
Expand All @@ -1350,7 +1370,7 @@ int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
*/
max_optlen = max_t(int, 16, *optlen);

max_optlen = sockopt_alloc_buf(&ctx, max_optlen);
max_optlen = sockopt_alloc_buf(&ctx, max_optlen, &buf);
if (max_optlen < 0)
return max_optlen;

Expand Down Expand Up @@ -1390,14 +1410,31 @@ int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
*/
if (ctx.optlen != 0) {
*optlen = ctx.optlen;
*kernel_optval = ctx.optval;
/* We've used bpf_sockopt_kern->buf as an intermediary
* storage, but the BPF program indicates that we need
* to pass this data to the kernel setsockopt handler.
* No way to export on-stack buf, have to allocate a
* new buffer.
*/
if (!sockopt_buf_allocated(&ctx, &buf)) {
void *p = kmalloc(ctx.optlen, GFP_USER);

if (!p) {
ret = -ENOMEM;
goto out;
}
memcpy(p, ctx.optval, ctx.optlen);
*kernel_optval = p;
} else {
*kernel_optval = ctx.optval;
}
/* export and don't free sockopt buf */
return 0;
}
}

out:
sockopt_free_buf(&ctx);
sockopt_free_buf(&ctx, &buf);
return ret;
}

Expand All @@ -1407,6 +1444,7 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
int retval)
{
struct cgroup *cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
struct bpf_sockopt_buf buf = {};
struct bpf_sockopt_kern ctx = {
.sk = sk,
.level = level,
Expand All @@ -1425,7 +1463,7 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,

ctx.optlen = max_optlen;

max_optlen = sockopt_alloc_buf(&ctx, max_optlen);
max_optlen = sockopt_alloc_buf(&ctx, max_optlen, &buf);
if (max_optlen < 0)
return max_optlen;

Expand Down Expand Up @@ -1483,7 +1521,7 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
ret = ctx.retval;

out:
sockopt_free_buf(&ctx);
sockopt_free_buf(&ctx, &buf);
return ret;
}

Expand Down

0 comments on commit 20f2505

Please sign in to comment.