Skip to content

Commit

Permalink
perf: Slightly reduce bandwidth usage when there are few nodes.
Browse files Browse the repository at this point in the history
This mainly saves spam in test logs, but may save some packets here and
there, if nodes are randomly selected twice for GET_NODES and onion
routing packets.
  • Loading branch information
iphydf committed Dec 16, 2023
1 parent 89b6450 commit 7dba867
Show file tree
Hide file tree
Showing 24 changed files with 246 additions and 83 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ unit_test(toxcore bin_pack)
unit_test(toxcore crypto_core)
unit_test(toxcore group_announce)
unit_test(toxcore group_moderation)
unit_test(toxcore list)
unit_test(toxcore mem)
unit_test(toxcore mono_time)
unit_test(toxcore ping_array)
Expand Down
8 changes: 4 additions & 4 deletions auto_tests/onion_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ static int handle_test_1(void *object, const IP_Port *source, const uint8_t *pac
res_packet[0] = NET_PACKET_ANNOUNCE_RESPONSE;
memcpy(res_packet + 1, res_message, sizeof(res_message));

if (send_onion_response(onion->net, source, res_packet, sizeof(res_packet),
if (send_onion_response(onion->log, onion->net, source, res_packet, sizeof(res_packet),
packet + sizeof(res_packet)) == -1) {
return 1;
}
Expand Down Expand Up @@ -293,7 +293,7 @@ static void test_basic(void)
uint64_t s;
memcpy(&s, sb_data, sizeof(uint64_t));
memcpy(test_3_pub_key, nodes[3].public_key, CRYPTO_PUBLIC_KEY_SIZE);
int ret = send_announce_request(onion1->net, rng, &path, &nodes[3],
int ret = send_announce_request(log1, onion1->net, rng, &path, &nodes[3],
dht_get_self_public_key(onion1->dht),
dht_get_self_secret_key(onion1->dht),
zeroes,
Expand All @@ -315,7 +315,7 @@ static void test_basic(void)
memcpy(onion_announce_entry_public_key(onion2_a, 1), dht_get_self_public_key(onion2->dht), CRYPTO_PUBLIC_KEY_SIZE);
onion_announce_entry_set_time(onion2_a, 1, mono_time_get(mono_time2));
networking_registerhandler(onion1->net, NET_PACKET_ONION_DATA_RESPONSE, &handle_test_4, onion1);
send_announce_request(onion1->net, rng, &path, &nodes[3],
send_announce_request(log1, onion1->net, rng, &path, &nodes[3],
dht_get_self_public_key(onion1->dht),
dht_get_self_secret_key(onion1->dht),
test_3_ping_id,
Expand All @@ -340,7 +340,7 @@ static void test_basic(void)
ck_assert_msg((onion3 != nullptr), "Onion failed initializing.");

random_nonce(rng, nonce);
ret = send_data_request(onion3->net, rng, &path, &nodes[3].ip_port,
ret = send_data_request(log3, onion3->net, rng, &path, &nodes[3].ip_port,
dht_get_self_public_key(onion1->dht),
dht_get_self_public_key(onion1->dht),
nonce, (const uint8_t *)"Install gentoo", sizeof("Install gentoo"));
Expand Down
2 changes: 1 addition & 1 deletion other/bootstrap_daemon/docker/tox-bootstrapd.sha256
Original file line number Diff line number Diff line change
@@ -1 +1 @@
5aac1df4d6c1de289e8e9f646d06099c84fd4d9b80d19f45e3254eec3ece2bff /usr/local/bin/tox-bootstrapd
befa926c070acc701cab4131bd258d0cec2186013230da3bc6f6c42d8bb695a5 /usr/local/bin/tox-bootstrapd
1 change: 1 addition & 0 deletions toxav/msi.c
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,7 @@ static void handle_pop(MSICall *call, const MSIMessage *msg)
switch (call->state) {
case MSI_CALL_INACTIVE: {
LOGGER_FATAL(call->session->messenger->log, "Handling what should be impossible case");
break;

Check warning on line 824 in toxav/msi.c

View check run for this annotation

Codecov / codecov/patch

toxav/msi.c#L824

Added line #L824 was not covered by tests
}

case MSI_CALL_ACTIVE: {
Expand Down
2 changes: 2 additions & 0 deletions toxcore/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ cc_library(
":attributes",
":ccompat",
":mem",
":util",
"@pthread",
],
)
Expand Down Expand Up @@ -230,6 +231,7 @@ cc_library(
deps = [
":ccompat",
":crypto_core",
":logger",
":mem",
":mono_time",
],
Expand Down
20 changes: 14 additions & 6 deletions toxcore/DHT.c
Original file line number Diff line number Diff line change
Expand Up @@ -1828,7 +1828,9 @@ static uint8_t do_ping_and_sendnode_requests(DHT *dht, uint64_t *lastgetnode, co
++not_kill;

if (mono_time_is_timeout(dht->mono_time, assoc->last_pinged, PING_INTERVAL)) {
dht_getnodes(dht, &assoc->ip_port, client->public_key, public_key);
const IP_Port *target = &assoc->ip_port;
const uint8_t *target_key = client->public_key;
dht_getnodes(dht, target, target_key, public_key);
assoc->last_pinged = temp_time;
}

Expand Down Expand Up @@ -1861,7 +1863,9 @@ static uint8_t do_ping_and_sendnode_requests(DHT *dht, uint64_t *lastgetnode, co
rand_node += random_range_u32(dht->rng, num_nodes - (rand_node + 1));
}

dht_getnodes(dht, &assoc_list[rand_node]->ip_port, client_list[rand_node]->public_key, public_key);
const IP_Port *target = &assoc_list[rand_node]->ip_port;
const uint8_t *target_key = client_list[rand_node]->public_key;
dht_getnodes(dht, target, target_key, public_key);

*lastgetnode = temp_time;
++*bootstrap_times;
Expand Down Expand Up @@ -1890,8 +1894,7 @@ static void do_dht_friends(DHT *dht)
dht_friend->num_to_bootstrap = 0;

do_ping_and_sendnode_requests(dht, &dht_friend->lastgetnode, dht_friend->public_key, dht_friend->client_list,
MAX_FRIEND_CLIENTS,
&dht_friend->bootstrap_times, true);
MAX_FRIEND_CLIENTS, &dht_friend->bootstrap_times, true);
}
}

Expand Down Expand Up @@ -2634,6 +2637,7 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
DHT *const dht = (DHT *)mem_alloc(mem, sizeof(DHT));

if (dht == nullptr) {
LOGGER_ERROR(log, "failed to allocate DHT struct (%ld bytes)", (unsigned long)sizeof(DHT));

Check warning on line 2640 in toxcore/DHT.c

View check run for this annotation

Codecov / codecov/patch

toxcore/DHT.c#L2640

Added line #L2640 was not covered by tests
return nullptr;
}

Expand All @@ -2651,6 +2655,7 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
dht->ping = ping_new(mem, mono_time, rng, dht);

if (dht->ping == nullptr) {
LOGGER_ERROR(log, "failed to initialise ping");

Check warning on line 2658 in toxcore/DHT.c

View check run for this annotation

Codecov / codecov/patch

toxcore/DHT.c#L2658

Added line #L2658 was not covered by tests
kill_dht(dht);
return nullptr;
}
Expand All @@ -2667,10 +2672,11 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw

crypto_new_keypair(rng, dht->self_public_key, dht->self_secret_key);

dht->shared_keys_recv = shared_key_cache_new(mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
dht->shared_keys_sent = shared_key_cache_new(mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
dht->shared_keys_recv = shared_key_cache_new(log, mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
dht->shared_keys_sent = shared_key_cache_new(log, mono_time, mem, dht->self_secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);

if (dht->shared_keys_recv == nullptr || dht->shared_keys_sent == nullptr) {
LOGGER_ERROR(log, "failed to initialise shared key cache");

Check warning on line 2679 in toxcore/DHT.c

View check run for this annotation

Codecov / codecov/patch

toxcore/DHT.c#L2679

Added line #L2679 was not covered by tests
kill_dht(dht);
return nullptr;
}
Expand All @@ -2679,6 +2685,7 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw
dht->dht_ping_array = ping_array_new(mem, DHT_PING_ARRAY_SIZE, PING_TIMEOUT);

if (dht->dht_ping_array == nullptr) {
LOGGER_ERROR(log, "failed to initialise ping array");

Check warning on line 2688 in toxcore/DHT.c

View check run for this annotation

Codecov / codecov/patch

toxcore/DHT.c#L2688

Added line #L2688 was not covered by tests
kill_dht(dht);
return nullptr;
}
Expand All @@ -2691,6 +2698,7 @@ DHT *new_dht(const Logger *log, const Memory *mem, const Random *rng, const Netw

uint32_t token; // We don't intend to delete these ever, but need to pass the token
if (dht_addfriend(dht, random_public_key_bytes, nullptr, nullptr, 0, &token) != 0) {
LOGGER_ERROR(log, "failed to add initial random seed DHT friends");

Check warning on line 2701 in toxcore/DHT.c

View check run for this annotation

Codecov / codecov/patch

toxcore/DHT.c#L2701

Added line #L2701 was not covered by tests
kill_dht(dht);
return nullptr;
}
Expand Down
11 changes: 11 additions & 0 deletions toxcore/Messenger.c
Original file line number Diff line number Diff line change
Expand Up @@ -3583,6 +3583,7 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
mem_delete(mem, m);

if (error != nullptr && net_err == 1) {
LOGGER_WARNING(m->log, "network initialisation failed (no ports available)");

Check warning on line 3586 in toxcore/Messenger.c

View check run for this annotation

Codecov / codecov/patch

toxcore/Messenger.c#L3586

Added line #L3586 was not covered by tests
*error = MESSENGER_ERROR_PORT;
}

Expand All @@ -3602,6 +3603,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
m->net_crypto = new_net_crypto(m->log, m->mem, m->rng, m->ns, m->mono_time, m->dht, &options->proxy_info);

if (m->net_crypto == nullptr) {
LOGGER_WARNING(m->log, "net_crypto initialisation failed");

Check warning on line 3606 in toxcore/Messenger.c

View check run for this annotation

Codecov / codecov/patch

toxcore/Messenger.c#L3606

Added line #L3606 was not covered by tests

kill_dht(m->dht);
kill_networking(m->net);
friendreq_kill(m->fr);
Expand All @@ -3614,6 +3617,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
m->group_announce = new_gca_list();

if (m->group_announce == nullptr) {
LOGGER_WARNING(m->log, "DHT group chats initialisation failed");

Check warning on line 3620 in toxcore/Messenger.c

View check run for this annotation

Codecov / codecov/patch

toxcore/Messenger.c#L3620

Added line #L3620 was not covered by tests

kill_net_crypto(m->net_crypto);
kill_dht(m->dht);
kill_networking(m->net);
Expand Down Expand Up @@ -3642,6 +3647,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *

if ((options->dht_announcements_enabled && (m->forwarding == nullptr || m->announce == nullptr)) ||
m->onion == nullptr || m->onion_a == nullptr || m->onion_c == nullptr || m->fr_c == nullptr) {
LOGGER_WARNING(m->log, "onion initialisation failed");

Check warning on line 3650 in toxcore/Messenger.c

View check run for this annotation

Codecov / codecov/patch

toxcore/Messenger.c#L3650

Added line #L3650 was not covered by tests

kill_onion(m->onion);
kill_onion_announce(m->onion_a);
kill_onion_client(m->onion_c);
Expand All @@ -3666,6 +3673,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
m->group_handler = new_dht_groupchats(m);

if (m->group_handler == nullptr) {
LOGGER_WARNING(m->log, "conferences initialisation failed");

Check warning on line 3676 in toxcore/Messenger.c

View check run for this annotation

Codecov / codecov/patch

toxcore/Messenger.c#L3676

Added line #L3676 was not covered by tests

kill_onion(m->onion);
kill_onion_announce(m->onion_a);
kill_onion_client(m->onion_c);
Expand All @@ -3690,6 +3699,8 @@ Messenger *new_messenger(Mono_Time *mono_time, const Memory *mem, const Random *
m->onion, m->forwarding);

if (m->tcp_server == nullptr) {
LOGGER_WARNING(m->log, "TCP server initialisation failed");

Check warning on line 3702 in toxcore/Messenger.c

View check run for this annotation

Codecov / codecov/patch

toxcore/Messenger.c#L3702

Added line #L3702 was not covered by tests

kill_onion(m->onion);
kill_onion_announce(m->onion_a);
#ifndef VANILLA_NACL
Expand Down
6 changes: 5 additions & 1 deletion toxcore/TCP_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,11 @@ int read_tcp_packet(
const uint16_t count = net_socket_data_recv_buffer(ns, sock);

if (count < length) {
LOGGER_TRACE(logger, "recv buffer has %d bytes, but requested %d bytes", count, length);
if (count != 0) {
// Only log when there are some bytes available, as empty buffer
// is a very common case and this spams our logs.
LOGGER_TRACE(logger, "recv buffer has %d bytes, but requested %d bytes", count, length);
}
return -1;
}

Expand Down
2 changes: 1 addition & 1 deletion toxcore/TCP_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ bool tcp_get_random_conn_ip_port(const TCP_Connections *tcp_c, IP_Port *ip_port)
* return -1 on failure.
*/
non_null()
int tcp_send_onion_request(TCP_Connections *tcp_c, unsigned int tcp_connections_number, const uint8_t *data,
int tcp_send_onion_request(TCP_Connections *tcp_c, uint32_t tcp_connections_number, const uint8_t *data,
uint16_t length);

/** @brief Set if we want TCP_connection to allocate some connection for onion use.
Expand Down
2 changes: 1 addition & 1 deletion toxcore/announce.c
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ Announcements *new_announcements(const Logger *log, const Memory *mem, const Ran
announce->public_key = dht_get_self_public_key(announce->dht);
announce->secret_key = dht_get_self_secret_key(announce->dht);
new_hmac_key(announce->rng, announce->hmac_key);
announce->shared_keys = shared_key_cache_new(mono_time, mem, announce->secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
announce->shared_keys = shared_key_cache_new(log, mono_time, mem, announce->secret_key, KEYS_TIMEOUT, MAX_KEYS_PER_SLOT);
if (announce->shared_keys == nullptr) {
free(announce);
return nullptr;
Expand Down
5 changes: 5 additions & 0 deletions toxcore/logger.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,8 @@ void logger_write(const Logger *log, Logger_Level level, const char *file, int l

log->callback(log->context, level, file, line, func, msg, log->userdata);
}

void logger_abort(void)

Check warning on line 121 in toxcore/logger.c

View check run for this annotation

Codecov / codecov/patch

toxcore/logger.c#L121

Added line #L121 was not covered by tests
{
abort();

Check warning on line 123 in toxcore/logger.c

View check run for this annotation

Codecov / codecov/patch

toxcore/logger.c#L123

Added line #L123 was not covered by tests
}
5 changes: 4 additions & 1 deletion toxcore/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ void logger_write(
const Logger *log, Logger_Level level, const char *file, int line, const char *func,
const char *format, ...);

/* @brief Terminate the program with a signal. */
void logger_abort(void);


#define LOGGER_WRITE(log, level, ...) \
do { \
Expand All @@ -85,7 +88,7 @@ void logger_write(
#define LOGGER_FATAL(log, ...) \
do { \
LOGGER_ERROR(log, __VA_ARGS__); \
abort(); \
logger_abort(); \
} while (0)

#define LOGGER_ASSERT(log, cond, ...) \
Expand Down
5 changes: 4 additions & 1 deletion toxcore/mono_time.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <time.h>

#include "ccompat.h"
#include "util.h"

/** don't call into system billions of times for no reason */
struct Mono_Time {
Expand Down Expand Up @@ -165,7 +166,9 @@ Mono_Time *mono_time_new(const Memory *mem, mono_time_current_time_cb *current_t
// Maximum reproducibility. Never return time = 0.
mono_time->base_time = 1;
#else
mono_time->base_time = (uint64_t)time(nullptr) * 1000ULL - current_time_monotonic(mono_time);
// Never return time = 0 in case time() returns 0 (e.g. on microcontrollers
// without battery-powered RTC or ones where NTP didn't initialise it yet).
mono_time->base_time = max_u64(1, (uint64_t)time(nullptr)) * 1000ULL - current_time_monotonic(mono_time);
#endif

mono_time_update(mono_time);
Expand Down
8 changes: 4 additions & 4 deletions toxcore/network.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* SPDX-License-Identifier: GPL-3.0-or-later
* Copyright © 2016-2018 The TokTok team.
* Copyright © 2016-2023 The TokTok team.
* Copyright © 2013 Tox project.
*/

Expand Down Expand Up @@ -778,22 +778,22 @@ static void loglogdata(const Logger *log, const char *message, const uint8_t *bu
Ip_Ntoa ip_str;
const int error = net_error();
char *strerror = net_new_strerror(error);
LOGGER_TRACE(log, "[%02x = %-20s] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
LOGGER_TRACE(log, "[%02x = %-21s] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
buffer[0], net_packet_type_name((Net_Packet_Type)buffer[0]), message,
min_u16(buflen, 999), 'E',
net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), error,
strerror, data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]);
net_kill_strerror(strerror);
} else if ((res > 0) && ((size_t)res <= buflen)) {
Ip_Ntoa ip_str;
LOGGER_TRACE(log, "[%02x = %-20s] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
LOGGER_TRACE(log, "[%02x = %-21s] %s %3u%c %s:%u (%u: %s) | %08x%08x...%02x",
buffer[0], net_packet_type_name((Net_Packet_Type)buffer[0]), message,
min_u16(res, 999), (size_t)res < buflen ? '<' : '=',
net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), 0, "OK",
data_0(buflen, buffer), data_1(buflen, buffer), buffer[buflen - 1]);
} else { /* empty or overwrite */
Ip_Ntoa ip_str;
LOGGER_TRACE(log, "[%02x = %-20s] %s %lu%c%u %s:%u (%u: %s) | %08x%08x...%02x",
LOGGER_TRACE(log, "[%02x = %-21s] %s %lu%c%u %s:%u (%u: %s) | %08x%08x...%02x",

Check warning on line 796 in toxcore/network.c

View check run for this annotation

Codecov / codecov/patch

toxcore/network.c#L796

Added line #L796 was not covered by tests
buffer[0], net_packet_type_name((Net_Packet_Type)buffer[0]), message,
res, res == 0 ? '!' : '>', buflen,
net_ip_ntoa(&ip_port->ip, &ip_str), net_ntohs(ip_port->port), 0, "OK",
Expand Down
Loading

0 comments on commit 7dba867

Please sign in to comment.