Skip to content

Commit 3ffba47

Browse files
committed
Tests: Replace the WORKAROUND(globals) HashMaps with real @bpfglobal scalars
The selftest spike substituted a one-entry HashMap for every upstream global because PythonBPF had no global variable support. Integer-scalar @bpfglobal support has since landed, so the three affected ports now declare the upstream globals directly: get_cgroup_id reads expected_pid and writes cg_id, autoattach shares prog1_called/prog2_called between two programs, and perf_skip reads ip. perf_skip stays a strict xfail on the nested ctx.regs.ip access. Update the tests README and porting notes accordingly; the WORKAROUND tag no longer exists in the tree. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DLMW3X7opqAPH6bjmTVkr7
1 parent 708b5af commit 3ffba47

5 files changed

Lines changed: 90 additions & 68 deletions

File tree

‎tests/README.md‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -115,20 +115,13 @@ So a passing test here says PythonBPF emits a loadable, verifiable object for
115115
that program type and feature mix. It does not say the program behaves the way
116116
the kernel's version does. Treat it as a compiler assertion, not a semantic one.
117117

118-
### `WORKAROUND(globals)`
118+
### Globals
119119

120120
The selftest corpus overwhelmingly reports results through global variables: the
121-
program writes a global and the driver reads it back. PythonBPF has no global
122-
variable support, so each becomes a one-entry `HashMap` keyed by index, tagged in
123-
a comment naming the variable it replaces:
124-
125-
```bash
126-
grep -rn "WORKAROUND(globals)" tests/kernel_selftest_equivalent/
127-
```
128-
129-
This is deliberate scaffolding, not the intended shape — the tag exists so the
130-
sweep is mechanical once real globals land. It is not a cosmetic substitution
131-
either: it changes what a future userspace driver would read.
121+
program writes a global and the driver reads it back. Ports keep that shape with
122+
`@bpfglobal` scalars, so a future userspace driver reads the same `.bss`/`.data`
123+
values the kernel's driver does. Anything a global cannot yet hold (arrays,
124+
structs, strings) is a roadmap test, not a workaround.
132125

133126
Anything importing from `vmlinux` belongs in `vmlinux/`, which is registered in
134127
`VMLINUX_TEST_DIRS_PASSING` so it is skipped rather than failed where no

‎tests/kernel_selftest_equivalent/PORTING-NOTES.md‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,18 @@ the features would just produce a large pile of xfails.
5151

5252
## 2. What real globals must support
5353

54-
Every port that touches a global currently substitutes a one-entry `HashMap`, tagged
55-
`WORKAROUND(globals)`. Four programs produced four distinct shapes:
56-
57-
| Shape | Example | What globals must support |
58-
|---|---|---|
59-
| none | `tracepoint_sched_switch` | — (control case) |
60-
| scalar in + scalar out | `get_cgroup_id` | read a global, write a different one |
61-
| flags across programs | `autoattach` | two programs in one object sharing global state |
62-
| scalar in, compared against ctx | `perf_skip` | read-only input set by userspace before attach |
63-
| array + cursor *(next increment)* | `cgroup_preorder` | indexed writes and read-modify-write on a global |
54+
At the time of the spike every port that touched a global substituted a one-entry
55+
`HashMap`, tagged `WORKAROUND(globals)`. Integer-scalar `@bpfglobal` support has since
56+
landed and the sweep is done: the three ports below now declare the upstream globals
57+
directly. Four programs produced four distinct shapes:
58+
59+
| Shape | Example | What globals must support | Status |
60+
|---|---|---|---|
61+
| none | `tracepoint_sched_switch` | — (control case) | passes |
62+
| scalar in + scalar out | `get_cgroup_id` | read a global, write a different one | passes with `@bpfglobal` |
63+
| flags across programs | `autoattach` | two programs in one object sharing global state | passes with `@bpfglobal` |
64+
| scalar in, compared against ctx | `perf_skip` | read-only input set by userspace before attach | global fine; still xfail on `ctx.regs.ip` |
65+
| array + cursor *(next increment)* | `cgroup_preorder` | indexed writes and read-modify-write on a global | needs array globals |
6466

