From 5ba1e458c68e463f5625622b97bbf2a4b2f2fa2a Mon Sep 17 00:00:00 2001 From: zzz Date: Tue, 15 Apr 2008 18:27:43 +0000 Subject: [PATCH] * SSU Reachability/PeerTestManager: - Back out strict peer ordering until we fix SSU - Back out persistent lease selection until we fix SSU - Fix detection of UDP REJECT_UNSOLICITED by recording status on expiration - Increase known Charlie time to 10m; 3m wasn't enough - Don't continue retransmitting peer test if we know Charlie - Don't run multiple peer tests at once - Tighten test frequency range to 6.5-19.5m, was 0-26m --- history.txt | 10 ++++++ .../src/net/i2p/router/RouterVersion.java | 2 +- .../OutboundClientMessageOneShotJob.java | 9 ++++- .../router/transport/udp/PeerTestManager.java | 36 ++++++++++++++----- .../router/transport/udp/UDPTransport.java | 2 +- .../tunnel/pool/ClientPeerSelector.java | 3 ++ .../tunnel/pool/ExploratoryPeerSelector.java | 3 ++ 7 files changed, 53 insertions(+), 12 deletions(-) diff --git a/history.txt b/history.txt index a593addad..1279ffe7d 100644 --- a/history.txt +++ b/history.txt @@ -1,3 +1,13 @@ +2008-04-15 zzz + * SSU Reachability/PeerTestManager: + - Back out strict peer ordering until we fix SSU + - Back out persistent lease selection until we fix SSU + - Fix detection of UDP REJECT_UNSOLICITED by recording status on expiration + - Increase known Charlie time to 10m; 3m wasn't enough + - Don't continue retransmitting peer test if we know Charlie + - Don't run multiple peer tests at once + - Tighten test frequency range to 6.5-19.5m, was 0-26m + 2008-04-12 zzz * Addressbook: Disallow '.-' and '-.' in host names * NTCP: Don't drop a connection unless both directions are idle; diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index 8682eb41a..b4c410afb 100644 --- a/router/java/src/net/i2p/router/RouterVersion.java +++ b/router/java/src/net/i2p/router/RouterVersion.java @@ -17,7 +17,7 @@ import net.i2p.CoreVersion; public class RouterVersion { public final static String ID = "$Revision: 1.548 $ $Date: 2008-02-10 15:00:00 $"; public final static String VERSION = "0.6.1.32"; - public final static long BUILD = 16; + public final static long BUILD = 17; public static void main(String args[]) { System.out.println("I2P Router version: " + VERSION + "-" + BUILD); System.out.println("Router ID: " + RouterVersion.ID); diff --git a/router/java/src/net/i2p/router/message/OutboundClientMessageOneShotJob.java b/router/java/src/net/i2p/router/message/OutboundClientMessageOneShotJob.java index eb1fcf2a9..04f6a67f1 100644 --- a/router/java/src/net/i2p/router/message/OutboundClientMessageOneShotJob.java +++ b/router/java/src/net/i2p/router/message/OutboundClientMessageOneShotJob.java @@ -190,6 +190,9 @@ public class OutboundClientMessageOneShotJob extends JobImpl { _log.warn(getJobId() + ": Bundle leaseSet probability overridden incorrectly [" + str + "]", nfe); } + if (probability >= 100) + return true; + _log.error(getJobId() + ": Bundle leaseSet probability is " + probability); if (probability >= getContext().random().nextInt(100)) return true; else @@ -247,6 +250,7 @@ public class OutboundClientMessageOneShotJob extends JobImpl { } long now = getContext().clock().now(); +/*** removed until we fix SSU reachability // Use the same lease if it's still good // Even if _leaseSet changed, _leaseSet.getEncryptionKey() didn't... synchronized (_leaseCache) { @@ -274,7 +278,7 @@ public class OutboundClientMessageOneShotJob extends JobImpl { _leaseCache.remove(_to); } } - +***/ // get the possible leases List leases = new ArrayList(_leaseSet.getLeaseCount()); for (int i = 0; i < _leaseSet.getLeaseCount(); i++) { @@ -317,9 +321,11 @@ public class OutboundClientMessageOneShotJob extends JobImpl { ****/ _lease = (Lease)leases.get(0); // } +/*** removed until we fix SSU reachability synchronized (_leaseCache) { _leaseCache.put(_to, _lease); } +***/ if (_log.shouldLog(Log.WARN)) _log.warn("Added to cache - lease for " + _toString); return true; @@ -606,6 +612,7 @@ public class OutboundClientMessageOneShotJob extends JobImpl { long sendTime = getContext().clock().now() - _start; if (_log.shouldLog(Log.WARN)) _log.warn(getJobId() + ": Failed to send the message " + _clientMessageId + " to " + _toString + + " out " + _outTunnel + " in " + _lease + " ack " + _inTunnel + " after " + sendTime + "ms"); long messageDelay = getContext().throttle().getMessageDelay(); diff --git a/router/java/src/net/i2p/router/transport/udp/PeerTestManager.java b/router/java/src/net/i2p/router/transport/udp/PeerTestManager.java index e893b9cd9..424b6e027 100644 --- a/router/java/src/net/i2p/router/transport/udp/PeerTestManager.java +++ b/router/java/src/net/i2p/router/transport/udp/PeerTestManager.java @@ -56,6 +56,11 @@ class PeerTestManager { private static final long MAX_NONCE = (1l << 32) - 1l; //public void runTest(InetAddress bobIP, int bobPort, SessionKey bobIntroKey) { public void runTest(InetAddress bobIP, int bobPort, SessionKey bobCipherKey, SessionKey bobMACKey) { + if (_currentTest != null) { + if (_log.shouldLog(Log.WARN)) + _log.warn("We are already running a test with bob = " + _currentTest.getBobIP() + ", aborting test with bob = " + bobIP); + return; + } PeerTestState test = new PeerTestState(); test.setNonce(_context.random().nextLong(MAX_NONCE)); test.setBobIP(bobIP); @@ -137,11 +142,15 @@ class PeerTestManager { } /** - * If we have sent a packet to charlie within the last 3 minutes, ignore any test + * If we have sent a packet to charlie within the last 10 minutes, ignore any test * results we get from them, as our NAT will have poked a hole anyway + * NAT idle timeouts vary widely, from 30s to 10m or more. + * Set this too high and a high-traffic router may rarely get a good test result. + * Set it too low and a router will think it is reachable when it isn't. + * Maybe a router should need two consecutive OK results before believing it? * */ - private static final long CHARLIE_RECENT_PERIOD = 3*60*1000; + private static final long CHARLIE_RECENT_PERIOD = 10*60*1000; /** * Receive a PeerTest message which contains the correct nonce for our current @@ -175,14 +184,15 @@ class PeerTestManager { PeerState charlieSession = _transport.getPeerState(from); long recentBegin = _context.clock().now() - CHARLIE_RECENT_PERIOD; if ( (charlieSession != null) && - (charlieSession.getLastACKSend() > recentBegin) && - (charlieSession.getLastSendTime() > recentBegin) ) { + ( (charlieSession.getLastACKSend() > recentBegin) || + (charlieSession.getLastSendTime() > recentBegin) ) ) { if (_log.shouldLog(Log.WARN)) _log.warn("Bob chose a charlie we already have a session to, cancelling the test and rerunning (bob: " + _currentTest + ", charlie: " + from + ")"); _currentTestComplete = true; _context.statManager().addRateData("udp.statusKnownCharlie", 1, 0); honorStatus(CommSystemFacade.STATUS_UNKNOWN); + _currentTest = null; return; } @@ -198,7 +208,7 @@ class PeerTestManager { _log.debug("Receive test reply from charlie @ " + test.getCharlieIP() + " via our " + test.getAlicePort() + "/" + test.getAlicePortFromCharlie()); if (test.getReceiveBobTime() > 0) - testComplete(false); + testComplete(true); } catch (UnknownHostException uhe) { if (_log.shouldLog(Log.ERROR)) _log.error("Charlie @ " + from + " said we were an invalid IP address: " + uhe.getMessage(), uhe); @@ -211,6 +221,10 @@ class PeerTestManager { return; } + if (_log.shouldLog(Log.INFO) && charlieSession != null) + _log.info("Bob chose a charlie we last acked " + DataHelper.formatDuration(_context.clock().now() - charlieSession.getLastACKSend()) + " last sent " + DataHelper.formatDuration(_context.clock().now() - charlieSession.getLastSendTime()) + " (bob: " + + _currentTest + ", charlie: " + from + ")"); + // ok, first charlie. send 'em a packet test.setReceiveCharlieTime(_context.clock().now()); SessionKey charlieIntroKey = new SessionKey(new byte[SessionKey.KEYSIZE_BYTES]); @@ -239,10 +253,14 @@ class PeerTestManager { _currentTestComplete = true; short status = -1; PeerTestState test = _currentTest; - if (expired()) { - _currentTest = null; - return; - } + + // Don't do this or we won't call honorStatus() + // to set the status to UNKNOWN or REJECT_UNSOLICITED + // if (expired()) { + // _currentTest = null; + // return; + // } + if (test.getAlicePortFromCharlie() > 0) { // we received a second message from charlie if ( (test.getAlicePort() == test.getAlicePortFromCharlie()) && diff --git a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java index a6d95288c..38f6ee7d3 100644 --- a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java +++ b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java @@ -2060,7 +2060,7 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority } } if (_alive) { - long delay = _context.random().nextInt(2*TEST_FREQUENCY); + long delay = (TEST_FREQUENCY / 2) + _context.random().nextInt(TEST_FREQUENCY); if (delay <= 0) throw new RuntimeException("wtf, delay is " + delay); SimpleTimer.getInstance().addEvent(PeerTestEvent.this, delay); diff --git a/router/java/src/net/i2p/router/tunnel/pool/ClientPeerSelector.java b/router/java/src/net/i2p/router/tunnel/pool/ClientPeerSelector.java index a5c905f47..e851a68f0 100644 --- a/router/java/src/net/i2p/router/tunnel/pool/ClientPeerSelector.java +++ b/router/java/src/net/i2p/router/tunnel/pool/ClientPeerSelector.java @@ -29,7 +29,10 @@ class ClientPeerSelector extends TunnelPeerSelector { matches.remove(ctx.routerHash()); ArrayList rv = new ArrayList(matches); if (rv.size() > 1) +/*** removed until we fix SSU reachability orderPeers(rv, settings.getRandomKey()); +***/ + Collections.shuffle(rv, ctx.random()); if (settings.isInbound()) rv.add(0, ctx.routerHash()); else diff --git a/router/java/src/net/i2p/router/tunnel/pool/ExploratoryPeerSelector.java b/router/java/src/net/i2p/router/tunnel/pool/ExploratoryPeerSelector.java index 4ce344ea4..815f06443 100644 --- a/router/java/src/net/i2p/router/tunnel/pool/ExploratoryPeerSelector.java +++ b/router/java/src/net/i2p/router/tunnel/pool/ExploratoryPeerSelector.java @@ -50,7 +50,10 @@ class ExploratoryPeerSelector extends TunnelPeerSelector { matches.remove(ctx.routerHash()); ArrayList rv = new ArrayList(matches); if (rv.size() > 1) +/*** removed until we fix SSU reachability orderPeers(rv, settings.getRandomKey()); +***/ + Collections.shuffle(rv, ctx.random()); if (settings.isInbound()) rv.add(0, ctx.routerHash()); else