From 50710f8066a19472f9d9fbe47cebe294976b9f9f Mon Sep 17 00:00:00 2001 From: zzz Date: Fri, 27 Apr 2018 17:41:47 +0000 Subject: [PATCH] Tunnels: Fix and consolidate allow-zero-hop logic, prevent zero-hop client tunnels even when no active peers Remove buildRequestZeroHopTime stat --- history.txt | 8 +++++++ .../src/net/i2p/router/RouterVersion.java | 2 +- .../net/i2p/router/TunnelPoolSettings.java | 21 +++++++++++++++---- .../i2p/router/tunnel/pool/BuildExecutor.java | 9 ++++---- .../i2p/router/tunnel/pool/TunnelPool.java | 17 ++++----------- 5 files changed, 34 insertions(+), 23 deletions(-) diff --git a/history.txt b/history.txt index f17cf42dae..de225554ba 100644 --- a/history.txt +++ b/history.txt @@ -1,3 +1,7 @@ +2018-04-27 zzz + * Tunnels: Fix and consolidate allow-zero-hop logic, + prevent zero-hop client tunnels when no active peers + 2018-04-24 meeh * Mac OS X launcher is still WIP, but: - Building the I2P.app bundle is mostly done @@ -6,6 +10,10 @@ * Added new entries to mtn-ignore so we avoid any PEBCAK with commiting build directories * Added an SBT AutoPlugin named IconHelper to generate valid ICNS images for Mac OS X +2018-04-23 zzz + * Clock: Fix early NPE via DoH + * EepGet: Handle HTTP response line with no status text + 2018-04-23 meeh * Added launchers for Browser Bundle and Mac OS X diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index c10128fe62..fecba78d69 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 = 6; + public final static long BUILD = 7; /** for example "-test" */ public final static String EXTRA = ""; diff --git a/router/java/src/net/i2p/router/TunnelPoolSettings.java b/router/java/src/net/i2p/router/TunnelPoolSettings.java index 6e5152f9e3..2c781ed092 100644 --- a/router/java/src/net/i2p/router/TunnelPoolSettings.java +++ b/router/java/src/net/i2p/router/TunnelPoolSettings.java @@ -79,7 +79,7 @@ public class TunnelPoolSettings { private static final int DEFAULT_OB_EXPL_LENGTH_VARIANCE = 0; //private static final int DEFAULT_OB_EXPL_LENGTH_VARIANCE = isSlow ? 0 : 1; - public static final boolean DEFAULT_ALLOW_ZERO_HOP = true; + public static final boolean DEFAULT_ALLOW_ZERO_HOP = false; public static final int DEFAULT_IP_RESTRICTION = 2; // class B (/16) private static final int MIN_PRIORITY = -25; private static final int MAX_PRIORITY = 25; @@ -161,16 +161,29 @@ public class TunnelPoolSettings { /** * If there are no tunnels to build with, will this pool allow 0 hop tunnels? + * * Always true for exploratory. - * Generally true for client, but should probably be ignored... - * use getLength() + getLengthVariance() > 0 instead. + * Prior to 0.9.35, generally true for client. + * As of 0.9.35, generally false for client, but true if + * getLength() + Math.min(getLengthVariance(), 0) <= 0, + * OR if getLengthOverride() == 0 + * OR if setAllowZeroHop(true) was called or set in properties. */ - public boolean getAllowZeroHop() { return _allowZeroHop; } + public boolean getAllowZeroHop() { + return _allowZeroHop || + _length + Math.min(_lengthVariance, 0) <= 0 || + _lengthOverride == 0; + } /** * If there are no tunnels to build with, will this pool allow 0 hop tunnels? * No effect on exploratory (always true) + * + * @param ok if true, getAllowZeroHop() will always return true + * if false, getAllowZeroHop will return as documented. + * @deprecated unused */ + @Deprecated public void setAllowZeroHop(boolean ok) { if (!_isExploratory) _allowZeroHop = ok; diff --git a/router/java/src/net/i2p/router/tunnel/pool/BuildExecutor.java b/router/java/src/net/i2p/router/tunnel/pool/BuildExecutor.java index de2ad652b3..23cd195676 100644 --- a/router/java/src/net/i2p/router/tunnel/pool/BuildExecutor.java +++ b/router/java/src/net/i2p/router/tunnel/pool/BuildExecutor.java @@ -66,7 +66,7 @@ class BuildExecutor implements Runnable { _context.statManager().createRequiredRateStat("tunnel.buildClientReject", "Response time for rejection (ms)", "Tunnels", new long[] { 10*60*1000, 60*60*1000 }); _context.statManager().createRequiredRateStat("tunnel.buildRequestTime", "Time to build a tunnel request (ms)", "Tunnels", new long[] { 60*1000, 10*60*1000 }); _context.statManager().createRateStat("tunnel.buildConfigTime", "Time to build a tunnel request (ms)", "Tunnels", new long[] { 60*1000, 10*60*1000 }); - _context.statManager().createRateStat("tunnel.buildRequestZeroHopTime", "How long it takes to build a zero hop tunnel", "Tunnels", new long[] { 60*1000, 10*60*1000 }); + //_context.statManager().createRateStat("tunnel.buildRequestZeroHopTime", "How long it takes to build a zero hop tunnel", "Tunnels", new long[] { 60*1000, 10*60*1000 }); //_context.statManager().createRateStat("tunnel.pendingRemaining", "How many inbound requests are pending after a pass (period is how long the pass takes)?", "Tunnels", new long[] { 60*1000, 10*60*1000 }); _context.statManager().createRateStat("tunnel.buildFailFirstHop", "How often we fail to build a OB tunnel because we can't contact the first hop", "Tunnels", new long[] { 60*1000, 10*60*1000 }); _context.statManager().createRateStat("tunnel.buildReplySlow", "Build reply late, but not too late", "Tunnels", new long[] { 10*60*1000 }); @@ -535,11 +535,10 @@ class BuildExecutor implements Runnable { boolean ok = BuildRequestor.request(_context, pool, cfg, this); if (!ok) return; - long buildTime = System.currentTimeMillis() - beforeBuild; - if (cfg.getLength() <= 1) - _context.statManager().addRateData("tunnel.buildRequestZeroHopTime", buildTime, 0); - else + if (cfg.getLength() > 1) { + long buildTime = System.currentTimeMillis() - beforeBuild; _context.statManager().addRateData("tunnel.buildRequestTime", buildTime, 0); + } long id = cfg.getReplyMessageId(); if (id > 0) { synchronized (_recentBuildIds) { diff --git a/router/java/src/net/i2p/router/tunnel/pool/TunnelPool.java b/router/java/src/net/i2p/router/tunnel/pool/TunnelPool.java index 7c9cb893f0..5f884f987a 100644 --- a/router/java/src/net/i2p/router/tunnel/pool/TunnelPool.java +++ b/router/java/src/net/i2p/router/tunnel/pool/TunnelPool.java @@ -176,8 +176,7 @@ public class TunnelPool { TunnelInfo selectTunnel() { return selectTunnel(true); } private TunnelInfo selectTunnel(boolean allowRecurseOnFail) { - boolean avoidZeroHop = getSettings().getLength() > 0 && - getSettings().getLength() + getSettings().getLengthVariance() > 0; + boolean avoidZeroHop = !_settings.getAllowZeroHop(); long period = curPeriod(); synchronized (_tunnels) { @@ -243,7 +242,7 @@ public class TunnelPool { } } - if (_alive && _settings.getAllowZeroHop()) + if (_alive && !avoidZeroHop) buildFallback(); if (allowRecurseOnFail) return selectTunnel(false); @@ -264,8 +263,7 @@ public class TunnelPool { * @since 0.8.10 */ TunnelInfo selectTunnel(Hash closestTo) { - boolean avoidZeroHop = getSettings().getLength() > 0 && - getSettings().getLength() + getSettings().getLengthVariance() > 0; + boolean avoidZeroHop = !_settings.getAllowZeroHop(); TunnelInfo rv = null; synchronized (_tunnels) { if (!_tunnels.isEmpty()) { @@ -659,13 +657,6 @@ public class TunnelPool { return false; if (_settings.getAllowZeroHop()) { - if ( (_settings.getLength() + _settings.getLengthVariance() > 0) && - (!_settings.isExploratory()) && - (_context.profileOrganizer().countActivePeers() > 0) ) { - // if it is a client tunnel pool and our variance doesn't allow 0 hop, prefer failure to - // 0 hop operation (unless our router is offline) - return false; - } if (_log.shouldLog(Log.INFO)) _log.info(toString() + ": building a fallback tunnel (usable: " + usable + " needed: " + quantity + ")"); @@ -847,7 +838,7 @@ public class TunnelPool { } int wanted = getAdjustedTotalQuantity(); - boolean allowZeroHop = ((getSettings().getLength() + getSettings().getLengthVariance()) <= 0); + boolean allowZeroHop = _settings.getAllowZeroHop(); /** * This algorithm builds based on the previous average length of time it takes