commit 1e77376e1ac316ec2612b385c6e05af39d9113a8 Author: rl1987 <rl1987@sdf.lonestar.org> Date: Fri Aug 24 18:26:27 2018 +0300 Avoid calling node_get_all_orports() from node_is_a_configured_bridge() All node_get_all_orports() does is allocate and return a smartlist with at most two tor_addr_port_t members that match ORPort's of node configuration. This is harmful for memory efficiency, as it allocates the same stuff every time it is called. However, node_is_a_configured_bridge() does not need to call it, as it already has all the information to check if there is configured bridge for a given node. The new code is arranged in a way that hopefully makes each succeeding linear search through bridge_list less likely. --- changes/bug27224 | 5 ++ src/feature/client/bridges.c | 106 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 102 insertions(+), 9 deletions(-) diff --git a/changes/bug27224 b/changes/bug27224 new file mode 100644 index 000000000..d43890b81 --- /dev/null +++ b/changes/bug27224 @@ -0,0 +1,5 @@ + o Minor bugfixes (performance):: + - Rework node_is_a_configured_bridge() to no longer + call node_get_all_orports(), which was performing too + many memory allocations. Fixes bug 27224; bugfix on + 0.2.3.9. diff --git a/src/feature/client/bridges.c b/src/feature/client/bridges.c index 4bffe1603..7f4422f78 100644 --- a/src/feature/client/bridges.c +++ b/src/feature/client/bridges.c @@ -31,6 +31,7 @@ #include "feature/nodelist/node_st.h" #include "feature/nodelist/routerinfo_st.h" #include "feature/nodelist/routerstatus_st.h" +#include "feature/nodelist/microdesc_st.h" /** Information about a configured bridge. Currently this just matches the * ones in the torrc file, but one day we may be able to learn about new @@ -283,17 +284,105 @@ routerinfo_is_a_configured_bridge(const routerinfo_t *ri) return get_configured_bridge_by_routerinfo(ri) ? 1 : 0; } -/** Return 1 if <b>node</b> is one of our configured bridges, else 0. */ +/** + * Return 1 iff <b>bridge_list</b> contains entry matching + * given; IPv4 address in host byte order (<b>ipv4_addr</b> + * and <b>port</b> (and no identity digest) OR it contains an + * entry whose identity matches <b>digest</b>. Otherwise, + * return 0. + */ +static int +bridge_exists_with_ipv4h_addr_and_port(const uint32_t ipv4_addr, + const uint16_t port, + const char *digest) +{ + tor_addr_t node_ipv4; + + if (tor_addr_port_is_valid_ipv4h(ipv4_addr, port, 0)) { + tor_addr_from_ipv4h(&node_ipv4, ipv4_addr); + + bridge_info_t *bridge = + get_configured_bridge_by_addr_port_digest(&node_ipv4, + port, + digest); + + return (bridge != NULL); + } + + return 0; +} + +/** + * Return 1 iff <b>bridge_list</b> contains entry matching given + * <b>ipv6_addr</b> and <b>port</b> (and no identity digest) OR + * it contains an entry whose identity matches <b>digest</b>. + * Otherwise, return 0. + */ +static int +bridge_exists_with_ipv6_addr_and_port(const tor_addr_t *ipv6_addr, + const uint16_t port, + const char *digest) +{ + if (!tor_addr_port_is_valid(ipv6_addr, port, 0)) + return 0; + + bridge_info_t *bridge = + get_configured_bridge_by_addr_port_digest(ipv6_addr, + port, + digest); + + return (bridge != NULL); +} + +/** Return 1 if <b>node</b> is one of our configured bridges, else 0. + * More specifically, return 1 iff: a bridge_info_t object exists in + * <b>bridge_list</b> such that: 1) It's identity is equal to node + * identity OR 2) It's identity digest is zero, but it matches + * address and port of any ORPort in the node. + */ int node_is_a_configured_bridge(const node_t *node) { - int retval = 0; - smartlist_t *orports = node_get_all_orports(node); - retval = get_configured_bridge_by_orports_digest(node->identity, - orports) != NULL; - SMARTLIST_FOREACH(orports, tor_addr_port_t *, p, tor_free(p)); - smartlist_free(orports); - return retval; + /* First, let's try searching for a bridge with matching identity. */ + if (BUG(tor_digest_is_zero(node->identity))) + return 0; + + if (find_bridge_by_digest(node->identity) != NULL) + return 1; + + /* At this point, we have established that no bridge exists with + * matching identity digest. However, we still pass it into + * bridge_exists_* functions because we want further code to + * check for absence of identity digest in a bridge. + */ + if (node->ri) { + if (bridge_exists_with_ipv4h_addr_and_port(node->ri->addr, + node->ri->or_port, + node->identity)) + return 1; + + if (bridge_exists_with_ipv6_addr_and_port(&node->ri->ipv6_addr, + node->ri->ipv6_orport, + node->identity)) + return 1; + } else if (node->rs) { + if (bridge_exists_with_ipv4h_addr_and_port(node->rs->addr, + node->rs->or_port, + node->identity)) + return 1; + + if (bridge_exists_with_ipv6_addr_and_port(&node->rs->ipv6_addr, + node->rs->ipv6_orport, + node->identity)) + return 1; + } else if (node->md) { + if (bridge_exists_with_ipv6_addr_and_port(&node->md->ipv6_addr, + node->md->ipv6_orport, + node->identity)) + return 1; + } + + return 0; } /** We made a connection to a router at <b>addr</b>:<b>port</b> @@ -934,4 +1023,3 @@ bridges_free_all(void) smartlist_free(bridge_list); bridge_list = NULL; } -