Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: Fix dumping ISOCHRONOUS IN transfers
To:
tech@openbsd.org
Date:
Thu, 19 Dec 2024 14:18:51 +0100

Download raw body.

Thread
On Thu, 19 Dec 2024 11:00:26 +0100,
Martin Pieuchot <mpi@grenadille.net> wrote:
> 
> On 19/12/24(Thu) 02:27, Kirill A. Korinsky wrote:
> > tech@,
> > 
> > tcpdump can be used to capture USB traffic, and it works well with one
> > exception: ISOCHRONOUS IN transfers.
> > 
> > While OUT transfers work as expected, IN transfers only capture the first
> > packet that can be read.
> > 
> > The root cause is that the same array is being used to store both the
> > offsets in the data buffer and the resulting lengths. As a result, the dump
> > records the length of the packets read from the device as offsets in the
> > data buffer.
> 
> I don't see any offset being saved.  It is just the length of every
> frame, no?
> 

The usbpcap_pkt_hdr has two fields: uip_offset and uip_length. Wireshark
splits frames into packets, with each packet's payload starting at
uip_offset and having a length equal to uip_length.

> > When the device returns full packets, everything works correctly. However,
> > when it returns partial packets, only the first packet is useful, and all
> > subsequent packets are lost.
> 
> I'm not sure what you mean with "lost".  Are you saying that as soon as
> there is a frame missed the offset calculation is incorrect?
> 
> Are you testing on ehci(4) or xhci(4)?  If it is xhci(4) is it because
> `frlengths' is set to 0 in xhci_device_isoc_start()?
>

I’m using xhci(4), but as far as I understand, this doesn't matter here.

To illustrate, let's look at the uvideo driver as an example. It prepares a
transfer as follows:

	for (i = 0; i < sc->sc_nframes; i++)
		ixfer->size[i] = sc->sc_vs_cur->psize;

	usbd_setup_isoc_xfer(
	    ixfer->xfer,
	    sc->sc_vs_cur->pipeh,
	    ixfer,
	    ixfer->size,
	    sc->sc_nframes,
	    USBD_NO_COPY | USBD_SHORT_XFER_OK,
	    uvideo_vs_cb);

Here, size is an array that contains the maximum size of each packet.

Next, in the callback, the driver reads packets as follows:

	for (i = 0; i < sc->sc_nframes; i++) {
		frame = ixfer->buf + (i * sc->sc_vs_cur->psize);
		frame_size = ixfer->size[i];

In this context, the old maximum size of the packet is used as an offset in
the data buffer to access the actual package (here it's called frame).

On the other hand, usb.c prepares the packet to be dumped as follows:

		for (i = 0; i < nframes; i++) {
			h.uih.uih_frames[i].uip_offset = offset;
			h.uih.uih_frames[i].uip_length = xfer->frlengths[i];
			/* See above, we don't have per-frame error */
			h.uih.uih_frames[i].uip_status = 0;
			offset += xfer->frlengths[i];
		}

This code runs for both IN and OUT transfers.

For OUT transfers, this works correctly because xfer->frlengths[i] is the
actual size of the packet in the data buffer, and all packets are placed
continuously in the data buffer, without any gaps.

However, the situation is different for IN transfers. The kernel reads
packets from the device, but they may be shorter than the expected size. As
a result, the kernel tracks the total length of packets read using
xfer->actlen and updates xfer->frlengths[i] accordingly.

The following code handles the IN transfer case:

	} else { /* USBTAP_DIR_IN */
		bpfdir = BPF_DIRECTION_IN;
		uph->uph_info = USBPCAP_INFO_DIRECTION_IN;
		if (usbd_xfer_isread(xfer)) {
			data = KERNADDR(&xfer->dmabuf, 0);
			uph->uph_dlen = xfer->actlen;
			if (xfer->rqflags & URQ_REQUEST)
				h.uch.uch_stage = USBPCAP_CONTROL_STAGE_DATA;
		} else {
			data = NULL;
			uph->uph_dlen = 0;
			if (xfer->rqflags & URQ_REQUEST)
				h.uch.uch_stage = USBPCAP_CONTROL_STAGE_STATUS;
		}
	}

	/* Dump bulk/intr/iso data, ctrl DATA or STATUS stage. */
	bpf_tap_hdr(bpf, uph, uph->uph_hlen, data, uph->uph_dlen, bpfdir);

Here, In the case of an IN transfer, it dumps xfer->actlen bytes, which will
definitely contain the first packet. As a result, the pcap frame includes
all packets but with incorrect offsets.

When you open the dump in Wireshark, it reads the packets continuously,
without gaps. However, the driver reads the packets in larger steps that
correspond to the original sizes. The cause of this differences is incorrect
offsets, and as result only the first packet is interpreted correctly.

I hope this explanation clarifies the issue.

-- 
wbr, Kirill