Message ID | 20250609090218.2269790-1-amorenoz@redhat.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,v1] utilities: upcall_monitor: Avoid optimizing size. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/cirrus-robot | success | cirrus build: passed |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
Adrian Moreno via dev <ovs-dev@openvswitch.org> writes: > Use the barrier trick to avoid clang from optimizing the 'size' variable > that can cause the verifier to think that 'size' can be negative: > > ./upcall_monitor.py > 119: (85) call bpf_probe_read_kernel#113 > R2 min value is negative, either use unsigned or 'var &= const' > processed 131 insns (limit 1000000) max_states_per_insn 0 total_states > peak_states 9 mark_read 3 > Traceback (most recent call last): > File "/ovs/utilities/usdt-scripts/./upcall_monitor.py", line 729, in > odule> > main() > File "/ovs/utilities/usdt-scripts/./upcall_monitor.py", line 697, in > in > b = BPF(text=source, usdt_contexts=usdt, debug=options.debug) > File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 373, in > init__ > self._trace_autoload() > File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 1241, in > race_autoload > fn = self.load_func(func_name, BPF.KPROBE) > File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 412, in > ad_func > raise Exception("Failed to load BPF program %s: %s" % > Exception: Failed to load BPF program b'kretprobe__ovs_dp_upcall': > permission denied. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- Looks fine to me, but I think it would be useful to include which kernel version(s) you observed this issue with. For example, was this on RHEL9/Centos9? Ubuntu 22.04? Debian? Kernel 4.18, 5.14, latest upstream?
On Tue, Jun 10, 2025 at 06:16:38AM -0400, Aaron Conole wrote: > Adrian Moreno via dev <ovs-dev@openvswitch.org> writes: > > > Use the barrier trick to avoid clang from optimizing the 'size' variable > > that can cause the verifier to think that 'size' can be negative: > > > > ./upcall_monitor.py > > 119: (85) call bpf_probe_read_kernel#113 > > R2 min value is negative, either use unsigned or 'var &= const' > > processed 131 insns (limit 1000000) max_states_per_insn 0 total_states > > peak_states 9 mark_read 3 > > Traceback (most recent call last): > > File "/ovs/utilities/usdt-scripts/./upcall_monitor.py", line 729, in > > odule> > > main() > > File "/ovs/utilities/usdt-scripts/./upcall_monitor.py", line 697, in > > in > > b = BPF(text=source, usdt_contexts=usdt, debug=options.debug) > > File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 373, in > > init__ > > self._trace_autoload() > > File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 1241, in > > race_autoload > > fn = self.load_func(func_name, BPF.KPROBE) > > File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 412, in > > ad_func > > raise Exception("Failed to load BPF program %s: %s" % > > Exception: Failed to load BPF program b'kretprobe__ovs_dp_upcall': > > permission denied. > > > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > > --- > > Looks fine to me, but I think it would be useful to include which kernel > version(s) you observed this issue with. For example, was this on > RHEL9/Centos9? Ubuntu 22.04? Debian? Kernel 4.18, 5.14, latest > upstream? > Good point, I missed that. I found this in a RHEL kernel (kernel-5.14.0-427.72.1.el9_4). Should I repost and update the commit message? I did try to reproduce in latest upstream kernels but bcc is just completely failing on those. I Need to look deeper. Thanks. Adrián
Adrián Moreno via dev <ovs-dev@openvswitch.org> writes: > On Tue, Jun 10, 2025 at 06:16:38AM -0400, Aaron Conole wrote: >> Adrian Moreno via dev <ovs-dev@openvswitch.org> writes: >> >> > Use the barrier trick to avoid clang from optimizing the 'size' variable >> > that can cause the verifier to think that 'size' can be negative: >> > >> > ./upcall_monitor.py >> > 119: (85) call bpf_probe_read_kernel#113 >> > R2 min value is negative, either use unsigned or 'var &= const' >> > processed 131 insns (limit 1000000) max_states_per_insn 0 total_states >> > peak_states 9 mark_read 3 >> > Traceback (most recent call last): >> > File "/ovs/utilities/usdt-scripts/./upcall_monitor.py", line 729, in >> > odule> >> > main() >> > File "/ovs/utilities/usdt-scripts/./upcall_monitor.py", line 697, in >> > in >> > b = BPF(text=source, usdt_contexts=usdt, debug=options.debug) >> > File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 373, in >> > init__ >> > self._trace_autoload() >> > File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 1241, in >> > race_autoload >> > fn = self.load_func(func_name, BPF.KPROBE) >> > File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 412, in >> > ad_func >> > raise Exception("Failed to load BPF program %s: %s" % >> > Exception: Failed to load BPF program b'kretprobe__ovs_dp_upcall': >> > permission denied. >> > >> > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> > --- >> >> Looks fine to me, but I think it would be useful to include which kernel >> version(s) you observed this issue with. For example, was this on >> RHEL9/Centos9? Ubuntu 22.04? Debian? Kernel 4.18, 5.14, latest >> upstream? >> > > Good point, I missed that. > I found this in a RHEL kernel (kernel-5.14.0-427.72.1.el9_4). > > Should I repost and update the commit message? That would be great. Please include details of the bcc / clang / gcc and python that you used. I do consider this as a kind of fix, so we probably should be backporting it. > I did try to reproduce in latest upstream kernels but bcc is just > completely failing on those. > I Need to look deeper. > > Thanks. > Adrián > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py index 5b78e833f..390abdfe0 100755 --- a/utilities/usdt-scripts/upcall_monitor.py +++ b/utilities/usdt-scripts/upcall_monitor.py @@ -151,6 +151,8 @@ ebpf_source = """ #define MAX_PACKET <MAX_PACKET_VAL> #define MAX_KEY <MAX_KEY_VAL> +#define barrier_var(var) asm volatile("" : "=r"(var) : "0"(var)) + struct event_t { int result; u32 cpu; @@ -213,6 +215,11 @@ int do_trace(struct pt_regs *ctx) { size = MAX_KEY; else size = event->key_size; + + /* Prevent clang from using register mirroring (or any optimization) on + * the 'size' variable. */ + barrier_var(size); + bpf_usdt_readarg(5, ctx, &addr); bpf_probe_read(&event->key, size, (void *)addr); @@ -287,6 +294,10 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx) if (size > MAX_PACKET) size = MAX_PACKET; + /* Prevent clang from using register mirroring (or any optimization) on + * the 'size' variable. */ + barrier_var(size); + bpf_probe_read_kernel(event->pkt, size, upcall->skb->data); events.ringbuf_submit(event, 0); return 0;
Use the barrier trick to avoid clang from optimizing the 'size' variable that can cause the verifier to think that 'size' can be negative: ./upcall_monitor.py 119: (85) call bpf_probe_read_kernel#113 R2 min value is negative, either use unsigned or 'var &= const' processed 131 insns (limit 1000000) max_states_per_insn 0 total_states peak_states 9 mark_read 3 Traceback (most recent call last): File "/ovs/utilities/usdt-scripts/./upcall_monitor.py", line 729, in odule> main() File "/ovs/utilities/usdt-scripts/./upcall_monitor.py", line 697, in in b = BPF(text=source, usdt_contexts=usdt, debug=options.debug) File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 373, in init__ self._trace_autoload() File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 1241, in race_autoload fn = self.load_func(func_name, BPF.KPROBE) File "/usr/lib/python3.9/site-packages/bcc/__init__.py", line 412, in ad_func raise Exception("Failed to load BPF program %s: %s" % Exception: Failed to load BPF program b'kretprobe__ovs_dp_upcall': permission denied. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- utilities/usdt-scripts/upcall_monitor.py | 11 +++++++++++ 1 file changed, 11 insertions(+)