mbox series

[bpf-next,v4,0/3] selftests/bpf: convert test_dev_cgroup to test_progs

Message ID 20240731-convert_dev_cgroup-v4-0-849425d90de6@bootlin.com (mailing list archive)
Headers show
Series selftests/bpf: convert test_dev_cgroup to test_progs | expand

Message

Alexis Lothoré (eBPF Foundation) July 31, 2024, 6:37 a.m. UTC
Hello,
this small series aims to integrate test_dev_cgroup in test_progs so it
could be run automatically in CI. The new version brings a few differences
with the current one:
- test now uses directly syscalls instead of wrapping commandline tools
  into system() calls
- test_progs manipulates /dev/null (eg: redirecting test logs into it), so
  disabling access to it in the bpf program confuses the tests. To fix this,
  the first commit modifies the bpf program to allow access to char devices
  1:3 (/dev/null), and disable access to char devices 1:5 (/dev/zero)
- once test is converted, add a small subtest to also check for device type
  interpretation (char or block)
- paths used in mknod tests are now in /dev instead of /tmp: due to the CI
  runner organisation and mountpoints manipulations, trying to create nodes
  in /tmp leads to errors unrelated to the test (ie, mknod calls refused by
  kernel, not the bpf program). I don't understand exactly the root cause
  at the deepest point (all I see in CI is an -ENXIO error on mknod when trying to
  create the node in tmp, and I can not make sense out of it neither
  replicate it locally), so I would gladly take inputs from anyone more
  educated than me about this.

The new test_progs part has been tested in a local qemu environment as well
as in upstream CI:

  ./test_progs -a cgroup_dev
  47/1    cgroup_dev/allow-mknod:OK
  47/2    cgroup_dev/allow-read:OK
  47/3    cgroup_dev/allow-write:OK
  47/4    cgroup_dev/deny-mknod:OK
  47/5    cgroup_dev/deny-read:OK
  47/6    cgroup_dev/deny-write:OK
  47/7    cgroup_dev/deny-mknod-wrong-type:OK
  47      cgroup_dev:OK
  Summary: 1/7 PASSED, 0 SKIPPED, 0 FAILED

---
Changes in v4:
- Fix mixup between ret and errno by testing both
- Properly apply ack tag from Stanislas
- Link to v3: https://lore.kernel.org/r/20240730-convert_dev_cgroup-v3-0-93e573b74357@bootlin.com

Changes in v3:
- delete mknod file only if it has been created
- use bpf_program__attach_cgroup() instead of bpf_prog_attach
- reorganize subtests order
- collect review/ack tags from Alan and Stanislas
- Link to v2: https://lore.kernel.org/r/20240729-convert_dev_cgroup-v2-0-4c1fc0520545@bootlin.com

Changes in v2:
- directly pass expected ret code to subtests instead of boolean pass/not
  pass
- fix faulty fd check in subtest expected to fail on open
- fix wrong subtest name
- pass test buffer and corresponding size to read/write subtests
- use correct series prefix
- Link to v1: https://lore.kernel.org/r/20240725-convert_dev_cgroup-v1-0-2c8cbd487c44@bootlin.com

---
Alexis Lothoré (eBPF Foundation) (3):
      selftests/bpf: do not disable /dev/null device access in cgroup dev test
      selftests/bpf: convert test_dev_cgroup to test_progs
      selftests/bpf: add wrong type test to cgroup dev

 tools/testing/selftests/bpf/.gitignore             |   1 -
 tools/testing/selftests/bpf/Makefile               |   2 -
 .../testing/selftests/bpf/prog_tests/cgroup_dev.c  | 125 +++++++++++++++++++++
 tools/testing/selftests/bpf/progs/dev_cgroup.c     |   4 +-
 tools/testing/selftests/bpf/test_dev_cgroup.c      |  85 --------------
 5 files changed, 127 insertions(+), 90 deletions(-)
---
base-commit: 2107cb4bff1c21110ebf7a17458a918282c1a8c9
change-id: 20240723-convert_dev_cgroup-6464b0d37f1a

Best regards,

Comments

patchwork-bot+netdevbpf@kernel.org July 31, 2024, 6:40 p.m. UTC | #1
Hello:

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

