From 3bd641abd0d170df0968ac792ac19aa38913ff93 Mon Sep 17 00:00:00 2001 From: zzz Date: Tue, 6 Dec 2011 21:50:33 +0000 Subject: [PATCH] * UDP: Fix major MTU bug introduced in 0.8.9. - Change large MTU from 1492 to 1484 and small from 608 to 620 for encryption padding efficiency - Enforce sent MTU limit - Increase receive buffer size from 1536 to 1572 so that excessive-sized packets sent by 0.8.9-0.8.11 routers aren't dropped - Limit the max acks in a data packet - Limit the duplicate acks in successive data packets - Only include acks that will fit in the mtu in a data packet - Correctly remove acks from the pending set after they are sent, so they aren't sent repeatedly - Don't pad data packets unless necessary - Debug logging and javadocs --- history.txt | 20 ++ .../src/net/i2p/router/RouterVersion.java | 2 +- .../i2p/router/transport/udp/ACKSender.java | 2 +- .../transport/udp/InboundMessageState.java | 26 ++- .../udp/OutboundMessageFragments.java | 2 +- .../router/transport/udp/PacketBuilder.java | 218 +++++++++++++----- .../i2p/router/transport/udp/PeerState.java | 110 ++++++--- .../i2p/router/transport/udp/UDPPacket.java | 17 +- .../i2p/router/transport/udp/UDPSender.java | 5 +- 9 files changed, 310 insertions(+), 92 deletions(-) diff --git a/history.txt b/history.txt index 6aed39dce..924400a90 100644 --- a/history.txt +++ b/history.txt @@ -1,3 +1,23 @@ +2011-12-06 zzz + * Router: + - More refactoring tasks to their own files + - Adjust some thread priorities + * Susimail: Adjust login form sizes + * Tunnels: Increase next hop send timeout + * UDP: Fix major MTU bug introduced in 0.8.9. + - Change large MTU from 1492 to 1484 and small from 608 to 620 + for encryption padding efficiency + - Enforce sent MTU limit + - Increase receive buffer size from 1536 to 1572 so that excessive-sized + packets sent by 0.8.9-0.8.11 routers aren't dropped + - Limit the max acks in a data packet + - Limit the duplicate acks in successive data packets + - Only include acks that will fit in the mtu in a data packet + - Correctly remove acks from the pending set after they are sent, + so they aren't sent repeatedly + - Don't pad data packets unless necessary + - Debug logging and javadocs + 2011-12-04 zzz * Console: - Less icons on configclients.jsp diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index e1f87e077..8ab24107c 100644 --- a/router/java/src/net/i2p/router/RouterVersion.java +++ b/router/java/src/net/i2p/router/RouterVersion.java @@ -18,7 +18,7 @@ public class RouterVersion { /** deprecated */ public final static String ID = "Monotone"; public final static String VERSION = CoreVersion.VERSION; - public final static long BUILD = 16; + public final static long BUILD = 17; /** for example "-test" */ public final static String EXTRA = ""; diff --git a/router/java/src/net/i2p/router/transport/udp/ACKSender.java b/router/java/src/net/i2p/router/transport/udp/ACKSender.java index 3c56d11ab..da5e1034e 100644 --- a/router/java/src/net/i2p/router/transport/udp/ACKSender.java +++ b/router/java/src/net/i2p/router/transport/udp/ACKSender.java @@ -161,7 +161,7 @@ class ACKSender implements Runnable { continue; } - if ( (ackBitfields != null) && (!ackBitfields.isEmpty()) ) { + if (!ackBitfields.isEmpty()) { _context.statManager().addRateData("udp.sendACKCount", ackBitfields.size(), 0); if (remaining > 0) _context.statManager().addRateData("udp.sendACKRemaining", remaining, 0); diff --git a/router/java/src/net/i2p/router/transport/udp/InboundMessageState.java b/router/java/src/net/i2p/router/transport/udp/InboundMessageState.java index b810faff9..16a194d13 100644 --- a/router/java/src/net/i2p/router/transport/udp/InboundMessageState.java +++ b/router/java/src/net/i2p/router/transport/udp/InboundMessageState.java @@ -153,29 +153,43 @@ class InboundMessageState { return new PartialBitfield(_messageId, _fragments); } + /** + * A true partial bitfield that is not complete. + */ private static final class PartialBitfield implements ACKBitfield { - private long _bitfieldMessageId; - private boolean _fragmentsReceived[]; + private final long _bitfieldMessageId; + private final boolean _fragmentsReceived[]; + /** + * @param data each element is non-null or null for received or not + */ public PartialBitfield(long messageId, Object data[]) { _bitfieldMessageId = messageId; + boolean fragmentsRcvd[] = null; for (int i = data.length - 1; i >= 0; i--) { if (data[i] != null) { - if (_fragmentsReceived == null) - _fragmentsReceived = new boolean[i+1]; - _fragmentsReceived[i] = true; + if (fragmentsRcvd == null) + fragmentsRcvd = new boolean[i+1]; + fragmentsRcvd[i] = true; } } - if (_fragmentsReceived == null) + if (fragmentsRcvd == null) _fragmentsReceived = new boolean[0]; + else + _fragmentsReceived = fragmentsRcvd; } + public int fragmentCount() { return _fragmentsReceived.length; } + public long getMessageId() { return _bitfieldMessageId; } + public boolean received(int fragmentNum) { if ( (fragmentNum < 0) || (fragmentNum >= _fragmentsReceived.length) ) return false; return _fragmentsReceived[fragmentNum]; } + + /** @return false always */ public boolean receivedComplete() { return false; } @Override diff --git a/router/java/src/net/i2p/router/transport/udp/OutboundMessageFragments.java b/router/java/src/net/i2p/router/transport/udp/OutboundMessageFragments.java index 26ce9c44a..1ce3db3a4 100644 --- a/router/java/src/net/i2p/router/transport/udp/OutboundMessageFragments.java +++ b/router/java/src/net/i2p/router/transport/udp/OutboundMessageFragments.java @@ -372,11 +372,11 @@ class OutboundMessageFragments { // ok, simplest possible thing is to always tack on the bitfields if List msgIds = peer.getCurrentFullACKs(); - if (msgIds == null) msgIds = new ArrayList(); List partialACKBitfields = new ArrayList(); peer.fetchPartialACKs(partialACKBitfields); int piggybackedPartialACK = partialACKBitfields.size(); // getCurrentFullACKs() already makes a copy, do we need to copy again? + // YES because buildPacket() now removes them (maybe) List remaining = new ArrayList(msgIds); int sparseCount = 0; UDPPacket rv[] = new UDPPacket[fragments]; //sparse diff --git a/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java b/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java index 3fd9c146c..3d77e9c50 100644 --- a/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java +++ b/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java @@ -4,6 +4,7 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.util.Arrays; import java.util.Collections; +import java.util.Iterator; import java.util.List; import net.i2p.I2PAppContext; @@ -124,6 +125,31 @@ class PacketBuilder { /** we only talk to people of the right version */ static final int PROTOCOL_VERSION = 0; + /** if no extended options or rekey data, which we don't support = 37 */ + public static final int HEADER_SIZE = UDPPacket.MAC_SIZE + UDPPacket.IV_SIZE + 1 + 4; + + /** not including acks. 46 */ + public static final int DATA_HEADER_SIZE = HEADER_SIZE + 9; + + /** IPv4 only */ + public static final int IP_HEADER_SIZE = 20; + public static final int UDP_HEADER_SIZE = 8; + + /** 74 */ + public static final int MIN_DATA_PACKET_OVERHEAD = IP_HEADER_SIZE + UDP_HEADER_SIZE + DATA_HEADER_SIZE; + + /** + * Only for data packets. No limit in ack-only packets. + * This directly affects data packet overhead. + */ + private static final int MAX_EXPLICIT_ACKS_LARGE = 9; + + /** + * Only for data packets. No limit in ack-only packets. + * This directly affects data packet overhead. + */ + private static final int MAX_EXPLICIT_ACKS_SMALL = 4; + public PacketBuilder(I2PAppContext ctx, UDPTransport transport) { _context = ctx; _transport = transport; @@ -142,18 +168,51 @@ class PacketBuilder { * This builds a data packet (PAYLOAD_TYPE_DATA). * See the methods below for the other message types. * + * Note that while the UDP message spec allows for more than one fragment in a message, + * this method writes exactly one fragment. + * For no fragments use buildAck(). + * Multiple fragments in a single packet is not supported. + * Rekeying and extended options are not supported. + * + * Packet format: + *
+     *    16 byte MAC
+     *    16 byte IV
+     *     1 byte flag
+     *     4 byte date
+     *     1 byte flag
+     *     1 byte explicit ack count IF included
+     *   4*n byte explict acks IF included
+     *     1 byte ack bitfield count IF included
+     *   4*n + ?? ack bitfields IF included
+     *     1 byte fragment count (always 1)
+     *     4 byte message ID
+     *     3 byte fragment info
+     *     n byte fragment
+     *  0-15 bytes padding
+     *
+ * + * So ignoring the ack bitfields, and assuming we have explicit acks, + * it's (47 + 4*explict acks + padding) added to the + * fragment length. + * * @param ackIdsRemaining list of messageIds (Long) that should be acked by this packet. * The list itself is passed by reference, and if a messageId is - * transmitted and the sender does not want the ID to be included - * in subsequent acks, it should be removed from the list. NOTE: - * right now this does NOT remove the IDs, which means it assumes - * that the IDs will be transmitted potentially multiple times, - * and should otherwise be removed from the list. + * transmitted it will be removed from the list. + * Not all message IDs will necessarily be sent, there may not be room. + * non-null. + * * @param partialACKsRemaining list of messageIds (ACKBitfield) that should be acked by this packet. * The list itself is passed by reference, and if a messageId is * included, it should be removed from the list. + * Full acks in this list are skipped, they are NOT transmitted. + * non-null. + * Not all acks will necessarily be sent, there may not be room. + * + * @return null on error */ - public UDPPacket buildPacket(OutboundMessageState state, int fragment, PeerState peer, List ackIdsRemaining, List partialACKsRemaining) { + public UDPPacket buildPacket(OutboundMessageState state, int fragment, PeerState peer, + List ackIdsRemaining, List partialACKsRemaining) { UDPPacket packet = buildPacketHeader((byte)(UDPPacket.PAYLOAD_TYPE_DATA << 4)); byte data[] = packet.getPacket().getData(); int off = HEADER_SIZE; @@ -162,48 +221,93 @@ class PacketBuilder { boolean acksIncluded = false; if (_log.shouldLog(Log.INFO)) { msg = new StringBuilder(128); - msg.append("Send to ").append(peer.getRemotePeer().toBase64()); - msg.append(" msg ").append(state.getMessageId()).append(":").append(fragment); - if (fragment == state.getFragmentCount() - 1) - msg.append("*"); + msg.append("Data pkt to ").append(peer.getRemotePeer().toBase64()); + msg.append(" msg ").append(state.getMessageId()).append(" frag:").append(fragment); + msg.append('/').append(state.getFragmentCount()); } + int dataSize = state.fragmentSize(fragment); + if (dataSize < 0) { + packet.release(); + return null; + } + int currentMTU = peer.getMTU(); + int availableForAcks = currentMTU - MIN_DATA_PACKET_OVERHEAD - dataSize; + int availableForExplicitAcks = availableForAcks; + // ok, now for the body... // just always ask for an ACK for now... data[off] |= UDPPacket.DATA_FLAG_WANT_REPLY; - // we should in theory only include explicit ACKs if the expected packet size - // is under the MTU, but for now, since the # of packets acked is so few (usually - // just one or two), and since the packets are so small anyway, an additional five - // or ten bytes doesn't hurt. - if ( (ackIdsRemaining != null) && (!ackIdsRemaining.isEmpty()) ) + + // partial acks have priority but they are after explicit acks in the packet + // so we have to compute the space in advance + int partialAcksToSend = 0; + if (availableForExplicitAcks >= 6 && !partialACKsRemaining.isEmpty()) { + for (ACKBitfield bf : partialACKsRemaining) { + if (bf.receivedComplete()) + continue; + int acksz = 4 + (bf.fragmentCount() / 7) + 1; + if (partialAcksToSend == 0) + acksz++; // ack count + if (availableForExplicitAcks >= acksz) { + availableForExplicitAcks -= acksz; + partialAcksToSend++; + } else { + break; + } + } + if (partialAcksToSend > 0) + data[off] |= UDPPacket.DATA_FLAG_ACK_BITFIELDS; + } + + + // Only include acks if we have at least 5 bytes available and at least + // one ack is requested. + if (availableForExplicitAcks >= 5 && !ackIdsRemaining.isEmpty()) { data[off] |= UDPPacket.DATA_FLAG_EXPLICIT_ACK; - if ( (partialACKsRemaining != null) && (!partialACKsRemaining.isEmpty()) ) - data[off] |= UDPPacket.DATA_FLAG_ACK_BITFIELDS; + } off++; - if ( (ackIdsRemaining != null) && (!ackIdsRemaining.isEmpty()) ) { - DataHelper.toLong(data, off, 1, ackIdsRemaining.size()); + if (msg != null) { + msg.append(" data: ").append(dataSize).append(" bytes, mtu: ") + .append(currentMTU).append(", ") + .append(ackIdsRemaining.size()).append(" full acks requested, ") + .append(partialACKsRemaining.size()).append(" partial acks requested, ") + .append(availableForAcks).append(" avail. for all acks, ") + .append(availableForExplicitAcks).append(" for full acks, "); + } + + int explicitToSend = Math.min(currentMTU > PeerState.MIN_MTU ? MAX_EXPLICIT_ACKS_LARGE : MAX_EXPLICIT_ACKS_SMALL, + Math.min((availableForExplicitAcks - 1) / 4, ackIdsRemaining.size())); + if (explicitToSend > 0) { + if (msg != null) + msg.append(explicitToSend).append(" full acks included:"); + DataHelper.toLong(data, off, 1, explicitToSend); off++; - for (int i = 0; i < ackIdsRemaining.size(); i++) { - //while (ackIdsRemaining.size() > 0) { - Long ackId = ackIdsRemaining.get(i);//(Long)ackIdsRemaining.remove(0); + Iterator iter = ackIdsRemaining.iterator(); + for (int i = 0; i < explicitToSend && iter.hasNext(); i++) { + Long ackId = iter.next(); + iter.remove(); // NPE here, how did a null get in the List? DataHelper.toLong(data, off, 4, ackId.longValue()); off += 4; if (msg != null) // logging it msg.append(" full ack: ").append(ackId.longValue()); - acksIncluded = true; } + //acksIncluded = true; } - if ( (partialACKsRemaining != null) && (!partialACKsRemaining.isEmpty()) ) { + if (partialAcksToSend > 0) { + if (msg != null) + msg.append(partialAcksToSend).append(" partial acks included:"); int origNumRemaining = partialACKsRemaining.size(); int numPartialOffset = off; // leave it blank for now, since we could skip some off++; - for (int i = 0; i < partialACKsRemaining.size(); i++) { - ACKBitfield bitfield = partialACKsRemaining.get(i); + Iterator iter = partialACKsRemaining.iterator(); + for (int i = 0; i < partialAcksToSend && iter.hasNext(); i++) { + ACKBitfield bitfield = iter.next(); if (bitfield.receivedComplete()) continue; DataHelper.toLong(data, off, 4, bitfield.getMessageId()); off += 4; @@ -219,18 +323,17 @@ class PacketBuilder { } off++; } - partialACKsRemaining.remove(i); + iter.remove(); if (msg != null) // logging it msg.append(" partial ack: ").append(bitfield); - acksIncluded = true; - i--; } + //acksIncluded = true; // now jump back and fill in the number of bitfields *actually* included DataHelper.toLong(data, numPartialOffset, 1, origNumRemaining - partialACKsRemaining.size()); } - if ( (msg != null) && (acksIncluded) ) - _log.debug(msg.toString()); + //if ( (msg != null) && (acksIncluded) ) + // _log.debug(msg.toString()); DataHelper.toLong(data, off, 1, 1); // only one fragment in this message off++; @@ -243,57 +346,66 @@ class PacketBuilder { data[off] |= 1; // isLast off++; - int size = state.fragmentSize(fragment); - if (size < 0) { - packet.release(); - return null; - } - DataHelper.toLong(data, off, 2, size); + DataHelper.toLong(data, off, 2, dataSize); data[off] &= (byte)0x3F; // 2 highest bits are reserved off += 2; int sizeWritten = state.writeFragment(data, off, fragment); - if (sizeWritten != size) { + if (sizeWritten != dataSize) { if (sizeWritten < 0) { // probably already freed from OutboundMessageState if (_log.shouldLog(Log.WARN)) _log.warn("Write failed for fragment " + fragment + " of " + state.getMessageId()); } else { - _log.error("Size written: " + sizeWritten + " but size: " + size + _log.error("Size written: " + sizeWritten + " but size: " + dataSize + " for fragment " + fragment + " of " + state.getMessageId()); } packet.release(); return null; - } else if (_log.shouldLog(Log.DEBUG)) - _log.debug("Size written: " + sizeWritten + " for fragment " + fragment - + " of " + state.getMessageId()); - size = sizeWritten; - if (size < 0) { - packet.release(); - return null; + //} else if (_log.shouldLog(Log.DEBUG)) { + // _log.debug("Size written: " + sizeWritten + " for fragment " + fragment + // + " of " + state.getMessageId()); } - off += size; + off += dataSize; - // we can pad here if we want, maybe randomized? // pad up so we're on the encryption boundary - int padSize = 16 - (off % 16); - if (padSize > 0) { + // we could do additional random padding here if desired + int mod = off % 16; + if (mod > 0) { + int padSize = 16 - mod; _context.random().nextBytes(data, off, padSize); off += padSize; } packet.getPacket().setLength(off); + authenticate(packet, peer.getCurrentCipherKey(), peer.getCurrentMACKey()); setTo(packet, peer.getRemoteIPAddress(), peer.getRemotePort()); if (_log.shouldLog(Log.INFO)) { + msg.append(" pkt size ").append(off + (IP_HEADER_SIZE + UDP_HEADER_SIZE)); _log.info(msg.toString()); } + // the packet could have been built before the current mtu got lowered, so + // compare to LARGE_MTU + if (off + (IP_HEADER_SIZE + UDP_HEADER_SIZE) > PeerState.LARGE_MTU) { + _log.error("Size is " + off + " for " + packet + + " fragment " + fragment + + " data size " + dataSize + + " pkt size " + (off + (IP_HEADER_SIZE + UDP_HEADER_SIZE)) + + " MTU " + currentMTU + + ' ' + availableForAcks + " for all acks " + + availableForExplicitAcks + " for full acks " + + explicitToSend + " full acks included " + + partialAcksToSend + " partial acks included " + + " OMS " + state, new Exception()); + } return packet; } /** + * An ACK packet with no acks. * We use this for keepalive purposes. * It doesn't generate a reply, but that's ok. */ @@ -307,6 +419,7 @@ class PacketBuilder { * Build the ack packet. The list need not be sorted into full and partial; * this method will put all fulls before the partials in the outgoing packet. * An ack packet is just a data packet with no data. + * See buildPacket() for format. * * @param ackBitfields list of ACKBitfield instances to either fully or partially ACK */ @@ -903,7 +1016,7 @@ class PacketBuilder { // challenge... DataHelper.toLong(data, off, 1, 0); off++; - off += 0; // *cough* + //off += 0; // *cough* System.arraycopy(ourIntroKey.getData(), 0, data, off, SessionKey.KEYSIZE_BYTES); off += SessionKey.KEYSIZE_BYTES; @@ -1059,9 +1172,6 @@ class PacketBuilder { return packet; } - /** if no extended options or rekey data, which we don't support */ - private static final int HEADER_SIZE = UDPPacket.MAC_SIZE + UDPPacket.IV_SIZE + 1 + 4; - /** * Create a new packet and add the flag byte and the time stamp. * Caller should add data starting at HEADER_SIZE. diff --git a/router/java/src/net/i2p/router/transport/udp/PeerState.java b/router/java/src/net/i2p/router/transport/udp/PeerState.java index 8d31df663..d7dbed469 100644 --- a/router/java/src/net/i2p/router/transport/udp/PeerState.java +++ b/router/java/src/net/i2p/router/transport/udp/PeerState.java @@ -216,6 +216,7 @@ class PeerState { private static final int DEFAULT_SEND_WINDOW_BYTES = 8*1024; private static final int MINIMUM_WINDOW_BYTES = DEFAULT_SEND_WINDOW_BYTES; private static final int MAX_SEND_WINDOW_BYTES = 1024*1024; + /* * 596 gives us 588 IP byes, 568 UDP bytes, and with an SSU data message, * 522 fragment bytes, which is enough to send a tunnel data message in 2 @@ -228,9 +229,15 @@ class PeerState { * 1 + (4 * MAX_RESEND_ACKS_SMALL) which can take up a significant amount of space. * We reduce the max acks when using the small MTU but it may not be enough... * + * Goal: VTBM msg fragments 2646 / (620 - 87) fits nicely. + * + * Assuming that we can enforce an MTU correctly, this % 16 should be 12, + * as the IP/UDP header is 28 bytes and data max should be mulitple of 16 for padding efficiency, + * and so PacketBuilder.buildPacket() works correctly. */ - private static final int MIN_MTU = 608;//600; //1500; + public static final int MIN_MTU = 620; private static final int DEFAULT_MTU = MIN_MTU; + /* * based on measurements, 1350 fits nearly all reasonably small I2NP messages * (larger I2NP messages may be up to 1900B-4500B, which isn't going to fit @@ -241,8 +248,16 @@ class PeerState { * 2646 / 2 = 1323 * 1323 + 74 + 46 + 1 + (4 * 9) = 1480 * So why not make it 1492 (old ethernet is 1492, new is 1500) + * Changed to 1492 in 0.8.9 + * + * BUT through 0.8.11, + * size estimate was bad, actual packet was up to 48 bytes bigger + * To be figured out. Curse the ACKs. + * Assuming that we can enforce an MTU correctly, this % 16 should be 12, + * as the IP/UDP header is 28 bytes and data max should be mulitple of 16 for padding efficiency, + * and so PacketBuilder.buildPacket() works correctly. */ - private static final int LARGE_MTU = 1492; + public static final int LARGE_MTU = 1484; private static final int MIN_RTO = 100 + ACKSender.ACK_FREQUENCY; private static final int MAX_RTO = 3000; // 5000; @@ -699,21 +714,41 @@ class PeerState { * "want to send" list. If the message id is transmitted to the peer, * removeACKMessage(Long) should be called. * + * The returned list contains acks not yet sent, followed by + * a random assortment of acks already sent. + * The caller should NOT transmit all of them all the time, + * even if there is room, + * or the packets will have way too much overhead. + * + * @return a new list, do as you like with it */ public List getCurrentFullACKs() { // no such element exception seen here - ArrayList rv = new ArrayList(_currentACKs); + List rv = new ArrayList(_currentACKs); // include some for retransmission - rv.addAll(_currentACKsResend); + List randomResends = new ArrayList(_currentACKsResend); + Collections.shuffle(randomResends, _context.random()); + rv.addAll(randomResends); + if (_log.shouldLog(Log.INFO)) + _log.info("Returning " + _currentACKs.size() + " current and " + randomResends.size() + " resend acks"); return rv; } + /** the ack was sent */ public void removeACKMessage(Long messageId) { - _currentACKs.remove(messageId); - _currentACKsResend.offer(messageId); - // trim down the resends - while (_currentACKsResend.size() > MAX_RESEND_ACKS) - _currentACKsResend.poll(); + boolean removed = _currentACKs.remove(messageId); + if (removed) { + // only add if reoved from current, as this may be called for + // acks already in _currentACKsResend. + _currentACKsResend.offer(messageId); + // trim down the resends + while (_currentACKsResend.size() > MAX_RESEND_ACKS) + _currentACKsResend.poll(); + if (_log.shouldLog(Log.INFO)) + _log.info("Sent ack " + messageId + " now " + _currentACKs.size() + " current and " + + _currentACKsResend.size() + " resend acks"); + } + // should we only do this if removed? _lastACKSend = _context.clock().now(); } @@ -722,15 +757,13 @@ class PeerState { */ private static final int MAX_RESEND_ACKS = 16; /** - * The number of duplicate acks sent in each messge - - * Warning, this directly affects network overhead - * Was 16 but that's too much (64 bytes in a max 608 byte packet, - * and often much smaller) + * The max number of duplicate acks sent in each ack-only messge. + * Doesn't really matter, we have plenty of room... * @since 0.7.13 */ - private static final int MAX_RESEND_ACKS_LARGE = 9; + private static final int MAX_RESEND_ACKS_LARGE = MAX_RESEND_ACKS; /** for small MTU */ - private static final int MAX_RESEND_ACKS_SMALL = 4; + private static final int MAX_RESEND_ACKS_SMALL = MAX_RESEND_ACKS; /** * grab a list of ACKBitfield instances, some of which may fully @@ -740,9 +773,18 @@ class PeerState { * ACKed with this call. Be sure to check getWantedACKSendSince() which * will be unchanged if there are ACKs remaining. * + * @return non-null, possibly empty + * @deprecated unused */ public List retrieveACKBitfields() { return retrieveACKBitfields(true); } + /** + * See above. Only called by ACKSender with alwaysIncludeRetransmissions = false. + * So this is only for ACK-only packets, so all the size limiting is useless. + * FIXME. + * + * @return non-null, possibly empty + */ public List retrieveACKBitfields(boolean alwaysIncludeRetransmissions) { List rv = new ArrayList(MAX_RESEND_ACKS); int bytesRemaining = countMaxACKData(); @@ -821,13 +863,17 @@ class PeerState { } _lastACKSend = _context.clock().now(); - if (rv == null) - rv = Collections.EMPTY_LIST; + //if (rv == null) + // rv = Collections.EMPTY_LIST; if (partialIncluded > 0) _context.statManager().addRateData("udp.sendACKPartial", partialIncluded, rv.size() - partialIncluded); return rv; } + /** + * @param rv out parameter, populated with true partial ACKBitfields. + * no full bitfields are included. + */ void fetchPartialACKs(List rv) { InboundMessageState states[] = null; int curState = 0; @@ -853,6 +899,7 @@ class PeerState { } } if (states != null) { + // _inboundMessages is a Map (unordered), so why bother going backwards? for (int i = curState-1; i >= 0; i--) { if (states[i] != null) rv.add(states[i].createACKBitfield()); @@ -860,10 +907,14 @@ class PeerState { } } - /** represent a full ACK of a message */ + /** + * A dummy "partial" ack which represents a full ACK of a message + */ private static class FullACKBitfield implements ACKBitfield { - private long _msgId; + private final long _msgId; + public FullACKBitfield(long id) { _msgId = id; } + public int fragmentCount() { return 0; } public long getMessageId() { return _msgId; } public boolean received(int fragmentNum) { return true; } @@ -1062,6 +1113,7 @@ class PeerState { public long getLastACKSend() { return _lastACKSend; } public void setLastACKSend(long when) { _lastACKSend = when; } public long getWantedACKSendSince() { return _wantACKSendSince; } + public boolean unsentACKThresholdReached() { int threshold = countMaxACKData() / 4; return _currentACKs.size() >= threshold; @@ -1070,8 +1122,8 @@ class PeerState { /** @return MTU - 83 */ private int countMaxACKData() { return _mtu - - IP_HEADER_SIZE - - UDP_HEADER_SIZE + - PacketBuilder.IP_HEADER_SIZE + - PacketBuilder.UDP_HEADER_SIZE - UDPPacket.IV_SIZE - UDPPacket.MAC_SIZE - 1 // type flag @@ -1335,16 +1387,22 @@ class PeerState { */ private static final boolean THROTTLE_INITIAL_SEND = true; - private static final int SSU_HEADER_SIZE = 46; - static final int UDP_HEADER_SIZE = 8; - static final int IP_HEADER_SIZE = 20; + /** + * Always leave room for this many explicit acks. + * Only for data packets. Does not affect ack-only packets. + * This directly affects data packet overhead, adjust with care. + */ + private static final int MIN_EXPLICIT_ACKS = 3; + /** this is room for three explicit acks or two partial acks or one of each = 13 */ + private static final int MIN_ACK_SIZE = 1 + (4 * MIN_EXPLICIT_ACKS); /** * how much payload data can we shove in there? - * @return MTU - 74 + * @return MTU - 87, i.e. 521 or 1401 */ private static final int fragmentSize(int mtu) { - return mtu - SSU_HEADER_SIZE - UDP_HEADER_SIZE - IP_HEADER_SIZE; + // 46 + 20 + 8 + 13 = 74 + 13 = 87 + return mtu - (PacketBuilder.MIN_DATA_PACKET_OVERHEAD + MIN_ACK_SIZE); } private boolean locked_shouldSend(OutboundMessageState state) { diff --git a/router/java/src/net/i2p/router/transport/udp/UDPPacket.java b/router/java/src/net/i2p/router/transport/udp/UDPPacket.java index 3dd97a704..c45a29bdb 100644 --- a/router/java/src/net/i2p/router/transport/udp/UDPPacket.java +++ b/router/java/src/net/i2p/router/transport/udp/UDPPacket.java @@ -56,8 +56,14 @@ class UDPPacket { * if a received packet is this big it is truncated. * This is bigger than PeerState.LARGE_MTU, as the far-end's * LARGE_MTU may be larger than ours. + * + * Due to longstanding bugs, a packet may be larger than LARGE_MTU + * (acks and padding). Together with an increase in the LARGE_MTU to + * 1492 in release 0.8.9, routers from 0.8.9 - 0.8.11 can generate + * packets up to 1536. Data packets are always a multiple of 16, + * so make this 4 + a multiple of 16. */ - static final int MAX_PACKET_SIZE = 1536; + static final int MAX_PACKET_SIZE = 1572; public static final int IV_SIZE = 16; public static final int MAC_SIZE = 16; @@ -116,12 +122,16 @@ class UDPPacket { _released = false; } + /**** public void writeData(byte src[], int offset, int len) { verifyNotReleased(); System.arraycopy(src, offset, _data, 0, len); _packet.setLength(len); resetBegin(); } + ****/ + + /** */ public DatagramPacket getPacket() { verifyNotReleased(); return _packet; } public short getPriority() { verifyNotReleased(); return _priority; } public long getExpiration() { verifyNotReleased(); return _expiration; } @@ -263,7 +273,10 @@ class UDPPacket { buf.append(" byte packet with "); buf.append(_packet.getAddress().getHostAddress()).append(":"); buf.append(_packet.getPort()); - buf.append(" id=").append(System.identityHashCode(this)); + //buf.append(" id=").append(System.identityHashCode(this)); + buf.append(" msg type=").append(_messageType); + buf.append(" mark type=").append(_markedType); + buf.append(" frag count=").append(_fragmentCount); buf.append(" sinceEnqueued=").append((_enqueueTime > 0 ? _context.clock().now()-_enqueueTime : -1)); buf.append(" sinceReceived=").append((_receivedTime > 0 ? _context.clock().now()-_receivedTime : -1)); diff --git a/router/java/src/net/i2p/router/transport/udp/UDPSender.java b/router/java/src/net/i2p/router/transport/udp/UDPSender.java index b39e78f45..b431ab4c5 100644 --- a/router/java/src/net/i2p/router/transport/udp/UDPSender.java +++ b/router/java/src/net/i2p/router/transport/udp/UDPSender.java @@ -158,6 +158,9 @@ class UDPSender { public int add(UDPPacket packet) { if (packet == null || !_keepRunning) return 0; int size = 0; + int psz = packet.getPacket().getLength(); + if (psz > PeerState.LARGE_MTU) + _log.error("Sending large UDP packet " + psz + " bytes: " + packet); _outboundQueue.offer(packet); //size = _outboundQueue.size(); //_context.statManager().addRateData("udp.sendQueueSize", size, lifetime); @@ -187,7 +190,7 @@ class UDPSender { _log.debug("Packet to send known: " + packet); long acquireTime = _context.clock().now(); int size = packet.getPacket().getLength(); - int size2 = packet.getPacket().getLength(); + // ?? int size2 = packet.getPacket().getLength(); if (size > 0) { //_context.bandwidthLimiter().requestOutbound(req, size, "UDP sender"); req = _context.bandwidthLimiter().requestOutbound(size, "UDP sender");