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

13 files changed, 64 insertions, 39 deletions

diff --git a/src/or/buffers.c b/src/or/buffers.c
index ad855b1478..df6b3ed6a0 100644
--- a/src/or/buffers.c
+++ b/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
diff --git a/src/or/command.c b/src/or/command.c
index 8f941e2a99..5b14257181 100644
--- a/src/or/command.c
+++ b/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
- * to 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
- * have in common, and continue the negotiation from there.
+/** Process a 'versions' cell.  The current link protocol version must be 0 to
+ * 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 have
+ * in common, and continue the negotiation from there.
  */
 static void
 command_process_versions_cell(var_cell_t *cell, or_connection_t *conn)
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 8f38b615f9..16403bd527 100644
--- a/src/or/connection_edge.c
+++ b/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;
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 9950bebb9a..50f25e3b2c 100644
--- a/src/or/connection_or.c
+++ b/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
-       * track is_canonical properly. */
+      /* XXXX020 arma: this is stupid, and it's the reason we need real_addr
+       * 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,
-                              conn->tls, &identity_rcvd);
+    int v = tor_tls_verify(started_here?severity:LOG_INFO,
+                           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.",
diff --git a/src/or/dnsserv.c b/src/or/dnsserv.c
index ca0c0ce3ab..799f473974 100644
--- a/src/or/dnsserv.c
+++ b/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.
diff --git a/src/or/geoip.c b/src/or/geoip.c
index 5ad4df1f08..9f93159e30 100644
--- a/src/or/geoip.c
+++ b/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];
-  unsigned total;
+  char country[3]; /**< two-leter country code. */
+  unsigned total; /**< total ips seen in this country. */
 } c_hist_t;
 
 /** Sorting helper: return -1, 1, or 0 based on comparison of two
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 6ac4e2c6ad..ccff1937e8 100644
--- a/src/or/networkstatus.c
+++ b/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.
  */
 
diff --git a/src/or/ntmain.c b/src/or/ntmain.c
index 97c9a4299b..06d0af4e00 100644
--- a/src/or/ntmain.c
+++ b/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;
 
diff --git a/src/or/or.h b/src/or/or.h
index 8e652d452c..5ae2c00737 100644
--- a/src/or/or.h
+++ b/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 */
-  unsigned int have_renegotiated:1; /**< DOCDOC */
+  /** Should this connection be used for extending circuits to the server
+   * 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;
diff --git a/src/or/relay.c b/src/or/relay.c
index c0e308b983..cc5ee4f13b 100644
--- a/src/or/relay.c
+++ b/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>. */
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 9a18ee7258..2891fc1e72 100644
--- a/src/or/rendservice.c
+++ 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 */
+  int descriptor_version; /**< Rendezvous descriptor version that will be
+                           * published. */
   /* 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];
+  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;
+  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;
-  time_t desc_is_dirty;
-  time_t next_upload_time;
-  int descriptor_version; /**< Rendezvous descriptor version that will be
-                           * published. */
+  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.
diff --git a/src/or/rephist.c b/src/or/rephist.c
index edeb43444f..a5746697e6 100644
--- a/src/or/rephist.c
+++ b/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 {
diff --git a/src/or/test.c b/src/or/test.c
index 1177e5a82e..7ec883c30b 100644
--- a/src/or/test.c
+++ b/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);