Re: [PATCH] usb: gadget: Add gadgetmon traffic monitor

[Alan Stern]

On Fri, Jul 25, 2025 at 05:25:49PM +0200, Olivier Tuchon wrote:
> The Linux kernel lacks a tool equivalent to usbmon for the gadget side,
> making it difficult to debug the lifecycle and performance of usb_requests.
> This forces developers into using ad-hoc printk statements for
> introspection.
> 
> This commit introduces "gadgetmon", a comprehensive framework for
> monitoring USB gadget traffic. It consists of two main parts: a new
> monitoring interface in the UDC core and a loadable module that
> implements this interface to provide data to userspace.
> 
> The UDC core is modified to add an optional monitoring interface defined
> by struct usb_gadget_mon_operations in <linux/usb/gadget.h>.

This does not appear in the patch.  Was it left out by mistake?

>  An
> RCU-protected global pointer, gadget_mon_ops, is defined and exported
> to allow monitoring modules to register. Hooks are placed in
> usb_ep_queue() and usb_gadget_giveback_request() to call this interface,
> capturing request submission and completion events.

Do you expect that other gadget monitoring modules will be written?
If they are, assignment to the global pointer should be handled by a
routine in the gadget core, not in the monitoring module as done here.

> + /*
> + * optimization: for an OUT submission (host-to-device), the data
> + * has not yet arrived from the host. The buffer is an empty
> + * placeholder, so its content is not captured to save space.
> + */
> + if (event_type == GADGETMON_EVENT_SUBMIT && hdr.dir == USB_DIR_OUT)
> + payload_len = 0;

There should be a similar optimization for IN givebacks.  The data to
be transferred to the host was already recorded by the submission
hook, so you can save space by not copying it a second time during the
giveback.

> +
> + hdr.data_len = payload_len;
> + total_len = sizeof(hdr) + payload_len;
> +
> + /* lock and queue the event into the FIFO */
> + spin_lock_irqsave(&mon_lock, flags);
> +
> + if (kfifo_avail(&mon_fifo) < total_len) {
> + /* not enough space, drop the event silently */

Would it be better to keep the event but drop the tail end of the data?

Alan Stern