From cc0dff6dc3b44e33cd6b935893db66563ef15ba0 Mon Sep 17 00:00:00 2001 From: Jiong Wang Date: Tue, 26 Jun 2018 19:48:52 -0700 Subject: nfp: bpf: allow source ptr type be map ptr in memcpy optimization Map read has been supported on NFP, this patch enables optimization for memcpy from map to packet. This patch also fixed one latent bug which will cause copying from unexpected address once memcpy for map pointer enabled. The fixed code path was not exercised before. Reported-by: Mary Pham Reported-by: David Beckett Signed-off-by: Jiong Wang Reviewed-by: Jakub Kicinski Acked-by: Song Liu Signed-off-by: Daniel Borkmann --- drivers/net/ethernet/netronome/nfp/bpf/jit.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c index 8a92088df0d7..33111739b210 100644 --- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c +++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c @@ -670,7 +670,7 @@ static int nfp_cpp_memcpy(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta) xfer_num = round_up(len, 4) / 4; if (src_40bit_addr) - addr40_offset(nfp_prog, meta->insn.src_reg, off, &src_base, + addr40_offset(nfp_prog, meta->insn.src_reg * 2, off, &src_base, &off); /* Setup PREV_ALU fields to override memory read length. */ @@ -3299,7 +3299,8 @@ curr_pair_is_memcpy(struct nfp_insn_meta *ld_meta, if (!is_mbpf_load(ld_meta) || !is_mbpf_store(st_meta)) return false; - if (ld_meta->ptr.type != PTR_TO_PACKET) + if (ld_meta->ptr.type != PTR_TO_PACKET && + ld_meta->ptr.type != PTR_TO_MAP_VALUE) return false; if (st_meta->ptr.type != PTR_TO_PACKET) -- cgit v1.2.3 From 22adedd304668f0a2001284ddf225b49e9c05c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= Date: Mon, 25 Jun 2018 14:25:02 +0200 Subject: trace_helpers.c: Add helpers to poll multiple perf FDs for events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two new helper functions to trace_helpers that supports polling multiple perf file descriptors for events. These are used to the XDP perf_event_output example, which needs to work with one perf fd per CPU. Reviewed-by: Jakub Kicinski Signed-off-by: Toke Høiland-Jørgensen Acked-by: Song Liu Signed-off-by: Daniel Borkmann --- tools/testing/selftests/bpf/trace_helpers.c | 48 +++++++++++++++++++++++++++-- tools/testing/selftests/bpf/trace_helpers.h | 4 +++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c index 3868dcb63420..cabe2a3a3b30 100644 --- a/tools/testing/selftests/bpf/trace_helpers.c +++ b/tools/testing/selftests/bpf/trace_helpers.c @@ -88,7 +88,7 @@ static int page_size; static int page_cnt = 8; static struct perf_event_mmap_page *header; -int perf_event_mmap(int fd) +int perf_event_mmap_header(int fd, struct perf_event_mmap_page **header) { void *base; int mmap_size; @@ -102,10 +102,15 @@ int perf_event_mmap(int fd) return -1; } - header = base; + *header = base; return 0; } +int perf_event_mmap(int fd) +{ + return perf_event_mmap_header(fd, &header); +} + static int perf_event_poll(int fd) { struct pollfd pfd = { .fd = fd, .events = POLLIN }; @@ -163,3 +168,42 @@ int perf_event_poller(int fd, perf_event_print_fn output_fn) return ret; } + +int perf_event_poller_multi(int *fds, struct perf_event_mmap_page **headers, + int num_fds, perf_event_print_fn output_fn) +{ + enum bpf_perf_event_ret ret; + struct pollfd *pfds; + void *buf = NULL; + size_t len = 0; + int i; + + pfds = calloc(num_fds, sizeof(*pfds)); + if (!pfds) + return LIBBPF_PERF_EVENT_ERROR; + + for (i = 0; i < num_fds; i++) { + pfds[i].fd = fds[i]; + pfds[i].events = POLLIN; + } + + for (;;) { + poll(pfds, num_fds, 1000); + for (i = 0; i < num_fds; i++) { + if (!pfds[i].revents) + continue; + + ret = bpf_perf_event_read_simple(headers[i], + page_cnt * page_size, + page_size, &buf, &len, + bpf_perf_event_print, + output_fn); + if (ret != LIBBPF_PERF_EVENT_CONT) + break; + } + } + free(buf); + free(pfds); + + return ret; +} diff --git a/tools/testing/selftests/bpf/trace_helpers.h b/tools/testing/selftests/bpf/trace_helpers.h index 3b4bcf7f5084..18924f23db1b 100644 --- a/tools/testing/selftests/bpf/trace_helpers.h +++ b/tools/testing/selftests/bpf/trace_helpers.h @@ -3,6 +3,7 @@ #define __TRACE_HELPER_H #include +#include struct ksym { long addr; @@ -16,6 +17,9 @@ long ksym_get_addr(const char *name); typedef enum bpf_perf_event_ret (*perf_event_print_fn)(void *data, int size); int perf_event_mmap(int fd); +int perf_event_mmap_header(int fd, struct perf_event_mmap_page **header); /* return LIBBPF_PERF_EVENT_DONE or LIBBPF_PERF_EVENT_ERROR */ int perf_event_poller(int fd, perf_event_print_fn output_fn); +int perf_event_poller_multi(int *fds, struct perf_event_mmap_page **headers, + int num_fds, perf_event_print_fn output_fn); #endif -- cgit v1.2.3 From 1e54ad251a93a524f1a2950b1d65bc7437c57a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= Date: Mon, 25 Jun 2018 14:25:02 +0200 Subject: samples/bpf: Add xdp_sample_pkts example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an example program showing how to sample packets from XDP using the perf event buffer. The example userspace program just prints the ethernet header for every packet sampled. Reviewed-by: Jakub Kicinski Signed-off-by: Toke Høiland-Jørgensen Acked-by: Song Liu Signed-off-by: Daniel Borkmann --- samples/bpf/Makefile | 4 + samples/bpf/xdp_sample_pkts_kern.c | 66 +++++++++++++++ samples/bpf/xdp_sample_pkts_user.c | 169 +++++++++++++++++++++++++++++++++++++ 3 files changed, 239 insertions(+) create mode 100644 samples/bpf/xdp_sample_pkts_kern.c create mode 100644 samples/bpf/xdp_sample_pkts_user.c diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 1303af10e54d..9ea2f7b64869 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -52,6 +52,7 @@ hostprogs-y += xdp_adjust_tail hostprogs-y += xdpsock hostprogs-y += xdp_fwd hostprogs-y += task_fd_query +hostprogs-y += xdp_sample_pkts # Libbpf dependencies LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a @@ -107,6 +108,7 @@ xdp_adjust_tail-objs := xdp_adjust_tail_user.o xdpsock-objs := bpf_load.o xdpsock_user.o xdp_fwd-objs := bpf_load.o xdp_fwd_user.o task_fd_query-objs := bpf_load.o task_fd_query_user.o $(TRACE_HELPERS) +xdp_sample_pkts-objs := xdp_sample_pkts_user.o $(TRACE_HELPERS) # Tell kbuild to always build the programs always := $(hostprogs-y) @@ -163,6 +165,7 @@ always += xdp_adjust_tail_kern.o always += xdpsock_kern.o always += xdp_fwd_kern.o always += task_fd_query_kern.o +always += xdp_sample_pkts_kern.o HOSTCFLAGS += -I$(objtree)/usr/include HOSTCFLAGS += -I$(srctree)/tools/lib/ @@ -179,6 +182,7 @@ HOSTCFLAGS_spintest_user.o += -I$(srctree)/tools/lib/bpf/ HOSTCFLAGS_trace_event_user.o += -I$(srctree)/tools/lib/bpf/ HOSTCFLAGS_sampleip_user.o += -I$(srctree)/tools/lib/bpf/ HOSTCFLAGS_task_fd_query_user.o += -I$(srctree)/tools/lib/bpf/ +HOSTCFLAGS_xdp_sample_pkts_user.o += -I$(srctree)/tools/lib/bpf/ HOST_LOADLIBES += $(LIBBPF) -lelf HOSTLOADLIBES_tracex4 += -lrt diff --git a/samples/bpf/xdp_sample_pkts_kern.c b/samples/bpf/xdp_sample_pkts_kern.c new file mode 100644 index 000000000000..f7ca8b850978 --- /dev/null +++ b/samples/bpf/xdp_sample_pkts_kern.c @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: GPL-2.0 +#include +#include +#include +#include "bpf_helpers.h" + +#define SAMPLE_SIZE 64ul +#define MAX_CPUS 128 + +#define bpf_printk(fmt, ...) \ +({ \ + char ____fmt[] = fmt; \ + bpf_trace_printk(____fmt, sizeof(____fmt), \ + ##__VA_ARGS__); \ +}) + +struct bpf_map_def SEC("maps") my_map = { + .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY, + .key_size = sizeof(int), + .value_size = sizeof(u32), + .max_entries = MAX_CPUS, +}; + +SEC("xdp_sample") +int xdp_sample_prog(struct xdp_md *ctx) +{ + void *data_end = (void *)(long)ctx->data_end; + void *data = (void *)(long)ctx->data; + + /* Metadata will be in the perf event before the packet data. */ + struct S { + u16 cookie; + u16 pkt_len; + } __packed metadata; + + if (data < data_end) { + /* The XDP perf_event_output handler will use the upper 32 bits + * of the flags argument as a number of bytes to include of the + * packet payload in the event data. If the size is too big, the + * call to bpf_perf_event_output will fail and return -EFAULT. + * + * See bpf_xdp_event_output in net/core/filter.c. + * + * The BPF_F_CURRENT_CPU flag means that the event output fd + * will be indexed by the CPU number in the event map. + */ + u64 flags = BPF_F_CURRENT_CPU; + u16 sample_size; + int ret; + + metadata.cookie = 0xdead; + metadata.pkt_len = (u16)(data_end - data); + sample_size = min(metadata.pkt_len, SAMPLE_SIZE); + flags |= (u64)sample_size << 32; + + ret = bpf_perf_event_output(ctx, &my_map, flags, + &metadata, sizeof(metadata)); + if (ret) + bpf_printk("perf_event_output failed: %d\n", ret); + } + + return XDP_PASS; +} + +char _license[] SEC("license") = "GPL"; +u32 _version SEC("version") = LINUX_VERSION_CODE; diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c new file mode 100644 index 000000000000..8dd87c1eb560 --- /dev/null +++ b/samples/bpf/xdp_sample_pkts_user.c @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: GPL-2.0 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "perf-sys.h" +#include "trace_helpers.h" + +#define MAX_CPUS 128 +static int pmu_fds[MAX_CPUS], if_idx; +static struct perf_event_mmap_page *headers[MAX_CPUS]; +static char *if_name; + +static int do_attach(int idx, int fd, const char *name) +{ + int err; + + err = bpf_set_link_xdp_fd(idx, fd, 0); + if (err < 0) + printf("ERROR: failed to attach program to %s\n", name); + + return err; +} + +static int do_detach(int idx, const char *name) +{ + int err; + + err = bpf_set_link_xdp_fd(idx, -1, 0); + if (err < 0) + printf("ERROR: failed to detach program from %s\n", name); + + return err; +} + +#define SAMPLE_SIZE 64 + +static int print_bpf_output(void *data, int size) +{ + struct { + __u16 cookie; + __u16 pkt_len; + __u8 pkt_data[SAMPLE_SIZE]; + } __packed *e = data; + int i; + + if (e->cookie != 0xdead) { + printf("BUG cookie %x sized %d\n", + e->cookie, size); + return LIBBPF_PERF_EVENT_ERROR; + } + + printf("Pkt len: %-5d bytes. Ethernet hdr: ", e->pkt_len); + for (i = 0; i < 14 && i < e->pkt_len; i++) + printf("%02x ", e->pkt_data[i]); + printf("\n"); + + return LIBBPF_PERF_EVENT_CONT; +} + +static void test_bpf_perf_event(int map_fd, int num) +{ + struct perf_event_attr attr = { + .sample_type = PERF_SAMPLE_RAW, + .type = PERF_TYPE_SOFTWARE, + .config = PERF_COUNT_SW_BPF_OUTPUT, + .wakeup_events = 1, /* get an fd notification for every event */ + }; + int i; + + for (i = 0; i < num; i++) { + int key = i; + + pmu_fds[i] = sys_perf_event_open(&attr, -1/*pid*/, i/*cpu*/, + -1/*group_fd*/, 0); + + assert(pmu_fds[i] >= 0); + assert(bpf_map_update_elem(map_fd, &key, + &pmu_fds[i], BPF_ANY) == 0); + ioctl(pmu_fds[i], PERF_EVENT_IOC_ENABLE, 0); + } +} + +static void sig_handler(int signo) +{ + do_detach(if_idx, if_name); + exit(0); +} + +int main(int argc, char **argv) +{ + struct bpf_prog_load_attr prog_load_attr = { + .prog_type = BPF_PROG_TYPE_XDP, + }; + struct bpf_object *obj; + struct bpf_map *map; + int prog_fd, map_fd; + char filename[256]; + int ret, err, i; + int numcpus; + + if (argc < 2) { + printf("Usage: %s \n", argv[0]); + return 1; + } + + numcpus = get_nprocs(); + if (numcpus > MAX_CPUS) + numcpus = MAX_CPUS; + + snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]); + prog_load_attr.file = filename; + + if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd)) + return 1; + + if (!prog_fd) { + printf("load_bpf_file: %s\n", strerror(errno)); + return 1; + } + + map = bpf_map__next(NULL, obj); + if (!map) { + printf("finding a map in obj file failed\n"); + return 1; + } + map_fd = bpf_map__fd(map); + + if_idx = if_nametoindex(argv[1]); + if (!if_idx) + if_idx = strtoul(argv[1], NULL, 0); + + if (!if_idx) { + fprintf(stderr, "Invalid ifname\n"); + return 1; + } + if_name = argv[1]; + err = do_attach(if_idx, prog_fd, argv[1]); + if (err) + return err; + + if (signal(SIGINT, sig_handler) || + signal(SIGHUP, sig_handler) || + signal(SIGTERM, sig_handler)) { + perror("signal"); + return 1; + } + + test_bpf_perf_event(map_fd, numcpus); + + for (i = 0; i < numcpus; i++) + if (perf_event_mmap_header(pmu_fds[i], &headers[i]) < 0) + return 1; + + ret = perf_event_poller_multi(pmu_fds, headers, numcpus, + print_bpf_output); + kill(0, SIGINT); + return ret; +} -- cgit v1.2.3 From a7f7547f5e2b90554b0f3d1604899a4f26dba92d Mon Sep 17 00:00:00 2001 From: Andrey Ignatov Date: Tue, 26 Jun 2018 14:22:41 -0700 Subject: selftests/bpf: Test sys_connect BPF hooks with TFO TCP Fast Open is triggered by sys_sendmsg with MSG_FASTOPEN flag for SOCK_STREAM socket. Even though it's sys_sendmsg, it eventually calls __inet_stream_connect the same way sys_connect does for TCP. __inet_stream_connect, in turn, already has BPF hooks for sys_connect. That means TFO is already covered by BPF_CGROUP_INET{4,6}_CONNECT and the only missing piece is selftest. The patch adds selftest for TFO. Signed-off-by: Andrey Ignatov Signed-off-by: Daniel Borkmann --- tools/testing/selftests/bpf/test_sock_addr.c | 37 +++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/bpf/test_sock_addr.c b/tools/testing/selftests/bpf/test_sock_addr.c index a5e76b9219b9..2e45c92d1111 100644 --- a/tools/testing/selftests/bpf/test_sock_addr.c +++ b/tools/testing/selftests/bpf/test_sock_addr.c @@ -998,8 +998,9 @@ int init_pktinfo(int domain, struct cmsghdr *cmsg) return 0; } -static int sendmsg_to_server(const struct sockaddr_storage *addr, - socklen_t addr_len, int set_cmsg, int *syscall_err) +static int sendmsg_to_server(int type, const struct sockaddr_storage *addr, + socklen_t addr_len, int set_cmsg, int flags, + int *syscall_err) { union { char buf[CMSG_SPACE(sizeof(struct in6_pktinfo))]; @@ -1022,7 +1023,7 @@ static int sendmsg_to_server(const struct sockaddr_storage *addr, goto err; } - fd = socket(domain, SOCK_DGRAM, 0); + fd = socket(domain, type, 0); if (fd == -1) { log_err("Failed to create client socket"); goto err; @@ -1052,7 +1053,7 @@ static int sendmsg_to_server(const struct sockaddr_storage *addr, } } - if (sendmsg(fd, &hdr, 0) != sizeof(data)) { + if (sendmsg(fd, &hdr, flags) != sizeof(data)) { log_err("Fail to send message to server"); *syscall_err = errno; goto err; @@ -1066,6 +1067,15 @@ out: return fd; } +static int fastconnect_to_server(const struct sockaddr_storage *addr, + socklen_t addr_len) +{ + int sendmsg_err; + + return sendmsg_to_server(SOCK_STREAM, addr, addr_len, /*set_cmsg*/0, + MSG_FASTOPEN, &sendmsg_err); +} + static int recvmsg_from_client(int sockfd, struct sockaddr_storage *src_addr) { struct timeval tv; @@ -1185,6 +1195,20 @@ static int run_connect_test_case(const struct sock_addr_test *test) if (cmp_local_ip(clientfd, &expected_src_addr)) goto err; + if (test->type == SOCK_STREAM) { + /* Test TCP Fast Open scenario */ + clientfd = fastconnect_to_server(&requested_addr, addr_len); + if (clientfd == -1) + goto err; + + /* Make sure src and dst addrs were overridden properly */ + if (cmp_peer_addr(clientfd, &expected_addr)) + goto err; + + if (cmp_local_ip(clientfd, &expected_src_addr)) + goto err; + } + goto out; err: err = -1; @@ -1222,8 +1246,9 @@ static int run_sendmsg_test_case(const struct sock_addr_test *test) if (clientfd >= 0) close(clientfd); - clientfd = sendmsg_to_server(&requested_addr, addr_len, - set_cmsg, &err); + clientfd = sendmsg_to_server(test->type, &requested_addr, + addr_len, set_cmsg, /*flags*/0, + &err); if (err) goto out; else if (clientfd == -1) -- cgit v1.2.3 From 0d25c43ab988766ad52ff2930af3bf47d92c20ac Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Mon, 25 Jun 2018 16:27:43 +0200 Subject: samples/bpf: extend xdp_rxq_info to read packet payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is a cost associated with reading the packet data payload that this test ignored. Add option --read to allow enabling reading part of the payload. This sample/tool helps us analyse an issue observed with a NIC mlx5 (ConnectX-5 Ex) and an Intel(R) Xeon(R) CPU E5-1650 v4. With no_touch of data: Running XDP on dev:mlx5p1 (ifindex:8) action:XDP_DROP options:no_touch XDP stats CPU pps issue-pps XDP-RX CPU 0 14,465,157 0 XDP-RX CPU 1 14,464,728 0 XDP-RX CPU 2 14,465,283 0 XDP-RX CPU 3 14,465,282 0 XDP-RX CPU 4 14,464,159 0 XDP-RX CPU 5 14,465,379 0 XDP-RX CPU total 86,789,992 When not touching data, we observe that the CPUs have idle cycles. When reading data the CPUs are 100% busy in softirq. With reading data: Running XDP on dev:mlx5p1 (ifindex:8) action:XDP_DROP options:read XDP stats CPU pps issue-pps XDP-RX CPU 0 9,620,639 0 XDP-RX CPU 1 9,489,843 0 XDP-RX CPU 2 9,407,854 0 XDP-RX CPU 3 9,422,289 0 XDP-RX CPU 4 9,321,959 0 XDP-RX CPU 5 9,395,242 0 XDP-RX CPU total 56,657,828 The effect seen above is a result of cache-misses occuring when more RXQs are being used. Based on perf-event observations, our conclusion is that the CPUs DDIO (Direct Data I/O) choose to deliver packet into main memory, instead of L3-cache. We also found, that this can be mitigated by either using less RXQs or by reducing NICs the RX-ring size. Signed-off-by: Jesper Dangaard Brouer Signed-off-by: Toke Høiland-Jørgensen Acked-by: Song Liu Signed-off-by: Daniel Borkmann --- samples/bpf/xdp_rxq_info_kern.c | 19 +++++++++++++++++++ samples/bpf/xdp_rxq_info_user.c | 34 ++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c index 3fd209291653..61af6210df2f 100644 --- a/samples/bpf/xdp_rxq_info_kern.c +++ b/samples/bpf/xdp_rxq_info_kern.c @@ -4,6 +4,8 @@ * Example howto extract XDP RX-queue info */ #include +#include +#include #include "bpf_helpers.h" /* Config setup from with userspace @@ -14,6 +16,11 @@ struct config { __u32 action; int ifindex; + __u32 options; +}; +enum cfg_options_flags { + NO_TOUCH = 0x0U, + READ_MEM = 0x1U, }; struct bpf_map_def SEC("maps") config_map = { .type = BPF_MAP_TYPE_ARRAY, @@ -90,6 +97,18 @@ int xdp_prognum0(struct xdp_md *ctx) if (key == MAX_RXQs) rxq_rec->issue++; + /* Default: Don't touch packet data, only count packets */ + if (unlikely(config->options & READ_MEM)) { + struct ethhdr *eth = data; + + if (eth + 1 > data_end) + return XDP_ABORTED; + + /* Avoid compiler removing this: Drop non 802.3 Ethertypes */ + if (ntohs(eth->h_proto) < ETH_P_802_3_MIN) + return XDP_ABORTED; + } + return config->action; } diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c index e4e9ba52bff0..435485d4f49e 100644 --- a/samples/bpf/xdp_rxq_info_user.c +++ b/samples/bpf/xdp_rxq_info_user.c @@ -50,6 +50,7 @@ static const struct option long_options[] = { {"sec", required_argument, NULL, 's' }, {"no-separators", no_argument, NULL, 'z' }, {"action", required_argument, NULL, 'a' }, + {"readmem", no_argument, NULL, 'r' }, {0, 0, NULL, 0 } }; @@ -66,6 +67,11 @@ static void int_exit(int sig) struct config { __u32 action; int ifindex; + __u32 options; +}; +enum cfg_options_flags { + NO_TOUCH = 0x0U, + READ_MEM = 0x1U, }; #define XDP_ACTION_MAX (XDP_TX + 1) #define XDP_ACTION_MAX_STRLEN 11 @@ -109,6 +115,16 @@ static void list_xdp_actions(void) printf("\n"); } +static char* options2str(enum cfg_options_flags flag) +{ + if (flag == NO_TOUCH) + return "no_touch"; + if (flag & READ_MEM) + return "read"; + fprintf(stderr, "ERR: Unknown config option flags"); + exit(EXIT_FAIL); +} + static void usage(char *argv[]) { int i; @@ -305,7 +321,7 @@ static __u64 calc_errs_pps(struct datarec *r, static void stats_print(struct stats_record *stats_rec, struct stats_record *stats_prev, - int action) + int action, __u32 cfg_opt) { unsigned int nr_rxqs = bpf_map__def(rx_queue_index_map)->max_entries; unsigned int nr_cpus = bpf_num_possible_cpus(); @@ -316,8 +332,8 @@ static void stats_print(struct stats_record *stats_rec, int i; /* Header */ - printf("\nRunning XDP on dev:%s (ifindex:%d) action:%s\n", - ifname, ifindex, action2str(action)); + printf("\nRunning XDP on dev:%s (ifindex:%d) action:%s options:%s\n", + ifname, ifindex, action2str(action), options2str(cfg_opt)); /* stats_global_map */ { @@ -399,7 +415,7 @@ static inline void swap(struct stats_record **a, struct stats_record **b) *b = tmp; } -static void stats_poll(int interval, int action) +static void stats_poll(int interval, int action, __u32 cfg_opt) { struct stats_record *record, *prev; @@ -410,7 +426,7 @@ static void stats_poll(int interval, int action) while (1) { swap(&prev, &record); stats_collect(record); - stats_print(record, prev, action); + stats_print(record, prev, action, cfg_opt); sleep(interval); } @@ -421,6 +437,7 @@ static void stats_poll(int interval, int action) int main(int argc, char **argv) { + __u32 cfg_options= NO_TOUCH ; /* Default: Don't touch packet memory */ struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY}; struct bpf_prog_load_attr prog_load_attr = { .prog_type = BPF_PROG_TYPE_XDP, @@ -435,6 +452,7 @@ int main(int argc, char **argv) int interval = 2; __u32 key = 0; + char action_str_buf[XDP_ACTION_MAX_STRLEN + 1 /* for \0 */] = { 0 }; int action = XDP_PASS; /* Default action */ char *action_str = NULL; @@ -496,6 +514,9 @@ int main(int argc, char **argv) action_str = (char *)&action_str_buf; strncpy(action_str, optarg, XDP_ACTION_MAX_STRLEN); break; + case 'r': + cfg_options |= READ_MEM; + break; case 'h': error: default: @@ -522,6 +543,7 @@ int main(int argc, char **argv) } } cfg.action = action; + cfg.options = cfg_options; /* Trick to pretty printf with thousands separators use %' */ if (use_separators) @@ -542,6 +564,6 @@ int main(int argc, char **argv) return EXIT_FAIL_XDP; } - stats_poll(interval, action); + stats_poll(interval, action, cfg_options); return EXIT_OK; } -- cgit v1.2.3 From 509fda105ba8f9a1a5c6f8b79e4c7fc50b35c1e3 Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Mon, 25 Jun 2018 16:27:48 +0200 Subject: samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XDP_TX requires also changing the MAC-addrs, else some hardware may drop the TX packet before reaching the wire. This was observed with driver mlx5. If xdp_rxq_info select --action XDP_TX the swapmac functionality is activated. It is also possible to manually enable via cmdline option --swapmac. This is practical if wanting to measure the overhead of writing/updating payload for other action types. Signed-off-by: Jesper Dangaard Brouer Signed-off-by: Toke Høiland-Jørgensen Acked-by: Song Liu Signed-off-by: Daniel Borkmann --- samples/bpf/xdp_rxq_info_kern.c | 26 +++++++++++++++++++++++++- samples/bpf/xdp_rxq_info_user.c | 11 +++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c index 61af6210df2f..222a83eed1cb 100644 --- a/samples/bpf/xdp_rxq_info_kern.c +++ b/samples/bpf/xdp_rxq_info_kern.c @@ -21,6 +21,7 @@ struct config { enum cfg_options_flags { NO_TOUCH = 0x0U, READ_MEM = 0x1U, + SWAP_MAC = 0x2U, }; struct bpf_map_def SEC("maps") config_map = { .type = BPF_MAP_TYPE_ARRAY, @@ -52,6 +53,23 @@ struct bpf_map_def SEC("maps") rx_queue_index_map = { .max_entries = MAX_RXQs + 1, }; +static __always_inline +void swap_src_dst_mac(void *data) +{ + unsigned short *p = data; + unsigned short dst[3]; + + dst[0] = p[0]; + dst[1] = p[1]; + dst[2] = p[2]; + p[0] = p[3]; + p[1] = p[4]; + p[2] = p[5]; + p[3] = dst[0]; + p[4] = dst[1]; + p[5] = dst[2]; +} + SEC("xdp_prog0") int xdp_prognum0(struct xdp_md *ctx) { @@ -98,7 +116,7 @@ int xdp_prognum0(struct xdp_md *ctx) rxq_rec->issue++; /* Default: Don't touch packet data, only count packets */ - if (unlikely(config->options & READ_MEM)) { + if (unlikely(config->options & (READ_MEM|SWAP_MAC))) { struct ethhdr *eth = data; if (eth + 1 > data_end) @@ -107,6 +125,12 @@ int xdp_prognum0(struct xdp_md *ctx) /* Avoid compiler removing this: Drop non 802.3 Ethertypes */ if (ntohs(eth->h_proto) < ETH_P_802_3_MIN) return XDP_ABORTED; + + /* XDP_TX requires changing MAC-addrs, else HW may drop. + * Can also be enabled with --swapmac (for test purposes) + */ + if (unlikely(config->options & SWAP_MAC)) + swap_src_dst_mac(data); } return config->action; diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c index 435485d4f49e..248a7eab9531 100644 --- a/samples/bpf/xdp_rxq_info_user.c +++ b/samples/bpf/xdp_rxq_info_user.c @@ -51,6 +51,7 @@ static const struct option long_options[] = { {"no-separators", no_argument, NULL, 'z' }, {"action", required_argument, NULL, 'a' }, {"readmem", no_argument, NULL, 'r' }, + {"swapmac", no_argument, NULL, 'm' }, {0, 0, NULL, 0 } }; @@ -72,6 +73,7 @@ struct config { enum cfg_options_flags { NO_TOUCH = 0x0U, READ_MEM = 0x1U, + SWAP_MAC = 0x2U, }; #define XDP_ACTION_MAX (XDP_TX + 1) #define XDP_ACTION_MAX_STRLEN 11 @@ -119,6 +121,8 @@ static char* options2str(enum cfg_options_flags flag) { if (flag == NO_TOUCH) return "no_touch"; + if (flag & SWAP_MAC) + return "swapmac"; if (flag & READ_MEM) return "read"; fprintf(stderr, "ERR: Unknown config option flags"); @@ -517,6 +521,9 @@ int main(int argc, char **argv) case 'r': cfg_options |= READ_MEM; break; + case 'm': + cfg_options |= SWAP_MAC; + break; case 'h': error: default: @@ -543,6 +550,10 @@ int main(int argc, char **argv) } } cfg.action = action; + + /* XDP_TX requires changing MAC-addrs, else HW may drop */ + if (action == XDP_TX) + cfg_options |= SWAP_MAC; cfg.options = cfg_options; /* Trick to pretty printf with thousands separators use %' */ -- cgit v1.2.3 From c256429fbd01d6993a2e19ff24f252514a0b84a5 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 28 Jun 2018 14:41:35 -0700 Subject: tools: bpftool: use correct make variable type to improve compilation time Commit 4bfe3bd3cc35 ("tools/bpftool: use version from the kernel source tree") added version to bpftool. The version used is equal to the kernel version and obtained by running make kernelversion against kernel source tree. Version is then communicated to the sources with a command line define set in CFLAGS. Use a simply expanded variable for the version, otherwise the recursive make will run every time CFLAGS are used. This brings the single-job compilation time for me from almost 16 sec down to less than 4 sec. Signed-off-by: Jakub Kicinski Reviewed-by: Quentin Monnet Signed-off-by: Daniel Borkmann --- tools/bpf/bpftool/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile index 892dbf095bff..0911b00b25cc 100644 --- a/tools/bpf/bpftool/Makefile +++ b/tools/bpf/bpftool/Makefile @@ -23,7 +23,7 @@ endif LIBBPF = $(BPF_PATH)libbpf.a -BPFTOOL_VERSION=$(shell make --no-print-directory -sC ../../.. kernelversion) +BPFTOOL_VERSION := $(shell make --no-print-directory -sC ../../.. kernelversion) $(LIBBPF): FORCE $(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) $(OUTPUT)libbpf.a FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) -- cgit v1.2.3 From d9b683d7464ec9e8298d82c00b9033c96e27a187 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 28 Jun 2018 14:41:36 -0700 Subject: tools: libbpf: add section names for missing program types Specify default section names for BPF_PROG_TYPE_LIRC_MODE2 and BPF_PROG_TYPE_LWT_SEG6LOCAL, these are the only two missing right now. Signed-off-by: Jakub Kicinski Reviewed-by: Quentin Monnet Signed-off-by: Daniel Borkmann --- tools/lib/bpf/libbpf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index a1e96b5de5ff..a1491e95edd0 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -2037,9 +2037,11 @@ static const struct { BPF_PROG_SEC("lwt_in", BPF_PROG_TYPE_LWT_IN), BPF_PROG_SEC("lwt_out", BPF_PROG_TYPE_LWT_OUT), BPF_PROG_SEC("lwt_xmit", BPF_PROG_TYPE_LWT_XMIT), + BPF_PROG_SEC("lwt_seg6local", BPF_PROG_TYPE_LWT_SEG6LOCAL), BPF_PROG_SEC("sockops", BPF_PROG_TYPE_SOCK_OPS), BPF_PROG_SEC("sk_skb", BPF_PROG_TYPE_SK_SKB), BPF_PROG_SEC("sk_msg", BPF_PROG_TYPE_SK_MSG), + BPF_PROG_SEC("lirc_mode2", BPF_PROG_TYPE_LIRC_MODE2), BPF_SA_PROG_SEC("cgroup/bind4", BPF_CGROUP_INET4_BIND), BPF_SA_PROG_SEC("cgroup/bind6", BPF_CGROUP_INET6_BIND), BPF_SA_PROG_SEC("cgroup/connect4", BPF_CGROUP_INET4_CONNECT), -- cgit v1.2.3 From 9aba36139a5f9ee1d11a60d0a3a90944b8d56385 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 28 Jun 2018 14:41:37 -0700 Subject: tools: libbpf: allow setting ifindex for programs and maps Users of bpf_object__open()/bpf_object__load() APIs may want to load the programs and maps onto a device for offload. Allow setting ifindex on those sub-objects. Signed-off-by: Jakub Kicinski Reviewed-by: Quentin Monnet Signed-off-by: Daniel Borkmann --- tools/lib/bpf/libbpf.c | 10 ++++++++++ tools/lib/bpf/libbpf.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index a1491e95edd0..7bc02d93e277 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -1896,6 +1896,11 @@ void *bpf_program__priv(struct bpf_program *prog) return prog ? prog->priv : ERR_PTR(-EINVAL); } +void bpf_program__set_ifindex(struct bpf_program *prog, __u32 ifindex) +{ + prog->prog_ifindex = ifindex; +} + const char *bpf_program__title(struct bpf_program *prog, bool needs_copy) { const char *title; @@ -2122,6 +2127,11 @@ void *bpf_map__priv(struct bpf_map *map) return map ? map->priv : ERR_PTR(-EINVAL); } +void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex) +{ + map->map_ifindex = ifindex; +} + struct bpf_map * bpf_map__next(struct bpf_map *prev, struct bpf_object *obj) { diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index 09976531aa74..564f4be9bae0 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -109,6 +109,7 @@ int bpf_program__set_priv(struct bpf_program *prog, void *priv, bpf_program_clear_priv_t clear_priv); void *bpf_program__priv(struct bpf_program *prog); +void bpf_program__set_ifindex(struct bpf_program *prog, __u32 ifindex); const char *bpf_program__title(struct bpf_program *prog, bool needs_copy); @@ -251,6 +252,7 @@ typedef void (*bpf_map_clear_priv_t)(struct bpf_map *, void *); int bpf_map__set_priv(struct bpf_map *map, void *priv, bpf_map_clear_priv_t clear_priv); void *bpf_map__priv(struct bpf_map *map); +void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex); int bpf_map__pin(struct bpf_map *map, const char *path); long libbpf_get_error(const void *ptr); -- cgit v1.2.3 From 9a94f277c4fb84815f450418d37bfc568d57b5cc Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 28 Jun 2018 14:41:38 -0700 Subject: tools: libbpf: restore the ability to load programs from .text section libbpf used to be able to load programs from the default section called '.text'. It's not very common to leave sections unnamed, but if it happens libbpf will fail to load the programs reporting -EINVAL from the kernel. The -EINVAL comes from bpf_obj_name_cpy() because since 48cca7e44f9f ("libbpf: add support for bpf_call") libbpf does not resolve program names for programs in '.text', defaulting to '.text'. '.text', however, does not pass the (isalnum(*src) || *src == '_') check in bpf_obj_name_cpy(). With few extra lines of code we can limit the pseudo call assumptions only to objects which actually contain code relocations. Signed-off-by: Jakub Kicinski Reviewed-by: Quentin Monnet Signed-off-by: Daniel Borkmann --- tools/lib/bpf/libbpf.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 7bc02d93e277..e2401b95f08d 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -234,6 +234,7 @@ struct bpf_object { size_t nr_maps; bool loaded; + bool has_pseudo_calls; /* * Information when doing elf related work. Only valid if fd @@ -400,10 +401,6 @@ bpf_object__init_prog_names(struct bpf_object *obj) const char *name = NULL; prog = &obj->programs[pi]; - if (prog->idx == obj->efile.text_shndx) { - name = ".text"; - goto skip_search; - } for (si = 0; si < symbols->d_size / sizeof(GElf_Sym) && !name; si++) { @@ -426,12 +423,15 @@ bpf_object__init_prog_names(struct bpf_object *obj) } } + if (!name && prog->idx == obj->efile.text_shndx) + name = ".text"; + if (!name) { pr_warning("failed to find sym for prog %s\n", prog->section_name); return -EINVAL; } -skip_search: + prog->name = strdup(name); if (!prog->name) { pr_warning("failed to allocate memory for prog sym %s\n", @@ -981,6 +981,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr, prog->reloc_desc[i].type = RELO_CALL; prog->reloc_desc[i].insn_idx = insn_idx; prog->reloc_desc[i].text_off = sym.st_value; + obj->has_pseudo_calls = true; continue; } @@ -1426,6 +1427,12 @@ out: return err; } +static bool bpf_program__is_function_storage(struct bpf_program *prog, + struct bpf_object *obj) +{ + return prog->idx == obj->efile.text_shndx && obj->has_pseudo_calls; +} + static int bpf_object__load_progs(struct bpf_object *obj) { @@ -1433,7 +1440,7 @@ bpf_object__load_progs(struct bpf_object *obj) int err; for (i = 0; i < obj->nr_programs; i++) { - if (obj->programs[i].idx == obj->efile.text_shndx) + if (bpf_program__is_function_storage(&obj->programs[i], obj)) continue; err = bpf_program__load(&obj->programs[i], obj->license, @@ -2247,7 +2254,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr, bpf_program__set_expected_attach_type(prog, expected_attach_type); - if (prog->idx != obj->efile.text_shndx && !first_prog) + if (!bpf_program__is_function_storage(prog, obj) && !first_prog) first_prog = prog; } -- cgit v1.2.3 From eac7d84519a35fc9fc8071b7d93803b2b075e2a7 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 28 Jun 2018 14:41:39 -0700 Subject: tools: libbpf: don't return '.text' as a program for multi-function programs Make bpf_program__next() skip over '.text' section if object file has pseudo calls. The '.text' section is hardly a program in that case, it's more of a storage for code of functions other than main. Signed-off-by: Jakub Kicinski Reviewed-by: Quentin Monnet Signed-off-by: Daniel Borkmann --- tools/lib/bpf/libbpf.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index e2401b95f08d..38ed3e92e393 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -1865,8 +1865,8 @@ void *bpf_object__priv(struct bpf_object *obj) return obj ? obj->priv : ERR_PTR(-EINVAL); } -struct bpf_program * -bpf_program__next(struct bpf_program *prev, struct bpf_object *obj) +static struct bpf_program * +__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj) { size_t idx; @@ -1887,6 +1887,18 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj) return &obj->programs[idx]; } +struct bpf_program * +bpf_program__next(struct bpf_program *prev, struct bpf_object *obj) +{ + struct bpf_program *prog = prev; + + do { + prog = __bpf_program__next(prog, obj); + } while (prog && bpf_program__is_function_storage(prog, obj)); + + return prog; +} + int bpf_program__set_priv(struct bpf_program *prog, void *priv, bpf_program_clear_priv_t clear_priv) { -- cgit v1.2.3 From 71e07ddcdc03000e37acfc6e757f70c81a963d58 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 28 Jun 2018 14:41:40 -0700 Subject: tools: bpftool: drop unnecessary Author comments Drop my author comments, those are from the early days of bpftool and make little sense in tree, where we have quite a few people contributing and git to attribute the work. While at it bump some copyrights. Signed-off-by: Jakub Kicinski Reviewed-by: Quentin Monnet Signed-off-by: Daniel Borkmann --- tools/bpf/bpftool/common.c | 2 -- tools/bpf/bpftool/main.c | 4 +--- tools/bpf/bpftool/main.h | 2 -- tools/bpf/bpftool/map.c | 2 -- tools/bpf/bpftool/prog.c | 4 +--- 5 files changed, 2 insertions(+), 12 deletions(-) diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c index 32f9e397a6c0..b432daea4520 100644 --- a/tools/bpf/bpftool/common.c +++ b/tools/bpf/bpftool/common.c @@ -31,8 +31,6 @@ * SOFTWARE. */ -/* Author: Jakub Kicinski */ - #include #include #include diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index eea7f14355f3..d15a62be6cf0 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2017 Netronome Systems, Inc. + * Copyright (C) 2017-2018 Netronome Systems, Inc. * * This software is dual licensed under the GNU General License Version 2, * June 1991 as shown in the file COPYING in the top-level directory of this @@ -31,8 +31,6 @@ * SOFTWARE. */ -/* Author: Jakub Kicinski */ - #include #include #include diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h index 63fdb310b9a4..d39f7ef01d23 100644 --- a/tools/bpf/bpftool/main.h +++ b/tools/bpf/bpftool/main.h @@ -31,8 +31,6 @@ * SOFTWARE. */ -/* Author: Jakub Kicinski */ - #ifndef __BPF_TOOL_H #define __BPF_TOOL_H diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c index 097b1a5e046b..5989e1575ae4 100644 --- a/tools/bpf/bpftool/map.c +++ b/tools/bpf/bpftool/map.c @@ -31,8 +31,6 @@ * SOFTWARE. */ -/* Author: Jakub Kicinski */ - #include #include #include diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index 05f42a46d6ed..fd8cd9b51621 100644 --- a/tools/bpf/bpftool/prog.c +++ b/tools/bpf/bpftool/prog.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2017 Netronome Systems, Inc. + * Copyright (C) 2017-2018 Netronome Systems, Inc. * * This software is dual licensed under the GNU General License Version 2, * June 1991 as shown in the file COPYING in the top-level directory of this @@ -31,8 +31,6 @@ * SOFTWARE. */ -/* Author: Jakub Kicinski */ - #include #include #include -- cgit v1.2.3 From ef347a340b1a8507c22ee3cf981cd5cd64188431 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 28 Jun 2018 14:41:41 -0700 Subject: tools: bpftool: add missing --bpffs to completions --bpffs is not suggested by bash completions. Signed-off-by: Jakub Kicinski Reviewed-by: Quentin Monnet Signed-off-by: Daniel Borkmann --- tools/bpf/bpftool/bash-completion/bpftool | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool index 1e1083321643..b0b8022d3570 100644 --- a/tools/bpf/bpftool/bash-completion/bpftool +++ b/tools/bpf/bpftool/bash-completion/bpftool @@ -182,7 +182,7 @@ _bpftool() if [[ -z $object ]]; then case $cur in -*) - local c='--version --json --pretty' + local c='--version --json --pretty --bpffs' COMPREPLY=( $( compgen -W "$c" -- "$cur" ) ) return 0 ;; -- cgit v1.2.3 From 121c58bed01a45043381ce52b8556e851dbaa123 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 28 Jun 2018 14:41:42 -0700 Subject: tools: bpftool: deal with options upfront Remove options (in getopt() sense, i.e. starting with a dash like -n or --NAME) while parsing arguments for bash completions. This allows us to refer to position-dependent parameters better, and complete options at any point. Signed-off-by: Jakub Kicinski Reviewed-by: Quentin Monnet Signed-off-by: Daniel Borkmann --- tools/bpf/bpftool/bash-completion/bpftool | 32 ++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool index b0b8022d3570..fffd76f4998b 100644 --- a/tools/bpf/bpftool/bash-completion/bpftool +++ b/tools/bpf/bpftool/bash-completion/bpftool @@ -153,6 +153,13 @@ _bpftool() local cur prev words objword _init_completion || return + # Deal with options + if [[ ${words[cword]} == -* ]]; then + local c='--version --json --pretty --bpffs' + COMPREPLY=( $( compgen -W "$c" -- "$cur" ) ) + return 0 + fi + # Deal with simplest keywords case $prev in help|hex|opcodes|visual) @@ -172,20 +179,23 @@ _bpftool() ;; esac - # Search for object and command - local object command cmdword - for (( cmdword=1; cmdword < ${#words[@]}-1; cmdword++ )); do - [[ -n $object ]] && command=${words[cmdword]} && break - [[ ${words[cmdword]} != -* ]] && object=${words[cmdword]} + # Remove all options so completions don't have to deal with them. + local i + for (( i=1; i < ${#words[@]}; )); do + if [[ ${words[i]::1} == - ]]; then + words=( "${words[@]:0:i}" "${words[@]:i+1}" ) + [[ $i -le $cword ]] && cword=$(( cword - 1 )) + else + i=$(( ++i )) + fi done + cur=${words[cword]} + prev=${words[cword - 1]} - if [[ -z $object ]]; then + local object=${words[1]} command=${words[2]} + + if [[ -z $object || $cword -eq 1 ]]; then case $cur in - -*) - local c='--version --json --pretty --bpffs' - COMPREPLY=( $( compgen -W "$c" -- "$cur" ) ) - return 0 - ;; *) COMPREPLY=( $( compgen -W "$( bpftool help 2>&1 | \ command sed \ -- cgit v1.2.3