commit da1e44ee51084f5fbd1ebaf9c8ffd199c06b9019 Author: Karsten Loesing karsten.loesing@gmx.net Date: Thu Jan 17 10:45:19 2013 +0100

Remove dirreq-v2-* lines from extra-info descriptors.

Implements the rest of #5823. --- changes/bug5823 | 3 +- src/or/directory.c | 27 +++++++------ src/or/geoip.c | 111 +++++++++++---------------------------------------- src/or/geoip.h | 7 +-- src/test/test.c | 5 +- 5 files changed, 45 insertions(+), 108 deletions(-)

diff --git a/changes/bug5823 b/changes/bug5823 index 8838538..d76b590 100644 --- a/changes/bug5823 +++ b/changes/bug5823 @@ -1,4 +1,5 @@ o Removed featurs: - Stop exporting estimates of v2 and v3 directory traffic shares in extrainfo documents. They were unneeded and sometimes inaccurate. - Resolves ticket 5823. + Also stop exporting any v2 directory request statistics. Resolves + ticket 5823. diff --git a/src/or/directory.c b/src/or/directory.c index 692a6d6..c68a4e3 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2800,8 +2800,6 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers, /* v2 or v3 network status fetch. */ smartlist_t *dir_fps = smartlist_new(); int is_v3 = !strcmpstart(url, "/tor/status-vote"); - geoip_client_action_t act = - is_v3 ? GEOIP_CLIENT_NETWORKSTATUS : GEOIP_CLIENT_NETWORKSTATUS_V2; const char *request_type = NULL; const char *key = url + strlen("/tor/status/"); long lifetime = NETWORKSTATUS_CACHE_LIFETIME; @@ -2851,7 +2849,7 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers, write_http_status_line(conn, 404, "Consensus not signed by sufficient " "number of requested authorities"); smartlist_free(dir_fps); - geoip_note_ns_response(act, GEOIP_REJECT_NOT_ENOUGH_SIGS); + geoip_note_ns_response(GEOIP_REJECT_NOT_ENOUGH_SIGS); tor_free(flavor); goto done; } @@ -2870,7 +2868,8 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers, if (!smartlist_len(dir_fps)) { /* we failed to create/cache cp */ write_http_status_line(conn, 503, "Network status object unavailable"); smartlist_free(dir_fps); - geoip_note_ns_response(act, GEOIP_REJECT_UNAVAILABLE); + if (is_v3) + geoip_note_ns_response(GEOIP_REJECT_UNAVAILABLE); goto done; }

@@ -2878,13 +2877,15 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers, write_http_status_line(conn, 404, "Not found"); SMARTLIST_FOREACH(dir_fps, char *, cp, tor_free(cp)); smartlist_free(dir_fps); - geoip_note_ns_response(act, GEOIP_REJECT_NOT_FOUND); + if (is_v3) + geoip_note_ns_response(GEOIP_REJECT_NOT_FOUND); goto done; } else if (!smartlist_len(dir_fps)) { write_http_status_line(conn, 304, "Not modified"); SMARTLIST_FOREACH(dir_fps, char *, cp, tor_free(cp)); smartlist_free(dir_fps); - geoip_note_ns_response(act, GEOIP_REJECT_NOT_MODIFIED); + if (is_v3) + geoip_note_ns_response(GEOIP_REJECT_NOT_MODIFIED); goto done; }

@@ -2896,24 +2897,24 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers, write_http_status_line(conn, 503, "Directory busy, try again later"); SMARTLIST_FOREACH(dir_fps, char *, fp, tor_free(fp)); smartlist_free(dir_fps); - geoip_note_ns_response(act, GEOIP_REJECT_BUSY); + if (is_v3) + geoip_note_ns_response(GEOIP_REJECT_BUSY); goto done; }

