On Tue, May 3, 2022 at 10:15 AM Maxim Mikityanskiy <maximmi@xxxxxxxxxx> wrote:
>
> This commits allows the new BPF helpers to work in SKB context (in TC
> BPF programs): bpf_tcp_raw_{gen,check}_syncookie_ipv{4,6}.
>
> The sample application and selftest are updated to support the TC mode.
> It's not the recommended mode of operation, because the SKB is already
> created at this point, and it's unlikely that the BPF program will
> provide any substantional speedup compared to regular SYN cookies or
> synproxy.
>
> Signed-off-by: Maxim Mikityanskiy <maximmi@xxxxxxxxxx>
> Reviewed-by: Tariq Toukan <tariqt@xxxxxxxxxx>
> ---
> net/core/filter.c | 10 ++
> .../selftests/bpf/prog_tests/xdp_synproxy.c | 53 +++++--
> .../selftests/bpf/progs/xdp_synproxy_kern.c | 141 +++++++++++++-----
> tools/testing/selftests/bpf/xdp_synproxy.c | 94 +++++++++---
please split selftests and kernel code into separate patches (and use
selftests/bpf: prefix for selftests)
> 4 files changed, 230 insertions(+), 68 deletions(-)
>
[...]
> @@ -87,7 +112,11 @@ void test_xdp_synproxy(void)
> if (!ASSERT_OK_PTR(ns, "setns"))
> goto out;
>
> - ctrl_file = SYS_OUT("./xdp_synproxy --iface tmp1 --single");
> + if (xdp)
> + ctrl_file = SYS_OUT("./xdp_synproxy --iface tmp1 --single");
> + else
> + ctrl_file = SYS_OUT("./xdp_synproxy --prog %s --single",
> + prog_id);
> size = fread(buf, 1, sizeof(buf), ctrl_file);
> pclose(ctrl_file);
> if (!ASSERT_TRUE(expect_str(buf, size, "Total SYNACKs generated: 1\n"),
> @@ -107,3 +136,9 @@ void test_xdp_synproxy(void)
> system("ip link del tmp0");
> system("ip netns del synproxy");
> }
> +
> +void test_xdp_synproxy(void)
> +{
> + test_synproxy(true);
> + test_synproxy(false);
let's model this as subtests instead? See test__start_subtest() use in
other selftests
> +}
> diff --git a/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c b/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c
> index 9ae85b189072..f70b5f776dcf 100644
> --- a/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c
> +++ b/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c
> @@ -7,6 +7,9 @@
> #include <bpf/bpf_endian.h>
> #include <asm/errno.h>
>
[...]
> @@ -201,21 +220,50 @@ static int syncookie_attach(const char *argv0, unsigned int ifindex)
> fprintf(stderr, "Error: bpf_obj_get_info_by_fd: %s\n", strerror(-err));
> goto out;
> }
> + attached_tc = tc;
> attached_prog_id = info.id;
> signal(SIGINT, cleanup);
> signal(SIGTERM, cleanup);
> - err = bpf_xdp_attach(ifindex, prog_fd, XDP_FLAGS_UPDATE_IF_NOEXIST, NULL);
> - if (err < 0) {
> - fprintf(stderr, "Error: bpf_set_link_xdp_fd: %s\n", strerror(-err));
> - signal(SIGINT, SIG_DFL);
> - signal(SIGTERM, SIG_DFL);
> - attached_prog_id = 0;
> - goto out;
> + if (tc) {
> + DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook,
nit: LIBBPF_OPTS is shorter, DECLARE_LIBBPF_OPTS is discouraged
> + .ifindex = ifindex,
> + .attach_point = BPF_TC_INGRESS);
> + DECLARE_LIBBPF_OPTS(bpf_tc_opts, opts,
> + .handle = 1,
> + .priority = 1,
> + .prog_fd = prog_fd);
> +
[...]