6567
The last row is not in this spike but is the recommended next port precisely because it is
6668
the most demanding shape: `result[idx++] = N` needs an array global *and* a read-modify-write

‎tests/kernel_selftest_equivalent/tracing/autoattach.py‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,58 @@
11
# Ported from Linux tools/testing/selftests/bpf/progs/test_autoattach.c
22
#
33
# Two programs on different raw tracepoints, each recording that it ran. The
4-
# upstream test asserts both fired after bpf_object__attach_skeleton().
4+
# upstream test asserts both fired after bpf_object__attach_skeleton():
55
#
6-
# WORKAROUND(globals): upstream uses `bool prog1_called` / `bool prog2_called`.
7-
# PythonBPF has no global variable support yet, so both live in one HashMap
8-
# keyed by program number. Replace with real globals once they land.
6+
# bool prog1_called = false;
7+
# bool prog2_called = false;
8+
#
9+
# SEC("raw_tp/sys_enter")
10+
# int prog1(const void *ctx)
11+
# {
12+
# prog1_called = true;
13+
# return 0;
14+
# }
15+
#
16+
# SEC("raw_tp/sys_exit")
17+
# int prog2(const void *ctx)
18+
# {
19+
# prog2_called = true;
20+
# return 0;
21+
# }
22+
#
23+
# Both flags are @bpfglobal scalars shared by the two programs in one object.
24+
# They are c_uint64 rather than bool because integer scalars are the only
25+
# global type today; the driver-side check is the same either way.
26+
27+
from pythonbpf import bpf, section, bpfglobal, compile
28+
from ctypes import c_void_p, c_int64, c_uint64
929

10-
from pythonbpf import bpf, map, section, bpfglobal, compile
11-
from pythonbpf.maps import HashMap
12-
from ctypes import c_void_p, c_int64, c_int32, c_uint64
30+
31+
@bpf
32+
@bpfglobal
33+
def prog1_called() -> c_uint64:
34+
return c_uint64(0)
1335

1436

15-
# WORKAROUND(globals): key 1 -> prog1_called, key 2 -> prog2_called
1637
@bpf
17-
@map
18-
def called() -> HashMap:
19-
return HashMap(key=c_int32, value=c_uint64, max_entries=2)
38+
@bpfglobal
39+
def prog2_called() -> c_uint64:
40+
return c_uint64(0)
2041

2142

2243
@bpf
2344
@section("raw_tp/sys_enter")
2445
def prog1(ctx: c_void_p) -> c_int64:
25-
called.update(1, 1)
46+
global prog1_called
47+
prog1_called = 1
2648
return c_int64(0)
2749

2850

2951
@bpf
3052
@section("raw_tp/sys_exit")
3153
def prog2(ctx: c_void_p) -> c_int64:
32-
called.update(2, 1)
54+
global prog2_called
55+
prog2_called = 1
3356
return c_int64(0)
3457

3558

‎tests/kernel_selftest_equivalent/tracing/get_cgroup_id.py‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,48 @@
11
# Ported from Linux tools/testing/selftests/bpf/progs/get_cgroup_id_kern.c
22
#
33
# Upstream records the cgroup id of a process whose pid matches one the
4-
# userspace half of the test set beforehand.
4+
# userspace half of the test set beforehand:
55
#
6-
# WORKAROUND(globals): upstream uses the file-scope variables `cg_id` and
7-
# `expected_pid` to pass values in and out. PythonBPF has no global variable
8-
# support yet, so each becomes a one-entry HashMap keyed by 0. Replace these
9-
# with real globals once they land; grep for WORKAROUND(globals).
6+
# __u64 cg_id;
7+
# __u64 expected_pid;
8+
#
9+
# SEC("tracepoint/syscalls/sys_enter_nanosleep")
10+
# int trace(void *ctx)
11+
# {
12+
# __u32 pid = bpf_get_current_pid_tgid();
13+
#
14+
# if (expected_pid == pid)
15+
# cg_id = bpf_get_current_cgroup_id();
16+
#
17+
# return 0;
18+
# }
19+
#
20+
# Both file-scope variables are @bpfglobal scalars, so the userspace half reads
21+
# `cg_id` back out of the object's .bss exactly as the kernel's driver does.
1022

