Re: [PATCH bpf-next v3 5/6] selftests/bpf: add test for XDP metadata support in tun driver

[Stanislav Fomichev <stfomichev@gmail.com>]

On 02/26, Marcus Wichelmann wrote:
> Am 24.02.25 um 18:14 schrieb Stanislav Fomichev:
> > On 02/24, Marcus Wichelmann wrote:
> > > Add a selftest that creates a tap device, attaches XDP and TC programs,
> > > writes a packet with a test payload into the tap device and checks the
> > > test result. This test ensures that the XDP metadata support in the tun
> > > driver is enabled and that the metadata size is correctly passed to the
> > > skb.
> > > 
> > > See the previous commit ("selftests/bpf: refactor xdp_context_functional
> > > test and bpf program") for details about the test design.
> > > 
> > > Signed-off-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de>
> > > ---
> > >   .../bpf/prog_tests/xdp_context_test_run.c     | 64 +++++++++++++++++++
> > >   1 file changed, 64 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> > > index 4043f220d7c0..60aad6bd8882 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> > > @@ -8,6 +8,7 @@
> > >   #define TX_NAME "veth1"
> > >   #define TX_NETNS "xdp_context_tx"
> > >   #define RX_NETNS "xdp_context_rx"
> > > +#define TAP_NAME "tap0"
> > >   #define TEST_PAYLOAD_LEN 32
> > >   static const __u8 test_payload[TEST_PAYLOAD_LEN] = {
> > > @@ -251,3 +252,66 @@ void test_xdp_context_veth(void)
> > >   	netns_free(tx_ns);
> > >   }
> > > +void test_xdp_context_tuntap(void)
> > 
> > tap0 is already used by lwt tests, so there is a chance this new test
> > will clash with it? Should we run your new test in a net namespace
> > to be safe? Bastien recently added a change where you can make
> > your test run in a net ns by naming the function test_ns_xxx.
> > 
> 
> Ah, cool, I didn't know of that feature.
> 
> For reference, you probably mean this one?
> commit c047e0e0e435 ("selftests/bpf: Optionally open a dedicated namespace to run test in it")

Yes.
 
> As long as the tests are not run in parallel, the test function SHOULD
> clean up well enough behind itself that no conflicts should occur.
> But having that bit of extra safety won't hurt.

The test you're adding is gonna be running in parallel with most of the
other test cases. If you want it to be executed in a serial fashion
(which I don't think you do, running in parallel in a ns is a better
option), the function as to be called serial_test_xxx.