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

Pass Facedancer stress test, adding NAK support #1

Merged
merged 8 commits into from
Jun 6, 2024

Conversation

kauwua
Copy link
Member

@kauwua kauwua commented Jun 4, 2024

Hydradancer was built initially by always priming IN buffers (sending data before the host requests it). This worked for the different test devices (mouse, mass-storage, loopback, speedtest) because the device was already aware of what to send or the protocol on top of the USB protocol set the requirements in advance. The advantage is that it makes things faster, as data is already there when the host needs it. The issue is the device is not aware of the host requesting data. While this worked for a while, it meant the USB protocol was not fully implemented, which made the new USB stress test fail.

Speedtests with this new version are 50% slower : the order of magnitude is still the same compared with Facedancer21/GreatFET but waiting for the host to request data makes things slower. The previous speeds can still be reached for devices that support priming.

A lot of things had to be changed in this PR

  • NAK interrupts were not implemented, and are not used in any examples from WCH. NAK can easily trigger loads of interrupts, a lot of testing was needed to avoid missing other interrupts
  • the buffer status system used to share state between Facedancer and the firmware could not work anymore. Switching to an event system made things easiers and clearer.
  • dropping hydrausb3 firmwares, firmware_control_board and firmware_emulation_board : maintaining these with all the NAK changes is too much, it was already unstable with having to exchange buffers and events through HSPI, adding NAKs won't help. those firmwares can be found in v1.0.0 in a somewhat working state, and can be used with the v1.0.0 version the Facedancer backend.
  • fixing ZLPs : to make the stress test work for Facedancer, the corresponding Facedancer device was first implemented directly for Hydradancer to improve the tests (instead of the simple loopback, which was not testing control requests anyways). This uncovered issues dealing with ZLPs.
  • trying to recover from missed interrupts : very rarely, one interrupt can be missed, which breaks everything. For instance, not receiving confirmation than a buffer has indeed been sent to Facedancer prevents the firmware from being able to receive new buffer. Ideally, interrupts should not be missed and it seems to happen occasionnally in the highly stressed stress test. To recover from that, both the firmware and backend define a maximum delay after which the buffer has been considered to be sent. This can free deadlocks, but seems to cause some issues with overflows which is still somehow better than a deadlock as this might be improved later.

@kauwua kauwua changed the title Add naks rethink events Pass Facedancer stress test, adding NAK support Jun 4, 2024
@kauwua kauwua force-pushed the add_naks_rethink_events branch from 3a39153 to 03d9c4f Compare June 5, 2024 16:26
@kauwua kauwua merged commit 68526f4 into main Jun 6, 2024
6 checks passed
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.

1 participant