Subject: Re: [PATCH v2 bpf-next 14/17] selftests: xsk: implement bpf_link test

From: Maciej Fijalkowski <[email-masked]>
Date: Mon, 22 Mar 2021 21:11:02 +0100

On Mon, Mar 15, 2021 at 10:39:38PM -0700, Andrii Nakryiko wrote:
> On Thu, Mar 11, 2021 at 7:43 AM Maciej Fijalkowski
> <[email-masked]> wrote:
> >
> > Introduce a test that is supposed to verify the persistence of BPF
> > resources based on underlying bpf_link usage.
> >
> > Test will:
> > 1) create and bind two sockets on queue ids 0 and 1
> > 2) run a traffic on queue ids 0
> > 3) remove xsk sockets from queue 0 on both veth interfaces
> > 4) run a traffic on queues ids 1
> >
> > Running traffic successfully on qids 1 means that BPF resources were
> > not removed on step 3).
> >
> > In order to make it work, change the command that creates veth pair to
> > have the 4 queue pairs by default.
> >
> > Introduce the arrays of xsks and umems to ifobject struct but keep a
> > pointers to single entities, so rest of the logic around Rx/Tx can be
> > kept as-is.
> >
> > For umem handling, double the size of mmapped space and split that
> > between the two sockets.
> >
> > Rename also bidi_pass to a variable 'second_step' of a boolean type as
> > it's now used also for the test that is introduced here and it doesn't
> > have anything in common with bi-directional testing.
> >
> > Drop opt_queue command line argument as it wasn't working before anyway.
> >
> > Signed-off-by: Maciej Fijalkowski <[email-masked]>
> > ---
> >  tools/testing/selftests/bpf/test_xsk.sh  |   2 +-
> >  tools/testing/selftests/bpf/xdpxceiver.c | 179 +++++++++++++++++------
> >  tools/testing/selftests/bpf/xdpxceiver.h |   7 +-
> >  3 files changed, 138 insertions(+), 50 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > index 56d4474e2c83..2a00b8222475 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -107,7 +107,7 @@ setup_vethPairs() {
> >                 echo "setting up ${VETH0}: namespace: ${NS0}"
> >         fi
> >         ip netns add ${NS1}
> > -       ip link add ${VETH0} type veth peer name ${VETH1}
> > +       ip link add ${VETH0} numtxqueues 4 numrxqueues 4 type veth peer name ${VETH1} numtxqueues 4 numrxqueues 4
> >         if [ -f /proc/net/if_inet6 ]; then
> >                 echo 1 > /proc/sys/net/ipv6/conf/${VETH0}/disable_ipv6
> >         fi
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> > index aef5840e1c24..dc775ee139c5 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.c
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> > @@ -41,8 +41,12 @@
> >   *            Reduce the size of the RX ring to a fraction of the fill ring size.
> >   *       iv.  fill queue empty
> >   *            Do not populate the fill queue and then try to receive pkts.
> > + *    f. bpf_link resource persitence
> 
> typo: persistence

I'll fix it.

> 
> > + *       Configure sockets at indexes 0 and 1,run a traffic on queue ids 0,
> 
> while I'm here nitting :) space after comma?

I like nitpicking, yeah let's add that space.

> 
> > + *       then remove xsk sockets from queue 0 on both veth interfaces and
> > + *       finally run a traffic on queues ids 1
> >   *
> > - * Total tests: 10
> > + * Total tests: 12
> >   *
> >   * Flow:
> >   * -----
> 
> [...]