r18051@catbus: nickm | 2008-02-12 15:20:43 -0500

Re-tune mempool parametes based on testing on peacetime: use smaller chuncks, free them a little more aggressively, and try very hard to concentrate allocations on fuller chunks.  Also, lots of new documentation.


svn:r13484

ChangeLog

@@ -1,4 +1,8 @@
 Changes in version 0.2.0.20-?? - 2008-02-??
+  o Minor features (performance):
+    - Tune parameters for cell pool allocation to minimize amount of
+      RAM overhead used.
+
   o Minor bugfixes:
     - Log the correct memory chunk sizes for empty RAM chunks in mempool.c
     - Directory mirrors no longer include a guess at the client's IP

doc/tor.1.in

@@ -979,6 +979,17 @@ This option only affects name lookups that your server does on
 behalf of clients, and only takes effect if Tor was built with
 eventdns support.
 (Default: 0)
+.LP
+.TP
+\fBBridgeRecordUsageByCountry \fR\fB0\fR|\fB1\fR\fP
+When this option is enabled and BridgeRelay is also enabled, and we
+have GeoIP data, Tor keeps a keep a per-country count of how many
+client addresses have contacted it so that it can help the bridge
+authority guess which countries have blocked access to it.
+.LP
+.TP
+\fBGeoIPFile \fR\fIfilename\fP
+A filename containing GeoIP data, for use with BridgeRecordUsageByCountry.
 
 .SH DIRECTORY SERVER OPTIONS
 .PP

src/common/aes.c

@@ -28,8 +28,13 @@ const char aes_c_id[] = "$Id$";
 /* We have 3 strategies for getting AES: Via OpenSSL's AES_encrypt function,
  * via OpenSSL's EVP_EncryptUpdate function, or via the built-in AES
  * implementation below. */
+
+/** Defined iff we're using openssl's AES functions for AES. */
 #undef USE_OPENSSL_AES
+/** Defined iff we're using openssl's EVP code for AES. */
 #undef USE_OPENSSL_EVP
+/** Defined iff we're using Tor's internal AES implementation, defined
+ * below. */
 #undef USE_BUILTIN_AES
 
 /* Figure out our CPU type.  We use this to pick an AES implementation.
@@ -130,6 +135,7 @@ static void rijndaelEncrypt(const u32 rk[/*4*(Nr + 1)*/], int Nr,
 /*======================================================================*/
 /* Interface to AES code, and counter implementation */
 
