mbox series

[bpf-next,v2,00/15] selftests/xsk: speed-ups, fixes, and new XDP programs

Message ID 20230104121744.2820-1-magnus.karlsson@gmail.com (mailing list archive)
Headers show
Series selftests/xsk: speed-ups, fixes, and new XDP programs | expand

Message

Magnus Karlsson Jan. 4, 2023, 12:17 p.m. UTC
This is a patch set of various performance improvements, fixes and the
introduction of more than one XDP program to the xsk selftests
framework so we can test more things in the future such as upcoming
multi-buffer and metadata support for AF_XDP. The new programs just
reuses the framework that all the other eBPF selftests use. The new
feature is used to implement one new test that does XDP_DROP on every
other packet. More tests using this will be added in future commits.

Contents:

* The run-time of the test suite is cut by 10x when executing the
  tests on a real NIC, by only attaching the XDP program once per mode
  tested, instead of once per test program.

* Over 700 lines of code have been removed. The xsk.c control file was
  moved straight over from libbpf when the xsk support was deprecated
  there. As it is now not used as library code that has to work with
  all kinds of versions of Linux, a lot of code could be dropped or
  simplified.

* Add a new command line option "-d" that can be used when a test
  fails and you want to debug it with gdb or some other debugger. The
  option creates the two veth netdevs and prints them to the screen
  without deleting them afterwards. This way these veth netdevs can be
  used when running xskxceiver in a debugger.

* Implemented the possibility to load external XDP programs so we can
  have more than the default one. This feature is used to implement a
  test where every other packet is dropped. Good exercise for the
  recycling mechanism of the xsk buffer pool used in zero-copy mode.

* Various clean-ups and small fixes in patches 1 to 5. None of these
  fixes has any impact on the correct execution of the tests when they
  pass, though they can be irritating when a test fails. IMHO, they do
  not need to go to bpf as they will not fix anything there. The first
  version of patches 1, 2, and 4 where previously sent to bpf, but has
  now been included here.

v1 -> v2:
* Fixed spelling error in commit message of patch #6 [Björn]
* Added explanation on why it is safe to use C11 atomics in patch #7
  [Daniel]
* Put all XDP programs in the same file so that adding more XDP
  programs to xskxceiver.c becomes more scalable in patches #11 and
  #12 [Maciej]
* Removed more dead code in patch #8 [Maciej]
* Removed stale %s specifier in error print, patch #9 [Maciej]
* Changed name of XDP_CONSUMES_SOME_PACKETS to XDP_DROP_HALF to
  hopefully make it clearer [Maciej]
* ifobj_rx and ifobj_tx name changes in patch #13 [Maciej]
* Simplified XDP attachment code in patch #15 [Maciej]

Patches:
1-5:   Small fixes and clean-ups
6:     New convenient debug option when using a debugger such as gdb
7-8:   Removal of unnecessary code
9:     Add the ability to load external XDP programs
10-11: Removal of more unnecessary code
12:    Implement a new test where every other packet is XDP_DROP:ed
13:    Unify the thread dispatching code
14-15: Simplify the way tests are written when using custom packet_streams
       or custom XDP programs

Thanks: Magnus

