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 <amore...@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
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
