speedup 'hf mf chk' #901
speedup 'hf mf chk' #901
Conversation
pwpiwi
commented
Dec 23, 2019
pwpiwi
commented
- add separate timeout for tag response to nr_ar
- measure response time and use it for response timeout
- don't drop field between keyblocks
- some reformatting
- some whitespace fixes
* add separate timeout for tag response to nr_ar * measure response time and use it for response timeout * don't drop field between keyblocks * some reformatting * some whitespace fixes
* fishing for microseconds in TransmitFor14443a() * some reformatting
| @@ -1284,9 +1273,6 @@ static void TransmitFor14443a(const uint8_t *cmd, uint16_t len, uint32_t *timing | |||
| LastTimeProxToAirStart = ThisTransferTime; | |||
| } | |||
|
|
|||
| // clear TXRDY | |||
There was a problem hiding this comment.
This sends a 0x00 byte just to clear TXREADY. In the following loop we are waiting for TXREADY to be set again. This wastes 9.4us.
|
Tested the idea of setting / restoring timeout. Got 206/sec. Impressive speedups. |
armsrc/mifarecmd.c
Outdated
| if (res >= 0) { | ||
| cmd_send(CMD_ACK, 1, res, 0, keyIndex, 80); | ||
| } else { | ||
| cmd_send(CMD_ACK, 0, res, 0, NULL, 0); | ||
| drop_field = true; | ||
| } | ||
| } else { | ||
| int res = MifareChkBlockKeys(datain, keyCount, blockNo, keyType, OLD_MF_DBGLEVEL); | ||
| int res = MifareChkBlockKeys(datain, keyCount, blockNo, keyType, &auth_timeout, OLD_MF_DBGLEVEL); | ||
|
|
||
| if (res > 0) { | ||
| cmd_send(CMD_ACK, 1, res, 0, datain + (res - 1) * 6, 6); | ||
| } else { | ||
| cmd_send(CMD_ACK, 0, res, 0, NULL, 0); | ||
| drop_field = true; | ||
| } |
There was a problem hiding this comment.
Isn't this drop_field logic backwards? As written it always drops the field when the key is not found (and we're about to come back here again with a new set of keys to check) but does not drop it when the key is found and we're finished.
There was a problem hiding this comment.
Absolutely. Pushed a fix.
|
Field should be dropped on every return value except 0 as >0 is success and <0 is error Before: After: We get even more by pulling the (incorrect) And slightly more still by pulling the button press detection out of the loop completely. |
|
I was looking at the lights blink and thought a lot of time is wasted in waiting for the first set of keys to complete before transferring the next set, so I re-wrote it to "queue up" and USB transfer the next batch of keys while it's checking and got another ~16 keys/sec out of it. I'll put together a PR for this one. |
|
Not sure if this is worth the effort. And I am exploring another check functions tailored to fixed nonces. This should be faster anyway (I hope). |
|
I like the new drop_field check however MifareMultisectorChk() needs to be modified to return 1 on success for it to work. I went ahead and made a PR @ pwpiwi#1 anyway since I was basically done. The keys/sec calculation needs some tweaking as it now thinks 1 block more than actual has been completed. |
|
Hmm, I am getting lot's of connection errors: |
|
Probably because there is no way for the Proxmark to tell the client when it is ready to receive another bunch of keys. And it cannot receive any commands while it is working on the current key block,.. |
|
Really? It's working fine on mine. I thought the USB side was a hardware peripheral? Sure it can't process it while working, but I thought it would just sit in the buffer in that case. |
The hardware buffer is only 64 Bytes. Maybe the Operating System makes the difference here. You are on Linux? I am on Windows. |
|
Yep, on an antique version of Fedora. |
|
Confirmed. Works on Kali without any changes. The speed measurement for first keyblock is still off (but now it is too fast instead of too slow). |
|
Different timeouts or error handling in uart_win32.c vs uart_posix.c ? |
|
Possibly, I remember those files were way different and when I was working on disconnect/reconnect they needed some changes. I don't have a Windows box so I can't test easily. |
|
Which reminds me of PR#516 which is still open. Can you rebase it? |
|
Well, it turns out that moving the button press check doesn't work so well. I put it back and fixed the keys/sec but when trying to make a new PR it's wanting to pull all the changes in again. pwpiwi@3b62d28 I'll see if I can get to #516 in the next day or 2. |
* revert queuing change * instead allow arbitrary number of keys in MifareChkKeys() * and move progress printing to MifareChkKeys()
|
For the time being I have reverted your queuing patch. I propose to merge this PR (if there aren't any more issues) and open a new one for queuing when the comms issue is sorted out. |
* fix abort by pressing the PM3 button
|
Yep, sounds good. I'll try to get to both that and #516, though I'm not sure when as I'm back home and don't have much free time at the moment. |