Magnus Karlsson (15):
  selftests/xsk: print correct payload for packet dump
  selftests/xsk: do not close unused file descriptors
  selftests/xsk: submit correct number of frames in populate_fill_ring
  selftests/xsk: print correct error codes when exiting
  selftests/xsk: remove unused variable outstanding_tx
  selftests/xsk: add debug option for creating netdevs
  selftests/xsk: replace asm acquire/release implementations
  selftests/xsk: remove namespaces
  selftests/xsk: load and attach XDP program only once per mode
  selftests/xsk: remove unnecessary code in control path
  selftests/xsk: get rid of built-in XDP program
  selftests/xsk: add test when some packets are XDP_DROPed
  selftests/xsk: merge dual and single thread dispatchers
  selftests/xsk: automatically restore packet stream
  selftests/xsk: automatically switch XDP programs

 tools/testing/selftests/bpf/Makefile          |   2 +-
 .../selftests/bpf/progs/xsk_xdp_progs.c       |  30 +
 tools/testing/selftests/bpf/test_xsk.sh       |  42 +-
 tools/testing/selftests/bpf/xsk.c             | 674 +-----------------
 tools/testing/selftests/bpf/xsk.h             |  97 +--
 tools/testing/selftests/bpf/xsk_prereqs.sh    |  12 +-
 tools/testing/selftests/bpf/xskxceiver.c      | 382 +++++-----
 tools/testing/selftests/bpf/xskxceiver.h      |  17 +-
 8 files changed, 308 insertions(+), 948 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c


base-commit: bb5747cfbc4b7fe29621ca6cd4a695d2723bf2e8
--
2.34.1

Comments

Fijalkowski, Maciej Jan. 10, 2023, noon UTC | #1
On Wed, Jan 04, 2023 at 01:17:29PM +0100, Magnus Karlsson wrote:
> This is a patch set of various performance improvements, fixes and the
> introduction of more than one XDP program to the xsk selftests
> framework so we can test more things in the future such as upcoming
> multi-buffer and metadata support for AF_XDP. The new programs just
> reuses the framework that all the other eBPF selftests use. The new
> feature is used to implement one new test that does XDP_DROP on every
> other packet. More tests using this will be added in future commits.
> 
> Contents:
> 
> * The run-time of the test suite is cut by 10x when executing the
>   tests on a real NIC, by only attaching the XDP program once per mode
>   tested, instead of once per test program.
> 
> * Over 700 lines of code have been removed. The xsk.c control file was
>   moved straight over from libbpf when the xsk support was deprecated
>   there. As it is now not used as library code that has to work with
>   all kinds of versions of Linux, a lot of code could be dropped or
>   simplified.
> 
> * Add a new command line option "-d" that can be used when a test
>   fails and you want to debug it with gdb or some other debugger. The
>   option creates the two veth netdevs and prints them to the screen
>   without deleting them afterwards. This way these veth netdevs can be
>   used when running xskxceiver in a debugger.
> 
> * Implemented the possibility to load external XDP programs so we can
>   have more than the default one. This feature is used to implement a
>   test where every other packet is dropped. Good exercise for the
>   recycling mechanism of the xsk buffer pool used in zero-copy mode.
> 
> * Various clean-ups and small fixes in patches 1 to 5. None of these
>   fixes has any impact on the correct execution of the tests when they
>   pass, though they can be irritating when a test fails. IMHO, they do
>   not need to go to bpf as they will not fix anything there. The first
>   version of patches 1, 2, and 4 where previously sent to bpf, but has
>   now been included here.
> 
> v1 -> v2:
> * Fixed spelling error in commit message of patch #6 [Björn]
> * Added explanation on why it is safe to use C11 atomics in patch #7
>   [Daniel]
> * Put all XDP programs in the same file so that adding more XDP
>   programs to xskxceiver.c becomes more scalable in patches #11 and
>   #12 [Maciej]
> * Removed more dead code in patch #8 [Maciej]
> * Removed stale %s specifier in error print, patch #9 [Maciej]
> * Changed name of XDP_CONSUMES_SOME_PACKETS to XDP_DROP_HALF to
>   hopefully make it clearer [Maciej]
> * ifobj_rx and ifobj_tx name changes in patch #13 [Maciej]
> * Simplified XDP attachment code in patch #15 [Maciej]

I had minor comments on last patch which you can take or not.
From my side it is an ack for whole series:

Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

However you probably would like to ping Daniel against patch 7.
Also, usage of printf vs ksft_print_msg seems sort of random throughout
this series but it's not a big deal.

Thanks!

