[tor-commits] [tor/master] Merge remote-tracking branch 'public/bug11553_024' into bug11553_025

[nickm at torproject.org]

commit 17ad309d33561ee255cac70bdb9a19803f2d8c08
Merge: fd9961d a770b74
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Apr 23 12:44:18 2014 -0400

    Merge remote-tracking branch 'public/bug11553_024' into bug11553_025
    
    Conflicts:
    	src/or/circuitbuild.c

 src/or/channel.c      |    3 +++
 src/or/channel.h      |    6 ++++--
 src/or/circuitbuild.c |   19 +++++++++++++------
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --cc src/or/channel.h
index de19fad,63da0c1..bd9a02f
--- a/src/or/channel.h
+++ b/src/or/channel.h
@@@ -146,16 -145,14 +146,14 @@@ struct channel_s 
     * When we send CREATE cells along this connection, which half of the
     * space should we use?
     */
 -  ENUM_BF(circ_id_type_t) circ_id_type:2;
 +  circ_id_type_bitfield_t circ_id_type:2;
    /** DOCDOC*/
    unsigned wide_circ_ids:1;
-   /** Have we logged a warning about circID exhaustion on this channel? */
-   unsigned warned_circ_ids_exhausted:1;
  
 -  /* For how many circuits are we n_chan?  What about p_chan? */
 +  /** For how many circuits are we n_chan?  What about p_chan? */
    unsigned int num_n_circuits, num_p_circuits;
  
 -  /*
 +  /**
     * True iff this channel shouldn't get any new circs attached to it,
     * because the connection is too old, or because there's a better one.
     * More generally, this flag is used to note an unhealthy connection;
diff --cc src/or/circuitbuild.c
index 550ed1c,8d6aad6..9e11a0b
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@@ -87,9 -102,14 +87,15 @@@ channel_connect_for_circuit(const tor_a
  static circid_t
  get_unique_circ_id_by_chan(channel_t *chan)
  {
+ /* This number is chosen somewhat arbitrarily; see comment below for more
+  * info.  When the space is 80% full, it gives a one-in-a-million failure
+  * chance; when the space is 90% full, it gives a one-in-850 chance; and when
+  * the space is 95% full, it gives a one-in-26 failure chance.  That seems
+  * okay, though you could make a case IMO for anything between N=32 and
+  * N=256. */
  #define MAX_CIRCID_ATTEMPTS 64
 -
 +  int in_use;
 +  unsigned n_with_circ = 0, n_pending_destroy = 0;
    circid_t test_circ_id;
    circid_t attempts=0;
    circid_t high_bit, max_range, mask;
@@@ -123,30 -143,22 +129,31 @@@
         * whole circuit ID space every time we extend a circuit, which is
         * not so great either.
         */
-       if (! chan->warned_circ_ids_exhausted) {
-         chan->warned_circ_ids_exhausted = 1;
-         log_warn(LD_CIRC,"No unused circIDs found on channel %s wide "
+       log_fn_ratelim(&chan->last_warned_circ_ids_exhausted, LOG_WARN,
+                  LD_CIRC,"No unused circIDs found on channel %s wide "
                   "circID support, with %u inbound and %u outbound circuits. "
 +                 "Found %u circuit IDs in use by circuits, and %u with "
 +                 "pending destroy cells."
                   "Failing a circuit.",
                   chan->wide_circ_ids ? "with" : "without",
 -                 chan->num_p_circuits, chan->num_n_circuits);
 +                 chan->num_p_circuits, chan->num_n_circuits,
 +                 n_with_circ, n_pending_destroy);
        return 0;
      }
  
-     crypto_rand((char*) &test_circ_id, sizeof(test_circ_id));
-     test_circ_id &= mask;
+     do {
+       crypto_rand((char*) &test_circ_id, sizeof(test_circ_id));
+       test_circ_id &= mask;
+     } while (test_circ_id == 0);
+ 
      test_circ_id |= high_bit;
 -  } while (circuit_id_in_use_on_channel(test_circ_id, chan));
 +
 +    in_use = circuit_id_in_use_on_channel(test_circ_id, chan);
 +    if (in_use == 1)
 +      ++n_with_circ;
 +    else if (in_use == 2)
 +      ++n_pending_destroy;
 +  } while (in_use);
    return test_circ_id;
  }