+/** Implements an aes counter-mode cipher. */
 struct aes_cnt_cipher {
   /** This next element (howevever it's defined) is the AES key. */
 #if defined(USE_OPENSSL_EVP)

src/common/compat.h

@@ -330,6 +330,7 @@ struct sockaddr_in6 {
 
 typedef uint8_t maskbits_t;
 struct in_addr;
+/** DOCDOC */
 typedef struct tor_addr_t
 {
   sa_family_t family;

src/common/mempool.c

@@ -12,7 +12,7 @@
 #define MEMPOOL_PRIVATE
 #include "mempool.h"
 
-//#define LAZY_CHUNK_SORT
+#define LAZY_CHUNK_SORT
 
 /* OVERVIEW:
  *
@@ -193,7 +193,9 @@ mp_chunk_new(mp_pool_t *pool)
   return chunk;
 }
 
-/** DOCDOC */
+/** Take a <b>chunk</b> that has just been allocated or removed from
+ * <b>pool</b>'s empty chunk list, and add it to the head of the used chunk
+ * list. */
 static INLINE void
 add_newly_used_chunk_to_used_list(mp_pool_t *pool, mp_chunk_t *chunk)
 {
@@ -347,7 +349,6 @@ mp_pool_release(void *item)
 
     ++pool->n_empty_chunks;
   }
-
   --chunk->n_allocated;
 }
 
@@ -404,7 +405,8 @@ mp_pool_new(size_t item_size, size_t chunk_capacity)
 }
 
 #ifdef LAZY_CHUNK_SORT
-/** DOCDOC */
+/** Helper function for qsort: used to sort pointers to mp_chunk_t into
+ * descending order of fullness. */
 static int
 mp_pool_sort_used_chunks_helper(const void *_a, const void *_b)
 {
@@ -413,7 +415,9 @@ mp_pool_sort_used_chunks_helper(const void *_a, const void *_b)
   return b->n_allocated - a->n_allocated;
 }
 
-/** DOCDOC */
+/** Sort the used chunks in <b>pool</b> into descending order of fullness,
+ * so that we preferentially fill up mostly full chunks before we make
+ * nearly empty chunks less nearly empty. */
 static void
 mp_pool_sort_used_chunks(mp_pool_t *pool)
 {
@@ -426,7 +430,6 @@ mp_pool_sort_used_chunks(mp_pool_t *pool)
   }
   if (!inverted)
     return;
-  ASSERT(n);
   //printf("Sort %d/%d\n",inverted,n);
   chunks = ALLOC(sizeof(mp_chunk_t *)*n);
 #ifdef ALLOC_CAN_RETURN_NULL
@@ -456,12 +459,9 @@ mp_pool_sort_used_chunks(mp_pool_t *pool)
 #endif
 
 /** If there are more than <b>n</b> empty chunks in <b>pool</b>, free the
- * excess ones that have been empty for the longest.  (If <b>n</b> is less
+ * excess ones that have been empty for the longest. If
- * than zero, free only empty chunks that were not used since the last
+ * <b>keep_recently_used</b> is true, do not free chunks unless they have been
- * call to mp_pool_clean(), leaving only -<b>n</b>.)
+ * empty since the last call to this function.
- * DOCDOC Keep_recently_used, n_to_keep
- * XXXX020 maybe dump negative n_to_keep behavior, if k_r_u turns out to be
- *   smarter.
  **/
 void
 mp_pool_clean(mp_pool_t *pool, int n_to_keep, int keep_recently_used)
@@ -471,12 +471,8 @@ mp_pool_clean(mp_pool_t *pool, int n_to_keep, int keep_recently_used)
 #ifdef LAZY_CHUNK_SORT
   mp_pool_sort_used_chunks(pool);
 #endif
+  ASSERT(n_to_keep >= 0);
 
-  if (n_to_keep < 0) {
-    /* As said in the documentation, "negative n" means "leave an additional
-     * -n chunks". So replace n with a positive number. */
-    n_to_keep = pool->min_empty_chunks + (-n_to_keep);
-  }
   if (keep_recently_used) {
     int n_recently_used = pool->n_empty_chunks - pool->min_empty_chunks;
     if (n_to_keep < n_recently_used)

src/common/mempool.h

@@ -3,7 +3,7 @@
 /* $Id$ */
 
 /**
- * \file util.h
+ * \file mempool.h
  * \brief Headers for mempool.c
  **/
 

src/common/tortls.c

@@ -1105,8 +1105,13 @@ log_cert_lifetime(X509 *cert, const char *problem)
     tor_free(s2);
 }
 
-/** DOCDOC helper.
+/** Helper function: try to extract a link certificate and an identity
- * cert_out needs to be freed. id_cert_out doesn't. */
+ * certificate from <b>tls</b>, and store them in *<b>cert_out</b> and
+ * *<b>id_cert_out</b> respectively.  Log all messages at level
+ * <b>severity</b>.
+ *
+ * Note that a reference is added to cert_out, so it needs to be
+ * freed. id_cert_out doesn't. */
 static void
 try_to_extract_certs_from_tls(int severity, tor_tls_t *tls,
                               X509 **cert_out, X509 **id_cert_out)
@@ -1141,12 +1146,12 @@ try_to_extract_certs_from_tls(int severity, tor_tls_t *tls,
 }
 
 /** If the provided tls connection is authenticated and has a
- * certificate that is currently valid and signed, then set
+ * certificate chain that is currently valid and signed, then set
  * *<b>identity_key</b> to the identity certificate's key and return
  * 0.  Else, return -1 and log complaints with log-level <b>severity</b>.
  */
 int
-tor_tls_verify_v1(int severity, tor_tls_t *tls, crypto_pk_env_t **identity_key)
+tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_env_t **identity_key)
 {
   X509 *cert = NULL, *id_cert = NULL;
   EVP_PKEY *id_pkey = NULL;
@@ -1279,7 +1284,8 @@ _check_no_tls_errors(const char *fname, int line)
   tls_log_errors(LOG_WARN, NULL);
 }
 
