Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

usb device enumeration issues // usbip incompatibility #861

Closed
gozu42 opened this issue Mar 23, 2021 · 5 comments · Fixed by #987
Closed

usb device enumeration issues // usbip incompatibility #861

gozu42 opened this issue Mar 23, 2021 · 5 comments · Fixed by #987
Assignees
Labels
bug technical support request for technical support

Comments

@gozu42
Copy link
Contributor

gozu42 commented Mar 23, 2021

Steps to reproduce

minimal steps // devolved repro:
host OS is a fedora.

  1. dnf install usbip
  2. systemctl start usbip-server
  3. systemctl start usbip-client
  4. usbip list -l
  5. usbip bind -b 2-1.3 (busid from the list output)
  6. usbip list -r 127.0.0.1 (to see its actualy there)
  7. usbip attach -r 127.0.0.1 -b 2-1.3

at that point, dmesg should show some unhappy messages like ...
usb 4-1: device descriptor read/64, error -71
usb 4-1: device descriptor read/8, error -71
usb usb4-port1: Cannot enable. Maybe the USB cable is bad?

note it repeats with /64 a few times, then tries /8, indicating it is related to the probing mechanism described in the ancient mailpost here: https://www.spinics.net/lists/usb/msg02644.html

Expected behaviour

hackrf working through usbip

Actual behaviour

device enumeration fails

Version information

Operating system:
fedora-33 (shouldnt matter as long as it "some linux")

hackrf_info output:
build from current git head, so release 2020.
same symptoms with older versions, both git and release.

Guesses

the hackrf side usb stack seems to dislike the 64 byte sized read request in a way that makes the rest fail.
the reported error -71 seems to be a generic "something went wrong".
the libopencm3 issue tracker has a-plenty of people bumping into same/similar symptoms, the fixes seem to vary wildly depending on platform.
reading through firmware/common/usb* and hackrf_usb/usb* i can not spot any obvious places where 64 byte would be unsupported.

@straithe straithe added the technical support request for technical support label Mar 24, 2021
@gozu42
Copy link
Contributor Author

gozu42 commented Mar 24, 2021

USB captures of a good and bad initialisation. (zip because github supported file types)
usbshark-hackrf.zip

open with "wireshark usbshark-hackrf-good.pcapng"
i left a bit of the surrounding traffic in (mostly hub/portstatus things) in case it is relevant.
narrow it down to the hackrf itself with this filter: usb.addr == "2.55.0"

looking at the first "GET DESCRIPTOR Request DEVICE" in each, the main difference seems to be indeed the length:
18 for good, 64 for bad.

@gozu42
Copy link
Contributor Author

gozu42 commented Apr 18, 2021

with "a bit" of usart0-print-debugging i narrowed it down to this line:
https://github.com/mossmann/hackrf/blob/5f1536cc3631f79e6beafbd8d7bb6cf86dcf555f/firmware/common/usb_standard_request.c#L279

if i comment out that call to usb_set_address_immediate, hackRF works through usbip.

but i dont know what that line is supposed to do, or if it might be needed in some cases, so i dont feel comfortable making a PR for this worksforme kludge.

perhaps guarding it with some additional check that skips it if oldaddr==newaddr?

@mossmann mossmann self-assigned this Jun 13, 2021
@mossmann
Copy link
Member

Nice work tracking this down! I think you are right that we should remove that line (well, three lines and a comment), but I'm currently at the "how did this ever work?" phase of debugging. I'm pretty sure we should never set the address when handling a SET_CONFIGURATION request. I suspect that this may have caused other problems on platforms that set the configuration to zero (which I believe Linux does not normally do).

It seems that your wireshark captures don't quite allow comparison of the same condition. You can see the difference by filtering for (usb.addr == "2.55.0") || (usb.addr == "2.0.0"). The "good" capture includes a GET_DESCRIPTOR sent to address 0 before the address is initially set. The "bad" capture does not. Maybe you physically plugged in the HackRF during the "good" capture but not during the "bad" capture? Additionally I think it would be helpful to look at this from the PHY layer instead of from the OS layer, so I'm going to try to analyze it with a LUNA.

@gozu42
Copy link
Contributor Author

gozu42 commented Jun 14, 2021

the captures were the closest // most comparable i could get with the current setup. (PC with just one USB controller and usb-hid)
the "good" capture was most likely powercycling the port on the USB hub. (device enumeration by the stack with the physical device connection, aka "usbip server")
the "bad" happens after the good, when "reattaching" the device via usbip (the step 7 from the initial post). (device enumeration by the stack with the virtual connection, aka "usbip client")

really looking forward to getting my hands on a LUNA. while i dont think it would have helped me much in this particular case (except as a capture source), it looks like a real gamechanger for any usb-mitm usecases compared to my current "raspi zero using builtin-if in gadgetmode and a max3421e-spi as host-if" kludge.

what i could have done differently here is using "proper" jtag/swd tooling to look at/into the hackRF, but at the end of the day, using print-to-serial debugging revealed the code pathes the hackrf was taking well enough and didnt involve any half-pitch soldering or entirely new (to me) tool stacks.

@mossmann
Copy link
Member

mossmann commented Nov 8, 2021

I hadn't gotten around to testing it with LUNA, but I am confident this is the correct solution based on the USB specification. Thank you for pointing it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug technical support request for technical support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants