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

usb2.control: Support PING protocol in data OUT and status OUT stages. #172

Merged
merged 1 commit into from Jul 18, 2022

Conversation

zyp
Copy link
Contributor

@zyp zyp commented Jul 17, 2022

USB2.0 8.5.1 states:

High-speed devices must support an improved NAK mechanism for Bulk OUT and Control endpoints and transactions. Control endpoints must support this protocol for an OUT transaction in the data and status stages. The control Setup stage must not support the PING protocol.

We observe that hosts running Linux 5.18 sometimes issues PING tokens during control requests, and not handling them means those requests are unable to complete, resulting in the device failing to enumerate correctly.

This patch solves the issue by always returning ACK to PING tokens received during data OUT and status OUT stages. In most cases ACK is the correct response since we're ready to handle data, and in the few cases it's not, we'll just fall back to letting the request handler return a NAK to the following data packet.

@miek
Copy link
Member

miek commented Jul 18, 2022

Thanks, this looks great.

I'd quite like to reproduce the issue here but I'm not having much luck so far. I've tried the latest Arch ISO (2022.07.01 with kernel 5.18.7) on a couple of machines but it doesn't issue any PINGs and a stock ORBTrace enumerates OK. Have you got any more advice on how to trigger it?

@zyp
Copy link
Contributor Author

zyp commented Jul 18, 2022

Check whether sudo lsusb -vd 1209:3443 | grep iInterface is displaying interface strings or errors. Errors there is the first symptom of this issue.

I guess it could be driver dependent, but on the machine I've got running that same Arch ISO here, I see PINGs both on the USB2 ports going through EHCI and the USB3 ports going through XHCI.

@zyp
Copy link
Contributor Author

zyp commented Jul 18, 2022

Also, what appears to be the actual trigger is the SET_LINE_CODING request that's sent to the ACM interfaces. I haven't implemented a dummy handler for those yet, so they get neither ACKed nor NAKed, and the host appears to turn on PING protocol before retrying, as part of the error handling.

The ACM request failing is unproblematic in itself, but results in PING protocol still remaining enabled when the status stage for the following request is sent, and when we're not handling that properly, the standard request handler gets stuck.

@miek
Copy link
Member

miek commented Jul 18, 2022

Right, I see it now - thanks! For whatever reason, my system only sends two PING packets before giving up and carrying on, so I didn't see them before:

controlreq-ping-bug_000

@miek miek merged commit 4bc5899 into greatscottgadgets:main Jul 18, 2022
@miek
Copy link
Member

miek commented Jul 18, 2022

Cheers!

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

Successfully merging this pull request may close these issues.

None yet

2 participants