-/**DOCDOC */
+/** Return true iff the initial TLS connection at <b>tls</b> did not use a v2
+ * TLS handshake. Output undefined if the handshake isn't finished. */
 int
 tor_tls_used_v1_handshake(tor_tls_t *tls)
 {

src/common/tortls.h

@@ -55,8 +55,7 @@ void tor_tls_set_renegotiate_callback(tor_tls_t *tls,
 int tor_tls_is_server(tor_tls_t *tls);
 void tor_tls_free(tor_tls_t *tls);
 int tor_tls_peer_has_cert(tor_tls_t *tls);
-int tor_tls_verify_v1(int severity, tor_tls_t *tls,
+int tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_env_t **identity);
-                      crypto_pk_env_t **identity);
 int tor_tls_check_lifetime(tor_tls_t *tls, int tolerance);
 int tor_tls_read(tor_tls_t *tls, char *cp, size_t len);
 int tor_tls_write(tor_tls_t *tls, const char *cp, size_t n);

src/or/buffers.c

@@ -20,6 +20,8 @@ const char buffers_c_id[] =
 //#define NOINLINE
 
 #ifdef PARANOIA
+/** Helper: If PARANOIA is defined, assert that the buffer in local variable
+ * <b>buf</b> is well-formed. */
 #define check() STMT_BEGIN assert_buf_ok(buf); STMT_END
 #else
 #define check() STMT_NIL
@@ -124,6 +126,8 @@ static chunk_freelist_t freelists[] = {
   FL(0, 0, 0)
 };
 #undef FL
+/** How many times have we looked for a chunk of a size that no freelist
+ * could help with? */
 static uint64_t n_freelist_miss = 0;
 
 static void assert_freelist_ok(chunk_freelist_t *fl);
@@ -232,7 +236,7 @@ chunk_grow(chunk_t *chunk, size_t sz)
 #define MIN_READ_LEN 8
 /** Every chunk should take up at least this many bytes. */
 #define MIN_CHUNK_ALLOC 256
-/*XXXX020 enforce this maximum. */
+/** No chunk should take up more than this many bytes. */
 #define MAX_CHUNK_ALLOC 65536
 
 /** Return the allocation size we'd like to use to hold <b>target</b>
@@ -549,7 +553,7 @@ buf_add_chunk_with_capacity(buf_t *buf, size_t capacity, int capped)
 }
 
 /** Read up to <b>at_most</b> bytes from the socket <b>fd</b> into
- * <b>chunk</b> (which must be on <b>buf/b>). If we get an EOF, set
+ * <b>chunk</b> (which must be on <b>buf</b>). If we get an EOF, set
  * *<b>reached_eof</b> to 1.  Return -1 on error, 0 on eof or blocking,
  * and the number of bytes read otherwise. */
 static INLINE int

src/or/command.c

@@ -445,10 +445,10 @@ command_process_destroy_cell(cell_t *cell, or_connection_t *conn)
   }
 }
 
