Re: Patch to add usbmon

[Marcel Holtmann <marcel@holtmann.org>]

Hi Pete,

> > By accident I removed the debugfs option from my kernel config and this
> > makes usbmon totally useless. So I think the module approach is wrong
> > from my point of view. Why not compile it always and if debugfs is
> > available, then enable it when the usbcore gets loaded?
> 
> This may be a good idea, but I'm too lazy to think in trinary. Care to
> send a patch?

I can do this if you think that this is a good idea. Acutally I see
usbmon as an option of the USB core. This will also reduce the code a
little bit, because you don't need an registration interface. You can
call the usbmon functions directly.

> > And btw don't you think that you split your code into too much separate
> > files? I count less than 900 lines of code.
> 
> What does it hurt? I think the split was rather clean, along well defined
> functional lines, with minimum dependencies. They may even be separate
> modules later.

I think that separation is a good idea, but dividing such a minimum code
into four files didn't make it cleaner. Two much separation makes it
even worse and sometimes harder to understand. I personally vote for
having a single driver/usb/core/usbmon.c and link it into usbcore if
selected.

> > However it works fine for me, but I can't find a document that describes
> > the information (without looking at the code) you get, when running cat
> > on a specific bus.
> 
> I'm sorry about that. I will add a file in Documentation/ .

This would be great and btw the official mount point for debugfs should
be /sys/kernel/debug as Greg posted some time ago.

> > control urb endpoint 0x00 flags 0x00 buflen 3 actlen 0
> > bRequestType 0x20 bRequest 0x00 wValue 0 wIndex 0 wLength 3
> > pipe 0x80003900 flags 0x00 buffer f7f2e4e0 length 3 setup f64895a0 start 0 packets 0 interval 0
> > data 36 fc 00
> 
> You are right, the current text format (0t) is not easily readable.
> 
> My plan calls for the parsing of control messages and things like SCSI
> commands to be placed in user mode applications, like USBMon. This is how
> tcpdump and Ethereal operate. I don't like to keep too much of this in
> kernel. Do you think it's a wrong approach?

I think if cat is the prefered tool for viewing this file then it should
be more human readable. If not, then a binary format should be choosen.
Maybe we can implement both. Is this possible?

> > [...] I think it makes sense to present the device number of that device.
> 
> When I read dumps, I mentally fetch the device number from the pipe.
> Just use that for now, please.

I don't, because I need to check the encoding in the kernel source to do
that. I also think that we should abstract from the definition of pipe.
>From my view this is a kernel encoding and should not be visible to the
user. Maybe I am wrong, because I am not an expert of USB internals.

> But if you or someone else were to hack on something like usbdump(1),
> it would be peachy, I think.

I can start with usbdump if we agree on an interface. I personally would
prefer a binary interface for that.

Regards

Marcel