On Sat, 16 May 2020 21:02:01 -0700
Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote:
> On Thu, May 14, 2020 at 3:51 AM Jesper Dangaard Brouer
> <brouer@xxxxxxxxxx> wrote:
> >
> > Update the memory requirements, when adding xdp.frame_sz in BPF test_run
> > function bpf_prog_test_run_xdp() which e.g. is used by XDP selftests.
> >
> > Specifically add the expected reserved tailroom, but also allocated a
> > larger memory area to reflect that XDP frames usually comes in this
> > format. Limit the provided packet data size to 4096 minus headroom +
> > tailroom, as this also reflect a common 3520 bytes MTU limit with XDP.
> >
> > Note that bpf_test_init already use a memory allocation method that clears
> > memory. Thus, this already guards against leaking uninit kernel memory.
> >
> > Signed-off-by: Jesper Dangaard Brouer <brouer@xxxxxxxxxx>
> > ---
> > net/bpf/test_run.c | 16 ++++++++++++----
> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index 29dbdd4c29f6..30ba7d38941d 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -470,25 +470,34 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> > int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
> > union bpf_attr __user *uattr)
> > {
> > + u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> > + u32 headroom = XDP_PACKET_HEADROOM;
> > u32 size = kattr->test.data_size_in;
> > u32 repeat = kattr->test.repeat;
> > struct netdev_rx_queue *rxqueue;
> > struct xdp_buff xdp = {};
> > u32 retval, duration;
> > + u32 max_data_sz;
> > void *data;
> > int ret;
> >
> > if (kattr->test.ctx_in || kattr->test.ctx_out)
> > return -EINVAL;
> >
> > - data = bpf_test_init(kattr, size, XDP_PACKET_HEADROOM + NET_IP_ALIGN, 0);
> > + /* XDP have extra tailroom as (most) drivers use full page */
> > + max_data_sz = 4096 - headroom - tailroom;
> > + if (size > max_data_sz)
> > + return -EINVAL;
> > +
> > + data = bpf_test_init(kattr, max_data_sz, headroom, tailroom);
>
> Hi Jesper,
>
> above is buggy.
> max_data_sz is way more than what user space has.
> That causes xdp unit tests to fail sporadically with EFAULT.
> Like:
> ./test_progs -t xdp_perf
> test_xdp_perf:FAIL:xdp-perf err -1 errno 14 retval 0 size 0
> #89 xdp_perf:FAIL
>
> For that test max_data_sz will be 3520 while user space prepared only 128 bytes
> and copy_from_user will fault.
I'm looking into this... BUT
... I'm getting unrelated compile errors for selftests/bpf in
bpf-next tree (HEAD 96586dd9268d2).
The compile error, see below signature, happens in ./progs/bpf_iter_ipv6_route.c
(tools/testing/selftests/bpf/progs/bpf_iter_ipv6_route.c).
Related to commit:
7c128a6bbd4f ("tools/bpf: selftests: Add iterator programs for ipv6_route and netlink") (Author: Yonghong Song)
-
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
$ make
CLNG-LLC [test_maps] bpf_iter_ipv6_route.o
progs/bpf_iter_ipv6_route.c:18:28: warning: declaration of 'struct bpf_iter__ipv6_route' will not be visible outside of this function [-Wvisibility]
int dump_ipv6_route(struct bpf_iter__ipv6_route *ctx)
^
progs/bpf_iter_ipv6_route.c:20:28: error: incomplete definition of type 'struct bpf_iter__ipv6_route'
struct seq_file *seq = ctx->meta->seq;
~~~^
progs/bpf_iter_ipv6_route.c:18:28: note: forward declaration of 'struct bpf_iter__ipv6_route'
int dump_ipv6_route(struct bpf_iter__ipv6_route *ctx)
^
progs/bpf_iter_ipv6_route.c:21:28: error: incomplete definition of type 'struct bpf_iter__ipv6_route'
struct fib6_info *rt = ctx->rt;
~~~^
progs/bpf_iter_ipv6_route.c:18:28: note: forward declaration of 'struct bpf_iter__ipv6_route'
int dump_ipv6_route(struct bpf_iter__ipv6_route *ctx)
^
1 warning and 2 errors generated.
llc: error: llc: <stdin>:1:1: error: expected top-level entity
BPF obj compilation failed
^
make: *** [Makefile:365: /home/jbrouer/git/kernel/bpf-next/tools/testing/selftests/bpf/bpf_iter_ipv6_route.o] Error 1
