| Message ID | 20250227172340.4120887-4-amorenoz@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | aaron conole |
| Headers | show |
| Series | utilities: upcall_monitor: Monitor drops. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 27 Feb 2025, at 18:23, Adrian Moreno wrote: > This patch makes it possible to display only succeeded or errored > upcalls. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> One comment below… Don’t think it’s a blocker, however, the message might be misleading. > --- > utilities/usdt-scripts/upcall_monitor.py | 58 +++++++++++++++++++----- > 1 file changed, 46 insertions(+), 12 deletions(-) > > diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py > index 0c2518f54..ca304d7c9 100755 > --- a/utilities/usdt-scripts/upcall_monitor.py > +++ b/utilities/usdt-scripts/upcall_monitor.py > @@ -62,6 +62,9 @@ > # Set maximum packet size to capture, default 64 > # -w PCAP_FILE, --pcap PCAP_FILE > # Write upcall packets to specified pcap file. > +# -r, --result {error,ok,any} > +# Display only events with the given result, > +# default: any > # > # The following is an example of how to use the script on the running > # ovs-vswitchd process with a packet and flow key dump enabled: > @@ -157,6 +160,7 @@ void report_missed_event() { > __sync_fetch_and_add(value, 1); > } > > +#if <INSTALL_OVS_UPCALL_RECV_PROBE> > int do_trace(struct pt_regs *ctx) { > uint64_t addr; > uint64_t size; > @@ -198,7 +202,9 @@ int do_trace(struct pt_regs *ctx) { > events.ringbuf_submit(event, 0); > return 0; > }; > +#endif > > +#if <INSTALL_OVS_UPCALL_DROP_PROBE> > struct inflight_upcall { > u32 cpu; > u32 upcall_type; > @@ -264,6 +270,7 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx) > events.ringbuf_submit(event, 0); > return 0; > } > +#endif > """ > > > @@ -526,9 +533,29 @@ def main(): > parser.add_argument("-w", "--pcap", metavar="PCAP_FILE", > help="Write upcall packets to specified pcap file.", > type=str, default=None) > + parser.add_argument("-r", "--result", > + help="Display only events with the given result, " > + "default: any", > + choices=["error", "ok", "any"], default="any") > > options = parser.parse_args() > > + # > + # Check if current kernel supports error reporting. > + # > + upcall_tp_path = \ > + "/sys/kernel/debug/tracing/events/openvswitch/ovs_dp_upcall" I think this is nice, but not sure if /sys/kernel/debug is always mounted by default. If it’s not the message might be misleading. > + > + if not exists(upcall_tp_path): > + if options.result == "error": > + print("ERROR: Monitoring error upcalls is not supported by the " > + "running kernel.") > + sys.exit(-1) > + if options.result == "any": > + print("WARN: Monitoring error upcalls is not supported by the " > + "running kernel. Only successful ones will be monitored.") > + options.result = "ok" > + > # > # Find the PID of the ovs-vswitchd daemon if not specified. > # > @@ -560,20 +587,23 @@ def main(): > # > # Attach the usdt probe > # > - u = USDT(pid=int(options.pid)) > - try: > - u.enable_probe(probe="recv_upcall", fn_name="do_trace") > - except USDTException as e: > - print("ERROR: {}" > - "ovs-vswitchd!".format( > - (re.sub('^', ' ' * 7, str(e), flags=re.MULTILINE)).strip(). > - replace("--with-dtrace or --enable-dtrace", > - "--enable-usdt-probes"))) > - sys.exit(-1) > + usdt = [] > + if options.result in ["ok", "any"]: > + u = USDT(pid=int(options.pid)) > + try: > + u.enable_probe(probe="recv_upcall", fn_name="do_trace") > + usdt.append(u) > + except USDTException as e: > + print("ERROR: {}" > + "ovs-vswitchd!".format( > + (re.sub('^', ' ' * 7, str(e), flags=re.MULTILINE)). > + strip().replace("--with-dtrace or --enable-dtrace", > + "--enable-usdt-probes"))) > + sys.exit(-1) > > # > # Uncomment to see how arguments are decoded. > - # print(u.get_text()) > + # print(u.get_text()) > # > > # > @@ -583,8 +613,12 @@ def main(): > source = source.replace("<MAX_KEY_VAL>", str(options.flow_key_size)) > source = source.replace("<BUFFER_PAGE_CNT>", > str(options.buffer_page_count)) > + source = source.replace("<INSTALL_OVS_UPCALL_RECV_PROBE>", "1" > + if options.result in ["ok", "any"] else "0") > + source = source.replace("<INSTALL_OVS_UPCALL_DROP_PROBE>", "1" > + if options.result in ["error", "any"] else "0") > > - b = BPF(text=source, usdt_contexts=[u], debug=options.debug) > + b = BPF(text=source, usdt_contexts=usdt, debug=options.debug) > > # > # Print header > -- > 2.48.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Tue, Mar 11, 2025 at 04:01:33PM +0100, Eelco Chaudron wrote: > > > On 27 Feb 2025, at 18:23, Adrian Moreno wrote: > > > This patch makes it possible to display only succeeded or errored > > upcalls. > > > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > > One comment below… Don’t think it’s a blocker, however, the message might be misleading. > > > --- > > utilities/usdt-scripts/upcall_monitor.py | 58 +++++++++++++++++++----- > > 1 file changed, 46 insertions(+), 12 deletions(-) > > > > diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py > > index 0c2518f54..ca304d7c9 100755 > > --- a/utilities/usdt-scripts/upcall_monitor.py > > +++ b/utilities/usdt-scripts/upcall_monitor.py > > @@ -62,6 +62,9 @@ > > # Set maximum packet size to capture, default 64 > > # -w PCAP_FILE, --pcap PCAP_FILE > > # Write upcall packets to specified pcap file. > > +# -r, --result {error,ok,any} > > +# Display only events with the given result, > > +# default: any > > # > > # The following is an example of how to use the script on the running > > # ovs-vswitchd process with a packet and flow key dump enabled: > > @@ -157,6 +160,7 @@ void report_missed_event() { > > __sync_fetch_and_add(value, 1); > > } > > > > +#if <INSTALL_OVS_UPCALL_RECV_PROBE> > > int do_trace(struct pt_regs *ctx) { > > uint64_t addr; > > uint64_t size; > > @@ -198,7 +202,9 @@ int do_trace(struct pt_regs *ctx) { > > events.ringbuf_submit(event, 0); > > return 0; > > }; > > +#endif > > > > +#if <INSTALL_OVS_UPCALL_DROP_PROBE> > > struct inflight_upcall { > > u32 cpu; > > u32 upcall_type; > > @@ -264,6 +270,7 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx) > > events.ringbuf_submit(event, 0); > > return 0; > > } > > +#endif > > """ > > > > > > @@ -526,9 +533,29 @@ def main(): > > parser.add_argument("-w", "--pcap", metavar="PCAP_FILE", > > help="Write upcall packets to specified pcap file.", > > type=str, default=None) > > + parser.add_argument("-r", "--result", > > + help="Display only events with the given result, " > > + "default: any", > > + choices=["error", "ok", "any"], default="any") > > > > options = parser.parse_args() > > > > + # > > + # Check if current kernel supports error reporting. > > + # > > + upcall_tp_path = \ > > + "/sys/kernel/debug/tracing/events/openvswitch/ovs_dp_upcall" > > I think this is nice, but not sure if /sys/kernel/debug is always mounted by default. If it’s not the message might be misleading. > Arguably, it can also live in "/sys/kernel/tracing". But if neither are mounted, most of bcc tooling won't work. We can add a specific message to avoid confusion, is that what you mean? > > + > > + if not exists(upcall_tp_path): > > + if options.result == "error": > > + print("ERROR: Monitoring error upcalls is not supported by the " > > + "running kernel.") > > + sys.exit(-1) > > + if options.result == "any": > > + print("WARN: Monitoring error upcalls is not supported by the " > > + "running kernel. Only successful ones will be monitored.") > > + options.result = "ok" > > + > > # > > # Find the PID of the ovs-vswitchd daemon if not specified. > > # > > @@ -560,20 +587,23 @@ def main(): > > # > > # Attach the usdt probe > > # > > - u = USDT(pid=int(options.pid)) > > - try: > > - u.enable_probe(probe="recv_upcall", fn_name="do_trace") > > - except USDTException as e: > > - print("ERROR: {}" > > - "ovs-vswitchd!".format( > > - (re.sub('^', ' ' * 7, str(e), flags=re.MULTILINE)).strip(). > > - replace("--with-dtrace or --enable-dtrace", > > - "--enable-usdt-probes"))) > > - sys.exit(-1) > > + usdt = [] > > + if options.result in ["ok", "any"]: > > + u = USDT(pid=int(options.pid)) > > + try: > > + u.enable_probe(probe="recv_upcall", fn_name="do_trace") > > + usdt.append(u) > > + except USDTException as e: > > + print("ERROR: {}" > > + "ovs-vswitchd!".format( > > + (re.sub('^', ' ' * 7, str(e), flags=re.MULTILINE)). > > + strip().replace("--with-dtrace or --enable-dtrace", > > + "--enable-usdt-probes"))) > > + sys.exit(-1) > > > > # > > # Uncomment to see how arguments are decoded. > > - # print(u.get_text()) > > + # print(u.get_text()) > > # > > > > # > > @@ -583,8 +613,12 @@ def main(): > > source = source.replace("<MAX_KEY_VAL>", str(options.flow_key_size)) > > source = source.replace("<BUFFER_PAGE_CNT>", > > str(options.buffer_page_count)) > > + source = source.replace("<INSTALL_OVS_UPCALL_RECV_PROBE>", "1" > > + if options.result in ["ok", "any"] else "0") > > + source = source.replace("<INSTALL_OVS_UPCALL_DROP_PROBE>", "1" > > + if options.result in ["error", "any"] else "0") > > > > - b = BPF(text=source, usdt_contexts=[u], debug=options.debug) > > + b = BPF(text=source, usdt_contexts=usdt, debug=options.debug) > > > > # > > # Print header > > -- > > 2.48.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 21 Mar 2025, at 11:08, Adrián Moreno wrote: > On Tue, Mar 11, 2025 at 04:01:33PM +0100, Eelco Chaudron wrote: >> >> >> On 27 Feb 2025, at 18:23, Adrian Moreno wrote: >> >>> This patch makes it possible to display only succeeded or errored >>> upcalls. >>> >>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> >> One comment below… Don’t think it’s a blocker, however, the message might be misleading. >> >>> --- >>> utilities/usdt-scripts/upcall_monitor.py | 58 +++++++++++++++++++----- >>> 1 file changed, 46 insertions(+), 12 deletions(-) >>> >>> diff --git a/utilities/usdt-scripts/upcall_monitor.py b/utilities/usdt-scripts/upcall_monitor.py >>> index 0c2518f54..ca304d7c9 100755 >>> --- a/utilities/usdt-scripts/upcall_monitor.py >>> +++ b/utilities/usdt-scripts/upcall_monitor.py >>> @@ -62,6 +62,9 @@ >>> # Set maximum packet size to capture, default 64 >>> # -w PCAP_FILE, --pcap PCAP_FILE >>> # Write upcall packets to specified pcap file. >>> +# -r, --result {error,ok,any} >>> +# Display only events with the given result, >>> +# default: any >>> # >>> # The following is an example of how to use the script on the running >>> # ovs-vswitchd process with a packet and flow key dump enabled: >>> @@ -157,6 +160,7 @@ void report_missed_event() { >>> __sync_fetch_and_add(value, 1); >>> } >>> >>> +#if <INSTALL_OVS_UPCALL_RECV_PROBE> >>> int do_trace(struct pt_regs *ctx) { >>> uint64_t addr; >>> uint64_t size; >>> @@ -198,7 +202,9 @@ int do_trace(struct pt_regs *ctx) { >>> events.ringbuf_submit(event, 0); >>> return 0; >>> }; >>> +#endif >>> >>> +#if <INSTALL_OVS_UPCALL_DROP_PROBE> >>> struct inflight_upcall { >>> u32 cpu; >>> u32 upcall_type; >>> @@ -264,6 +270,7 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx) >>> events.ringbuf_submit(event, 0); >>> return 0; >>> } >>> +#endif >>> """ >>> >>> >>> @@ -526,9 +533,29 @@ def main(): >>> parser.add_argument("-w", "--pcap", metavar="PCAP_FILE", >>> help="Write upcall packets to specified pcap file.", >>> type=str, default=None) >>> + parser.add_argument("-r", "--result", >>> + help="Display only events with the given result, " >>> + "default: any", >>> + choices=["error", "ok", "any"], default="any") >>> >>> options = parser.parse_args() >>> >>> + # >>> + # Check if current kernel supports error reporting. >>> + # >>> + upcall_tp_path = \ >>> + "/sys/kernel/debug/tracing/events/openvswitch/ovs_dp_upcall" >> >> I think this is nice, but not sure if /sys/kernel/debug is always mounted by default. If it’s not the message might be misleading. >> > > Arguably, it can also live in "/sys/kernel/tracing". But if neither are > mounted, most of bcc tooling won't work. We can add a specific message > to avoid confusion, is that what you mean? I guess updating the error message with something like the below would be good enough for me: print("ERROR: Monitoring error upcalls is not supported by the " "running kernel (or the debugfs, /sys/kernel/debug, is not mounted).”) >>> + >>> + if not exists(upcall_tp_path): >>> + if options.result == "error": >>> + print("ERROR: Monitoring error upcalls is not supported by the " >>> + "running kernel.") >>> + sys.exit(-1) >>> + if options.result == "any": >>> + print("WARN: Monitoring error upcalls is not supported by the " >>> + "running kernel. Only successful ones will be monitored.") >>> + options.result = "ok" >>> + >>> # >>> # Find the PID of the ovs-vswitchd daemon if not specified. >>> # >>> @@ -560,20 +587,23 @@ def main(): >>> # >>> # Attach the usdt probe >>> # >>> - u = USDT(pid=int(options.pid)) >>> - try: >>> - u.enable_probe(probe="recv_upcall", fn_name="do_trace") >>> - except USDTException as e: >>> - print("ERROR: {}" >>> - "ovs-vswitchd!".format( >>> - (re.sub('^', ' ' * 7, str(e), flags=re.MULTILINE)).strip(). >>> - replace("--with-dtrace or --enable-dtrace", >>> - "--enable-usdt-probes"))) >>> - sys.exit(-1) >>> + usdt = [] >>> + if options.result in ["ok", "any"]: >>> + u = USDT(pid=int(options.pid)) >>> + try: >>> + u.enable_probe(probe="recv_upcall", fn_name="do_trace") >>> + usdt.append(u) >>> + except USDTException as e: >>> + print("ERROR: {}" >>> + "ovs-vswitchd!".format( >>> + (re.sub('^', ' ' * 7, str(e), flags=re.MULTILINE)). >>> + strip().replace("--with-dtrace or --enable-dtrace", >>> + "--enable-usdt-probes"))) >>> + sys.exit(-1) >>> >>> # >>> # Uncomment to see how arguments are decoded. >>> - # print(u.get_text()) >>> + # print(u.get_text()) >>> # >>> >>> # >>> @@ -583,8 +613,12 @@ def main(): >>> source = source.replace("<MAX_KEY_VAL>", str(options.flow_key_size)) >>> source = source.replace("<BUFFER_PAGE_CNT>", >>> str(options.buffer_page_count)) >>> + source = source.replace("<INSTALL_OVS_UPCALL_RECV_PROBE>", "1" >>> + if options.result in ["ok", "any"] else "0") >>> + source = source.replace("<INSTALL_OVS_UPCALL_DROP_PROBE>", "1" >>> + if options.result in ["error", "any"] else "0") >>> >>> - b = BPF(text=source, usdt_contexts=[u], debug=options.debug) >>> + b = BPF(text=source, usdt_contexts=usdt, debug=options.debug) >>> >>> # >>> # Print header >>> -- >>> 2.48.1 >>> >>> _______________________________________________ >>> 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 0c2518f54..ca304d7c9 100755 --- a/utilities/usdt-scripts/upcall_monitor.py +++ b/utilities/usdt-scripts/upcall_monitor.py @@ -62,6 +62,9 @@ # Set maximum packet size to capture, default 64 # -w PCAP_FILE, --pcap PCAP_FILE # Write upcall packets to specified pcap file. +# -r, --result {error,ok,any} +# Display only events with the given result, +# default: any # # The following is an example of how to use the script on the running # ovs-vswitchd process with a packet and flow key dump enabled: @@ -157,6 +160,7 @@ void report_missed_event() { __sync_fetch_and_add(value, 1); } +#if <INSTALL_OVS_UPCALL_RECV_PROBE> int do_trace(struct pt_regs *ctx) { uint64_t addr; uint64_t size; @@ -198,7 +202,9 @@ int do_trace(struct pt_regs *ctx) { events.ringbuf_submit(event, 0); return 0; }; +#endif +#if <INSTALL_OVS_UPCALL_DROP_PROBE> struct inflight_upcall { u32 cpu; u32 upcall_type; @@ -264,6 +270,7 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx) events.ringbuf_submit(event, 0); return 0; } +#endif """ @@ -526,9 +533,29 @@ def main(): parser.add_argument("-w", "--pcap", metavar="PCAP_FILE", help="Write upcall packets to specified pcap file.", type=str, default=None) + parser.add_argument("-r", "--result", + help="Display only events with the given result, " + "default: any", + choices=["error", "ok", "any"], default="any") options = parser.parse_args() + # + # Check if current kernel supports error reporting. + # + upcall_tp_path = \ + "/sys/kernel/debug/tracing/events/openvswitch/ovs_dp_upcall" + + if not exists(upcall_tp_path): + if options.result == "error": + print("ERROR: Monitoring error upcalls is not supported by the " + "running kernel.") + sys.exit(-1) + if options.result == "any": + print("WARN: Monitoring error upcalls is not supported by the " + "running kernel. Only successful ones will be monitored.") + options.result = "ok" + # # Find the PID of the ovs-vswitchd daemon if not specified. # @@ -560,20 +587,23 @@ def main(): # # Attach the usdt probe # - u = USDT(pid=int(options.pid)) - try: - u.enable_probe(probe="recv_upcall", fn_name="do_trace") - except USDTException as e: - print("ERROR: {}" - "ovs-vswitchd!".format( - (re.sub('^', ' ' * 7, str(e), flags=re.MULTILINE)).strip(). - replace("--with-dtrace or --enable-dtrace", - "--enable-usdt-probes"))) - sys.exit(-1) + usdt = [] + if options.result in ["ok", "any"]: + u = USDT(pid=int(options.pid)) + try: + u.enable_probe(probe="recv_upcall", fn_name="do_trace") + usdt.append(u) + except USDTException as e: + print("ERROR: {}" + "ovs-vswitchd!".format( + (re.sub('^', ' ' * 7, str(e), flags=re.MULTILINE)). + strip().replace("--with-dtrace or --enable-dtrace", + "--enable-usdt-probes"))) + sys.exit(-1) # # Uncomment to see how arguments are decoded. - # print(u.get_text()) + # print(u.get_text()) # # @@ -583,8 +613,12 @@ def main(): source = source.replace("<MAX_KEY_VAL>", str(options.flow_key_size)) source = source.replace("<BUFFER_PAGE_CNT>", str(options.buffer_page_count)) + source = source.replace("<INSTALL_OVS_UPCALL_RECV_PROBE>", "1" + if options.result in ["ok", "any"] else "0") + source = source.replace("<INSTALL_OVS_UPCALL_DROP_PROBE>", "1" + if options.result in ["error", "any"] else "0") - b = BPF(text=source, usdt_contexts=[u], debug=options.debug) + b = BPF(text=source, usdt_contexts=usdt, debug=options.debug) # # Print header
This patch makes it possible to display only succeeded or errored upcalls. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- utilities/usdt-scripts/upcall_monitor.py | 58 +++++++++++++++++++----- 1 file changed, 46 insertions(+), 12 deletions(-)