On Wed, 31 Jul 2024 08:37:24 +0200 you wrote:
> Hello,
> this small series aims to integrate test_dev_cgroup in test_progs so it
> could be run automatically in CI. The new version brings a few differences
> with the current one:
> - test now uses directly syscalls instead of wrapping commandline tools
>   into system() calls
> - test_progs manipulates /dev/null (eg: redirecting test logs into it), so
>   disabling access to it in the bpf program confuses the tests. To fix this,
>   the first commit modifies the bpf program to allow access to char devices
>   1:3 (/dev/null), and disable access to char devices 1:5 (/dev/zero)
> - once test is converted, add a small subtest to also check for device type
>   interpretation (char or block)
> - paths used in mknod tests are now in /dev instead of /tmp: due to the CI
>   runner organisation and mountpoints manipulations, trying to create nodes
>   in /tmp leads to errors unrelated to the test (ie, mknod calls refused by
>   kernel, not the bpf program). I don't understand exactly the root cause
>   at the deepest point (all I see in CI is an -ENXIO error on mknod when trying to
>   create the node in tmp, and I can not make sense out of it neither
>   replicate it locally), so I would gladly take inputs from anyone more
>   educated than me about this.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v4,1/3] selftests/bpf: do not disable /dev/null device access in cgroup dev test
    https://git.kernel.org/bpf/bpf-next/c/ba6a9018502e
  - [bpf-next,v4,2/3] selftests/bpf: convert test_dev_cgroup to test_progs
    https://git.kernel.org/bpf/bpf-next/c/d83d8230e415
  - [bpf-next,v4,3/3] selftests/bpf: add wrong type test to cgroup dev
    https://git.kernel.org/bpf/bpf-next/c/84cdbff4a935

You are awesome, thank you!
Alexis Lothoré (eBPF Foundation) July 31, 2024, 7:27 p.m. UTC | #2
On 7/31/24 20:40, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This series was applied to bpf/bpf-next.git (master)
> by Martin KaFai Lau <martin.lau@kernel.org>:
> 
> On Wed, 31 Jul 2024 08:37:24 +0200 you wrote:
>> Hello,
>> this small series aims to integrate test_dev_cgroup in test_progs so it
>> could be run automatically in CI. The new version brings a few differences
>> with the current one:
>> - test now uses directly syscalls instead of wrapping commandline tools
>>   into system() calls
>> - test_progs manipulates /dev/null (eg: redirecting test logs into it), so
>>   disabling access to it in the bpf program confuses the tests. To fix this,
>>   the first commit modifies the bpf program to allow access to char devices
>>   1:3 (/dev/null), and disable access to char devices 1:5 (/dev/zero)
>> - once test is converted, add a small subtest to also check for device type
>>   interpretation (char or block)
>> - paths used in mknod tests are now in /dev instead of /tmp: due to the CI
>>   runner organisation and mountpoints manipulations, trying to create nodes
>>   in /tmp leads to errors unrelated to the test (ie, mknod calls refused by
>>   kernel, not the bpf program). I don't understand exactly the root cause
>>   at the deepest point (all I see in CI is an -ENXIO error on mknod when trying to
>>   create the node in tmp, and I can not make sense out of it neither
>>   replicate it locally), so I would gladly take inputs from anyone more
>>   educated than me about this.
>>
>> [...]
> 
> Here is the summary with links:
>   - [bpf-next,v4,1/3] selftests/bpf: do not disable /dev/null device access in cgroup dev test
>     https://git.kernel.org/bpf/bpf-next/c/ba6a9018502e
>   - [bpf-next,v4,2/3] selftests/bpf: convert test_dev_cgroup to test_progs
>     https://git.kernel.org/bpf/bpf-next/c/d83d8230e415
>   - [bpf-next,v4,3/3] selftests/bpf: add wrong type test to cgroup dev
>     https://git.kernel.org/bpf/bpf-next/c/84cdbff4a935
> 
> You are awesome, thank you!

For the record, I am not receiving the notification about my patches being
merged (well, I receive it at least thanks to the mailing list, but not as the
author). I see that my email address looks pretty broken in the recipient field.
Could patchwork automation be confused by "customized" identity ?