netdev - Re: [PATCH bpf-next 00/10] seltests/xsk: prepare for AF

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJ8uoz3jf2xdmaDYiBZ3jcM4G1-9h5ngXXEpmnU8iwKwp9PZdw@mail.gmail.com>
Date: Tue, 16 May 2023 09:45:24 +0200
From: Magnus Karlsson <magnus.karlsson@...il.com>
To: Daniel Borkmann <daniel@...earbox.net>
Cc: magnus.karlsson@...el.com, bjorn@...nel.org, ast@...nel.org, 
	netdev@...r.kernel.org, maciej.fijalkowski@...el.com, bpf@...r.kernel.org, 
	yhs@...com, andrii@...nel.org, martin.lau@...ux.dev, song@...nel.org, 
	john.fastabend@...il.com, kpsingh@...nel.org, sdf@...gle.com, 
	haoluo@...gle.com, jolsa@...nel.org, tirthendu.sarkar@...el.com
Subject: Re: [PATCH bpf-next 00/10] seltests/xsk: prepare for AF_XDP
 multi-buffer testing

On Mon, 15 May 2023 at 23:12, Daniel Borkmann <daniel@...earbox.net> wrote:
>
> Hi Magnus,
>
> On 5/12/23 11:20 AM, Magnus Karlsson wrote:
> >
> > Prepare the AF_XDP selftests test framework code for the upcoming
> > multi-buffer support in AF_XDP. This so that the multi-buffer patch
> > set does not become way too large. In that upcoming patch set, we are
> > only including the multi-buffer tests together with any framework
> > code that depends on the new options bit introduced in the AF_XDP
> > multi-buffer implementation itself.
> >
> > Currently, the test framework is based on the premise that a packet
> > consists of a single fragment and thus occupies a single buffer and a
> > single descriptor. Multi-buffer breaks this assumption, as that is the
> > whole purpose of it. Now, a packet can consist of multiple buffers and
> > therefore consume multiple descriptors.
> >
> > The patch set starts with some clean-ups and simplifications followed
> > by patches that make sure that the current code works even when a
> > packet occupies multiple buffers. The actual code for sending and
> > receiving multi-buffer packets will be included in the AF_XDP
> > multi-buffer patch set as it depends on a new bit being used in the
> > options field of the descriptor.
> >
> > Patch set anatomy:
> > 1: The XDP program was unnecessarily changed many times. Fixes this.
> >
> > 2: There is no reason to generate a full UDP/IPv4 packet as it is
> >     never used. Simplify the code by just generating a valid Ethernet
> >     frame.
> >
> > 3: Introduce a more complicated payload pattern that can detect
> >     fragments out of bounds in a multi-buffer packet and other errors
> >     found in single-fragment packets.
> >
> > 4: As a convenience, dump the content of the faulty packet at error.
> >
> > 5: To simplify the code, make the usage of the packet stream for Tx
> >     and Rx more similar.
> >
> > 6: Store the offset of the packet in the buffer in the struct pkt
> >     definition instead of the address in the umem itself and introduce
> >     a simple buffer allocator. The address only made sense when all
> >     packets consumed a single buffer. Now, we do not know beforehand
> >     how many buffers a packet will consume, so we instead just allocate
> >     a buffer from the allocator and specify the offset within that
> >     buffer.
> >
> > 7: Test for huge pages only once instead of before each test that needs it.
> >
> > 8: Populate the fill ring based on how many frags are needed for each
> >     packet.
> >
> > 9: Change the data generation code so it can generate data for
> >     multi-buffer packets too.
> >
> > 10: Adjust the packet pacing algorithm so that it can cope with
> >      multi-buffer packets. The pacing algorithm is present so that Tx
> >      does not send too many packets/frames to Rx that it starts to drop
> >      packets. That would ruin the tests.
>
> This triggers build error in BPF CI:

Thanks Daniel. Will fix.

>    https://github.com/kernel-patches/bpf/actions/runs/4984982413/jobs/8924047266
>
>    [...]
>    xskxceiver.c:1881:2: error: variable 'ret' is used uninitialized whenever switch default is taken [-Werror,-Wsometimes-uninitialized]
>            default:
>            ^~~~~~~
>    xskxceiver.c:1885:6: note: uninitialized use occurs here
>            if (ret == TEST_PASS)
>                ^~~
>    xskxceiver.c:1779:9: note: initialize the variable 'ret' to silence this warning
>      GEN-SKEL [test_progs] test_subskeleton.skel.h
>      GEN-SKEL [test_progs] test_subskeleton_lib.skel.h
>            int ret;
>                   ^
>                    = 0
>    1 error generated.
>    make: *** [Makefile:617: /tmp/work/bpf/bpf/tools/testing/selftests/bpf/xskxceiver] Error 1
>    make: *** Waiting for unfinished jobs....
>      GEN-SKEL [test_progs] test_usdt.skel.h
>    make: Leaving directory '/tmp/work/bpf/bpf/tools/testing/selftests/bpf'
>
> Pls fix and respin, thanks.