- { + if (is_v3) { struct in_addr in; tor_addr_t addr; if (tor_inet_aton((TO_CONN(conn))->address, &in)) { tor_addr_from_ipv4h(&addr, ntohl(in.s_addr)); - geoip_note_client_seen(act, &addr, time(NULL)); - geoip_note_ns_response(act, GEOIP_SUCCESS); + geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS, &addr, time(NULL)); + geoip_note_ns_response(GEOIP_SUCCESS); /* Note that a request for a network status has started, so that we * can measure the download time later on. */ if (conn->dirreq_id) - geoip_start_dirreq(conn->dirreq_id, dlen, act, - DIRREQ_TUNNELED); + geoip_start_dirreq(conn->dirreq_id, dlen, DIRREQ_TUNNELED); else - geoip_start_dirreq(TO_CONN(conn)->global_identifier, dlen, act, + geoip_start_dirreq(TO_CONN(conn)->global_identifier, dlen, DIRREQ_DIRECT); } } diff --git a/src/or/geoip.c b/src/or/geoip.c index fd5529c..9ba1e31 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -38,7 +38,6 @@ typedef struct geoip_ipv6_entry_t { /** A per-country record for GeoIP request history. */ typedef struct geoip_country_t { char countrycode[3]; - uint32_t n_v2_ns_requests; uint32_t n_v3_ns_requests; } geoip_country_t;

@@ -549,17 +548,13 @@ geoip_note_client_seen(geoip_client_action_t action, else ent->last_seen_in_minutes = 0;

- if (action == GEOIP_CLIENT_NETWORKSTATUS || - action == GEOIP_CLIENT_NETWORKSTATUS_V2) { + if (action == GEOIP_CLIENT_NETWORKSTATUS) { int country_idx = geoip_get_country_by_addr(addr); if (country_idx < 0) country_idx = 0; /** unresolved requests are stored at index 0. */ if (country_idx >= 0 && country_idx < smartlist_len(geoip_countries)) { geoip_country_t *country = smartlist_get(geoip_countries, country_idx); - if (action == GEOIP_CLIENT_NETWORKSTATUS) - ++country->n_v3_ns_requests; - else - ++country->n_v2_ns_requests; + ++country->n_v3_ns_requests; } } } @@ -587,36 +582,24 @@ geoip_remove_old_clients(time_t cutoff) &cutoff); }

-/** How many responses are we giving to clients requesting v2 network - * statuses? */ -static uint32_t ns_v2_responses[GEOIP_NS_RESPONSE_NUM]; - /** How many responses are we giving to clients requesting v3 network * statuses? */ static uint32_t ns_v3_responses[GEOIP_NS_RESPONSE_NUM];

-/** Note that we've rejected a client's request for a v2 or v3 network - * status, encoded in <b>action</b> for reason <b>reason</b> at time - * <b>now</b>. */ +/** Note that we've rejected a client's request for a v3 network status + * for reason <b>reason</b> at time <b>now</b>. */ void -geoip_note_ns_response(geoip_client_action_t action, - geoip_ns_response_t response) +geoip_note_ns_response(geoip_ns_response_t response) { static int arrays_initialized = 0; if (!get_options()->DirReqStatistics) return; if (!arrays_initialized) { - memset(ns_v2_responses, 0, sizeof(ns_v2_responses)); memset(ns_v3_responses, 0, sizeof(ns_v3_responses)); arrays_initialized = 1; } - tor_assert(action == GEOIP_CLIENT_NETWORKSTATUS || - action == GEOIP_CLIENT_NETWORKSTATUS_V2); tor_assert(response < GEOIP_NS_RESPONSE_NUM); - if (action == GEOIP_CLIENT_NETWORKSTATUS) - ns_v3_responses[response]++; - else - ns_v2_responses[response]++; + ns_v3_responses[response]++; }

/** Do not mention any country from which fewer than this number of IPs have @@ -671,7 +654,6 @@ typedef struct dirreq_map_entry_t { unsigned int state:3; /**< State of this directory request. */ unsigned int type:1; /**< Is this a direct or a tunneled request? */ unsigned int completed:1; /**< Is this request complete? */ - unsigned int action:2; /**< Is this a v2 or v3 request? */ /** When did we receive the request and started sending the response? */ struct timeval request_time; size_t response_size; /**< What is the size of the response in bytes? */ @@ -740,12 +722,11 @@ dirreq_map_get_(dirreq_type_t type, uint64_t dirreq_id) }

