Re: Min Xu patches testing (Lukas Kuzmiak)

Hi Lukas I saw your email to my @sanjole.com address but since this address is on the mailing list I'll just reply here so everyone can see. --- your email below --- Hi Min Xu, first of all thanks for all the work you put into simtrace. I’ve been testing your patches last couple days, they have not yet been in any release which I think is a shame, so I’d like to push the community to fix that so it can be even further tested (by testing, reporting back, fixing building the firmware with latest arm-none-eabi, etc.) I have found one bug for which I fail to identify the source .. The whole trace is fine except sometimes there are 4 bytes inside the data which seems to be the simtrace_hdr (first line is my debug): USB MSG: sh->cmd: 1, sh->flags: 0, sh->res[9, 5], payload: 00 a4 00 04 02 a4 6f 07 61 2a 00 c0 00 00 2a c0 62 28 82 02 41 21 83 02 6f 07 a5 0f 80 01 71 c0 01 00 91 04 7f 20 6f 07 92 01 00 8a 01 05 8b 03 6f 06 03 80 02 00 09 88 01 38 90 00 01 00 09 05 00 b0 00 00 09 b0 08 29 03 30 10 66 03 91 12 90 00 APDU: 00 a4 00 04 02 6f 07 61 2a APDU: 00 c0 00 00 2a 62 28 82 02 41 21 83 02 6f 07 a5 0f 80 01 71 c0 01 00 91 04 7f 20 6f 07 92 01 00 8a 01 05 8b 03 6f 06 03 80 02 00 09 88 01 38 90 00 APDU: 01 00 09 05 00 b0 00 APDU: 00 09 b0 08 29 03 30 APDU: 10 66 03 91 12 90 00 The 01 00 09 05 (which seems to be sh->cmd, sh->flags and Fi/Di (9/5) just randomly appear in APDU data every now and then. If those 4 bytes were not there apdu_split would split it fine .. like this it breaks this into nonsense pieces and breaks the trace. I’ve gone through your patches in the firmware, fiddled around with some of them but didn’t manage to find the root cause yet - seems like the simtrace_hdr is inserted in the middle (but that does not seem possible), so perhaps 2 USB messages somehow get merged into one? So I figured I’ll try to write you, maybe it will ring a bell - I have not fully verified the merge of your patches went correctly but from a fast compare it seems like it. If you’ll find a minute to give me a few tips I’ll appreciate that, in the meantime I’ll keep digging. PS: the version of FW I’m using is latest master in the git (https://git.osmocom.org/openpcd). Thanks! Lukas --- END --- I believe the reason for this is actually in an email I sent to the list on Sep 10, 2013. Basically, the ATMEL chip can break up the req_ctx ( the usb response ) and combine as it see fit (if there's a large burst etc). So you cannot rely on the "natural break" between the calls to transmit. Therefore, since it's a stream you'll get on receiving side, then a natural packet header that accounts of subsequent bytes must be added. So I added extra bytes into the simtrace_hdr header so that the actual data payload can be correctly accounted for. The changes are: struct simtrace_hdr { u_int8_t cmd; u_int8_t flags; u_int8_t res[2]; + u_int16_t seq_num; + u_int16_t offset; + u_int16_t tot_len; u_int8_t data[0]; } __attribute__ ((packed)); So the desktop client will have to have equivalent changes to account for these extra fields. Let me know if this answers your question. If not I can try send you the full code I have for the firmware and a sample of the desktop receiving / parsing code so you'll have a baseline.