Index | Thread | Search

From:
Martin Pieuchot <mpi@grenadille.net>
Subject:
Re: Fix dumping ISOCHRONOUS IN transfers
To:
tech@openbsd.org
Date:
Fri, 20 Dec 2024 20:09:46 +0100

Download raw body.

Thread
On 20/12/24(Fri) 20:04, Kirill A. Korinsky wrote:
> On Fri, 20 Dec 2024 19:40:02 +0100,
> Martin Pieuchot <mpi@grenadille.net> wrote:
> > 
> > On 19/12/24(Thu) 14:18, Kirill A. Korinsky wrote:
> > > [...] 
> > > 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.
> > 
> > Understood.  I'd prefer if we could find another solution than adding an
> > array to every USB transfer descriptor.
> > It seems to me that all isochronous USB drivers use frames of the same size,
> > which mean we could calculate the offset with:
> > 
> > 	(xfer->length / xfer->nframes)
> > 
> > Could that work?
> > 
> > On a related note, is their any OS and/or driver that use isochronous
> > transfers with frames of different sizes?
> > 
> 
> I made a quick grep through the Linux sources and was able to locate an
> example with a different offset and length in a driver:
> https://github.com/torvalds/linux/blob/v6.12/drivers/isdn/hardware/mISDN/hfcsusb.c#L1245-L1321
> 
> Linux uses the following structure to describe each isochronous packet:
> 
>         struct usb_iso_packet_descriptor {
>                 unsigned int offset;
>                 unsigned int length;		/* expected length */
>                 unsigned int actual_length;
>                 int status;
>         };
> 
> Since we have a few isochronous drivers, what do you think about migrating
> the code to follow the same approach? I mean that the driver should allocate
> an array of usb_iso_packet_descriptor instead of an array of uint16_t.

Thanks for the digging!

I'd rather fix usb_tap() with a comment explaining why the hack I
described above work and that it should be revisited if we start
supporting a device like the one you found.