| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 28 Jan 2019 09:26:56 -0800
From: Song Liu <liu.song.a23@...il.com>
To: Stanislav Fomichev <sdf@...ichev.me>
Cc: Stanislav Fomichev <sdf@...gle.com>,
Networking <netdev@...r.kernel.org>,
"David S . Miller" <davem@...emloft.net>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH bpf-next v2 3/3] selftests/bpf: add simple
BPF_PROG_TEST_RUN examples for flow dissector
On Fri, Jan 25, 2019 at 4:58 PM Stanislav Fomichev <sdf@...ichev.me> wrote:
>
> On 01/25, Song Liu wrote:
> > On Thu, Jan 24, 2019 at 8:52 AM Stanislav Fomichev <sdf@...gle.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@...gle.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@...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
> > >
Powered by blists - more mailing lists