> 
> Patches:
> 1-5:   Small fixes and clean-ups
> 6:     New convenient debug option when using a debugger such as gdb
> 7-8:   Removal of unnecessary code
> 9:     Add the ability to load external XDP programs
> 10-11: Removal of more unnecessary code
> 12:    Implement a new test where every other packet is XDP_DROP:ed
> 13:    Unify the thread dispatching code
> 14-15: Simplify the way tests are written when using custom packet_streams
>        or custom XDP programs
> 
> Thanks: Magnus
> 
> Magnus Karlsson (15):
>   selftests/xsk: print correct payload for packet dump
>   selftests/xsk: do not close unused file descriptors
>   selftests/xsk: submit correct number of frames in populate_fill_ring
>   selftests/xsk: print correct error codes when exiting
>   selftests/xsk: remove unused variable outstanding_tx
>   selftests/xsk: add debug option for creating netdevs
>   selftests/xsk: replace asm acquire/release implementations
>   selftests/xsk: remove namespaces
>   selftests/xsk: load and attach XDP program only once per mode
>   selftests/xsk: remove unnecessary code in control path
>   selftests/xsk: get rid of built-in XDP program
>   selftests/xsk: add test when some packets are XDP_DROPed
>   selftests/xsk: merge dual and single thread dispatchers
>   selftests/xsk: automatically restore packet stream
>   selftests/xsk: automatically switch XDP programs
> 
>  tools/testing/selftests/bpf/Makefile          |   2 +-
>  .../selftests/bpf/progs/xsk_xdp_progs.c       |  30 +
>  tools/testing/selftests/bpf/test_xsk.sh       |  42 +-
>  tools/testing/selftests/bpf/xsk.c             | 674 +-----------------
>  tools/testing/selftests/bpf/xsk.h             |  97 +--
>  tools/testing/selftests/bpf/xsk_prereqs.sh    |  12 +-
>  tools/testing/selftests/bpf/xskxceiver.c      | 382 +++++-----
>  tools/testing/selftests/bpf/xskxceiver.h      |  17 +-
>  8 files changed, 308 insertions(+), 948 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> 
> 
> base-commit: bb5747cfbc4b7fe29621ca6cd4a695d2723bf2e8
> --
> 2.34.1
Magnus Karlsson Jan. 10, 2023, 12:10 p.m. UTC | #2
On Tue, Jan 10, 2023 at 1:00 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Wed, Jan 04, 2023 at 01:17:29PM +0100, Magnus Karlsson wrote:
> > This is a patch set of various performance improvements, fixes and the
> > introduction of more than one XDP program to the xsk selftests
> > framework so we can test more things in the future such as upcoming
> > multi-buffer and metadata support for AF_XDP. The new programs just
> > reuses the framework that all the other eBPF selftests use. The new
> > feature is used to implement one new test that does XDP_DROP on every
> > other packet. More tests using this will be added in future commits.
> >
> > Contents:
> >
> > * The run-time of the test suite is cut by 10x when executing the
> >   tests on a real NIC, by only attaching the XDP program once per mode
> >   tested, instead of once per test program.
> >
> > * Over 700 lines of code have been removed. The xsk.c control file was
> >   moved straight over from libbpf when the xsk support was deprecated
> >   there. As it is now not used as library code that has to work with
> >   all kinds of versions of Linux, a lot of code could be dropped or
> >   simplified.
> >
> > * Add a new command line option "-d" that can be used when a test
> >   fails and you want to debug it with gdb or some other debugger. The
> >   option creates the two veth netdevs and prints them to the screen
> >   without deleting them afterwards. This way these veth netdevs can be
> >   used when running xskxceiver in a debugger.
> >
> > * Implemented the possibility to load external XDP programs so we can
> >   have more than the default one. This feature is used to implement a
> >   test where every other packet is dropped. Good exercise for the
> >   recycling mechanism of the xsk buffer pool used in zero-copy mode.
> >
> > * Various clean-ups and small fixes in patches 1 to 5. None of these
> >   fixes has any impact on the correct execution of the tests when they
> >   pass, though they can be irritating when a test fails. IMHO, they do
> >   not need to go to bpf as they will not fix anything there. The first
> >   version of patches 1, 2, and 4 where previously sent to bpf, but has
> >   now been included here.
> >
> > v1 -> v2:
> > * Fixed spelling error in commit message of patch #6 [Björn]
> > * Added explanation on why it is safe to use C11 atomics in patch #7
> >   [Daniel]
> > * Put all XDP programs in the same file so that adding more XDP
> >   programs to xskxceiver.c becomes more scalable in patches #11 and
> >   #12 [Maciej]
> > * Removed more dead code in patch #8 [Maciej]
> > * Removed stale %s specifier in error print, patch #9 [Maciej]
> > * Changed name of XDP_CONSUMES_SOME_PACKETS to XDP_DROP_HALF to
> >   hopefully make it clearer [Maciej]
> > * ifobj_rx and ifobj_tx name changes in patch #13 [Maciej]
> > * Simplified XDP attachment code in patch #15 [Maciej]
>
> I had minor comments on last patch which you can take or not.
> From my side it is an ack for whole series:
>
> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>
> However you probably would like to ping Daniel against patch 7.
> Also, usage of printf vs ksft_print_msg seems sort of random throughout
> this series but it's not a big deal.

