Message ID | 20190124164953.29740-4-sdf@google.com |
---|---|
State | Changes Requested |
Delegated to: | BPF Maintainers |
Headers | show |
Series | support flow dissector in BPF_PROG_TEST_RUN | expand |
On Thu, Jan 24, 2019 at 8:52 AM Stanislav Fomichev <sdf@google.com> wrote: > > Use existing pkt_v4 and pkt_v6 to make sure flow_keys are what we want. > > Also, add new bpf_flow_load routine (and flow_dissector_load.h header) > that loads bpf_flow.o program and does all required setup. > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > --- > tools/testing/selftests/bpf/Makefile | 3 + > .../selftests/bpf/flow_dissector_load.c | 43 ++-------- > .../selftests/bpf/flow_dissector_load.h | 55 +++++++++++++ > tools/testing/selftests/bpf/test_progs.c | 78 ++++++++++++++++++- > 4 files changed, 139 insertions(+), 40 deletions(-) > create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index 70229de510f5..ade67470a152 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -19,6 +19,9 @@ all: $(TEST_CUSTOM_PROGS) > $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c > $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id > > +flow_dissector_load.c: flow_dissector_load.h > +test_run.c: flow_dissector_load.h > + > # Order correspond to 'make run_tests' order > TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \ > test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \ > diff --git a/tools/testing/selftests/bpf/flow_dissector_load.c b/tools/testing/selftests/bpf/flow_dissector_load.c > index ae8180b11d5f..77cafa66d048 100644 > --- a/tools/testing/selftests/bpf/flow_dissector_load.c > +++ b/tools/testing/selftests/bpf/flow_dissector_load.c > @@ -12,6 +12,7 @@ > #include <bpf/libbpf.h> > > #include "bpf_rlimit.h" > +#include "flow_dissector_load.h" > > const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector"; > const char *cfg_map_name = "jmp_table"; > @@ -21,46 +22,13 @@ char *cfg_path_name; > > static void load_and_attach_program(void) > { > - struct bpf_program *prog, *main_prog; > - struct bpf_map *prog_array; > - int i, fd, prog_fd, ret; > + int prog_fd, ret; > struct bpf_object *obj; > - int prog_array_fd; > > - ret = bpf_prog_load(cfg_path_name, BPF_PROG_TYPE_FLOW_DISSECTOR, &obj, > - &prog_fd); > + ret = bpf_flow_load(&obj, cfg_path_name, cfg_section_name, > + cfg_map_name, &prog_fd); > if (ret) > - error(1, 0, "bpf_prog_load %s", cfg_path_name); > - > - main_prog = bpf_object__find_program_by_title(obj, cfg_section_name); > - if (!main_prog) > - error(1, 0, "bpf_object__find_program_by_title %s", > - cfg_section_name); > - > - prog_fd = bpf_program__fd(main_prog); > - if (prog_fd < 0) > - error(1, 0, "bpf_program__fd"); > - > - prog_array = bpf_object__find_map_by_name(obj, cfg_map_name); > - if (!prog_array) > - error(1, 0, "bpf_object__find_map_by_name %s", cfg_map_name); > - > - prog_array_fd = bpf_map__fd(prog_array); > - if (prog_array_fd < 0) > - error(1, 0, "bpf_map__fd %s", cfg_map_name); > - > - i = 0; > - bpf_object__for_each_program(prog, obj) { > - fd = bpf_program__fd(prog); > - if (fd < 0) > - error(1, 0, "bpf_program__fd"); > - > - if (fd != prog_fd) { > - printf("%d: %s\n", i, bpf_program__title(prog, false)); > - bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); > - ++i; > - } > - } > + error(1, 0, "bpf_flow_load %s", cfg_path_name); > > ret = bpf_prog_attach(prog_fd, 0 /* Ignore */, BPF_FLOW_DISSECTOR, 0); > if (ret) > @@ -69,7 +37,6 @@ static void load_and_attach_program(void) > ret = bpf_object__pin(obj, cfg_pin_path); > if (ret) > error(1, 0, "bpf_object__pin %s", cfg_pin_path); > - > } > > static void detach_program(void) > diff --git a/tools/testing/selftests/bpf/flow_dissector_load.h b/tools/testing/selftests/bpf/flow_dissector_load.h > new file mode 100644 > index 000000000000..41dd6959feb0 > --- /dev/null > +++ b/tools/testing/selftests/bpf/flow_dissector_load.h > @@ -0,0 +1,55 @@ > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > +#ifndef FLOW_DISSECTOR_LOAD > +#define FLOW_DISSECTOR_LOAD > + > +#include <bpf/bpf.h> > +#include <bpf/libbpf.h> > + > +static inline int bpf_flow_load(struct bpf_object **obj, > + const char *path, > + const char *section_name, > + const char *map_name, > + int *prog_fd) > +{ > + struct bpf_program *prog, *main_prog; > + struct bpf_map *prog_array; > + int prog_array_fd; > + int ret, fd, i; > + > + ret = bpf_prog_load(path, BPF_PROG_TYPE_FLOW_DISSECTOR, obj, > + prog_fd); > + if (ret) > + return ret; > + > + main_prog = bpf_object__find_program_by_title(*obj, section_name); > + if (!main_prog) > + return ret; > + > + *prog_fd = bpf_program__fd(main_prog); > + if (*prog_fd < 0) > + return ret; > + > + prog_array = bpf_object__find_map_by_name(*obj, map_name); > + if (!prog_array) > + return ret; > + > + prog_array_fd = bpf_map__fd(prog_array); > + if (prog_array_fd < 0) > + return ret; > + > + i = 0; > + bpf_object__for_each_program(prog, *obj) { > + fd = bpf_program__fd(prog); > + if (fd < 0) > + return fd; > + > + if (fd != *prog_fd) { > + bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); > + ++i; > + } > + } > + > + return 0; > +} > + > +#endif /* FLOW_DISSECTOR_LOAD */ > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > index 126fc624290d..5f46680d4ad4 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -39,6 +39,7 @@ typedef __u16 __sum16; > #include "bpf_endian.h" > #include "bpf_rlimit.h" > #include "trace_helpers.h" > +#include "flow_dissector_load.h" > > static int error_cnt, pass_cnt; > static bool jit_enabled; > @@ -53,9 +54,10 @@ static struct { > } __packed pkt_v4 = { > .eth.h_proto = __bpf_constant_htons(ETH_P_IP), > .iph.ihl = 5, > - .iph.protocol = 6, > + .iph.protocol = IPPROTO_TCP, > .iph.tot_len = __bpf_constant_htons(MAGIC_BYTES), > .tcp.urg_ptr = 123, > + .tcp.doff = 5, > }; > > /* ipv6 test vector */ > @@ -65,9 +67,10 @@ static struct { > struct tcphdr tcp; > } __packed pkt_v6 = { > .eth.h_proto = __bpf_constant_htons(ETH_P_IPV6), > - .iph.nexthdr = 6, > + .iph.nexthdr = IPPROTO_TCP, > .iph.payload_len = __bpf_constant_htons(MAGIC_BYTES), > .tcp.urg_ptr = 123, > + .tcp.doff = 5, > }; Why do we need these changes? They seem unrelated to me. Thanks, Song > > #define _CHECK(condition, tag, duration, format...) ({ \ > @@ -1882,6 +1885,76 @@ static void test_queue_stack_map(int type) > bpf_object__close(obj); > } > > +#define CHECK_FLOW_KEYS(desc, got, expected) \ > + CHECK(memcmp(&got, &expected, sizeof(got)) != 0, \ > + desc, \ > + "nhoff=%u/%u " \ > + "thoff=%u/%u " \ > + "addr_proto=0x%x/0x%x " \ > + "is_frag=%u/%u " \ > + "is_first_frag=%u/%u " \ > + "is_encap=%u/%u " \ > + "n_proto=0x%x/0x%x " \ > + "sport=%u/%u " \ > + "dport=%u/%u\n", \ > + got.nhoff, expected.nhoff, \ > + got.thoff, expected.thoff, \ > + got.addr_proto, expected.addr_proto, \ > + got.is_frag, expected.is_frag, \ > + got.is_first_frag, expected.is_first_frag, \ > + got.is_encap, expected.is_encap, \ > + got.n_proto, expected.n_proto, \ > + got.sport, expected.sport, \ > + got.dport, expected.dport) > + > +static struct bpf_flow_keys pkt_v4_flow_keys = { > + .nhoff = 0, > + .thoff = sizeof(struct iphdr), > + .addr_proto = ETH_P_IP, > + .ip_proto = IPPROTO_TCP, > + .n_proto = bpf_htons(ETH_P_IP), > +}; > + > +static struct bpf_flow_keys pkt_v6_flow_keys = { > + .nhoff = 0, > + .thoff = sizeof(struct ipv6hdr), > + .addr_proto = ETH_P_IPV6, > + .ip_proto = IPPROTO_TCP, > + .n_proto = bpf_htons(ETH_P_IPV6), > +}; > + > +static void test_flow_dissector(void) > +{ > + struct bpf_flow_keys flow_keys; > + struct bpf_object *obj; > + __u32 duration, retval; > + int err, prog_fd; > + __u32 size; > + > + err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector", > + "jmp_table", &prog_fd); > + if (err) { > + error_cnt++; > + return; > + } > + > + err = bpf_prog_test_run(prog_fd, 10, &pkt_v4, sizeof(pkt_v4), > + &flow_keys, &size, &retval, &duration); > + CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv4", > + "err %d errno %d retval %d duration %d size %u/%lu\n", > + err, errno, retval, duration, size, sizeof(flow_keys)); > + CHECK_FLOW_KEYS("ipv4_flow_keys", flow_keys, pkt_v4_flow_keys); > + > + err = bpf_prog_test_run(prog_fd, 10, &pkt_v6, sizeof(pkt_v6), > + &flow_keys, &size, &retval, &duration); > + CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv6", > + "err %d errno %d retval %d duration %d size %u/%lu\n", > + err, errno, retval, duration, size, sizeof(flow_keys)); > + CHECK_FLOW_KEYS("ipv6_flow_keys", flow_keys, pkt_v6_flow_keys); > + > + bpf_object__close(obj); > +} > + > int main(void) > { > srand(time(NULL)); > @@ -1909,6 +1982,7 @@ int main(void) > test_reference_tracking(); > test_queue_stack_map(QUEUE); > test_queue_stack_map(STACK); > + test_flow_dissector(); > > printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt); > return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS; > -- > 2.20.1.321.g9e740568ce-goog >
On 01/25, Song Liu wrote: > On Thu, Jan 24, 2019 at 8:52 AM Stanislav Fomichev <sdf@google.com> wrote: > > > > Use existing pkt_v4 and pkt_v6 to make sure flow_keys are what we want. > > > > Also, add new bpf_flow_load routine (and flow_dissector_load.h header) > > that loads bpf_flow.o program and does all required setup. > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > --- > > tools/testing/selftests/bpf/Makefile | 3 + > > .../selftests/bpf/flow_dissector_load.c | 43 ++-------- > > .../selftests/bpf/flow_dissector_load.h | 55 +++++++++++++ > > tools/testing/selftests/bpf/test_progs.c | 78 ++++++++++++++++++- > > 4 files changed, 139 insertions(+), 40 deletions(-) > > create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > index 70229de510f5..ade67470a152 100644 > > --- a/tools/testing/selftests/bpf/Makefile > > +++ b/tools/testing/selftests/bpf/Makefile > > @@ -19,6 +19,9 @@ all: $(TEST_CUSTOM_PROGS) > > $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c > > $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id > > > > +flow_dissector_load.c: flow_dissector_load.h > > +test_run.c: flow_dissector_load.h > > + > > # Order correspond to 'make run_tests' order > > TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \ > > test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \ > > diff --git a/tools/testing/selftests/bpf/flow_dissector_load.c b/tools/testing/selftests/bpf/flow_dissector_load.c > > index ae8180b11d5f..77cafa66d048 100644 > > --- a/tools/testing/selftests/bpf/flow_dissector_load.c > > +++ b/tools/testing/selftests/bpf/flow_dissector_load.c > > @@ -12,6 +12,7 @@ > > #include <bpf/libbpf.h> > > > > #include "bpf_rlimit.h" > > +#include "flow_dissector_load.h" > > > > const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector"; > > const char *cfg_map_name = "jmp_table"; > > @@ -21,46 +22,13 @@ char *cfg_path_name; > > > > static void load_and_attach_program(void) > > { > > - struct bpf_program *prog, *main_prog; > > - struct bpf_map *prog_array; > > - int i, fd, prog_fd, ret; > > + int prog_fd, ret; > > struct bpf_object *obj; > > - int prog_array_fd; > > > > - ret = bpf_prog_load(cfg_path_name, BPF_PROG_TYPE_FLOW_DISSECTOR, &obj, > > - &prog_fd); > > + ret = bpf_flow_load(&obj, cfg_path_name, cfg_section_name, > > + cfg_map_name, &prog_fd); > > if (ret) > > - error(1, 0, "bpf_prog_load %s", cfg_path_name); > > - > > - main_prog = bpf_object__find_program_by_title(obj, cfg_section_name); > > - if (!main_prog) > > - error(1, 0, "bpf_object__find_program_by_title %s", > > - cfg_section_name); > > - > > - prog_fd = bpf_program__fd(main_prog); > > - if (prog_fd < 0) > > - error(1, 0, "bpf_program__fd"); > > - > > - prog_array = bpf_object__find_map_by_name(obj, cfg_map_name); > > - if (!prog_array) > > - error(1, 0, "bpf_object__find_map_by_name %s", cfg_map_name); > > - > > - prog_array_fd = bpf_map__fd(prog_array); > > - if (prog_array_fd < 0) > > - error(1, 0, "bpf_map__fd %s", cfg_map_name); > > - > > - i = 0; > > - bpf_object__for_each_program(prog, obj) { > > - fd = bpf_program__fd(prog); > > - if (fd < 0) > > - error(1, 0, "bpf_program__fd"); > > - > > - if (fd != prog_fd) { > > - printf("%d: %s\n", i, bpf_program__title(prog, false)); > > - bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); > > - ++i; > > - } > > - } > > + error(1, 0, "bpf_flow_load %s", cfg_path_name); > > > > ret = bpf_prog_attach(prog_fd, 0 /* Ignore */, BPF_FLOW_DISSECTOR, 0); > > if (ret) > > @@ -69,7 +37,6 @@ static void load_and_attach_program(void) > > ret = bpf_object__pin(obj, cfg_pin_path); > > if (ret) > > error(1, 0, "bpf_object__pin %s", cfg_pin_path); > > - > > } > > > > static void detach_program(void) > > diff --git a/tools/testing/selftests/bpf/flow_dissector_load.h b/tools/testing/selftests/bpf/flow_dissector_load.h > > new file mode 100644 > > index 000000000000..41dd6959feb0 > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/flow_dissector_load.h > > @@ -0,0 +1,55 @@ > > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > > +#ifndef FLOW_DISSECTOR_LOAD > > +#define FLOW_DISSECTOR_LOAD > > + > > +#include <bpf/bpf.h> > > +#include <bpf/libbpf.h> > > + > > +static inline int bpf_flow_load(struct bpf_object **obj, > > + const char *path, > > + const char *section_name, > > + const char *map_name, > > + int *prog_fd) > > +{ > > + struct bpf_program *prog, *main_prog; > > + struct bpf_map *prog_array; > > + int prog_array_fd; > > + int ret, fd, i; > > + > > + ret = bpf_prog_load(path, BPF_PROG_TYPE_FLOW_DISSECTOR, obj, > > + prog_fd); > > + if (ret) > > + return ret; > > + > > + main_prog = bpf_object__find_program_by_title(*obj, section_name); > > + if (!main_prog) > > + return ret; > > + > > + *prog_fd = bpf_program__fd(main_prog); > > + if (*prog_fd < 0) > > + return ret; > > + > > + prog_array = bpf_object__find_map_by_name(*obj, map_name); > > + if (!prog_array) > > + return ret; > > + > > + prog_array_fd = bpf_map__fd(prog_array); > > + if (prog_array_fd < 0) > > + return ret; > > + > > + i = 0; > > + bpf_object__for_each_program(prog, *obj) { > > + fd = bpf_program__fd(prog); > > + if (fd < 0) > > + return fd; > > + > > + if (fd != *prog_fd) { > > + bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); > > + ++i; > > + } > > + } > > + > > + return 0; > > +} > > + > > +#endif /* FLOW_DISSECTOR_LOAD */ > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > > index 126fc624290d..5f46680d4ad4 100644 > > --- a/tools/testing/selftests/bpf/test_progs.c > > +++ b/tools/testing/selftests/bpf/test_progs.c > > @@ -39,6 +39,7 @@ typedef __u16 __sum16; > > #include "bpf_endian.h" > > #include "bpf_rlimit.h" > > #include "trace_helpers.h" > > +#include "flow_dissector_load.h" > > > > static int error_cnt, pass_cnt; > > static bool jit_enabled; > > @@ -53,9 +54,10 @@ static struct { > > } __packed pkt_v4 = { > > .eth.h_proto = __bpf_constant_htons(ETH_P_IP), > > .iph.ihl = 5, > > - .iph.protocol = 6, > > + .iph.protocol = IPPROTO_TCP, > > .iph.tot_len = __bpf_constant_htons(MAGIC_BYTES), > > .tcp.urg_ptr = 123, > > + .tcp.doff = 5, > > }; > > > > /* ipv6 test vector */ > > @@ -65,9 +67,10 @@ static struct { > > struct tcphdr tcp; > > } __packed pkt_v6 = { > > .eth.h_proto = __bpf_constant_htons(ETH_P_IPV6), > > - .iph.nexthdr = 6, > > + .iph.nexthdr = IPPROTO_TCP, > > .iph.payload_len = __bpf_constant_htons(MAGIC_BYTES), > > .tcp.urg_ptr = 123, > > + .tcp.doff = 5, > > }; > > Why do we need these changes? They seem unrelated to me. doff = 5 is required for the current flow dissector program implementation (bpf_flow.c), without it it doesn't return success: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/bpf_flow.c#n188 6 to IPPROTO_TCP just to look nice, since I'm already touching it. > > Thanks, > Song > > > > > > #define _CHECK(condition, tag, duration, format...) ({ \ > > @@ -1882,6 +1885,76 @@ static void test_queue_stack_map(int type) > > bpf_object__close(obj); > > } > > > > +#define CHECK_FLOW_KEYS(desc, got, expected) \ > > + CHECK(memcmp(&got, &expected, sizeof(got)) != 0, \ > > + desc, \ > > + "nhoff=%u/%u " \ > > + "thoff=%u/%u " \ > > + "addr_proto=0x%x/0x%x " \ > > + "is_frag=%u/%u " \ > > + "is_first_frag=%u/%u " \ > > + "is_encap=%u/%u " \ > > + "n_proto=0x%x/0x%x " \ > > + "sport=%u/%u " \ > > + "dport=%u/%u\n", \ > > + got.nhoff, expected.nhoff, \ > > + got.thoff, expected.thoff, \ > > + got.addr_proto, expected.addr_proto, \ > > + got.is_frag, expected.is_frag, \ > > + got.is_first_frag, expected.is_first_frag, \ > > + got.is_encap, expected.is_encap, \ > > + got.n_proto, expected.n_proto, \ > > + got.sport, expected.sport, \ > > + got.dport, expected.dport) > > + > > +static struct bpf_flow_keys pkt_v4_flow_keys = { > > + .nhoff = 0, > > + .thoff = sizeof(struct iphdr), > > + .addr_proto = ETH_P_IP, > > + .ip_proto = IPPROTO_TCP, > > + .n_proto = bpf_htons(ETH_P_IP), > > +}; > > + > > +static struct bpf_flow_keys pkt_v6_flow_keys = { > > + .nhoff = 0, > > + .thoff = sizeof(struct ipv6hdr), > > + .addr_proto = ETH_P_IPV6, > > + .ip_proto = IPPROTO_TCP, > > + .n_proto = bpf_htons(ETH_P_IPV6), > > +}; > > + > > +static void test_flow_dissector(void) > > +{ > > + struct bpf_flow_keys flow_keys; > > + struct bpf_object *obj; > > + __u32 duration, retval; > > + int err, prog_fd; > > + __u32 size; > > + > > + err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector", > > + "jmp_table", &prog_fd); > > + if (err) { > > + error_cnt++; > > + return; > > + } > > + > > + err = bpf_prog_test_run(prog_fd, 10, &pkt_v4, sizeof(pkt_v4), > > + &flow_keys, &size, &retval, &duration); > > + CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv4", > > + "err %d errno %d retval %d duration %d size %u/%lu\n", > > + err, errno, retval, duration, size, sizeof(flow_keys)); > > + CHECK_FLOW_KEYS("ipv4_flow_keys", flow_keys, pkt_v4_flow_keys); > > + > > + err = bpf_prog_test_run(prog_fd, 10, &pkt_v6, sizeof(pkt_v6), > > + &flow_keys, &size, &retval, &duration); > > + CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv6", > > + "err %d errno %d retval %d duration %d size %u/%lu\n", > > + err, errno, retval, duration, size, sizeof(flow_keys)); > > + CHECK_FLOW_KEYS("ipv6_flow_keys", flow_keys, pkt_v6_flow_keys); > > + > > + bpf_object__close(obj); > > +} > > + > > int main(void) > > { > > srand(time(NULL)); > > @@ -1909,6 +1982,7 @@ int main(void) > > test_reference_tracking(); > > test_queue_stack_map(QUEUE); > > test_queue_stack_map(STACK); > > + test_flow_dissector(); > > > > printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt); > > return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS; > > -- > > 2.20.1.321.g9e740568ce-goog > >
On Fri, Jan 25, 2019 at 4:58 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > On 01/25, Song Liu wrote: > > On Thu, Jan 24, 2019 at 8:52 AM Stanislav Fomichev <sdf@google.com> wrote: > > > > > > Use existing pkt_v4 and pkt_v6 to make sure flow_keys are what we want. > > > > > > Also, add new bpf_flow_load routine (and flow_dissector_load.h header) > > > that loads bpf_flow.o program and does all required setup. > > > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > > --- > > > tools/testing/selftests/bpf/Makefile | 3 + > > > .../selftests/bpf/flow_dissector_load.c | 43 ++-------- > > > .../selftests/bpf/flow_dissector_load.h | 55 +++++++++++++ > > > tools/testing/selftests/bpf/test_progs.c | 78 ++++++++++++++++++- > > > 4 files changed, 139 insertions(+), 40 deletions(-) > > > create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h > > > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > > index 70229de510f5..ade67470a152 100644 > > > --- a/tools/testing/selftests/bpf/Makefile > > > +++ b/tools/testing/selftests/bpf/Makefile > > > @@ -19,6 +19,9 @@ all: $(TEST_CUSTOM_PROGS) > > > $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c > > > $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id > > > > > > +flow_dissector_load.c: flow_dissector_load.h > > > +test_run.c: flow_dissector_load.h > > > + > > > # Order correspond to 'make run_tests' order > > > TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \ > > > test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \ > > > diff --git a/tools/testing/selftests/bpf/flow_dissector_load.c b/tools/testing/selftests/bpf/flow_dissector_load.c > > > index ae8180b11d5f..77cafa66d048 100644 > > > --- a/tools/testing/selftests/bpf/flow_dissector_load.c > > > +++ b/tools/testing/selftests/bpf/flow_dissector_load.c > > > @@ -12,6 +12,7 @@ > > > #include <bpf/libbpf.h> > > > > > > #include "bpf_rlimit.h" > > > +#include "flow_dissector_load.h" > > > > > > const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector"; > > > const char *cfg_map_name = "jmp_table"; > > > @@ -21,46 +22,13 @@ char *cfg_path_name; > > > > > > static void load_and_attach_program(void) > > > { > > > - struct bpf_program *prog, *main_prog; > > > - struct bpf_map *prog_array; > > > - int i, fd, prog_fd, ret; > > > + int prog_fd, ret; > > > struct bpf_object *obj; > > > - int prog_array_fd; > > > > > > - ret = bpf_prog_load(cfg_path_name, BPF_PROG_TYPE_FLOW_DISSECTOR, &obj, > > > - &prog_fd); > > > + ret = bpf_flow_load(&obj, cfg_path_name, cfg_section_name, > > > + cfg_map_name, &prog_fd); > > > if (ret) > > > - error(1, 0, "bpf_prog_load %s", cfg_path_name); > > > - > > > - main_prog = bpf_object__find_program_by_title(obj, cfg_section_name); > > > - if (!main_prog) > > > - error(1, 0, "bpf_object__find_program_by_title %s", > > > - cfg_section_name); > > > - > > > - prog_fd = bpf_program__fd(main_prog); > > > - if (prog_fd < 0) > > > - error(1, 0, "bpf_program__fd"); > > > - > > > - prog_array = bpf_object__find_map_by_name(obj, cfg_map_name); > > > - if (!prog_array) > > > - error(1, 0, "bpf_object__find_map_by_name %s", cfg_map_name); > > > - > > > - prog_array_fd = bpf_map__fd(prog_array); > > > - if (prog_array_fd < 0) > > > - error(1, 0, "bpf_map__fd %s", cfg_map_name); > > > - > > > - i = 0; > > > - bpf_object__for_each_program(prog, obj) { > > > - fd = bpf_program__fd(prog); > > > - if (fd < 0) > > > - error(1, 0, "bpf_program__fd"); > > > - > > > - if (fd != prog_fd) { > > > - printf("%d: %s\n", i, bpf_program__title(prog, false)); > > > - bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); > > > - ++i; > > > - } > > > - } > > > + error(1, 0, "bpf_flow_load %s", cfg_path_name); > > > > > > ret = bpf_prog_attach(prog_fd, 0 /* Ignore */, BPF_FLOW_DISSECTOR, 0); > > > if (ret) > > > @@ -69,7 +37,6 @@ static void load_and_attach_program(void) > > > ret = bpf_object__pin(obj, cfg_pin_path); > > > if (ret) > > > error(1, 0, "bpf_object__pin %s", cfg_pin_path); > > > - > > > } > > > > > > static void detach_program(void) > > > diff --git a/tools/testing/selftests/bpf/flow_dissector_load.h b/tools/testing/selftests/bpf/flow_dissector_load.h > > > new file mode 100644 > > > index 000000000000..41dd6959feb0 > > > --- /dev/null > > > +++ b/tools/testing/selftests/bpf/flow_dissector_load.h > > > @@ -0,0 +1,55 @@ > > > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > > > +#ifndef FLOW_DISSECTOR_LOAD > > > +#define FLOW_DISSECTOR_LOAD > > > + > > > +#include <bpf/bpf.h> > > > +#include <bpf/libbpf.h> > > > + > > > +static inline int bpf_flow_load(struct bpf_object **obj, > > > + const char *path, > > > + const char *section_name, > > > + const char *map_name, > > > + int *prog_fd) > > > +{ > > > + struct bpf_program *prog, *main_prog; > > > + struct bpf_map *prog_array; > > > + int prog_array_fd; > > > + int ret, fd, i; > > > + > > > + ret = bpf_prog_load(path, BPF_PROG_TYPE_FLOW_DISSECTOR, obj, > > > + prog_fd); > > > + if (ret) > > > + return ret; > > > + > > > + main_prog = bpf_object__find_program_by_title(*obj, section_name); > > > + if (!main_prog) > > > + return ret; > > > + > > > + *prog_fd = bpf_program__fd(main_prog); > > > + if (*prog_fd < 0) > > > + return ret; > > > + > > > + prog_array = bpf_object__find_map_by_name(*obj, map_name); > > > + if (!prog_array) > > > + return ret; > > > + > > > + prog_array_fd = bpf_map__fd(prog_array); > > > + if (prog_array_fd < 0) > > > + return ret; > > > + > > > + i = 0; > > > + bpf_object__for_each_program(prog, *obj) { > > > + fd = bpf_program__fd(prog); > > > + if (fd < 0) > > > + return fd; > > > + > > > + if (fd != *prog_fd) { > > > + bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); > > > + ++i; > > > + } > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +#endif /* FLOW_DISSECTOR_LOAD */ > > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > > > index 126fc624290d..5f46680d4ad4 100644 > > > --- a/tools/testing/selftests/bpf/test_progs.c > > > +++ b/tools/testing/selftests/bpf/test_progs.c > > > @@ -39,6 +39,7 @@ typedef __u16 __sum16; > > > #include "bpf_endian.h" > > > #include "bpf_rlimit.h" > > > #include "trace_helpers.h" > > > +#include "flow_dissector_load.h" > > > > > > static int error_cnt, pass_cnt; > > > static bool jit_enabled; > > > @@ -53,9 +54,10 @@ static struct { > > > } __packed pkt_v4 = { > > > .eth.h_proto = __bpf_constant_htons(ETH_P_IP), > > > .iph.ihl = 5, > > > - .iph.protocol = 6, > > > + .iph.protocol = IPPROTO_TCP, > > > .iph.tot_len = __bpf_constant_htons(MAGIC_BYTES), > > > .tcp.urg_ptr = 123, > > > + .tcp.doff = 5, > > > }; > > > > > > /* ipv6 test vector */ > > > @@ -65,9 +67,10 @@ static struct { > > > struct tcphdr tcp; > > > } __packed pkt_v6 = { > > > .eth.h_proto = __bpf_constant_htons(ETH_P_IPV6), > > > - .iph.nexthdr = 6, > > > + .iph.nexthdr = IPPROTO_TCP, > > > .iph.payload_len = __bpf_constant_htons(MAGIC_BYTES), > > > .tcp.urg_ptr = 123, > > > + .tcp.doff = 5, > > > }; > > > > Why do we need these changes? They seem unrelated to me. > doff = 5 is required for the current flow dissector program > implementation (bpf_flow.c), without it it doesn't return success: > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/bpf_flow.c#n188 > > 6 to IPPROTO_TCP just to look nice, since I'm already touching it. Thanks for the explanation. Acked-by: Song Liu <songliubraving@fb.com> > > > > > Thanks, > > Song > > > > > > > > > > #define _CHECK(condition, tag, duration, format...) ({ \ > > > @@ -1882,6 +1885,76 @@ static void test_queue_stack_map(int type) > > > bpf_object__close(obj); > > > } > > > > > > +#define CHECK_FLOW_KEYS(desc, got, expected) \ > > > + CHECK(memcmp(&got, &expected, sizeof(got)) != 0, \ > > > + desc, \ > > > + "nhoff=%u/%u " \ > > > + "thoff=%u/%u " \ > > > + "addr_proto=0x%x/0x%x " \ > > > + "is_frag=%u/%u " \ > > > + "is_first_frag=%u/%u " \ > > > + "is_encap=%u/%u " \ > > > + "n_proto=0x%x/0x%x " \ > > > + "sport=%u/%u " \ > > > + "dport=%u/%u\n", \ > > > + got.nhoff, expected.nhoff, \ > > > + got.thoff, expected.thoff, \ > > > + got.addr_proto, expected.addr_proto, \ > > > + got.is_frag, expected.is_frag, \ > > > + got.is_first_frag, expected.is_first_frag, \ > > > + got.is_encap, expected.is_encap, \ > > > + got.n_proto, expected.n_proto, \ > > > + got.sport, expected.sport, \ > > > + got.dport, expected.dport) > > > + > > > +static struct bpf_flow_keys pkt_v4_flow_keys = { > > > + .nhoff = 0, > > > + .thoff = sizeof(struct iphdr), > > > + .addr_proto = ETH_P_IP, > > > + .ip_proto = IPPROTO_TCP, > > > + .n_proto = bpf_htons(ETH_P_IP), > > > +}; > > > + > > > +static struct bpf_flow_keys pkt_v6_flow_keys = { > > > + .nhoff = 0, > > > + .thoff = sizeof(struct ipv6hdr), > > > + .addr_proto = ETH_P_IPV6, > > > + .ip_proto = IPPROTO_TCP, > > > + .n_proto = bpf_htons(ETH_P_IPV6), > > > +}; > > > + > > > +static void test_flow_dissector(void) > > > +{ > > > + struct bpf_flow_keys flow_keys; > > > + struct bpf_object *obj; > > > + __u32 duration, retval; > > > + int err, prog_fd; > > > + __u32 size; > > > + > > > + err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector", > > > + "jmp_table", &prog_fd); > > > + if (err) { > > > + error_cnt++; > > > + return; > > > + } > > > + > > > + err = bpf_prog_test_run(prog_fd, 10, &pkt_v4, sizeof(pkt_v4), > > > + &flow_keys, &size, &retval, &duration); > > > + CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv4", > > > + "err %d errno %d retval %d duration %d size %u/%lu\n", > > > + err, errno, retval, duration, size, sizeof(flow_keys)); > > > + CHECK_FLOW_KEYS("ipv4_flow_keys", flow_keys, pkt_v4_flow_keys); > > > + > > > + err = bpf_prog_test_run(prog_fd, 10, &pkt_v6, sizeof(pkt_v6), > > > + &flow_keys, &size, &retval, &duration); > > > + CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv6", > > > + "err %d errno %d retval %d duration %d size %u/%lu\n", > > > + err, errno, retval, duration, size, sizeof(flow_keys)); > > > + CHECK_FLOW_KEYS("ipv6_flow_keys", flow_keys, pkt_v6_flow_keys); > > > + > > > + bpf_object__close(obj); > > > +} > > > + > > > int main(void) > > > { > > > srand(time(NULL)); > > > @@ -1909,6 +1982,7 @@ int main(void) > > > test_reference_tracking(); > > > test_queue_stack_map(QUEUE); > > > test_queue_stack_map(STACK); > > > + test_flow_dissector(); > > > > > > printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt); > > > return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS; > > > -- > > > 2.20.1.321.g9e740568ce-goog > > >
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 70229de510f5..ade67470a152 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -19,6 +19,9 @@ all: $(TEST_CUSTOM_PROGS) $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id +flow_dissector_load.c: flow_dissector_load.h +test_run.c: flow_dissector_load.h + # Order correspond to 'make run_tests' order TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \ test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \ diff --git a/tools/testing/selftests/bpf/flow_dissector_load.c b/tools/testing/selftests/bpf/flow_dissector_load.c index ae8180b11d5f..77cafa66d048 100644 --- a/tools/testing/selftests/bpf/flow_dissector_load.c +++ b/tools/testing/selftests/bpf/flow_dissector_load.c @@ -12,6 +12,7 @@ #include <bpf/libbpf.h> #include "bpf_rlimit.h" +#include "flow_dissector_load.h" const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector"; const char *cfg_map_name = "jmp_table"; @@ -21,46 +22,13 @@ char *cfg_path_name; static void load_and_attach_program(void) { - struct bpf_program *prog, *main_prog; - struct bpf_map *prog_array; - int i, fd, prog_fd, ret; + int prog_fd, ret; struct bpf_object *obj; - int prog_array_fd; - ret = bpf_prog_load(cfg_path_name, BPF_PROG_TYPE_FLOW_DISSECTOR, &obj, - &prog_fd); + ret = bpf_flow_load(&obj, cfg_path_name, cfg_section_name, + cfg_map_name, &prog_fd); if (ret) - error(1, 0, "bpf_prog_load %s", cfg_path_name); - - main_prog = bpf_object__find_program_by_title(obj, cfg_section_name); - if (!main_prog) - error(1, 0, "bpf_object__find_program_by_title %s", - cfg_section_name); - - prog_fd = bpf_program__fd(main_prog); - if (prog_fd < 0) - error(1, 0, "bpf_program__fd"); - - prog_array = bpf_object__find_map_by_name(obj, cfg_map_name); - if (!prog_array) - error(1, 0, "bpf_object__find_map_by_name %s", cfg_map_name); - - prog_array_fd = bpf_map__fd(prog_array); - if (prog_array_fd < 0) - error(1, 0, "bpf_map__fd %s", cfg_map_name); - - i = 0; - bpf_object__for_each_program(prog, obj) { - fd = bpf_program__fd(prog); - if (fd < 0) - error(1, 0, "bpf_program__fd"); - - if (fd != prog_fd) { - printf("%d: %s\n", i, bpf_program__title(prog, false)); - bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); - ++i; - } - } + error(1, 0, "bpf_flow_load %s", cfg_path_name); ret = bpf_prog_attach(prog_fd, 0 /* Ignore */, BPF_FLOW_DISSECTOR, 0); if (ret) @@ -69,7 +37,6 @@ static void load_and_attach_program(void) ret = bpf_object__pin(obj, cfg_pin_path); if (ret) error(1, 0, "bpf_object__pin %s", cfg_pin_path); - } static void detach_program(void) diff --git a/tools/testing/selftests/bpf/flow_dissector_load.h b/tools/testing/selftests/bpf/flow_dissector_load.h new file mode 100644 index 000000000000..41dd6959feb0 --- /dev/null +++ b/tools/testing/selftests/bpf/flow_dissector_load.h @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ +#ifndef FLOW_DISSECTOR_LOAD +#define FLOW_DISSECTOR_LOAD + +#include <bpf/bpf.h> +#include <bpf/libbpf.h> + +static inline int bpf_flow_load(struct bpf_object **obj, + const char *path, + const char *section_name, + const char *map_name, + int *prog_fd) +{ + struct bpf_program *prog, *main_prog; + struct bpf_map *prog_array; + int prog_array_fd; + int ret, fd, i; + + ret = bpf_prog_load(path, BPF_PROG_TYPE_FLOW_DISSECTOR, obj, + prog_fd); + if (ret) + return ret; + + main_prog = bpf_object__find_program_by_title(*obj, section_name); + if (!main_prog) + return ret; + + *prog_fd = bpf_program__fd(main_prog); + if (*prog_fd < 0) + return ret; + + prog_array = bpf_object__find_map_by_name(*obj, map_name); + if (!prog_array) + return ret; + + prog_array_fd = bpf_map__fd(prog_array); + if (prog_array_fd < 0) + return ret; + + i = 0; + bpf_object__for_each_program(prog, *obj) { + fd = bpf_program__fd(prog); + if (fd < 0) + return fd; + + if (fd != *prog_fd) { + bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); + ++i; + } + } + + return 0; +} + +#endif /* FLOW_DISSECTOR_LOAD */ diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 126fc624290d..5f46680d4ad4 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -39,6 +39,7 @@ typedef __u16 __sum16; #include "bpf_endian.h" #include "bpf_rlimit.h" #include "trace_helpers.h" +#include "flow_dissector_load.h" static int error_cnt, pass_cnt; static bool jit_enabled; @@ -53,9 +54,10 @@ static struct { } __packed pkt_v4 = { .eth.h_proto = __bpf_constant_htons(ETH_P_IP), .iph.ihl = 5, - .iph.protocol = 6, + .iph.protocol = IPPROTO_TCP, .iph.tot_len = __bpf_constant_htons(MAGIC_BYTES), .tcp.urg_ptr = 123, + .tcp.doff = 5, }; /* ipv6 test vector */ @@ -65,9 +67,10 @@ static struct { struct tcphdr tcp; } __packed pkt_v6 = { .eth.h_proto = __bpf_constant_htons(ETH_P_IPV6), - .iph.nexthdr = 6, + .iph.nexthdr = IPPROTO_TCP, .iph.payload_len = __bpf_constant_htons(MAGIC_BYTES), .tcp.urg_ptr = 123, + .tcp.doff = 5, }; #define _CHECK(condition, tag, duration, format...) ({ \ @@ -1882,6 +1885,76 @@ static void test_queue_stack_map(int type) bpf_object__close(obj); } +#define CHECK_FLOW_KEYS(desc, got, expected) \ + CHECK(memcmp(&got, &expected, sizeof(got)) != 0, \ + desc, \ + "nhoff=%u/%u " \ + "thoff=%u/%u " \ + "addr_proto=0x%x/0x%x " \ + "is_frag=%u/%u " \ + "is_first_frag=%u/%u " \ + "is_encap=%u/%u " \ + "n_proto=0x%x/0x%x " \ + "sport=%u/%u " \ + "dport=%u/%u\n", \ + got.nhoff, expected.nhoff, \ + got.thoff, expected.thoff, \ + got.addr_proto, expected.addr_proto, \ + got.is_frag, expected.is_frag, \ + got.is_first_frag, expected.is_first_frag, \ + got.is_encap, expected.is_encap, \ + got.n_proto, expected.n_proto, \ + got.sport, expected.sport, \ + got.dport, expected.dport) + +static struct bpf_flow_keys pkt_v4_flow_keys = { + .nhoff = 0, + .thoff = sizeof(struct iphdr), + .addr_proto = ETH_P_IP, + .ip_proto = IPPROTO_TCP, + .n_proto = bpf_htons(ETH_P_IP), +}; + +static struct bpf_flow_keys pkt_v6_flow_keys = { + .nhoff = 0, + .thoff = sizeof(struct ipv6hdr), + .addr_proto = ETH_P_IPV6, + .ip_proto = IPPROTO_TCP, + .n_proto = bpf_htons(ETH_P_IPV6), +}; + +static void test_flow_dissector(void) +{ + struct bpf_flow_keys flow_keys; + struct bpf_object *obj; + __u32 duration, retval; + int err, prog_fd; + __u32 size; + + err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector", + "jmp_table", &prog_fd); + if (err) { + error_cnt++; + return; + } + + err = bpf_prog_test_run(prog_fd, 10, &pkt_v4, sizeof(pkt_v4), + &flow_keys, &size, &retval, &duration); + CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv4", + "err %d errno %d retval %d duration %d size %u/%lu\n", + err, errno, retval, duration, size, sizeof(flow_keys)); + CHECK_FLOW_KEYS("ipv4_flow_keys", flow_keys, pkt_v4_flow_keys); + + err = bpf_prog_test_run(prog_fd, 10, &pkt_v6, sizeof(pkt_v6), + &flow_keys, &size, &retval, &duration); + CHECK(size != sizeof(flow_keys) || err || retval != 1, "ipv6", + "err %d errno %d retval %d duration %d size %u/%lu\n", + err, errno, retval, duration, size, sizeof(flow_keys)); + CHECK_FLOW_KEYS("ipv6_flow_keys", flow_keys, pkt_v6_flow_keys); + + bpf_object__close(obj); +} + int main(void) { srand(time(NULL)); @@ -1909,6 +1982,7 @@ int main(void) test_reference_tracking(); test_queue_stack_map(QUEUE); test_queue_stack_map(STACK); + test_flow_dissector(); printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt); return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
Use existing pkt_v4 and pkt_v6 to make sure flow_keys are what we want. Also, add new bpf_flow_load routine (and flow_dissector_load.h header) that loads bpf_flow.o program and does all required setup. Signed-off-by: Stanislav Fomichev <sdf@google.com> --- tools/testing/selftests/bpf/Makefile | 3 + .../selftests/bpf/flow_dissector_load.c | 43 ++-------- .../selftests/bpf/flow_dissector_load.h | 55 +++++++++++++ tools/testing/selftests/bpf/test_progs.c | 78 ++++++++++++++++++- 4 files changed, 139 insertions(+), 40 deletions(-) create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h