The Summer of Code firm pencils down deadline is about to expire. The below is concerned with the question "what must be changed to get this mergeable"...
zidel/packetFormat branch: 6a1f69c42d0f013e77253a3eb1bb39cc27e75c95 to 08e909d0697b7bb7c282705fda40c42aee873c02 - Update the encrypted sequence numbers to watch list incrementally - every time we use half the list, generate some more. - Put the counter in the last 4 bytes rather than the first 4 (after the nonce) for CTR mode encryption for packet IVs, this is more standard. - Remove @Override 1.6ism. - Logging. - Remove dead code and imports. - Trivial optimisation, use int instead of long when creating a packet. - Always start packet numbers from 0 for new packet format. - Store packet sequence numbers as ints. - Use PacketTracker for sequence numbers. This has the advantage of keeping the would block handling, but on the other hand it has a nasty packet window limit, lots of code related to old format retransmission code (which is bad); the retransmission logic e.g. is completely inappropriate. - Allocate sequence number after creating a packet. - Use 250ms as lower bound for RTT for resending. - Fix a range removal bug in SparseBitmap and add a unit test. - Add toString()'s. - Remove list of received messages. Replace it with separate windows for received and acked. Track the last message ID for which we have received all prior ID's. Loop back around (28 bits, so ++ doesn't do it) when reach 2^28; and when looping back, clear the SparseBitmap tracking as appropriate; same for acked. - Don't allow allocating a message ID outside the maximum window size, 64K concurrent messages. - Acknowledge messages outside the current message window. This is necessary because they may be retransmitted packets etc, and if they are out of range we should have received them etc. NewPacketFormat.tryDecipherPacket: - Please document the new variables. Pointer is the index within the watch list of the packet with the lowest number, offset is the sequence number of that packet, correct? - Might be clearer code if you moved the sequence number assignment out of the for(j), use continue outer instead of break. - It is critical that you be able to deal with multiple matches - parse each one until find one that is valid. - Should it be updated on every packet? Obviously it should never go backwards... - RuntimeException is very bad form, it is quite possible the protocol is busted, find a cleaner way to deal with it. 2e33916fee9106b0e985f1bf974ed3d8fda40f42 - Any particular reason for always starting from 0 on new format? 501bb6dfe50a6e54bd0c19d867c1136eed37128c - Race condition: Fetch highestReceivedIncomingSeqNumber once! 501bb6dfe50a6e54bd0c19d867c1136eed37128c - Not sure this is a good idea, old format is limited to 512 packets in flight (or was it 256?) and has lots of retransmit stuff etc you don't need. - This has the advantage of keeping the would block handling, but on the other hand it has a nasty packet window limit, lots of code related to old format retransmission code (which is bad); the retransmission logic e.g. is completely inappropriate. 209645bbbc3f3f5a1c2af5673951499c329d9c85 - What ensures we don't complete the same message twice then? 889a5cea3ed9b570f1f6e6784d34d61c9d932cd6 - Do we check for -1 in all getMessageID() callers? - Inconsistent synchronization in getMessageID()?? -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 197 bytes Desc: This is a digitally signed message part. URL: <https://emu.freenetproject.org/pipermail/devl/attachments/20100816/993ae5bd/attachment.pgp>
