On Mon, Apr 16, 2018 at 11:51:23PM -0700, Nikita V. Shirokov wrote:
> adding selftests for bpf_xdp_adjust_tail helper. in this syntetic test
> we are testing that 1) if data_end < data helper will return EINVAL
> 2) for normal use case packet's length would be reduced.
>
> aside from adding new tests i'm changing behaviour of bpf_prog_test_run
> so it would recalculate packet's length if only data_end pointer was
> changed
>
> Signed-off-by: Nikita V. Shirokov <tehn...@tehnerd.com>
> ---
> net/bpf/test_run.c | 3 ++-
> tools/include/uapi/linux/bpf.h | 11 ++++++++-
> tools/testing/selftests/bpf/Makefile | 2 +-
> tools/testing/selftests/bpf/bpf_helpers.h | 3 +++
> tools/testing/selftests/bpf/test_adjust_tail.c | 29 +++++++++++++++++++++++
> tools/testing/selftests/bpf/test_progs.c | 32
> ++++++++++++++++++++++++++
> 6 files changed, 77 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/test_adjust_tail.c
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 2ced48662c1f..68c3578343b4 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -170,7 +170,8 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const
> union bpf_attr *kattr,
> xdp.rxq = &rxqueue->xdp_rxq;
>
> retval = bpf_test_run(prog, &xdp, repeat, &duration);
> - if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN)
> + if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN ||
> + xdp.data_end != xdp.data + size)
please split fixing prog_test_run for adjust_tail into separate patch
and selftests into another one.
> size = xdp.data_end - xdp.data;
> ret = bpf_test_finish(kattr, uattr, xdp.data, size, retval, duration);
> kfree(data);
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 9d07465023a2..9a2d1a04eb24 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -755,6 +755,13 @@ union bpf_attr {
> * @addr: pointer to struct sockaddr to bind socket to
> * @addr_len: length of sockaddr structure
> * Return: 0 on success or negative error code
> + *
> + * int bpf_xdp_adjust_tail(xdp_md, delta)
> + * Adjust the xdp_md.data_end by delta. Only shrinking of packet's
> + * size is supported.
> + * @xdp_md: pointer to xdp_md
> + * @delta: A negative integer to be added to xdp_md.data_end
> + * Return: 0 on success or negative on error
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -821,7 +828,8 @@ union bpf_attr {
> FN(msg_apply_bytes), \
> FN(msg_cork_bytes), \
> FN(msg_pull_data), \
> - FN(bind),
> + FN(bind), \
> + FN(xdp_adjust_tail),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> @@ -864,6 +872,7 @@ enum bpf_func_id {
> /* BPF_FUNC_skb_set_tunnel_key flags. */
> #define BPF_F_ZERO_CSUM_TX (1ULL << 1)
> #define BPF_F_DONT_FRAGMENT (1ULL << 2)
> +#define BPF_F_SEQ_NUMBER (1ULL << 3)
William Tu missed adding it to tools/include/uapi/bpf.h when it was added
to main uapi/bpf.h
but don't add it as part of this patch.
I saw a separate patch for this passing by in tip tree from Arnaldo.
I'm not sure how quickly it will get into Linus tree,
let's not create extra merge conflicts.
>
> /* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
> * BPF_FUNC_perf_event_read_value flags.
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 0a315ddabbf4..3e819dc70bee 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -31,7 +31,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o
> test_tcp_estats.o test
> sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o \
> test_l4lb_noinline.o test_xdp_noinline.o test_stacktrace_map.o \
> sample_map_ret0.o test_tcpbpf_kern.o test_stacktrace_build_id.o \
> - sockmap_tcp_msg_prog.o connect4_prog.o connect6_prog.o
> + sockmap_tcp_msg_prog.o connect4_prog.o connect6_prog.o
> test_adjust_tail.o
>
> # Order correspond to 'make run_tests' order
> TEST_PROGS := test_kmod.sh \
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h
> b/tools/testing/selftests/bpf/bpf_helpers.h
> index d8223d99f96d..50c607014b22 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -96,6 +96,9 @@ static int (*bpf_msg_pull_data)(void *ctx, int start, int
> end, int flags) =
> (void *) BPF_FUNC_msg_pull_data;
> static int (*bpf_bind)(void *ctx, void *addr, int addr_len) =
> (void *) BPF_FUNC_bind;
> +static int (*bpf_xdp_adjust_tail)(void *ctx, int offset) =
> + (void *) BPF_FUNC_xdp_adjust_tail;
> +
>
> /* llvm builtin functions that eBPF C program may use to
> * emit BPF_LD_ABS and BPF_LD_IND instructions
> diff --git a/tools/testing/selftests/bpf/test_adjust_tail.c
> b/tools/testing/selftests/bpf/test_adjust_tail.c
> new file mode 100644
> index 000000000000..86239e792d6d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_adjust_tail.c
> @@ -0,0 +1,29 @@
> +/* Copyright (c) 2016,2017 Facebook
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + */
Please use SPDX header here and in other patches.
> +#include <linux/bpf.h>
> +#include <linux/if_ether.h>
> +#include "bpf_helpers.h"
> +
> +int _version SEC("version") = 1;
we really should fix libbpf to avoid requiring that for all program types.
It's annoying to see this in every networking test.
