From e0973a421c6e9d268db2157bcb8756e7ab4b4313 Mon Sep 17 00:00:00 2001 From: Toke Høiland-Jørgensen Date: Mon, 16 Sep 2019 14:33:42 +0200 Subject: libbpf: Remove getsockopt() check for XDP_OPTIONS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The xsk_socket__create() function fails and returns an error if it cannot get the XDP_OPTIONS through getsockopt(). However, support for XDP_OPTIONS was not added until kernel 5.3, so this means that creating XSK sockets always fails on older kernels. Since the option is just used to set the zero-copy flag in the xsk struct, and that flag is not really used for anything yet, just remove the getsockopt() call until a proper use for it is introduced. Suggested-by: Yonghong Song Signed-off-by: Toke Høiland-Jørgensen Acked-by: Andrii Nakryiko Signed-off-by: Daniel Borkmann --- tools/lib/bpf/xsk.c | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'tools') diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c index 842c4fd55859..24fa313524fb 100644 --- a/tools/lib/bpf/xsk.c +++ b/tools/lib/bpf/xsk.c @@ -65,7 +65,6 @@ struct xsk_socket { int xsks_map_fd; __u32 queue_id; char ifname[IFNAMSIZ]; - bool zc; }; struct xsk_nl_info { @@ -491,7 +490,6 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname, void *rx_map = NULL, *tx_map = NULL; struct sockaddr_xdp sxdp = {}; struct xdp_mmap_offsets off; - struct xdp_options opts; struct xsk_socket *xsk; socklen_t optlen; int err; @@ -611,15 +609,6 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname, xsk->prog_fd = -1; - optlen = sizeof(opts); - err = getsockopt(xsk->fd, SOL_XDP, XDP_OPTIONS, &opts, &optlen); - if (err) { - err = -errno; - goto out_mmap_tx; - } - - xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY; - if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) { err = xsk_setup_xdp_prog(xsk); if (err) -- cgit From 8a03222f508bf09e03cf38f6bd77b34b450c1d60 Mon Sep 17 00:00:00 2001 From: Stanislav Fomichev Date: Mon, 23 Sep 2019 11:41:12 -0700 Subject: selftests/bpf: test_progs: fix client/server race in tcp_rtt This is the same problem I found earlier in test_sockopt_inherit: there is a race between server thread doing accept() and client thread doing connect(). Let's explicitly synchronize them via pthread conditional variable. v2: * don't exit from server_thread without signaling condvar, fixes possible issue where main() would wait forever (Andrii Nakryiko) Fixes: b55873984dab ("selftests/bpf: test BPF_SOCK_OPS_RTT_CB") Signed-off-by: Stanislav Fomichev Signed-off-by: Daniel Borkmann --- tools/testing/selftests/bpf/prog_tests/tcp_rtt.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c b/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c index fdc0b3614a9e..a82da555b1b0 100644 --- a/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c +++ b/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c @@ -203,14 +203,24 @@ static int start_server(void) return fd; } +static pthread_mutex_t server_started_mtx = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t server_started = PTHREAD_COND_INITIALIZER; + static void *server_thread(void *arg) { struct sockaddr_storage addr; socklen_t len = sizeof(addr); int fd = *(int *)arg; int client_fd; + int err; + + err = listen(fd, 1); + + pthread_mutex_lock(&server_started_mtx); + pthread_cond_signal(&server_started); + pthread_mutex_unlock(&server_started_mtx); - if (CHECK_FAIL(listen(fd, 1)) < 0) { + if (CHECK_FAIL(err < 0)) { perror("Failed to listed on socket"); return NULL; } @@ -248,7 +258,14 @@ void test_tcp_rtt(void) if (CHECK_FAIL(server_fd < 0)) goto close_cgroup_fd; - pthread_create(&tid, NULL, server_thread, (void *)&server_fd); + if (CHECK_FAIL(pthread_create(&tid, NULL, server_thread, + (void *)&server_fd))) + goto close_cgroup_fd; + + pthread_mutex_lock(&server_started_mtx); + pthread_cond_wait(&server_started, &server_started_mtx); + pthread_mutex_unlock(&server_started_mtx); + CHECK_FAIL(run_test(cgroup_fd, server_fd)); close(server_fd); close_cgroup_fd: -- cgit From aef70a1f44c0b570e6345c02c2d240471859f0a4 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Wed, 25 Sep 2019 11:30:38 -0700 Subject: libbpf: fix false uninitialized variable warning Some compilers emit warning for potential uninitialized next_id usage. The code is correct, but control flow is too complicated for some compilers to figure this out. Re-initialize next_id to satisfy compiler. Signed-off-by: Andrii Nakryiko Signed-off-by: Daniel Borkmann --- tools/lib/bpf/btf_dump.c | 1 + 1 file changed, 1 insertion(+) (limited to 'tools') diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index 715967762312..84b0661db7f3 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -1167,6 +1167,7 @@ static void btf_dump_emit_type_chain(struct btf_dump *d, return; } + next_id = decls->ids[decls->cnt - 1]; next_t = btf__type_by_id(d->btf, next_id); multidim = btf_is_array(next_t); /* we need space if we have named non-pointer */ -- cgit From d778c30a056ac352d1c0c58b5850e0fcc5655a58 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Wed, 25 Sep 2019 11:36:14 -0700 Subject: selftests/bpf: delete unused variables in test_sysctl Remove no longer used variables and avoid compiler warnings. Signed-off-by: Andrii Nakryiko Signed-off-by: Daniel Borkmann --- tools/testing/selftests/bpf/test_sysctl.c | 1 - 1 file changed, 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/bpf/test_sysctl.c b/tools/testing/selftests/bpf/test_sysctl.c index 4f8ec1f10a80..a320e3844b17 100644 --- a/tools/testing/selftests/bpf/test_sysctl.c +++ b/tools/testing/selftests/bpf/test_sysctl.c @@ -1385,7 +1385,6 @@ static int fixup_sysctl_value(const char *buf, size_t buf_len, uint8_t raw[sizeof(uint64_t)]; uint64_t num; } value = {}; - uint8_t c, i; if (buf_len > sizeof(value)) { log_err("Value is too big (%zd) to use in fixup", buf_len); -- cgit From 4670d68b9254710fdeaf794cad54d8b2c9929e0a Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Wed, 25 Sep 2019 11:52:05 -0700 Subject: selftests/bpf: adjust strobemeta loop to satisfy latest clang Some recent changes in latest Clang started causing the following warning when unrolling strobemeta test case main loop: progs/strobemeta.h:416:2: warning: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering [-Wpass-failed=transform-warning] This patch simplifies loop's exit condition to depend only on constant max iteration number (STROBE_MAX_MAP_ENTRIES), while moving early termination logic inside the loop body. The changes are equivalent from program logic standpoint, but fixes the warning. It also appears to improve generated BPF code, as it fixes previously failing non-unrolled strobemeta test cases. Cc: Alexei Starovoitov Signed-off-by: Andrii Nakryiko Signed-off-by: Daniel Borkmann --- tools/testing/selftests/bpf/progs/strobemeta.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/bpf/progs/strobemeta.h b/tools/testing/selftests/bpf/progs/strobemeta.h index 8a399bdfd920..067eb625d01c 100644 --- a/tools/testing/selftests/bpf/progs/strobemeta.h +++ b/tools/testing/selftests/bpf/progs/strobemeta.h @@ -413,7 +413,10 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg, #else #pragma unroll #endif - for (int i = 0; i < STROBE_MAX_MAP_ENTRIES && i < map.cnt; ++i) { + for (int i = 0; i < STROBE_MAX_MAP_ENTRIES; ++i) { + if (i >= map.cnt) + break; + descr->key_lens[i] = 0; len = bpf_probe_read_str(payload, STROBE_MAX_STR_LEN, map.entries[i].key); -- cgit From 39529a9948d8f67f39cb72bec914c1adab38562d Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Wed, 25 Sep 2019 13:37:45 -0700 Subject: libbpf: Teach btf_dumper to emit stand-alone anonymous enum definitions BTF-to-C converter previously skipped anonymous enums in an assumption that those are embedded in struct's field definitions. This is not always the case and a lot of kernel constants are defined as part of anonymous enums. This change fixes the logic by eagerly marking all types as either referenced by any other type or not. This is enough to distinguish two classes of anonymous enums and emit previously omitted enum definitions. Signed-off-by: Andrii Nakryiko Signed-off-by: Daniel Borkmann Link: https://lore.kernel.org/bpf/20190925203745.3173184-1-andriin@fb.com --- tools/lib/bpf/btf_dump.c | 93 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 87 insertions(+), 6 deletions(-) (limited to 'tools') diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index 84b0661db7f3..ede55fec3618 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -48,6 +48,8 @@ struct btf_dump_type_aux_state { __u8 fwd_emitted: 1; /* whether unique non-duplicate name was already assigned */ __u8 name_resolved: 1; + /* whether type is referenced from any other type */ + __u8 referenced: 1; }; struct btf_dump { @@ -173,6 +175,7 @@ void btf_dump__free(struct btf_dump *d) free(d); } +static int btf_dump_mark_referenced(struct btf_dump *d); static int btf_dump_order_type(struct btf_dump *d, __u32 id, bool through_ptr); static void btf_dump_emit_type(struct btf_dump *d, __u32 id, __u32 cont_id); @@ -213,6 +216,11 @@ int btf_dump__dump_type(struct btf_dump *d, __u32 id) /* VOID is special */ d->type_states[0].order_state = ORDERED; d->type_states[0].emit_state = EMITTED; + + /* eagerly determine referenced types for anon enums */ + err = btf_dump_mark_referenced(d); + if (err) + return err; } d->emit_queue_cnt = 0; @@ -226,6 +234,79 @@ int btf_dump__dump_type(struct btf_dump *d, __u32 id) return 0; } +/* + * Mark all types that are referenced from any other type. This is used to + * determine top-level anonymous enums that need to be emitted as an + * independent type declarations. + * Anonymous enums come in two flavors: either embedded in a struct's field + * definition, in which case they have to be declared inline as part of field + * type declaration; or as a top-level anonymous enum, typically used for + * declaring global constants. It's impossible to distinguish between two + * without knowning whether given enum type was referenced from other type: + * top-level anonymous enum won't be referenced by anything, while embedded + * one will. + */ +static int btf_dump_mark_referenced(struct btf_dump *d) +{ + int i, j, n = btf__get_nr_types(d->btf); + const struct btf_type *t; + __u16 vlen; + + for (i = 1; i <= n; i++) { + t = btf__type_by_id(d->btf, i); + vlen = btf_vlen(t); + + switch (btf_kind(t)) { + case BTF_KIND_INT: + case BTF_KIND_ENUM: + case BTF_KIND_FWD: + break; + + case BTF_KIND_VOLATILE: + case BTF_KIND_CONST: + case BTF_KIND_RESTRICT: + case BTF_KIND_PTR: + case BTF_KIND_TYPEDEF: + case BTF_KIND_FUNC: + case BTF_KIND_VAR: + d->type_states[t->type].referenced = 1; + break; + + case BTF_KIND_ARRAY: { + const struct btf_array *a = btf_array(t); + + d->type_states[a->index_type].referenced = 1; + d->type_states[a->type].referenced = 1; + break; + } + case BTF_KIND_STRUCT: + case BTF_KIND_UNION: { + const struct btf_member *m = btf_members(t); + + for (j = 0; j < vlen; j++, m++) + d->type_states[m->type].referenced = 1; + break; + } + case BTF_KIND_FUNC_PROTO: { + const struct btf_param *p = btf_params(t); + + for (j = 0; j < vlen; j++, p++) + d->type_states[p->type].referenced = 1; + break; + } + case BTF_KIND_DATASEC: { + const struct btf_var_secinfo *v = btf_var_secinfos(t); + + for (j = 0; j < vlen; j++, v++) + d->type_states[v->type].referenced = 1; + break; + } + default: + return -EINVAL; + } + } + return 0; +} static int btf_dump_add_emit_queue_id(struct btf_dump *d, __u32 id) { __u32 *new_queue; @@ -395,7 +476,12 @@ static int btf_dump_order_type(struct btf_dump *d, __u32 id, bool through_ptr) } case BTF_KIND_ENUM: case BTF_KIND_FWD: - if (t->name_off != 0) { + /* + * non-anonymous or non-referenced enums are top-level + * declarations and should be emitted. Same logic can be + * applied to FWDs, it won't hurt anyways. + */ + if (t->name_off != 0 || !tstate->referenced) { err = btf_dump_add_emit_queue_id(d, id); if (err) return err; @@ -536,11 +622,6 @@ static void btf_dump_emit_type(struct btf_dump *d, __u32 id, __u32 cont_id) t = btf__type_by_id(d->btf, id); kind = btf_kind(t); - if (top_level_def && t->name_off == 0) { - pr_warning("unexpected nameless definition, id:[%u]\n", id); - return; - } - if (tstate->emit_state == EMITTING) { if (tstate->fwd_emitted) return; -- cgit