Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] usbmon: Report device address assigned to USB device
@ 2020-01-06  9:37 Tomasz Moń
  2020-01-06 23:09 ` Pete Zaitcev
  2020-01-08 15:55 ` Greg Kroah-Hartman
  0 siblings, 2 replies; 6+ messages in thread
From: Tomasz Moń @ 2020-01-06  9:37 UTC (permalink / raw)
  To: linux-usb
  Cc: Tomasz Moń,
	Greg Kroah-Hartman, Jonathan Corbet, Pete Zaitcev, Alan Stern

Make USB device addresses match while sniffing USB communication
with usbmon and hardware USB sniffer (OpenVizsla) at the same time.
On xHCI root hubs the address is assigned by hardware and can be
different than devnum.

Signed-off-by: Tomasz Moń <desowin@gmail.com>
---
 Documentation/usb/usbmon.rst |  2 +-
 drivers/usb/mon/mon_bin.c    |  6 +++---
 drivers/usb/mon/mon_text.c   | 10 +++++-----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/usb/usbmon.rst b/Documentation/usb/usbmon.rst
index b0bd51080799..e603159f8a93 100644
--- a/Documentation/usb/usbmon.rst
+++ b/Documentation/usb/usbmon.rst
@@ -220,7 +220,7 @@ the following structure (its name is made up, so that we can refer to it)::
 	unsigned char type;	/*  8: Same as text; extensible. */
 	unsigned char xfer_type; /*    ISO (0), Intr, Control, Bulk (3) */
 	unsigned char epnum;	/*     Endpoint number and transfer direction */
-	unsigned char devnum;	/*     Device address */
+	unsigned char devaddr;	/*     Device address */
 	u16 busnum;		/* 12: Bus number */
 	char flag_setup;	/* 14: Same as text */
 	char flag_data;		/* 15: Same as text; Binary zero is OK. */
diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index f48a23adbc35..d7a55be5df68 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -91,7 +91,7 @@ struct mon_bin_hdr {
 	unsigned char type;	/* Same as in text API; extensible. */
 	unsigned char xfer_type;	/* ISO, Intr, Control, Bulk */
 	unsigned char epnum;	/* Endpoint number and transfer direction */
-	unsigned char devnum;	/* Device address */
+	unsigned char devaddr;	/* Device address */
 	unsigned short busnum;	/* Bus number */
 	char flag_setup;
 	char flag_data;
@@ -567,7 +567,7 @@ static void mon_bin_event(struct mon_reader_bin *rp, struct urb *urb,
 	ep->type = ev_type;
 	ep->xfer_type = xfer_to_pipe[usb_endpoint_type(epd)];
 	ep->epnum = dir | usb_endpoint_num(epd);
-	ep->devnum = urb->dev->devnum;
+	ep->devaddr = urb->dev->devaddr;
 	ep->busnum = urb->dev->bus->busnum;
 	ep->id = (unsigned long) urb;
 	ep->ts_sec = ts.tv_sec;
@@ -655,7 +655,7 @@ static void mon_bin_error(void *data, struct urb *urb, int error)
 	ep->xfer_type = xfer_to_pipe[usb_endpoint_type(&urb->ep->desc)];
 	ep->epnum = usb_urb_dir_in(urb) ? USB_DIR_IN : 0;
 	ep->epnum |= usb_endpoint_num(&urb->ep->desc);
-	ep->devnum = urb->dev->devnum;
+	ep->devaddr = urb->dev->devaddr;
 	ep->busnum = urb->dev->bus->busnum;
 	ep->id = (unsigned long) urb;
 	ep->ts_sec = ts.tv_sec;
diff --git a/drivers/usb/mon/mon_text.c b/drivers/usb/mon/mon_text.c
index bc5ecd5ff565..8f0d4a4416ef 100644
--- a/drivers/usb/mon/mon_text.c
+++ b/drivers/usb/mon/mon_text.c
@@ -59,7 +59,7 @@ struct mon_event_text {
 	unsigned long id;	/* From pointer, most of the time */
 	unsigned int tstamp;
 	int busnum;
-	char devnum;
+	char devaddr;
 	char epnum;
 	char is_in;
 	char xfertype;
@@ -210,7 +210,7 @@ static void mon_text_event(struct mon_reader_text *rp, struct urb *urb,
 	ep->type = ev_type;
 	ep->id = (unsigned long) urb;
 	ep->busnum = urb->dev->bus->busnum;
-	ep->devnum = urb->dev->devnum;
+	ep->devaddr = urb->dev->devaddr;
 	ep->epnum = usb_endpoint_num(&urb->ep->desc);
 	ep->xfertype = usb_endpoint_type(&urb->ep->desc);
 	ep->is_in = usb_urb_dir_in(urb);
@@ -282,7 +282,7 @@ static void mon_text_error(void *data, struct urb *urb, int error)
 	ep->type = 'E';
 	ep->id = (unsigned long) urb;
 	ep->busnum = urb->dev->bus->busnum;
-	ep->devnum = urb->dev->devnum;
+	ep->devaddr = urb->dev->devaddr;
 	ep->epnum = usb_endpoint_num(&urb->ep->desc);
 	ep->xfertype = usb_endpoint_type(&urb->ep->desc);
 	ep->is_in = usb_urb_dir_in(urb);
@@ -523,7 +523,7 @@ static void mon_text_read_head_t(struct mon_reader_text *rp,
 	p->cnt += snprintf(p->pbuf + p->cnt, p->limit - p->cnt,
 	    "%lx %u %c %c%c:%03u:%02u",
 	    ep->id, ep->tstamp, ep->type,
-	    utype, udir, ep->devnum, ep->epnum);
+	    utype, udir, ep->devaddr, ep->epnum);
 }
 
 static void mon_text_read_head_u(struct mon_reader_text *rp,
@@ -541,7 +541,7 @@ static void mon_text_read_head_u(struct mon_reader_text *rp,
 	p->cnt += snprintf(p->pbuf + p->cnt, p->limit - p->cnt,
 	    "%lx %u %c %c%c:%d:%03u:%u",
 	    ep->id, ep->tstamp, ep->type,
-	    utype, udir, ep->busnum, ep->devnum, ep->epnum);
+	    utype, udir, ep->busnum, ep->devaddr, ep->epnum);
 }
 
 static void mon_text_read_statset(struct mon_reader_text *rp,
-- 
2.24.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usbmon: Report device address assigned to USB device
  2020-01-06  9:37 [PATCH] usbmon: Report device address assigned to USB device Tomasz Moń
@ 2020-01-06 23:09 ` Pete Zaitcev
  2020-01-08 15:55 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 6+ messages in thread
From: Pete Zaitcev @ 2020-01-06 23:09 UTC (permalink / raw)
  To: Tomasz Moń
  Cc: linux-usb, Greg Kroah-Hartman, Jonathan Corbet, Alan Stern

On Mon,  6 Jan 2020 10:37:17 +0100
Tomasz Moń <desowin@gmail.com> wrote:

> Make USB device addresses match while sniffing USB communication
> with usbmon and hardware USB sniffer (OpenVizsla) at the same time.
> On xHCI root hubs the address is assigned by hardware and can be
> different than devnum.

> -	unsigned char devnum;	/*     Device address */
> +	unsigned char devaddr;	/*     Device address */

I think it's fine, the name of the field is not exported by a header
anyway, so there's no impact.

Acked-by: Pete Zaitcev <zaitcev@redhat.com>

-- Pete


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usbmon: Report device address assigned to USB device
  2020-01-06  9:37 [PATCH] usbmon: Report device address assigned to USB device Tomasz Moń
  2020-01-06 23:09 ` Pete Zaitcev
@ 2020-01-08 15:55 ` Greg Kroah-Hartman
  2020-01-11 23:52   ` Pete Zaitcev
  1 sibling, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-08 15:55 UTC (permalink / raw)
  To: Tomasz Moń; +Cc: linux-usb, Jonathan Corbet, Pete Zaitcev, Alan Stern

On Mon, Jan 06, 2020 at 10:37:17AM +0100, Tomasz Moń wrote:
> Make USB device addresses match while sniffing USB communication
> with usbmon and hardware USB sniffer (OpenVizsla) at the same time.
> On xHCI root hubs the address is assigned by hardware and can be
> different than devnum.

This breaks the userspace abi for matching up the devnum with the number
that is listed by the kernel to userspace, making it really hard to
match things up for xhci devices now :(

I understand the need to look at this data, but you can't do it in a way
that will change things like this.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usbmon: Report device address assigned to USB device
  2020-01-08 15:55 ` Greg Kroah-Hartman
@ 2020-01-11 23:52   ` Pete Zaitcev
  2020-01-13 15:05     ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: Pete Zaitcev @ 2020-01-11 23:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Tomasz Moń, linux-usb, Jonathan Corbet, Alan Stern, zaitcev

On Wed, 8 Jan 2020 16:55:13 +0100
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Mon, Jan 06, 2020 at 10:37:17AM +0100, Tomasz Moń wrote:
> > Make USB device addresses match while sniffing USB communication
> > with usbmon and hardware USB sniffer (OpenVizsla) at the same time.
> > On xHCI root hubs the address is assigned by hardware and can be
> > different than devnum.
> 
> This breaks the userspace abi for matching up the devnum with the number
> that is listed by the kernel to userspace, making it really hard to
> match things up for xhci devices now :(
> 
> I understand the need to look at this data, but you can't do it in a way
> that will change things like this.

This sounds reasonable, although perhaps unfortunate. I acked Tomasz's
patch because I thought that XHCI is new, and thus it's no big deal
if its results are different.

Of course I rushed to examine the packet structure, but yes, there's
no space in the header, not without some trickery. For example, the
upper 8 bits of the ID are probably the same for all packets, so
it may be stuffed in there. Or, it might be possible to create an
extra header and attach it at the end of ISO descriptors.

Alan's suggestion of leaving the physical address in /sys appeals
the most to me, honestly. One thing though, libpcap will need to
rifle through /sys and then store that address, so its serialization
has to be changed no matter what. Unfortunately, I'm wholly ignorant
as to what syntax it uses and how extensible it is.

-- Pete


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usbmon: Report device address assigned to USB device
  2020-01-11 23:52   ` Pete Zaitcev
@ 2020-01-13 15:05     ` Alan Stern
  2020-01-14 21:06       ` Pete Zaitcev
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2020-01-13 15:05 UTC (permalink / raw)
  To: Pete Zaitcev
  Cc: Greg Kroah-Hartman, Tomasz Moń, linux-usb, Jonathan Corbet

On Sat, 11 Jan 2020, Pete Zaitcev wrote:

> On Wed, 8 Jan 2020 16:55:13 +0100
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > On Mon, Jan 06, 2020 at 10:37:17AM +0100, Tomasz Moń wrote:
> > > Make USB device addresses match while sniffing USB communication
> > > with usbmon and hardware USB sniffer (OpenVizsla) at the same time.
> > > On xHCI root hubs the address is assigned by hardware and can be
> > > different than devnum.
> > 
> > This breaks the userspace abi for matching up the devnum with the number
> > that is listed by the kernel to userspace, making it really hard to
> > match things up for xhci devices now :(
> > 
> > I understand the need to look at this data, but you can't do it in a way
> > that will change things like this.
> 
> This sounds reasonable, although perhaps unfortunate. I acked Tomasz's
> patch because I thought that XHCI is new, and thus it's no big deal
> if its results are different.
> 
> Of course I rushed to examine the packet structure, but yes, there's
> no space in the header, not without some trickery. For example, the
> upper 8 bits of the ID are probably the same for all packets, so
> it may be stuffed in there. Or, it might be possible to create an
> extra header and attach it at the end of ISO descriptors.
> 
> Alan's suggestion of leaving the physical address in /sys appeals
> the most to me, honestly. One thing though, libpcap will need to
> rifle through /sys and then store that address, so its serialization
> has to be changed no matter what. Unfortunately, I'm wholly ignorant
> as to what syntax it uses and how extensible it is.

Alternatively, libpcap can ignore the issue and just display the device
numbers, as it does now.  A separate program or the user could convert
the number to a physical address, if necessary, using the information 
in sysfs.

Alan Stern


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usbmon: Report device address assigned to USB device
  2020-01-13 15:05     ` Alan Stern
@ 2020-01-14 21:06       ` Pete Zaitcev
  0 siblings, 0 replies; 6+ messages in thread
From: Pete Zaitcev @ 2020-01-14 21:06 UTC (permalink / raw)
  To: Alan Stern
  Cc: Greg Kroah-Hartman, Tomasz Moń, linux-usb, Jonathan Corbet, zaitcev

On Mon, 13 Jan 2020 10:05:42 -0500 (EST)
Alan Stern <stern@rowland.harvard.edu> wrote:

> > Alan's suggestion of leaving the physical address in /sys appeals
> > the most to me, honestly. One thing though, libpcap will need to
> > rifle through /sys and then store that address, so its serialization
> > has to be changed no matter what. Unfortunately, I'm wholly ignorant
> > as to what syntax it uses and how extensible it is.  
> 
> Alternatively, libpcap can ignore the issue and just display the device
> numbers, as it does now.  A separate program or the user could convert
> the number to a physical address, if necessary, using the information 
> in sysfs.

I thoght that people sometimes saved the traces and replayed them
on other machines, which obviously have their own /sys or even run
Windows or iOS.

-- Pete


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-06  9:37 [PATCH] usbmon: Report device address assigned to USB device Tomasz Moń
2020-01-06 23:09 ` Pete Zaitcev
2020-01-08 15:55 ` Greg Kroah-Hartman
2020-01-11 23:52   ` Pete Zaitcev
2020-01-13 15:05     ` Alan Stern
2020-01-14 21:06       ` Pete Zaitcev

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git