First, thanks for the Joe for very good TSV review for this draft. I am now going through this draft to see whether it is ready for WGLC, and while reviewing it I have some comments on it. On the other hand I think it is starting to get ready for WGLC...
In section 2.2.3 fragmentation etc, the draft talks about replay protection, but in the ESP replay attack protection is optional, and does not need to be done by the receiver, altough the sequence number generation MUST be done by sender. Also normally in ESP seqeuence numbers are not required to be back to back, there could be cases where they would be incrementing by larger value, and cases where the sequence numbers are not sent out in order. This can happen for example if multiple encryption boards are used, and each of them have separate coordinated sequence number generation. This can also cause reordering in some cases. The draft now says that "sender MUST Send the inner-packet fragments back-to-back", which adds a new requirement for ESP. It might be good idea to add note, that says this changes requirement from RFC4303, which only requires that sequence numbers are incremented, not by how much (for example draft-ietf-lwig-minimal-esp proposes that in some enviroments it might be better to use time for sequence number generation so they do not need to store sequence number while sleeping). Section 2.2.3 does not change the replay protection parts, just talks about that similar method than what is used in the replay protection can be used for reassembly. This may cause issues if the ESP level does not do replay protection, but the reassembly code assumes it is done. The text also seems to indicate that the window sizes for the reassembly and replay protection could be different. If reassembly and replay windows are not same, I think the smaller one of them is used, as if replay window is smaller, then too old packets are dropped before they would reach the larger reassembly window, and same happens for other way around. I think we could just say that when using this detection of replays is mandatory and that replay window and the reassembly window are same. Also note, that increasing replay window size does not have that much effect on the memory consumption of the SA, but increasing reassembly window size does have. I.e., if the quite often used 1024 packet replay window size is matched with same sized reassembly window of 1024 * 1024 byte packets, that will mean each SA could consume up to 1 MB for reassembly window. Also even when the 2.2.3 says in that there is no need for timers, I disagree with that, as during rekey or other operations it might be possible that the traffic from the SA is moved to the newly created SA, and then few minutes later the old SA is deleted. Keeping the reassembly buffers for that long just because there is no incoming packets is just wasting data. In section 2.2.2 and 2.2.3.1 the draft tries to fill the packets to full, even when there would only be one byte fitting on the frame, and I think this is bad idea. I would think it would be better to say that if less than 5% (or some other fixed configurable amount) of the packet is available and the packet to be added would be fragmented, then packet is padded instead of fragmenting, and the packet to be added is moved to next outer packet. I think there is too much emphasis on not wasting the bandwidth and too much emphasis on the robustness of the protocol. Small amount of padding that will avoid fragmentation is much more desirable than not wasting few % of the bandwidth. In section 2.2.5 it says that ECN value is not mapped, because of constant-send-rate, but it leaves out the other part, i.e., if some device is sending inner data faster than the constant-send-rate then the IPTFS should set the ECN for inner packets when adding them to the aggregated frame, just to indicate this information to the other end. In section 2.2.7 the draft says there is no effective MTU, which is true, but the question is that should there be one? I mean even if the original sender will automatically detect that 63kB UDP packets go through without fragmentation that does not mean that it will be good idea to use 63kB packets as that will cause latency for all other packets sharing the same SA. I think it would be useful to be able to restrict the effective MTU of the tunnel. In section 2.3 why does the text say: In other words, an SA that has AGGFRAG_PAYLOAD enabled MUST NOT have non-AGGFRAG_PAYLOAD payloads such as IP (IP protocol 4), TCP transport (IP protocol 6), or ESP pad packets (protocol 59) intermixed with non-empty AGGFRAG_PAYLOAD payloads. why is that restriction with only non-empty AGGFRAG_PAYLOAD payloads? When can empty AGGFRAG_PAYLOAD used intermixed with other payloads? The rekeying of the SA is not specified fully. It does say that rekeying cannot change the USE_AGGFRAG information, but it does not say that rekeyed SA actually inherits the USE_AGGFRAG flag. It should explictly mention that. Also it does not mention whether frame is allowed to to be fragmented across the old and new SA. It should say that as sequence numbers are different for the SAs, the packet can never be fragmented so that part of it is in old SA, and part of it is in the newly created SA. In section 2.2.4 draft says: ESP payload length is equal to the AGGFRAG_PAYLOAD header length. This would indicate that this packet is not the same length than other frames. I think it would be better to say that this has just the AGGFRAG_PAYLOAD header, and then rest of the frame is padding. -- kivi...@iki.fi _______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
