| Message ID | 1554814174-28765-1-git-send-email-magnus.karlsson@intel.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] libbpf: fix crash in XDP socket part with new larger BPF_LOG_BUF_SIZE | expand |
On 9 Apr 2019, at 5:49, Magnus Karlsson wrote: > In commit da11b417583e ("libbpf: teach libbpf about log_level bit 2"), > the BPF_LOG_BUF_SIZE was increased to 16M. The XDP socket part of > libbpf allocated the log_buf on the stack, but for the new 16M buffer > size this is not going to work. Change the code to allocate the > log_buf using malloc instead. If the allocation fails, we go ahead and > load the program without a log buffer. > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
On 04/09/2019 02:49 PM, Magnus Karlsson wrote: > In commit da11b417583e ("libbpf: teach libbpf about log_level bit 2"), > the BPF_LOG_BUF_SIZE was increased to 16M. The XDP socket part of > libbpf allocated the log_buf on the stack, but for the new 16M buffer > size this is not going to work. Change the code to allocate the > log_buf using malloc instead. If the allocation fails, we go ahead and > load the program without a log buffer. > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> For these few 17 insn, wouldn't it make more sense to rather shrink the log buffer size? malloc'ing 16M seems like overkill for the tiny program. I don't think there is a specific need to stick with BPF_LOG_BUF_SIZE. Why not simplifying it like the following where we also don't need to cope will allocation failure, for example: diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c index 8d0078b..557ef8d 100644 --- a/tools/lib/bpf/xsk.c +++ b/tools/lib/bpf/xsk.c @@ -259,7 +259,8 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size, static int xsk_load_xdp_prog(struct xsk_socket *xsk) { - char bpf_log_buf[BPF_LOG_BUF_SIZE]; + static const int log_buf_size = 16 * 1024; + char log_buf[log_buf_size]; int err, prog_fd; /* This is the C-program: @@ -308,10 +309,10 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk) size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn); prog_fd = bpf_load_program(BPF_PROG_TYPE_XDP, prog, insns_cnt, - "LGPL-2.1 or BSD-2-Clause", 0, bpf_log_buf, - BPF_LOG_BUF_SIZE); + "LGPL-2.1 or BSD-2-Clause", 0, log_buf, + log_buf_size); if (prog_fd < 0) { - pr_warning("BPF log buffer:\n%s", bpf_log_buf); + pr_warning("BPF log buffer:\n%s", log_buf); return prog_fd; } Thanks, Daniel
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c index 8d0078b..412531b 100644 --- a/tools/lib/bpf/xsk.c +++ b/tools/lib/bpf/xsk.c @@ -259,7 +259,7 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size, static int xsk_load_xdp_prog(struct xsk_socket *xsk) { - char bpf_log_buf[BPF_LOG_BUF_SIZE]; + char *bpf_log_buf; int err, prog_fd; /* This is the C-program: @@ -307,22 +307,27 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk) }; size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn); + bpf_log_buf = malloc(BPF_LOG_BUF_SIZE); prog_fd = bpf_load_program(BPF_PROG_TYPE_XDP, prog, insns_cnt, "LGPL-2.1 or BSD-2-Clause", 0, bpf_log_buf, - BPF_LOG_BUF_SIZE); + bpf_log_buf ? BPF_LOG_BUF_SIZE : 0); if (prog_fd < 0) { - pr_warning("BPF log buffer:\n%s", bpf_log_buf); - return prog_fd; + if (bpf_log_buf) + pr_warning("BPF log buffer:\n%s", bpf_log_buf); + err = prog_fd; + goto out; } err = bpf_set_link_xdp_fd(xsk->ifindex, prog_fd, xsk->config.xdp_flags); if (err) { close(prog_fd); - return err; + goto out; } xsk->prog_fd = prog_fd; - return 0; +out: + free(bpf_log_buf); + return err; } static int xsk_get_max_queues(struct xsk_socket *xsk)
In commit da11b417583e ("libbpf: teach libbpf about log_level bit 2"), the BPF_LOG_BUF_SIZE was increased to 16M. The XDP socket part of libbpf allocated the log_buf on the stack, but for the new 16M buffer size this is not going to work. Change the code to allocate the log_buf using malloc instead. If the allocation fails, we go ahead and load the program without a log buffer. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> --- tools/lib/bpf/xsk.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)