-/** Process a 'versions' cell.  The current link protocol version must be 0
+/** Process a 'versions' cell.  The current link protocol version must be 0 to
- * to indicate that no version has yet been negotiated.  We compare the versions
+ * indicate that no version has yet been negotiated.  We compare the versions
- * cell to the list of versions we support, pick the highest version we
+ * cell to the list of versions we support, pick the highest version we have
- * have in common, and continue the negotiation from there.
+ * in common, and continue the negotiation from there.
  */
 static void
 command_process_versions_cell(var_cell_t *cell, or_connection_t *conn)

src/or/connection_edge.c

@@ -925,6 +925,7 @@ client_dns_set_reverse_addressmap(const char *address, const char *v,
  *
  * These options are configured by parse_virtual_addr_network().
  */
+/*DOCDOC options */
 static uint32_t virtual_addr_network = 0x7fc00000u;
 static maskbits_t virtual_addr_netmask_bits = 10;
 static uint32_t next_virtual_addr    = 0x7fc00000u;

src/or/connection_or.c

@@ -404,8 +404,8 @@ connection_or_init_conn_from_address(or_connection_t *conn,
       /* Override the addr/port, so our log messages will make sense.
        * This is dangerous, since if we ever try looking up a conn by
        * its actual addr/port, we won't remember. Careful! */
-      /* XXXX020 this is stupid, and it's the reason we need real_addr to
+      /* XXXX020 arma: this is stupid, and it's the reason we need real_addr
-       * track is_canonical properly. */
+       * to track is_canonical properly.  What requires it? */
       conn->_base.addr = r->addr;
       conn->_base.port = r->or_port;
     }
@@ -724,8 +724,8 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn,
   check_no_tls_errors();
 
   if (has_cert) {
-    int v = tor_tls_verify_v1(started_here?severity:LOG_INFO,
+    int v = tor_tls_verify(started_here?severity:LOG_INFO,
-                              conn->tls, &identity_rcvd);
+                           conn->tls, &identity_rcvd);
     if (started_here && v<0) {
       log_fn(severity,LD_OR,"Tried connecting to router at %s:%d: It"
              " has a cert but it's invalid. Closing.",

src/or/dnsserv.c

@@ -5,7 +5,7 @@ const char dnsserv_c_id[] =
   "$Id$";
 
 /**
- * \file dnservs.c \brief Implements client-side DNS proxy server code.  Note:
+ * \file dnsserv.c \brief Implements client-side DNS proxy server code.  Note:
  * this is the DNS Server code, not the Server DNS code.  Confused?  This code
  * runs on client-side, and acts as a DNS server.  The code in dns.c, on the
  * other hand, runs on Tor servers, and acts as a DNS client.

src/or/geoip.c

@@ -294,10 +294,10 @@ geoip_get_history_start(void)
   return client_history_starts;
 }
 
-/* Helper type: used to sort results by value. */
+/** Helper type: used to sort per-country totals by value. */
 typedef struct c_hist_t {
-  char country[3];
+  char country[3]; /**< two-leter country code. */
-  unsigned total;
+  unsigned total; /**< total ips seen in this country. */
 } c_hist_t;
 
 /** Sorting helper: return -1, 1, or 0 based on comparison of two

src/or/networkstatus.c

@@ -8,7 +8,8 @@ const char networkstatus_c_id[] =
   "$Id$";
 
 /**
- * \file Functions and structures for handling network status documents as a
+ * \file networkstatus.c
+ * \brief Functions and structures for handling network status documents as a
  * client or cache.
  */
 

src/or/ntmain.c

@@ -45,6 +45,8 @@ static int nt_service_remove(void);
 static int nt_service_cmd_start(void);
 static int nt_service_cmd_stop(void);
 
+/** Struct to hold dynamically loaded NT-service related function pointers.
+ */
 struct service_fns {
   int loaded;
 

src/or/or.h

@@ -916,13 +916,20 @@ typedef struct or_connection_t {
    * recent, we can rate limit it further. */
   time_t client_used;
 
-  uint32_t real_addr; /**DOCDOC */
+  uint32_t real_addr; /**< The actual address that this connection came from
+                       * or went to.  The <b>addr</b> field is prone to
+                       * getting overridden by the address from the router
+                       * descriptor matching <b>identity_digest</b>. */
 
   circ_id_type_t circ_id_type:2; /**< When we send CREATE cells along this
                                   * connection, which half of the space should
                                   * we use? */
-  unsigned int is_canonical:1; /**< DOCDOC */
+  /** Should this connection be used for extending circuits to the server
-  unsigned int have_renegotiated:1; /**< DOCDOC */
+   * matching the <b>identity_digest</b> field?  Set to true if we're pretty
+   * sure we aren't getting MITMed, either because we're connected to an
+   * address listed in a server descriptor, or because an authenticated
+   * NETINFO cell listed the address we're connected to as recognized. */
+  unsigned int is_canonical:1;
   uint8_t link_proto; /**< What protocol version are we using? 0 for
                        * "none negotiated yet." */
   uint16_t next_circ_id; /**< Which circ_id do we try to use next on
@@ -1445,7 +1452,7 @@ typedef struct vote_routerstatus_t {
                   * running. */
 } vote_routerstatus_t;
 
-/* Information about a single voter in a vote or a consensus. */
+/** Information about a single voter in a vote or a consensus. */
 typedef struct networkstatus_voter_info_t {
   char *nickname; /**< Nickname of this voter */
   char identity_digest[DIGEST_LEN]; /**< Digest of this voter's identity key */
@@ -2335,8 +2342,11 @@ typedef struct {
    * cached. */
   char *FallbackNetworkstatusFile;
 
-  /** DOCDOC here and in tor.1 */
+  /** If true, and we have GeoIP data, and we're a bridge, keep a per-country
+   * count of how many client addresses have contacted us so that we can help
+   * the bridge authority guess which countries have blocked access to us. */
   int BridgeRecordUsageByCountry;
+  /** Optionally, a file with GeoIP data. */
   char *GeoIPFile;
 
 } or_options_t;

src/or/relay.c

@@ -1519,7 +1519,7 @@ void
 init_cell_pool(void)
 {
   tor_assert(!cell_pool);
-  cell_pool = mp_pool_new(sizeof(packed_cell_t), 1<<19);
+  cell_pool = mp_pool_new(sizeof(packed_cell_t), 128*1024);
 }
 
 /** Free all storage used to hold cells. */
@@ -1538,7 +1538,7 @@ void
 clean_cell_pool(void)
 {
   tor_assert(cell_pool);
-  mp_pool_clean(cell_pool, -1, 0);
+  mp_pool_clean(cell_pool, 0, 1);
 }
 
 /** Release storage held by <b>cell</b>. */

src/or/rendservice.c

@@ -42,26 +42,30 @@ typedef struct rend_service_port_config_t {
 
 /** Represents a single hidden service running at this OP. */
 typedef struct rend_service_t {
-  /** Fields specified in config file */
+  /* Fields specified in config file */
   char *directory; /**< where in the filesystem it stores it */
   smartlist_t *ports; /**< List of rend_service_port_config_t */
   char *intro_prefer_nodes; /**< comma-separated list of nicknames */
   char *intro_exclude_nodes; /**< comma-separated list of nicknames */
-  /* Other fields */
-  /* DOCDOC All of these fields */
-  crypto_pk_env_t *private_key;
-  char service_id[REND_SERVICE_ID_LEN_BASE32+1];
-  char pk_digest[DIGEST_LEN];
-  smartlist_t *intro_nodes; /**< List of rend_intro_point_t's we have,
-                             * or are trying to establish. */
-  time_t intro_period_started;
-  int n_intro_circuits_launched; /**< count of intro circuits we have
-                                  * established in this period. */
-  rend_service_descriptor_t *desc;
-  time_t desc_is_dirty;
-  time_t next_upload_time;
   int descriptor_version; /**< Rendezvous descriptor version that will be
                            * published. */
+  /* Other fields */
+  crypto_pk_env_t *private_key; /**< Permanent hidden-service key. */
+  char service_id[REND_SERVICE_ID_LEN_BASE32+1]; /**< Onion address without
+                                                  * '.onion' */
+  char pk_digest[DIGEST_LEN]; /**< Hash of permanent hidden-service key. */
+  smartlist_t *intro_nodes; /**< List of rend_intro_point_t's we have,
+                             * or are trying to establish. */
+  time_t intro_period_started; /**< Start of the current period to build
+                                * introduction points. */
+  int n_intro_circuits_launched; /**< count of intro circuits we have
+                                  * established in this period. */
+  rend_service_descriptor_t *desc; /**< Current hidden service descriptor. */
+  time_t desc_is_dirty; /**< Time at which changes to the hidden service
+                         * descriptor content occurred, or 0 if it's
+                         * up-to-date. */
+  time_t next_upload_time; /**< Scheduled next hidden service descriptor
+                            * upload time. */
 } rend_service_t;
 
 /** A list of rend_service_t's for services run on this OP.

src/or/rephist.c

@@ -1665,7 +1665,7 @@ typedef struct hs_usage_list_elem_t {
   struct hs_usage_list_elem_t *next;
 } hs_usage_list_elem_t;
 
-/* Ordered list that stores service ids and the number of observations. It is
+/** Ordered list that stores service ids and the number of observations. It is
  * ordered by the number of occurrences in descending order. Its purpose is to
  * calculate the frequency distribution when the period is over. */
 typedef struct hs_usage_list_t {

src/or/test.c

@@ -1042,7 +1042,9 @@ test_util(void)
   test_eq(round_to_power_of_2(0), 2);
 }
 
-/** DOCDOC */
+/** Helper: assert that IPv6 addresses <b>a</b> and <b>b</b> are the same.  On
+ * failure, reports an error, describing the addresses as <b>e1</b> and
+ * <b>e2</b>, and reporting the line number as <b>line</b>. */
 static void
 _test_eq_ip6(struct in6_addr *a, struct in6_addr *b, const char *e1,
              const char *e2, int line)
@@ -1077,8 +1079,6 @@ _test_eq_ip6(struct in6_addr *a, struct in6_addr *b, const char *e1,
     fflush(stdout);
   }
 }
-/** DOCDOC */
-#define test_eq_ip6(a,b) _test_eq_ip6((a),(b),#a,#b,__LINE__)
 
 /** Helper: Assert that two strings both decode as IPv6 addresses with
  * tor_inet_pton(), and both decode to the same address. */
@@ -1133,7 +1133,10 @@ _test_eq_ip6(struct in6_addr *a, struct in6_addr *b, const char *e1,
       test_fail_msg("failed: tor_addr_compare("a","b") "#op" 0"); \
   STMT_END
 
-/**DOCDOC*/
+/** Helper: assert that <b>xx</b> is parseable as a masked IPv6 address with
+ * ports by <b>tor_parse_mask_addr_ports(), with family <b>f</b>, IP address
+ * as 4 32-bit words <b>ip1...ip4</b>, mask bits as <b>mm</b>, and port range
+ * as <b>pt1..pt2</b>. */
 #define test_addr_mask_ports_parse(xx, f, ip1, ip2, ip3, ip4, mm, pt1, pt2) \
   STMT_BEGIN                                                                \
     test_eq(tor_addr_parse_mask_ports(xx, &t1, &mask, &port1, &port2), f);  \
@@ -3121,7 +3124,7 @@ test_util_mempool(void)
       //mp_pool_assert_ok(pool);
     }
     if (crypto_rand_int(777)==0)
-      mp_pool_clean(pool, -1, 0);
+      mp_pool_clean(pool, 1, 1);
 
     if (i % 777)
       mp_pool_assert_ok(pool);