From e3c222b5c1d51d7d6f805b56a6a8b63e5fef3d85 Mon Sep 17 00:00:00 2001 From: zzz Date: Sat, 10 Apr 2010 15:25:57 +0000 Subject: [PATCH 1/6] Lower per-torrent conn limits for large pieces --- .../java/src/org/klomp/snark/PeerCoordinator.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java b/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java index 9976d44d9..67ac95e55 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java +++ b/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java @@ -240,15 +240,19 @@ public class PeerCoordinator implements PeerListener } } - /** reduce max if huge pieces to keep from ooming */ + /** + * Reduce max if huge pieces to keep from ooming + * Todo: should we only reduce when leeching? + * @return 512K: 16; 1M: 11; 2M: 8 + */ private int getMaxConnections() { int size = metainfo.getPieceLength(0); int max = _util.getMaxConnections(); - if (size <= 1024*1024) + if (size <= 512*1024) return max; - if (size <= 2*1024*1024) - return (max + 1) / 2; - return (max + 3) / 4; + if (size <= 1024*1024) + return (max + max + 2) / 3; + return (max + 1) / 2; } public boolean halted() { return halted; } From 89d0d7b2662ce5d09f71e47ce01e17e8da9d123b Mon Sep 17 00:00:00 2001 From: zzz Date: Sat, 10 Apr 2010 15:26:23 +0000 Subject: [PATCH 2/6] Disconnect seeds that connect to a seed --- apps/i2psnark/java/src/org/klomp/snark/PeerState.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/i2psnark/java/src/org/klomp/snark/PeerState.java b/apps/i2psnark/java/src/org/klomp/snark/PeerState.java index 378cd8758..2fa597ce5 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/PeerState.java +++ b/apps/i2psnark/java/src/org/klomp/snark/PeerState.java @@ -152,7 +152,16 @@ class PeerState // XXX - Check for weird bitfield and disconnect? bitfield = new BitField(bitmap, metainfo.getPieces()); } - setInteresting(listener.gotBitField(peer, bitfield)); + boolean interest = listener.gotBitField(peer, bitfield); + setInteresting(interest); + if (bitfield.complete() && !interest) { + // They are seeding and we are seeding, + // why did they contact us? (robert) + // Dump them quick before we send our whole bitmap + if (_log.shouldLog(Log.WARN)) + _log.warn("Disconnecting seed that connects to seeds" + peer); + peer.disconnect(true); + } } void requestMessage(int piece, int begin, int length) From d7e2f39d25d34581deb12c6663bf1c0bacd9ebd4 Mon Sep 17 00:00:00 2001 From: zzz Date: Sat, 10 Apr 2010 15:28:31 +0000 Subject: [PATCH 3/6] * Startup: - Don't die horribly if there is a router.info file but no router.keys file http://forum.i2p/viewtopic.php?t=4424 - Log tweaks --- .../i2p/router/startup/CreateRouterInfoJob.java | 4 ++-- .../net/i2p/router/startup/LoadRouterInfoJob.java | 15 ++++++++++----- .../i2p/router/startup/RebuildRouterInfoJob.java | 8 ++++---- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/router/java/src/net/i2p/router/startup/CreateRouterInfoJob.java b/router/java/src/net/i2p/router/startup/CreateRouterInfoJob.java index 5fbce84cd..459cd0fb3 100644 --- a/router/java/src/net/i2p/router/startup/CreateRouterInfoJob.java +++ b/router/java/src/net/i2p/router/startup/CreateRouterInfoJob.java @@ -98,9 +98,9 @@ public class CreateRouterInfoJob extends JobImpl { _log.info("Router info created and stored at " + ifile.getAbsolutePath() + " with private keys stored at " + kfile.getAbsolutePath() + " [" + info + "]"); } catch (DataFormatException dfe) { - _log.error("Error building the new router information", dfe); + _log.log(Log.CRIT, "Error building the new router information", dfe); } catch (IOException ioe) { - _log.error("Error writing out the new router information", ioe); + _log.log(Log.CRIT, "Error writing out the new router information", ioe); } finally { if (fos1 != null) try { fos1.close(); } catch (IOException ioe) {} if (fos2 != null) try { fos2.close(); } catch (IOException ioe) {} diff --git a/router/java/src/net/i2p/router/startup/LoadRouterInfoJob.java b/router/java/src/net/i2p/router/startup/LoadRouterInfoJob.java index f3428c447..94b4f1d2e 100644 --- a/router/java/src/net/i2p/router/startup/LoadRouterInfoJob.java +++ b/router/java/src/net/i2p/router/startup/LoadRouterInfoJob.java @@ -68,11 +68,18 @@ public class LoadRouterInfoJob extends JobImpl { FileInputStream fis1 = null; FileInputStream fis2 = null; try { - if (_infoExists) { + // if we have a routerinfo but no keys, things go bad in a hurry: + // CRIT ...rkdb.PublishLocalRouterInfoJob: Internal error - signing private key not known? rescheduling publish for 30s + // CRIT net.i2p.router.Router : Internal error - signing private key not known? wtf + // CRIT ...sport.udp.EstablishmentManager: Error in the establisher java.lang.NullPointerException + // at net.i2p.router.transport.udp.PacketBuilder.buildSessionConfirmedPacket(PacketBuilder.java:574) + // so pretend the RI isn't there if there is no keyfile + if (_infoExists && _keysExist) { fis1 = new FileInputStream(rif); info = new RouterInfo(); info.readBytes(fis1); _log.debug("Reading in routerInfo from " + rif.getAbsolutePath() + " and it has " + info.getAddresses().size() + " addresses"); + _us = info; } if (_keysExist) { @@ -91,17 +98,15 @@ public class LoadRouterInfoJob extends JobImpl { getContext().keyManager().setPublicKey(pubkey); //info.getIdentity().getPublicKey()); getContext().keyManager().setSigningPublicKey(signingPubKey); // info.getIdentity().getSigningPublicKey()); } - - _us = info; } catch (IOException ioe) { - _log.error("Error reading the router info from " + rif.getAbsolutePath() + " and the keys from " + rkf.getAbsolutePath(), ioe); + _log.log(Log.CRIT, "Error reading the router info from " + rif.getAbsolutePath() + " and the keys from " + rkf.getAbsolutePath(), ioe); _us = null; rif.delete(); rkf.delete(); _infoExists = false; _keysExist = false; } catch (DataFormatException dfe) { - _log.error("Corrupt router info or keys at " + rif.getAbsolutePath() + " / " + rkf.getAbsolutePath(), dfe); + _log.log(Log.CRIT, "Corrupt router info or keys at " + rif.getAbsolutePath() + " / " + rkf.getAbsolutePath(), dfe); _us = null; rif.delete(); rkf.delete(); diff --git a/router/java/src/net/i2p/router/startup/RebuildRouterInfoJob.java b/router/java/src/net/i2p/router/startup/RebuildRouterInfoJob.java index fda82de8e..e3daf60ea 100644 --- a/router/java/src/net/i2p/router/startup/RebuildRouterInfoJob.java +++ b/router/java/src/net/i2p/router/startup/RebuildRouterInfoJob.java @@ -107,7 +107,7 @@ public class RebuildRouterInfoJob extends JobImpl { ident.setSigningPublicKey(signingPubKey); info.setIdentity(ident); } catch (Exception e) { - _log.error("Error reading in the key data from " + keyFile.getAbsolutePath(), e); + _log.log(Log.CRIT, "Error reading in the key data from " + keyFile.getAbsolutePath(), e); if (fis != null) try { fis.close(); } catch (IOException ioe) {} fis = null; keyFile.delete(); @@ -129,7 +129,7 @@ public class RebuildRouterInfoJob extends JobImpl { info.sign(getContext().keyManager().getSigningPrivateKey()); } catch (DataFormatException dfe) { - _log.error("Error rebuilding the new router info", dfe); + _log.log(Log.CRIT, "Error rebuilding the new router info", dfe); return; } @@ -138,9 +138,9 @@ public class RebuildRouterInfoJob extends JobImpl { fos = new FileOutputStream(infoFile); info.writeBytes(fos); } catch (DataFormatException dfe) { - _log.error("Error rebuilding the router information", dfe); + _log.log(Log.CRIT, "Error rebuilding the router information", dfe); } catch (IOException ioe) { - _log.error("Error writing out the rebuilt router information", ioe); + _log.log(Log.CRIT, "Error writing out the rebuilt router information", ioe); } finally { if (fos != null) try { fos.close(); } catch (IOException ioe) {} } From 24020302fd52fecc9403ebc5af26695788dba3da Mon Sep 17 00:00:00 2001 From: zzz Date: Sat, 10 Apr 2010 15:29:16 +0000 Subject: [PATCH 4/6] cleanup --- .../java/src/net/i2p/router/web/PluginStarter.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java b/apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java index d804e384c..f6aa9adb4 100644 --- a/apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java +++ b/apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java @@ -259,8 +259,7 @@ public class PluginStarter implements Runnable { String current = ctx.getProperty(CSSHelper.PROP_THEME_NAME); for (int i = 0; i < tfiles.length; i++) { String name = tfiles[i].getName(); - if (tfiles[i].isDirectory() && (!name.equals("images")) && (!name.equals("classic")) && - (!name.equals("dark")) && (!name.equals("light")) && (!name.equals("midnight"))) { + if (tfiles[i].isDirectory() && (!Arrays.asList(STANDARD_THEMES).contains(tfiles[i]))) { ctx.router().removeConfigSetting(ConfigUIHelper.PROP_THEME_PFX + name); if (name.equals(current)) ctx.router().setConfigSetting(CSSHelper.PROP_THEME_NAME, CSSHelper.DEFAULT_THEME); From 70e9cf5838f37b4defcccbdb6dbbcda0fa68ab8b Mon Sep 17 00:00:00 2001 From: zzz Date: Sat, 10 Apr 2010 15:41:42 +0000 Subject: [PATCH 5/6] add comments about the null privkey bug --- .../net/i2p/router/transport/udp/EstablishmentManager.java | 1 + .../net/i2p/router/transport/udp/OutboundEstablishState.java | 2 ++ .../java/src/net/i2p/router/transport/udp/PacketBuilder.java | 5 +++++ 3 files changed, 8 insertions(+) diff --git a/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java b/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java index 91c23b444..64e1bdea2 100644 --- a/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java +++ b/router/java/src/net/i2p/router/transport/udp/EstablishmentManager.java @@ -699,6 +699,7 @@ public class EstablishmentManager { // signs if we havent signed yet state.prepareSessionConfirmed(); + // BUG - handle null return UDPPacket packets[] = _builder.buildSessionConfirmedPackets(state, _context.router().getRouterInfo().getIdentity()); if (_log.shouldLog(Log.DEBUG)) diff --git a/router/java/src/net/i2p/router/transport/udp/OutboundEstablishState.java b/router/java/src/net/i2p/router/transport/udp/OutboundEstablishState.java index fa01bc026..6e3738f30 100644 --- a/router/java/src/net/i2p/router/transport/udp/OutboundEstablishState.java +++ b/router/java/src/net/i2p/router/transport/udp/OutboundEstablishState.java @@ -348,6 +348,8 @@ public class OutboundEstablishState { DataHelper.toLong(signed, off, 4, _receivedRelayTag); off += 4; DataHelper.toLong(signed, off, 4, _sentSignedOnTime); + // BUG - if SigningPrivateKey is null, _sentSignature will be null, leading to NPE later + // should we throw something from here? _sentSignature = _context.dsa().sign(signed, _context.keyManager().getSigningPrivateKey()); } 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 26b82e667..4b16d078d 100644 --- a/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java +++ b/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java @@ -512,6 +512,10 @@ public class PacketBuilder { * encrypting it as necessary. * * @return ready to send packets, or null if there was a problem + * + * TODO: doesn't really return null, and caller doesn't handle null return + * (null SigningPrivateKey should cause this?) + * Should probably return null if buildSessionConfirmedPacket() turns null for any fragment */ public UDPPacket[] buildSessionConfirmedPackets(OutboundEstablishState state, RouterIdentity ourIdentity) { byte identity[] = ourIdentity.toByteArray(); @@ -593,6 +597,7 @@ public class PacketBuilder { off++; } + // BUG: NPE here if null signature System.arraycopy(state.getSentSignature().getData(), 0, data, off, Signature.SIGNATURE_BYTES); packet.getPacket().setLength(off + Signature.SIGNATURE_BYTES); authenticate(packet, state.getCipherKey(), state.getMACKey()); From 8b6751f419870525822dbfbf80144da0bdbcab3b Mon Sep 17 00:00:00 2001 From: zzz Date: Sat, 10 Apr 2010 15:42:08 +0000 Subject: [PATCH 6/6] Streaming: Fix the window size increment logic so it does it much more often. The code increased the window size by MSS * MSS / N, like in RFC 2581, but it did it only once every N, so that was like MSS * MSS / N**2. Now do it all the time, except for isolated packets like keepalives that aren't using more than one message of the window. Seems to speed up outbound significantly, without any noticable increase in stream.sendsBeforeAck. --- .../client/streaming/ConnectionOptions.java | 2 ++ .../streaming/ConnectionPacketHandler.java | 34 +++++++++++++++---- history.txt | 13 +++++++ .../src/net/i2p/router/RouterVersion.java | 2 +- 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/apps/streaming/java/src/net/i2p/client/streaming/ConnectionOptions.java b/apps/streaming/java/src/net/i2p/client/streaming/ConnectionOptions.java index a3f691ece..ea4e1f069 100644 --- a/apps/streaming/java/src/net/i2p/client/streaming/ConnectionOptions.java +++ b/apps/streaming/java/src/net/i2p/client/streaming/ConnectionOptions.java @@ -386,8 +386,10 @@ public class ConnectionOptions extends I2PSocketOptionsImpl { private static final double RTT_DAMPENING = 0.875; public void updateRTT(int measuredValue) { + // the rttDev calculation matches that recommended in RFC 2988 (beta = 1/4) _rttDev = _rttDev + (int)(0.25d*(Math.abs(measuredValue-_rtt)-_rttDev)); int smoothed = (int)(RTT_DAMPENING*_rtt + (1-RTT_DAMPENING)*measuredValue); + // K = 4 _rto = smoothed + (_rttDev<<2); if (_rto < Connection.MIN_RESEND_DELAY) _rto = (int)Connection.MIN_RESEND_DELAY; diff --git a/apps/streaming/java/src/net/i2p/client/streaming/ConnectionPacketHandler.java b/apps/streaming/java/src/net/i2p/client/streaming/ConnectionPacketHandler.java index b9ebbb7d2..5292b5f09 100644 --- a/apps/streaming/java/src/net/i2p/client/streaming/ConnectionPacketHandler.java +++ b/apps/streaming/java/src/net/i2p/client/streaming/ConnectionPacketHandler.java @@ -328,7 +328,19 @@ public class ConnectionPacketHandler { } long lowest = con.getHighestAckedThrough(); - if (lowest >= con.getCongestionWindowEnd()) { + // RFC 2581 + // Why wait until we get a whole cwin to start updating the window? + // That means we don't start increasing the window until after 1 RTT. + // And whether we increase the window or not (probably not since 1/N), + // we reset the CongestionWindowEnd and have to wait another RTT. + // So we add the acked > 1 and UnackedPacketsSent > 0 cases, + // so we almost always go through the window adjustment code, + // unless we're just sending a single packet now and then. + // This keeps the window size from going sky-high from ping traffic alone. + // Since we don't adjust the window down after idle? (RFC 2581 sec. 4.1) + if (lowest >= con.getCongestionWindowEnd() || + acked > 1 || + con.getUnackedPacketsSent() > 0) { // new packet that ack'ed uncongested data, or an empty ack int oldWindow = con.getOptions().getWindowSize(); int newWindowSize = oldWindow; @@ -352,11 +364,12 @@ public class ConnectionPacketHandler { newWindowSize += acked / factor; if (_log.shouldLog(Log.DEBUG)) _log.debug("slow start acks = " + acked + " for " + con); - } else if (trend < 0) { - // rtt is shrinking, so lets increment the cwin - newWindowSize++; - if (_log.shouldLog(Log.DEBUG)) - _log.debug("trend < 0 for " + con); + // this is too fast since we mostly disabled the CongestionWindowEnd test above + //} else if (trend < 0) { + // // rtt is shrinking, so lets increment the cwin + // newWindowSize++; + // if (_log.shouldLog(Log.DEBUG)) + // _log.debug("trend < 0 for " + con); } else { // congestion avoidance // linear growth - increase window 1/N per RTT @@ -368,6 +381,10 @@ public class ConnectionPacketHandler { if (_log.shouldLog(Log.DEBUG)) _log.debug("cong. avoid acks = " + acked + " for " + con); } + } else { + if (_log.shouldLog(Log.DEBUG)) + _log.debug("No change to window: " + con.getOptions().getWindowSize() + + " congested? " + congested + " acked: " + acked + " resends: " + numResends); } if (newWindowSize <= 0) @@ -380,6 +397,11 @@ public class ConnectionPacketHandler { _log.debug("New window size " + newWindowSize + "/" + oldWindow + "/" + con.getOptions().getWindowSize() + " congestionSeenAt: " + con.getLastCongestionSeenAt() + " (#resends: " + numResends + ") for " + con); + } else { + if (_log.shouldLog(Log.DEBUG)) + _log.debug("No change to window: " + con.getOptions().getWindowSize() + + " highestAckedThrough: " + lowest + " congestionWindowEnd: " + con.getCongestionWindowEnd() + + " acked: " + acked + " unacked: " + con.getUnackedPacketsSent()); } con.windowAdjusted(); diff --git a/history.txt b/history.txt index 4a6e2d8ed..0b07ff332 100644 --- a/history.txt +++ b/history.txt @@ -1,3 +1,16 @@ +2010-04-10 zzz + * i2psnark: + - Disconnect seeds that connect to a seed + - Lower per-torrent conn limits for large pieces + * Startup: + - Don't die horribly if there is a router.info file + but no router.keys file + http://forum.i2p/viewtopic.php?t=4424 + - Log tweaks + * Streaming: + - Fix the window size increment logic so it + does it much more often + 2010-04-08 zzz * Key Manager: Hopefully avoid some races at startup http://forum.i2p/viewtopic.php?t=4424 diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index fecba78d6..a6204817e 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 = 7; + public final static long BUILD = 8; /** for example "-test" */ public final static String EXTRA = "";