mbox series

[bpf-next,v4,0/4] selftests/bpf: convert three other cgroup tests to test_progs

Message ID 20240813-convert_cgroup_tests-v4-0-a33c03458cf6@bootlin.com
Headers show
Series selftests/bpf: convert three other cgroup tests to test_progs | expand

Message

Alexis Lothoré Aug. 13, 2024, 12:45 p.m. UTC
Hello,
this series brings a new set of test converted to the test_progs framework.
Since the tests are quite small, I chose to group three tests conversion in
the same series, but feel free to let me know if I should keep one series
per test. The series focuses on cgroup testing and converts the following
tests:
- get_cgroup_id_user
- cgroup_storage
- test_skb_cgroup_id_user

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
Changes in v4:
- Fix test after netns addition by making sure loopack interface is up
- Link to v3: https://lore.kernel.org/r/20240812-convert_cgroup_tests-v3-0-47ac6ce4e88b@bootlin.com

Changes in v3:
- Fixed multiple leaks on cgroup file descriptors and sockets
- Used dedicated network namespaces for tests involving network
- Link to v2: https://lore.kernel.org/r/20240806-convert_cgroup_tests-v2-0-180c57e5b710@bootlin.com

Changes in v2:
- Use global variables instead of maps when possible
- Collect review tags from Alan
- Link to v1: https://lore.kernel.org/r/20240731-convert_cgroup_tests-v1-0-14cbc51b6947@bootlin.com

---
Alexis Lothoré (eBPF Foundation) (4):
      selftests/bpf: convert get_current_cgroup_id_user to test_progs
      selftests/bpf: convert test_cgroup_storage to test_progs
      selftests/bpf: add proper section name to bpf prog and rename it
      selftests/bpf: convert test_skb_cgroup_id_user to test_progs

 tools/testing/selftests/bpf/.gitignore             |   3 -
 tools/testing/selftests/bpf/Makefile               |   8 +-
 tools/testing/selftests/bpf/get_cgroup_id_user.c   | 151 -----------------
 .../selftests/bpf/prog_tests/cgroup_ancestor.c     | 169 +++++++++++++++++++
 .../bpf/prog_tests/cgroup_get_current_cgroup_id.c  |  46 ++++++
 .../selftests/bpf/prog_tests/cgroup_storage.c      |  96 +++++++++++
 ...test_skb_cgroup_id_kern.c => cgroup_ancestor.c} |  14 +-
 tools/testing/selftests/bpf/progs/cgroup_storage.c |  24 +++
 .../selftests/bpf/progs/get_cgroup_id_kern.c       |  26 +--
 tools/testing/selftests/bpf/test_cgroup_storage.c  | 174 --------------------
 tools/testing/selftests/bpf/test_skb_cgroup_id.sh  |  63 -------
 .../selftests/bpf/test_skb_cgroup_id_user.c        | 183 ---------------------
 12 files changed, 344 insertions(+), 613 deletions(-)
---
base-commit: ab2c4aa104050a184c3411a973b165285549f732
change-id: 20240725-convert_cgroup_tests-d07c66053225

Best regards,

Comments

patchwork-bot+netdevbpf@kernel.org Aug. 15, 2024, 2:10 a.m. UTC | #1
Hello:

This series was applied to bpf/bpf-next.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:

On Tue, 13 Aug 2024 14:45:04 +0200 you wrote:
> Hello,
> this series brings a new set of test converted to the test_progs framework.
> Since the tests are quite small, I chose to group three tests conversion in
> the same series, but feel free to let me know if I should keep one series
> per test. The series focuses on cgroup testing and converts the following
> tests:
> - get_cgroup_id_user
> - cgroup_storage
> - test_skb_cgroup_id_user
> 
> [...]

Here is the summary with links:
  - [bpf-next,v4,1/4] selftests/bpf: convert get_current_cgroup_id_user to test_progs
    https://git.kernel.org/bpf/bpf-next/c/a4ae5c31e0f2
  - [bpf-next,v4,2/4] selftests/bpf: convert test_cgroup_storage to test_progs
    https://git.kernel.org/bpf/bpf-next/c/37a14cfd667a
  - [bpf-next,v4,3/4] selftests/bpf: add proper section name to bpf prog and rename it
    https://git.kernel.org/bpf/bpf-next/c/7b4400a0a69b
  - [bpf-next,v4,4/4] selftests/bpf: convert test_skb_cgroup_id_user to test_progs
    https://git.kernel.org/bpf/bpf-next/c/f957c230e173

You are awesome, thank you!
Alexis Lothoré Aug. 15, 2024, 8:17 a.m. UTC | #2
Hello Martin,

On 8/15/24 04:20, Martin KaFai Lau wrote:
> On 8/13/24 5:45 AM, Alexis Lothoré (eBPF Foundation) wrote:
> 
>> +#define DST_ADDR "ff02::1"
> 
> [ ... ]
> 
>> +static int wait_local_ip(void)
>> +{
>> +    char *ping_cmd = ping_command(AF_INET6);
>> +    int i, err;
>> +
>> +    for (i = 0; i < WAIT_AUTO_IP_MAX_ATTEMPT; i++) {
>> +        err = SYS_NOFAIL("%s -c 1 -W 1 %s%%%s", ping_cmd, DST_ADDR,
>> +                 VETH_1);
> 
> I tried in my qemu. This loop takes at least 3-4 iteration to get the ping
> through. This test could become flaky if the CI is busy.
> 
> I have been always wondering why some of the (non) test_progs has this practice.
> I traced a little. I think it has something to do with the "ff02::1" used in the
> test and/or the local link address is not ready. I have not further nailed it
> down but I think it is close enough.
> 
> It will be easier to use a nodad configured v6 addr.
> 
> I take this chance to use an easier "::1" address for the test here instead of
> ff02::1. This also removed the need to add veth pair and no need to ping first.
> 
> Applied with the "::1" changes mentioned above.

Thanks for the improvement. So far I tried to preserve as much as possible the
original test behavior to avoid accidentally removing some important features,
but I guess the more tests I work on, the more confident I will be to
"challenge" some parts ;)

> Thanks for migrating the tests to test_progs. This is long overdue.

For the record, the next test I am targeting is test_xdp_features.sh. I have
undertaken the conversion and have a working base but it still needs some work.

Thanks,

Alexis