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 <[email protected]>

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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to