Yep, it has been more or less random from day one, unfortunately. I
will look into it for the next patch set of fixes.

> Thanks!
>
> >
> > Patches:
> > 1-5:   Small fixes and clean-ups
> > 6:     New convenient debug option when using a debugger such as gdb
> > 7-8:   Removal of unnecessary code
> > 9:     Add the ability to load external XDP programs
> > 10-11: Removal of more unnecessary code
> > 12:    Implement a new test where every other packet is XDP_DROP:ed
> > 13:    Unify the thread dispatching code
> > 14-15: Simplify the way tests are written when using custom packet_streams
> >        or custom XDP programs
> >
> > Thanks: Magnus
> >
> > Magnus Karlsson (15):
> >   selftests/xsk: print correct payload for packet dump
> >   selftests/xsk: do not close unused file descriptors
> >   selftests/xsk: submit correct number of frames in populate_fill_ring
> >   selftests/xsk: print correct error codes when exiting
> >   selftests/xsk: remove unused variable outstanding_tx
> >   selftests/xsk: add debug option for creating netdevs
> >   selftests/xsk: replace asm acquire/release implementations
> >   selftests/xsk: remove namespaces
> >   selftests/xsk: load and attach XDP program only once per mode
> >   selftests/xsk: remove unnecessary code in control path
> >   selftests/xsk: get rid of built-in XDP program
> >   selftests/xsk: add test when some packets are XDP_DROPed
> >   selftests/xsk: merge dual and single thread dispatchers
> >   selftests/xsk: automatically restore packet stream
> >   selftests/xsk: automatically switch XDP programs
> >
> >  tools/testing/selftests/bpf/Makefile          |   2 +-
> >  .../selftests/bpf/progs/xsk_xdp_progs.c       |  30 +
> >  tools/testing/selftests/bpf/test_xsk.sh       |  42 +-
> >  tools/testing/selftests/bpf/xsk.c             | 674 +-----------------
> >  tools/testing/selftests/bpf/xsk.h             |  97 +--
> >  tools/testing/selftests/bpf/xsk_prereqs.sh    |  12 +-
> >  tools/testing/selftests/bpf/xskxceiver.c      | 382 +++++-----
> >  tools/testing/selftests/bpf/xskxceiver.h      |  17 +-
> >  8 files changed, 308 insertions(+), 948 deletions(-)
> >  create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> >
> >
> > base-commit: bb5747cfbc4b7fe29621ca6cd4a695d2723bf2e8
> > --
> > 2.34.1