diff mbox series

[ovs-dev,v1] utilities: upcall_monitor: Avoid optimizing size.

Message ID 20250609090218.2269790-1-amorenoz@redhat.com
State Changes Requested
Headers show
Series [ovs-dev,v1] utilities: upcall_monitor: Avoid optimizing size. | expand

Checks

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

Commit Message

Adrián Moreno June 9, 2025, 9:02 a.m. UTC
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(+)

Comments

Aaron Conole June 10, 2025, 10:16 a.m. UTC | #1
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?
Adrián Moreno June 11, 2025, 8:23 a.m. UTC | #2
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
Aaron Conole June 11, 2025, 8:19 p.m. UTC | #3
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 mbox series

Patch

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;