* 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

@@ -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;

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);

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();

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.getLastACKSend() > recentBegin) ||
-                 (charlieSession.getLastSendTime() > 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;
+        // Don't do this or we won't call honorStatus()
-            return;
+        // 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()) &&

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);

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

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