Open desowin opened 5 years ago
This partly gets into "what OV is intended for vs what it isn't intended for". We originally built it for debugging these very low level issues, for which there previously was no open-source tools. It can certainly be extended for other use cases, but we didn't design the gateware with them in mind.
That said, I this is relatively straightforward to do, but would require changing the initialization process to add a filter setup step. The simplest mechanism would be to add a 16 bit vector that enables or disables capturing that PID.
For counter tracking, I'll think about it. The most straightfoward way would be to extend the timestamp field - but that would increase the capture bandwidth for small packet captures. Another alternative would be to stuff a frame on roll-over if no frame had been sent (~4hz).
I think the technically best solution would be to send an extended timestamp field on any that has a 24bit counter rollover from a previous packet. This has the same downsides as stuffed roll-over frames, but doesn't require a constant stream of information to the host.
This of course changes the packet/wire format (which takes us to the discussion in #30 ).
That said, I this is relatively straightforward to do, but would require changing the initialization process to add a filter setup step. The simplest mechanism would be to add a 16 bit vector that enables or disables capturing that PID.
Such simple solution could only work for SOFs. To effectively filter NAKed transactions a state machine is needed. Basically it is needed to recognize OUT/DATA/NAK or IN/NAK or PING/NAK and filter all these packets if they end in NAK (only if they are not directly after SPLIT packet).
However, a special care needs to be taken regarding the SPLIT transactions, as the simple recognition is not going to work correctly. I think that the NAK filtering of SPLIT transactions can be explicitly omitted - as doing so properly requires quite some memory: the minimal sequence for SPLITed NAK is SPLIT start/IN/ACK/anything/SPLIT complete/IN/NAK. The worst is that the anything can be either no transactions or as many as the host wishes. To effectively filter it, everything from SPLIT start up to the last NAK, excluding the anything would have to be removed from the trace. The simpliest way to filter SPLIT transactions, is to not generate them in first place, which is rather easy (sniff directly at the end full/low-speed device connection and not through a high speed hub)
Thanks for the refresher on that. Now I remember why I didn't implement it originally. Its been a few years since I worked with USB at that level, and the details aren't in my working memory anymore.
Would you consider writing up a design document for that state machine filter? Just states, transitions, and decisions on whether the packet should be discarded or forwarded? We could probably simulate that SM in python on existing captures to verify that it works, then I could write some RTL to make it work at wire-speed.
The gateware already has support for filters + discarding packets; we just need the filter itself.
The "filter NAKed transactions" state machine has 6 states (stDEFAULT, stSPLIT, stIN, stOUT, stOUTDATA, stPING) and requires queue that can hold up to two packets (and their respective FPGA clock timestamps). The design explicitly forwards all SPLIT transactions (even if they contain NAKed transaction) as filtering SPLIT transactions requires quite some memory and ability to deliver packets out of order to the analysis software. The "filter NAKed transactions" state machine starts in state stDEFAULT. In order to work properly all packets must enter the state machine (includes SOFs even if the "filter SOF" is enabled).
The state transitions are described in a table below. Table entries are sorted by current state. The PID value "other" denotes all PID values (including PID byte value where the negated form do not match due to transmission error) that do not have explicitly mentioned action originating from given state.
Current State | PID | Next State | Actions |
---|---|---|---|
stDEFAULT | SPLIT (0x78) | stSPLIT | forward packet |
stDEFAULT | IN (0x69) | stIN | queue packet |
stDEFAULT | OUT (0xE1) | stOUT | queue packet |
stDEFAULT | PING (0xB4) | stPING | queue packet |
stDEFAULT | other | stDEFAULT | forward packet |
stSPLIT | * | stDEFAULT | forward packet |
stIN | IN (0x69) | stIN | forward queued packets; queue packet |
stIN | OUT (0xE1) | stOUT | forward queued packets; queue packet |
stIN | PING (0xB4) | stPING | forward queued packets; queue packet |
stIN | SPLIT (0x78) | stSPLIT | forward queued packets; forward packet |
stIN | NAK (0x5A) | stDEFAULT | discard queued packets; discard packet |
stIN | other | stDEFAULT | forward queued packets; forward packet |
stOUT | IN (0x69) | stIN | forward queued packets; queue packet |
stOUT | OUT (0xE1) | stOUT | forward queued packets; queue packet |
stOUT | PING (0xB4) | stPING | forward queued packets; queue packet |
stOUT | SPLIT (0x78) | stSPLIT | forward queued packets; forward packet |
stOUT | DATA0 (0xC3) | stOUTDATA | queue packet |
stOUT | DATA1 (0x4B) | stOUTDATA | queue packet |
stOUT | other | stDEFAULT | forward queued packets; forward packet |
stOUTDATA | IN (0x69) | stIN | forward queued packets; queue packet |
stOUTDATA | OUT (0xE1) | stOUT | forward queued packets; queue packet |
stOUTDATA | PING (0xB4) | stPING | forward queued packets; queue packet |
stOUTDATA | SPLIT (0x78) | stSPLIT | forward queued packets; forward packet |
stOUTDATA | NAK (0x5A) | stDEFAULT | discard queued packets; discard packet |
stOUTDATA | other | stDEFAULT | forward queued packets; forward packet |
stPING | IN (0x69) | stIN | forward queued packets; queue packet |
stPING | OUT (0xE1) | stOUT | forward queued packets; queue packet |
stPING | PING (0xB4) | stPING | forward queued packets; queue packet |
stPING | SPLIT (0x78) | stSPLIT | forward queued packets; forward packet |
stPING | NAK (0x5A) | stDEFAULT | discard queued packets; discard packet |
stPING | other | stDEFAULT | forward queued packets; forward packet |
The "filter SOF" should operate on packets forwarded by the "filter NAKed transactions" state machine. "filter SOF" forwards packets whose PID is not SOF (0xA5).
This is excellent, thank you. I'll look into implementing it. The "dual packet queue" will probably end up being hybridized with the "packet whacker" code since we already have a mechanism for holding data while we think about what to do with it. As I'm unfortunately without a OV at the moment, would you be able to grab some packet captures (including NAK'ed + SPLIT transactions) that I can use to simulate and debug the filter? Otherwise, I can look into implementing this once I have hardware again.
In what format would you like the captures?
I believe PCAP should have everything I need.
I believe PCAP should have everything I need.
The https://bugs.wireshark.org/bugzilla/attachment.cgi?id=17250 contains blackmagicprobe-via-hs-hub-usbll.pcap that was generated using OpenVizsla - it contains NAKed and SPLIT transactions. This pcap file uses the LINKTYPE_USB_2_0, which does not contain the OpenVizsla flags. Each pcap packet starts with USB PID. The file can be viewed in latest Wireshark automated builds (only the raw packets, transfer reassembly is not implemented yet).
I have completely forgot about the PING. I have just updated the state machine transitions to handle PING.
I have implemented the NAK and SOF filter in C in https://github.com/matwey/libopenvizsla/commit/db4578e43bb9b7f5030bbd19293a4787ea4134c4
I am wondering if there was any progress on applying such a filer on the gateware?
The gateware already has support for filters + discarding packets; we just need the filter itself.
I saw that in the code and wanted to start implementing a simple "SOF" filter first - then found that "test_producer.py" is broken due to usage of SimActor etc. Just asking if anyone has already done that work?
Otherwise I am going to try myself
The state transitions are described in a table below. Table entries are sorted by current state. The PID value "other" denotes all PID values (including PID byte value where the negated form do not match due to transmission error) that do not have explicitly mentioned action originating from given state.
The easiest way to encounter the "other" PID value is using device that implements Link Power Management. Link Power Management uses PID 0xF0, followed by SubPID 0xC3 (this SubPID uses different CRC scheme than DATA0). According to USB 2.0 Link Power Management Addendum the Link Power Management transactions cannot be NAKed. There is no need to specifically handle them, because the default state handles it nicely.
sorry for the very late follow-up: I think the easiest way for now would be to do the filtering on the host PC side, between receiving the low-level-traces from the ov_ftdi and writing the pcap (or whatever format) files. That's easy to develop, and should be muhc quicker than changing the gateware in the FPGA.
sorry for the very late follow-up: I think the easiest way for now would be to do the filtering on the host PC side, between receiving the low-level-traces from the ov_ftdi and writing the pcap (or whatever format) files. That's easy to develop, and should be muhc quicker than changing the gateware in the FPGA.
This describes exactly what is implemented in https://github.com/matwey/libopenvizsla/commit/db4578e43bb9b7f5030bbd19293a4787ea4134c4
This describes exactly what is implemented in matwey/libopenvizsla@db4578e
Oh great. I wasn't even aware that there is a C language library now (excellent news!).
I would like to invite @matwey to migrate the repository within the openvizsla github project to give it more visibility. I think it's best if all openvizsla related code resided here.
@laf0rge I've never migrated the repos, so I have no idea how to proceed :-) Will I still have an access to the repo?
@laf0rge I've never migrated the repos, so I have no idea how to proceed :-) Will I still have an access to the repo?
You would of course get the same permissions (assigned by the openvizsla github "organization") as in your own "user" domain. The process is described at https://docs.github.com/en/github/administering-a-repository/transferring-a-repository#transferring-a-repository-owned-by-your-user-account
Once again, just a suggestion. Not trying to take anything away from you. I just thought it's good to have all OV related code in one place, next to each other.
Note: I am still waiting to get permissions on the openvizsla project, so please don't start any transfer yet. But feel free to give it some thought.
I am fine with migration, just curious how many things are going to break. Probably all CI stuff and deploy keys for windows builds?
When debugging USB issues in embedded devices, the importance of very low-level details (SOF and NAKs) lowers as the USB stack implementation improves. Once the low-level issues are solved, user focus shifts more into higher protocol layers. This is where Wireshark dissection engine is invaluable. Unfortunately the captures are growing very rapidly and thus the dissection is taking a lot of memory. For example, the capture attached to Wireshark bugzilla https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15908#c6 that contains data captured in less than 12 seconds contains over 1 million packets which require 18 MiB to store in pcap file. The same capture, but on the OS USB URB level (capture contains the payload with very little capture metadata) is merely 9 KiB. The captures are quickly growing to a point where Wireshark dissection engine starts slowing down significantly.
The solution to the capture files growing a lot is to make it possible to filter the low-level data that are of little interest to the user. In my opinion it would be best to implement such filtering in the FPGA gateware itself. The filtering would have a really big impact on reducing the analysis computer CPU uage. Of course, the filtering must be configurable by the user as issues like https://github.com/ARMmbed/mbed-os/pull/11103 are not possible to be detected when the "filter NAKed transactions" option is enabled.
Besides the actual filtering, we would need some time synchronization mechanism to indicate the 24-bit clock counter overflows when the SOFs are filtered, so the host software can track the realtive time with the accuracy of the 60 MHz clock.