On 3 Feb 2025, at 10:59, Adrián Moreno wrote:
> On Thu, Jan 30, 2025 at 02:31:33PM +0100, Eelco Chaudron wrote: >> >> >> On 17 Jan 2025, at 15:25, Adrian Moreno wrote: >> >>> This patch makes it possible to display only succeeded or errored >>> upcalls. >>> >>> Signed-off-by: Adrian Moreno <[email protected]> >> >> In general, this looks good to me. One comment below. >> >> //Eelco >> >>> --- >>> utilities/usdt-scripts/upcall_monitor.py | 42 +++++++++++++++++------- >>> 1 file changed, 30 insertions(+), 12 deletions(-) >>> >>> diff --git a/utilities/usdt-scripts/upcall_monitor.py >>> b/utilities/usdt-scripts/upcall_monitor.py >>> index c985dd5b3..333e23d51 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,succeed,both} >>> +# Display only events with the given result, >>> +# default: both >>> # >>> # 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; >>> @@ -267,6 +273,7 @@ int kretprobe__ovs_dp_upcall(struct pt_regs *ctx) >>> events.ringbuf_submit(event, 0); >>> return 0; >>> } >>> +#endif >>> """ >>> >>> >>> @@ -526,6 +533,10 @@ 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: both", >>> + choices=["error", "succeed", "both"], >>> default="both") >> >> I think this should be success, succeed sounds odd to me in this context. > > I agree, now that I re-read it. Maybe even "ok"? I’m fine with error/success/any or error/ok/any. I was thinking of any, instead of both (just in case we want to add more options in the future). >> >>> options = parser.parse_args() >>> >>> @@ -560,20 +571,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 ["succeed", "both"]: >>> + 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()) >> >> unnecessary change. >> > > If someone wants to uncomment this debug print, it should now be > indented more because it should go inside the > "if options.result in ["succeed", "both"]:" > > This change is to make it easy: just remove the "#". Ok, that makes sense. Let’s do it… >>> # >>> >>> # >>> @@ -583,8 +597,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 ["succeed", "both"] else >>> "0") >>> + source = source.replace("<INSTALL_OVS_UPCALL_DROP_PROBE>", "1" >>> + if options.result in ["error", "both"] 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 >>> [email protected] >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