/** Note that an either direct or tunneled (see <b>type</b>) directory - * request for a network status with unique ID <b>dirreq_id</b> of size - * <b>response_size</b> and action <b>action</b> (either v2 or v3) has - * started. */ + * request for a v3 network status with unique ID <b>dirreq_id</b> of size + * <b>response_size</b> has started. */ void geoip_start_dirreq(uint64_t dirreq_id, size_t response_size, - geoip_client_action_t action, dirreq_type_t type) + dirreq_type_t type) { dirreq_map_entry_t *ent; if (!get_options()->DirReqStatistics) @@ -754,7 +735,6 @@ geoip_start_dirreq(uint64_t dirreq_id, size_t response_size, ent->dirreq_id = dirreq_id; tor_gettimeofday(&ent->request_time); ent->response_size = response_size; - ent->action = action; ent->type = type; dirreq_map_put_(ent, type, dirreq_id); } @@ -795,8 +775,7 @@ geoip_change_dirreq_state(uint64_t dirreq_id, dirreq_type_t type, * times by deciles and quartiles. Return NULL if we have not observed * requests for long enough. */ static char * -geoip_get_dirreq_history(geoip_client_action_t action, - dirreq_type_t type) +geoip_get_dirreq_history(dirreq_type_t type) { char *result = NULL; smartlist_t *dirreq_completed = NULL; @@ -806,13 +785,10 @@ geoip_get_dirreq_history(geoip_client_action_t action, struct timeval now;

tor_gettimeofday(&now); - if (action != GEOIP_CLIENT_NETWORKSTATUS && - action != GEOIP_CLIENT_NETWORKSTATUS_V2) - return NULL; dirreq_completed = smartlist_new(); for (ptr = HT_START(dirreqmap, &dirreq_map); ptr; ptr = next) { ent = *ptr; - if (ent->action != action || ent->type != type) { + if (ent->type != type) { next = HT_NEXT(dirreqmap, &dirreq_map, ptr); continue; } else { @@ -998,18 +974,15 @@ geoip_get_client_history(geoip_client_action_t action, }

/** Return a newly allocated string holding the per-country request history - * for <b>action</b> in a format suitable for an extra-info document, or NULL - * on failure. */ + * for v3 network statuses in a format suitable for an extra-info document, + * or NULL on failure. */ char * -geoip_get_request_history(geoip_client_action_t action) +geoip_get_request_history(void) { smartlist_t *entries, *strings; char *result; unsigned granularity = IP_GRANULARITY;

- if (action != GEOIP_CLIENT_NETWORKSTATUS && - action != GEOIP_CLIENT_NETWORKSTATUS_V2) - return NULL; if (!geoip_countries) return NULL;

@@ -1017,8 +990,7 @@ geoip_get_request_history(geoip_client_action_t action) SMARTLIST_FOREACH_BEGIN(geoip_countries, geoip_country_t *, c) { uint32_t tot = 0; c_hist_t *ent; - tot = (action == GEOIP_CLIENT_NETWORKSTATUS) ? - c->n_v3_ns_requests : c->n_v2_ns_requests; + tot = c->n_v3_ns_requests; if (!tot) continue; ent = tor_malloc_zero(sizeof(c_hist_t)); @@ -1056,14 +1028,13 @@ void geoip_reset_dirreq_stats(time_t now) { SMARTLIST_FOREACH(geoip_countries, geoip_country_t *, c, { - c->n_v2_ns_requests = c->n_v3_ns_requests = 0; + c->n_v3_ns_requests = 0; }); { clientmap_entry_t **ent, **next, *this; for (ent = HT_START(clientmap, &client_history); ent != NULL; ent = next) { - if ((*ent)->action == GEOIP_CLIENT_NETWORKSTATUS || - (*ent)->action == GEOIP_CLIENT_NETWORKSTATUS_V2) { + if ((*ent)->action == GEOIP_CLIENT_NETWORKSTATUS) { this = *ent; next = HT_NEXT_RMV(clientmap, &client_history, ent); tor_free(this); @@ -1072,7 +1043,6 @@ geoip_reset_dirreq_stats(time_t now) } } } - memset(ns_v2_responses, 0, sizeof(ns_v2_responses)); memset(ns_v3_responses, 0, sizeof(ns_v3_responses)); { dirreq_map_entry_t **ent, **next, *this; @@ -1101,9 +1071,8 @@ geoip_format_dirreq_stats(time_t now) { char t[ISO_TIME_LEN+1]; int i; - char *v3_ips_string, *v2_ips_string, *v3_reqs_string, *v2_reqs_string, - *v3_direct_dl_string, *v2_direct_dl_string, - *v3_tunneled_dl_string, *v2_tunneled_dl_string; + char *v3_ips_string, *v3_reqs_string, *v3_direct_dl_string, + *v3_tunneled_dl_string; char *result;

if (!start_of_dirreq_stats_interval) @@ -1112,77 +1081,45 @@ geoip_format_dirreq_stats(time_t now) tor_assert(now >= start_of_dirreq_stats_interval);

format_iso_time(t, now); - geoip_get_client_history(GEOIP_CLIENT_NETWORKSTATUS_V2, &v2_ips_string, - NULL); geoip_get_client_history(GEOIP_CLIENT_NETWORKSTATUS, &v3_ips_string, NULL); - v2_reqs_string = geoip_get_request_history( - GEOIP_CLIENT_NETWORKSTATUS_V2); - v3_reqs_string = geoip_get_request_history(GEOIP_CLIENT_NETWORKSTATUS); + v3_reqs_string = geoip_get_request_history();

#define RESPONSE_GRANULARITY 8 for (i = 0; i < GEOIP_NS_RESPONSE_NUM; i++) { - ns_v2_responses[i] = round_uint32_to_next_multiple_of( - ns_v2_responses[i], RESPONSE_GRANULARITY); ns_v3_responses[i] = round_uint32_to_next_multiple_of( ns_v3_responses[i], RESPONSE_GRANULARITY); } #undef RESPONSE_GRANULARITY

- v2_direct_dl_string = geoip_get_dirreq_history( - GEOIP_CLIENT_NETWORKSTATUS_V2, DIRREQ_DIRECT); - v3_direct_dl_string = geoip_get_dirreq_history( - GEOIP_CLIENT_NETWORKSTATUS, DIRREQ_DIRECT); - - v2_tunneled_dl_string = geoip_get_dirreq_history( - GEOIP_CLIENT_NETWORKSTATUS_V2, DIRREQ_TUNNELED); - v3_tunneled_dl_string = geoip_get_dirreq_history( - GEOIP_CLIENT_NETWORKSTATUS, DIRREQ_TUNNELED); + v3_direct_dl_string = geoip_get_dirreq_history(DIRREQ_DIRECT); + v3_tunneled_dl_string = geoip_get_dirreq_history(DIRREQ_TUNNELED);

/* Put everything together into a single string. */ tor_asprintf(&result, "dirreq-stats-end %s (%d s)\n" "dirreq-v3-ips %s\n" - "dirreq-v2-ips %s\n" "dirreq-v3-reqs %s\n" - "dirreq-v2-reqs %s\n" "dirreq-v3-resp ok=%u,not-enough-sigs=%u,unavailable=%u," "not-found=%u,not-modified=%u,busy=%u\n" - "dirreq-v2-resp ok=%u,unavailable=%u," - "not-found=%u,not-modified=%u,busy=%u\n" "dirreq-v3-direct-dl %s\n" - "dirreq-v2-direct-dl %s\n" - "dirreq-v3-tunneled-dl %s\n" - "dirreq-v2-tunneled-dl %s\n", + "dirreq-v3-tunneled-dl %s\n", t, (unsigned) (now - start_of_dirreq_stats_interval), v3_ips_string ? v3_ips_string : "", - v2_ips_string ? v2_ips_string : "", v3_reqs_string ? v3_reqs_string : "", - v2_reqs_string ? v2_reqs_string : "", ns_v3_responses[GEOIP_SUCCESS], ns_v3_responses[GEOIP_REJECT_NOT_ENOUGH_SIGS], ns_v3_responses[GEOIP_REJECT_UNAVAILABLE], ns_v3_responses[GEOIP_REJECT_NOT_FOUND], ns_v3_responses[GEOIP_REJECT_NOT_MODIFIED], ns_v3_responses[GEOIP_REJECT_BUSY], - ns_v2_responses[GEOIP_SUCCESS], - ns_v2_responses[GEOIP_REJECT_UNAVAILABLE], - ns_v2_responses[GEOIP_REJECT_NOT_FOUND], - ns_v2_responses[GEOIP_REJECT_NOT_MODIFIED], - ns_v2_responses[GEOIP_REJECT_BUSY], v3_direct_dl_string ? v3_direct_dl_string : "", - v2_direct_dl_string ? v2_direct_dl_string : "", - v3_tunneled_dl_string ? v3_tunneled_dl_string : "", - v2_tunneled_dl_string ? v2_tunneled_dl_string : ""); + v3_tunneled_dl_string ? v3_tunneled_dl_string : "");

/* Free partial strings. */ tor_free(v3_ips_string); - tor_free(v2_ips_string); tor_free(v3_reqs_string); - tor_free(v2_reqs_string); tor_free(v3_direct_dl_string); - tor_free(v2_direct_dl_string); tor_free(v3_tunneled_dl_string); - tor_free(v2_tunneled_dl_string);

return result; } diff --git a/src/or/geoip.h b/src/or/geoip.h index 60d44a5..ebefee5 100644 --- a/src/or/geoip.h +++ b/src/or/geoip.h @@ -30,18 +30,17 @@ void geoip_note_client_seen(geoip_client_action_t action, const tor_addr_t *addr, time_t now); void geoip_remove_old_clients(time_t cutoff);

-void geoip_note_ns_response(geoip_client_action_t action, - geoip_ns_response_t response); +void geoip_note_ns_response(geoip_ns_response_t response); int geoip_get_client_history(geoip_client_action_t action, char **country_str, char **ipver_str); -char *geoip_get_request_history(geoip_client_action_t action); +char *geoip_get_request_history(void); int getinfo_helper_geoip(control_connection_t *control_conn, const char *question, char **answer, const char **errmsg); void geoip_free_all(void);

void geoip_start_dirreq(uint64_t dirreq_id, size_t response_size, - geoip_client_action_t action, dirreq_type_t type); + dirreq_type_t type); void geoip_change_dirreq_state(uint64_t dirreq_id, dirreq_type_t type, dirreq_state_t new_state);

diff --git a/src/test/test.c b/src/test/test.c index c219d98..1c8a64c 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1745,14 +1745,13 @@ test_geoip(void)

/* Note a successful network status response and make sure that it * appears in the history string. */ - geoip_note_ns_response(GEOIP_CLIENT_NETWORKSTATUS, GEOIP_SUCCESS); + geoip_note_ns_response(GEOIP_SUCCESS); s = geoip_format_dirreq_stats(now + 86400); test_streq(dirreq_stats_3, s); tor_free(s);

/* Start a tunneled directory request. */ - geoip_start_dirreq((uint64_t) 1, 1024, GEOIP_CLIENT_NETWORKSTATUS, - DIRREQ_TUNNELED); + geoip_start_dirreq((uint64_t) 1, 1024, DIRREQ_TUNNELED); s = geoip_format_dirreq_stats(now + 86400); test_streq(dirreq_stats_4, s);