11-
from pythonbpf import bpf, map, section, bpfglobal, compile
12-
from pythonbpf.maps import HashMap
23+
from pythonbpf import bpf, section, bpfglobal, compile
1324
from pythonbpf.helper import pid, get_current_cgroup_id
14-
from ctypes import c_void_p, c_int64, c_int32, c_uint64
25+
from ctypes import c_void_p, c_int64, c_uint64
1526

1627

17-
# WORKAROUND(globals): stands in for `__u64 expected_pid;`
1828
@bpf
19-
@map
20-
def expected_pid() -> HashMap:
21-
return HashMap(key=c_int32, value=c_uint64, max_entries=1)
29+
@bpfglobal
30+
def cg_id() -> c_uint64:
31+
return c_uint64(0)
2232

2333

24-
# WORKAROUND(globals): stands in for `__u64 cg_id;`
2534
@bpf
26-
@map
27-
def cg_id() -> HashMap:
28-
return HashMap(key=c_int32, value=c_uint64, max_entries=1)
35+
@bpfglobal
36+
def expected_pid() -> c_uint64:
37+
return c_uint64(0)
2938

3039

3140
@bpf
3241
@section("tracepoint/syscalls/sys_enter_nanosleep")
3342
def trace(ctx: c_void_p) -> c_int64:
34-
process_id = pid()
35-
want = expected_pid.lookup(0)
36-
if want == process_id:
37-
cg_id.update(0, get_current_cgroup_id())
43+
global cg_id
44+
if expected_pid == pid():
45+
cg_id = get_current_cgroup_id()
3846
return c_int64(0)
3947

4048

‎tests/kernel_selftest_equivalent/vmlinux/perf_skip.py‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
# return ip != PT_REGS_IP(&data->regs);
1313
# }
1414
#
15+
# `ip` is a @bpfglobal the driver sets before attaching; the program only
16+
# reads it, so no `global` statement is needed.
17+
#
1518
# ROADMAP: this is a strict expected failure. `ctx.regs.ip` is two levels of
1619
# struct field access, and PythonBPF supports only one --
1720
# `_allocate_for_attribute` in allocation_pass.py bails out unless the
@@ -23,30 +26,23 @@
2326
# allocation pass declines to allocate and logs at debug level, then the
2427
# expression pass fails later on the missing symbol. Worth improving alongside
2528
# nested access support.
26-
#
27-
# WORKAROUND(globals): upstream uses `uintptr_t ip` to receive the address to
28-
# compare against. PythonBPF has no global variable support yet, so it becomes a
29-
# one-entry HashMap keyed by 0. Replace with a real global once they land.
3029

31-
from pythonbpf import bpf, map, section, bpfglobal, compile
32-
from pythonbpf.maps import HashMap
30+
from pythonbpf import bpf, section, bpfglobal, compile
3331
from vmlinux import struct_bpf_perf_event_data
34-
from ctypes import c_int64, c_int32, c_uint64
32+
from ctypes import c_int64, c_uint64
3533

3634

37-
# WORKAROUND(globals): stands in for `uintptr_t ip;`
3835
@bpf
39-
@map
40-
def expected_ip() -> HashMap:
41-
return HashMap(key=c_int32, value=c_uint64, max_entries=1)
36+
@bpfglobal
37+
def ip() -> c_uint64:
38+
return c_uint64(0)
4239

4340

4441
@bpf
4542
@section("perf_event")
4643
def handler(ctx: struct_bpf_perf_event_data) -> c_int64:
47-
want = expected_ip.lookup(0)
4844
actual = ctx.regs.ip
49-
if want == actual:
45+
if ip == actual:
5046
return c_int64(0)
5147
return c_int64(1)
5248

0 commit comments

Comments
 (0)