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] [thread-next>] [day] [month] [year] [list]

Message-ID: <9e553914-3703-8f10-b3b8-7d7e90462417@iogearbox.net>
Date: Mon, 15 May 2023 23:12:52 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Magnus Karlsson <magnus.karlsson@...il.com>, 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

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:

   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.