From ec9082f2c382741c07fdbd81ed7362a99c747256 Mon Sep 17 00:00:00 2001 From: pyruvate Date: Thu, 7 Aug 2014 23:59:47 +0300 Subject: [PATCH 1/4] Remove DEFTESTCASE and DEFTESTCASE_SLOW redefinitions --- auto_tests/TCP_test.c | 10 ++-------- auto_tests/assoc_test.c | 12 ++---------- auto_tests/crypto_test.c | 12 ++---------- auto_tests/helpers.h | 15 +++++++++++++++ auto_tests/messenger_test.c | 7 ++----- auto_tests/network_test.c | 7 ++----- auto_tests/onion_test.c | 10 ++-------- auto_tests/skeleton_test.c | 8 ++------ auto_tests/tox_test.c | 10 ++-------- auto_tests/toxav_basic_test.c | 8 +++----- 10 files changed, 34 insertions(+), 65 deletions(-) create mode 100644 auto_tests/helpers.h diff --git a/auto_tests/TCP_test.c b/auto_tests/TCP_test.c index 8e75fae0..25827df3 100644 --- a/auto_tests/TCP_test.c +++ b/auto_tests/TCP_test.c @@ -14,6 +14,8 @@ #include "../toxcore/util.h" +#include "helpers.h" + #if defined(_WIN32) || defined(__WIN32__) || defined (WIN32) #define c_sleep(x) Sleep(1*x) #else @@ -489,14 +491,6 @@ START_TEST(test_client_invalid) } END_TEST -#define DEFTESTCASE(NAME) \ - TCase *tc_##NAME = tcase_create(#NAME); \ - tcase_add_test(tc_##NAME, test_##NAME); \ - suite_add_tcase(s, tc_##NAME); - -#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \ - DEFTESTCASE(NAME) \ - tcase_set_timeout(tc_##NAME, TIMEOUT); Suite *TCP_suite(void) { Suite *s = suite_create("TCP"); diff --git a/auto_tests/assoc_test.c b/auto_tests/assoc_test.c index 5f496ece..b377cadf 100644 --- a/auto_tests/assoc_test.c +++ b/auto_tests/assoc_test.c @@ -1,4 +1,3 @@ - #ifdef HAVE_CONFIG_H #include "config.h" #endif @@ -14,6 +13,8 @@ #include +#include "helpers.h" + START_TEST(test_basics) { /* TODO: real test */ @@ -132,15 +133,6 @@ START_TEST(test_fillup) } END_TEST -#define DEFTESTCASE(NAME) \ - TCase *tc_##NAME = tcase_create(#NAME); \ - tcase_add_test(tc_##NAME, test_##NAME); \ - suite_add_tcase(s, tc_##NAME); - -#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \ - DEFTESTCASE(NAME) \ - tcase_set_timeout(tc_##NAME, TIMEOUT); - Suite *Assoc_suite(void) { Suite *s = suite_create("Assoc"); diff --git a/auto_tests/crypto_test.c b/auto_tests/crypto_test.c index 252f7275..2fb08b8b 100644 --- a/auto_tests/crypto_test.c +++ b/auto_tests/crypto_test.c @@ -10,6 +10,8 @@ #include #include +#include "helpers.h" + void rand_bytes(uint8_t *b, size_t blen) { size_t i; @@ -270,16 +272,6 @@ START_TEST(test_large_data_symmetric) } END_TEST - -#define DEFTESTCASE(NAME) \ - TCase *NAME = tcase_create(#NAME); \ - tcase_add_test(NAME, test_##NAME); \ - suite_add_tcase(s, NAME); - -#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \ - DEFTESTCASE(NAME) \ - tcase_set_timeout(NAME, TIMEOUT); - Suite *crypto_suite(void) { Suite *s = suite_create("Crypto"); diff --git a/auto_tests/helpers.h b/auto_tests/helpers.h new file mode 100644 index 00000000..30fb7c8c --- /dev/null +++ b/auto_tests/helpers.h @@ -0,0 +1,15 @@ +#ifndef TOXCORE_TEST_HELPERS_H +#define TOXCORE_TEST_HELPERS_H + +#include + +#define DEFTESTCASE(NAME) \ + TCase *tc_##NAME = tcase_create(#NAME); \ + tcase_add_test(tc_##NAME, test_##NAME); \ + suite_add_tcase(s, tc_##NAME); + +#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \ + DEFTESTCASE(NAME) \ + tcase_set_timeout(tc_##NAME, TIMEOUT); + +#endif // TOXCORE_TEST_HELPERS_H \ No newline at end of file diff --git a/auto_tests/messenger_test.c b/auto_tests/messenger_test.c index 7ab7d674..3024ed1a 100644 --- a/auto_tests/messenger_test.c +++ b/auto_tests/messenger_test.c @@ -21,6 +21,8 @@ #include #include +#include "helpers.h" + #define REALLY_BIG_NUMBER ((1) << (sizeof(uint16_t) * 7)) #define STRINGS_EQUAL(X, Y) (strcmp(X, Y) == 0) @@ -298,11 +300,6 @@ START_TEST(test_messenger_state_saveloadsave) } END_TEST -#define DEFTESTCASE(NAME) \ - TCase *tc_##NAME = tcase_create(#NAME); \ - tcase_add_test(tc_##NAME, test_##NAME); \ - suite_add_tcase(s, tc_##NAME); - Suite *messenger_suite(void) { Suite *s = suite_create("Messenger"); diff --git a/auto_tests/network_test.c b/auto_tests/network_test.c index afd90e7a..9d07fbb4 100644 --- a/auto_tests/network_test.c +++ b/auto_tests/network_test.c @@ -11,6 +11,8 @@ #include "../toxcore/network.h" +#include "helpers.h" + START_TEST(test_addr_resolv_localhost) { #ifdef __CYGWIN__ @@ -141,11 +143,6 @@ START_TEST(test_struct_sizes) } END_TEST -#define DEFTESTCASE(NAME) \ - TCase *tc_##NAME = tcase_create(#NAME); \ - tcase_add_test(tc_##NAME, test_##NAME); \ - suite_add_tcase(s, tc_##NAME); - Suite *network_suite(void) { Suite *s = suite_create("Network"); diff --git a/auto_tests/onion_test.c b/auto_tests/onion_test.c index 2394e03d..ed297022 100644 --- a/auto_tests/onion_test.c +++ b/auto_tests/onion_test.c @@ -14,6 +14,8 @@ #include "../toxcore/onion_client.h" #include "../toxcore/util.h" +#include "helpers.h" + #if defined(_WIN32) || defined(__WIN32__) || defined (WIN32) #define c_sleep(x) Sleep(1*x) #else @@ -335,14 +337,6 @@ START_TEST(test_announce) } END_TEST -#define DEFTESTCASE(NAME) \ - TCase *tc_##NAME = tcase_create(#NAME); \ - tcase_add_test(tc_##NAME, test_##NAME); \ - suite_add_tcase(s, tc_##NAME); - -#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \ - DEFTESTCASE(NAME) \ - tcase_set_timeout(tc_##NAME, TIMEOUT); Suite *onion_suite(void) { Suite *s = suite_create("Onion"); diff --git a/auto_tests/skeleton_test.c b/auto_tests/skeleton_test.c index 89ef1b8b..27c9123a 100644 --- a/auto_tests/skeleton_test.c +++ b/auto_tests/skeleton_test.c @@ -9,6 +9,8 @@ #include #include +#include "helpers.h" + /* #include "../" */ @@ -20,12 +22,6 @@ START_TEST(test_creativetestnamegoeshere) } END_TEST - -#define DEFTESTCASE(NAME) \ - TCase *tc_##NAME = tcase_create(#NAME); \ - tcase_add_test(tc_##NAME, test_##NAME); \ - suite_add_tcase(s, tc_##NAME); - Suite *creativesuitenamegoeshere_suite(void) { Suite *s = suite_create("creativesuitedescritptiongoeshere"); diff --git a/auto_tests/tox_test.c b/auto_tests/tox_test.c index 43fb7a1c..f765fcd3 100644 --- a/auto_tests/tox_test.c +++ b/auto_tests/tox_test.c @@ -12,6 +12,8 @@ #include "../toxcore/tox.h" +#include "helpers.h" + #if defined(_WIN32) || defined(__WIN32__) || defined (WIN32) #define c_sleep(x) Sleep(1*x) #else @@ -360,14 +362,6 @@ loop_top: } END_TEST -#define DEFTESTCASE(NAME) \ - TCase *tc_##NAME = tcase_create(#NAME); \ - tcase_add_test(tc_##NAME, test_##NAME); \ - suite_add_tcase(s, tc_##NAME); - -#define DEFTESTCASE_SLOW(NAME, TIMEOUT) \ - DEFTESTCASE(NAME) \ - tcase_set_timeout(tc_##NAME, TIMEOUT); Suite *tox_suite(void) { Suite *s = suite_create("Tox"); diff --git a/auto_tests/toxav_basic_test.c b/auto_tests/toxav_basic_test.c index 57685bfc..c6481366 100644 --- a/auto_tests/toxav_basic_test.c +++ b/auto_tests/toxav_basic_test.c @@ -16,6 +16,8 @@ #include "../toxcore/crypto_core.h" #include "../toxav/toxav.h" +#include "helpers.h" + #if defined(_WIN32) || defined(__WIN32__) || defined (WIN32) #define c_sleep(x) Sleep(1*x) #else @@ -214,7 +216,6 @@ case 3: /* Wait for Both to have status ended */\ if (status_control.Alice.status == Ended && status_control.Bob.status == Ended) running = 0; break; } c_sleep(20); } } printf("\n"); START_TEST(test_AV_flows) -// int test_AV_flows() { long long unsigned int cur_time = time(NULL); Tox *bootstrap_node = tox_new(0); @@ -574,10 +575,7 @@ Suite *tox_suite(void) { Suite *s = suite_create("ToxAV"); - TCase *tc_av_flows = tcase_create("AV_flows"); - tcase_add_test(tc_av_flows, test_AV_flows); - tcase_set_timeout(tc_av_flows, 200); - suite_add_tcase(s, tc_av_flows); + DEFTESTCASE_SLOW(AV_flows, 200); return s; } From a460b9fbd0bd262b7e9c6d2b3f8d835a6b973093 Mon Sep 17 00:00:00 2001 From: pyruvate Date: Fri, 8 Aug 2014 15:40:21 +0300 Subject: [PATCH 2/4] Added tests for addto_lists function --- auto_tests/Makefile.inc | 10 +- auto_tests/dht_test.c | 349 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 357 insertions(+), 2 deletions(-) create mode 100644 auto_tests/dht_test.c diff --git a/auto_tests/Makefile.inc b/auto_tests/Makefile.inc index ecf3f31f..c68f313b 100644 --- a/auto_tests/Makefile.inc +++ b/auto_tests/Makefile.inc @@ -1,7 +1,7 @@ if BUILD_TESTS -TESTS = messenger_autotest crypto_test network_test assoc_test onion_test TCP_test tox_test -check_PROGRAMS = messenger_autotest crypto_test network_test assoc_test onion_test TCP_test tox_test +TESTS = messenger_autotest crypto_test network_test assoc_test onion_test TCP_test tox_test dht_autotest +check_PROGRAMS = messenger_autotest crypto_test network_test assoc_test onion_test TCP_test tox_test dht_autotest AUTOTEST_CFLAGS = \ $(LIBSODIUM_CFLAGS) \ @@ -74,6 +74,12 @@ tox_test_CFLAGS = $(AUTOTEST_CFLAGS) tox_test_LDADD = $(AUTOTEST_LDADD) +dht_autotest_SOURCES = ../auto_tests/dht_test.c + +dht_autotest_CFLAGS = $(AUTOTEST_CFLAGS) + +dht_autotest_LDADD = $(AUTOTEST_LDADD) + if BUILD_AV toxav_basic_test_SOURCES = ../auto_tests/toxav_basic_test.c diff --git a/auto_tests/dht_test.c b/auto_tests/dht_test.c new file mode 100644 index 00000000..a5a97233 --- /dev/null +++ b/auto_tests/dht_test.c @@ -0,0 +1,349 @@ +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include + +#include "../toxcore/tox.h" +#include "../toxcore/DHT.c" + +#include "helpers.h" + +#define swap(x,y) do \ + { unsigned char swap_temp[sizeof(x) == sizeof(y) ? (signed)sizeof(x) : -1]; \ + memcpy(swap_temp,&y,sizeof(x)); \ + memcpy(&y,&x, sizeof(x)); \ + memcpy(&x,swap_temp,sizeof(x)); \ + } while(0) + + +void mark_bad(IPPTsPng *ipptp) +{ + ipptp->timestamp = unix_time() - 2 * BAD_NODE_TIMEOUT; + ipptp->hardening.routes_requests_ok = 0; + ipptp->hardening.send_nodes_ok = 0; + ipptp->hardening.testing_requests = 0; +} + +void mark_possible_bad(IPPTsPng *ipptp) +{ + ipptp->timestamp = unix_time(); + ipptp->hardening.routes_requests_ok = 0; + ipptp->hardening.send_nodes_ok = 0; + ipptp->hardening.testing_requests = 0; +} + +void mark_good(IPPTsPng *ipptp) +{ + ipptp->timestamp = unix_time(); + ipptp->hardening.routes_requests_ok = (HARDENING_ALL_OK >> 0) & 1; + ipptp->hardening.send_nodes_ok = (HARDENING_ALL_OK >> 1) & 1; + ipptp->hardening.testing_requests = (HARDENING_ALL_OK >> 2) & 1; +} + +void mark_all_good(Client_data *list, uint32_t length, uint8_t ipv6) +{ + int i; + for (i = 0; i < length; ++i) { + if (ipv6) + mark_good(&list[i].assoc6); + else + mark_good(&list[i].assoc4); + } +} + +/* Returns 1 if client_id has a furthest distance to comp_client_id + than all client_id's in the list */ +uint8_t is_furthest(const uint8_t *comp_client_id, Client_data *list, uint32_t length, const uint8_t *client_id) +{ + int i; + for (i = 0; i < length; ++i) + if (id_closest(comp_client_id, client_id, list[i].client_id) == 1) + return 0; + return 1; +} + +int client_in_list(Client_data *list, uint32_t length, uint8_t *client_id) +{ + int i; + for (i = 0; i < length; ++i) + if (id_equal(client_id, list[i].client_id)) + return i; + return -1; +} + +void test_addto_lists_update(DHT *dht, + Client_data *list, + uint32_t length, + IP_Port *ip_port) +{ + int used, test = rand() % length, test1 = rand() % (length / 2), test2 = rand() % (length / 2) + length / 2; + uint8_t test_id[CLIENT_ID_SIZE]; + IP_Port test_ipp; + uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0; + + ipport_copy(&test_ipp, ipv6 ? &list[test].assoc6.ip_port : &list[test].assoc4.ip_port); + + // check id update for existing ip_port + randombytes(test_id, sizeof(test_id)); + used = addto_lists(dht, test_ipp, test_id); + ck_assert_msg(used >= 1, "Wrong number of added clients"); + ck_assert_msg(client_in_list(list, length, test_id) == test, "Client id is not in the list"); + ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test].assoc6.ip_port : &list[test].assoc4.ip_port), "Client IP_Port is incorrect"); + + // check ip_port update for existing id + test_ipp.port = rand() % TOX_PORT_DEFAULT; + used = addto_lists(dht, test_ipp, test_id); + ck_assert_msg(used >= 1, "Wrong number of added clients"); + ck_assert_msg(client_in_list(list, length, test_id) == test, "Client id is not in the list"); + ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test].assoc6.ip_port : &list[test].assoc4.ip_port), "Client IP_Port is incorrect"); + + // check ip_port update for existing id and ip_port (... port ... id ...) + ipport_copy(&test_ipp, ipv6 ? &list[test1].assoc6.ip_port : &list[test1].assoc4.ip_port); + id_copy(test_id, list[test2].client_id); + if (ipv6) list[test2].assoc6.ip_port.port = -1; else list[test2].assoc4.ip_port.port = -1; + used = addto_lists(dht, test_ipp, test_id); + ck_assert_msg(used >= 1, "Wrong number of added clients"); + ck_assert_msg(client_in_list(list, length, test_id) == test2, "Client id is not in the list"); + ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test2].assoc6.ip_port : &list[test2].assoc4.ip_port), "Client IP_Port is incorrect"); + + // check ip_port update for existing id and ip_port (... id ... port ...) + ipport_copy(&test_ipp, ipv6 ? &list[test2].assoc6.ip_port : &list[test2].assoc4.ip_port); + id_copy(test_id, list[test1].client_id); + if (ipv6) list[test1].assoc6.ip_port.port = -1; else list[test1].assoc4.ip_port.port = -1; + used = addto_lists(dht, test_ipp, test_id); + ck_assert_msg(used >= 1, "Wrong number of added clients"); + ck_assert_msg(client_in_list(list, length, test_id) == test1, "Client id is not in the list"); + ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test1].assoc6.ip_port : &list[test1].assoc4.ip_port), "Client IP_Port is incorrect"); +} + +void test_addto_lists_bad(DHT *dht, + Client_data *list, + uint32_t length, + IP_Port *ip_port) +{ + // check "bad" clients replacement + uint8_t client_id[CLIENT_ID_SIZE], test_id1[CLIENT_ID_SIZE], test_id2[CLIENT_ID_SIZE], test_id3[CLIENT_ID_SIZE]; + int test1, test2, test3; + uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0; + + randombytes(client_id, sizeof(client_id)); + mark_all_good(list, length, ipv6); + + test1 = rand() % (length / 3); + test2 = rand() % (length / 3) + length / 3; + test3 = rand() % (length / 3) + 2 * length / 3; + ck_assert_msg(!(test1 == test2 || test1 == test3 || test2 == test3), "Wrong test indices are chosen"); + + id_copy((uint8_t*)&test_id1, list[test1].client_id); + id_copy((uint8_t*)&test_id2, list[test2].client_id); + id_copy((uint8_t*)&test_id3, list[test3].client_id); + + // mark nodes as "bad" + if (ipv6) { + mark_bad(&list[test1].assoc6); + mark_bad(&list[test2].assoc6); + mark_bad(&list[test3].assoc6); + } else { + mark_bad(&list[test1].assoc4); + mark_bad(&list[test2].assoc4); + mark_bad(&list[test3].assoc4); + } + + ip_port->port += 1; + int used = addto_lists(dht, *ip_port, client_id); + ck_assert_msg(used >= 1, "Wrong number of added clients"); + + ck_assert_msg(client_in_list(list, length, client_id) >= 0, "Client id is not in the list"); + ck_assert_msg(id_equal(client_id, list[test1].client_id), "Wrong bad client removed"); + ck_assert_msg(id_equal(test_id2, list[test2].client_id), "Wrong bad client removed"); + ck_assert_msg(id_equal(test_id3, list[test3].client_id), "Wrong bad client removed"); +} + +void test_addto_lists_possible_bad(DHT *dht, + Client_data *list, + uint32_t length, + IP_Port *ip_port, + const uint8_t *comp_client_id) +{ + // check "possibly bad" clients replacement + uint8_t client_id[CLIENT_ID_SIZE], test_id1[CLIENT_ID_SIZE], test_id2[CLIENT_ID_SIZE], test_id3[CLIENT_ID_SIZE]; + int test1, test2, test3; + uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0; + + randombytes(client_id, sizeof(client_id)); + mark_all_good(list, length, ipv6); + + test1 = rand() % (length / 3); + test2 = rand() % (length / 3) + length / 3; + test3 = rand() % (length / 3) + 2 * length / 3; + ck_assert_msg(!(test1 == test2 || test1 == test3 || test2 == test3), "Wrong test indices are chosen"); + + id_copy((uint8_t*)&test_id1, list[test1].client_id); + id_copy((uint8_t*)&test_id2, list[test2].client_id); + id_copy((uint8_t*)&test_id3, list[test3].client_id); + + // mark nodes as "possibly bad" + if (ipv6) { + mark_possible_bad(&list[test1].assoc6); + mark_possible_bad(&list[test2].assoc6); + mark_possible_bad(&list[test3].assoc6); + } else { + mark_possible_bad(&list[test1].assoc4); + mark_possible_bad(&list[test2].assoc4); + mark_possible_bad(&list[test3].assoc4); + } + + ip_port->port += 1; + int used = addto_lists(dht, *ip_port, client_id); + ck_assert_msg(used >= 1, "Wrong number of added clients"); + + ck_assert_msg(client_in_list(list, length, client_id) >= 0, "Client id is not in the list"); + + int inlist_id1 = client_in_list(list, length, test_id1) >= 0; + int inlist_id2 = client_in_list(list, length, test_id2) >= 0; + int inlist_id3 = client_in_list(list, length, test_id3) >= 0; + + ck_assert_msg(inlist_id1 + inlist_id2 + inlist_id3 == 2, "Wrong client removed"); + + if (!inlist_id1) { + ck_assert_msg(id_closest(comp_client_id, test_id2, test_id1) == 1, "Id has been removed but is closer to than another one"); + ck_assert_msg(id_closest(comp_client_id, test_id3, test_id1) == 1, "Id has been removed but is closer to than another one"); + } else if (!inlist_id2) { + ck_assert_msg(id_closest(comp_client_id, test_id1, test_id2) == 1, "Id has been removed but is closer to than another one"); + ck_assert_msg(id_closest(comp_client_id, test_id3, test_id2) == 1, "Id has been removed but is closer to than another one"); + } else if (!inlist_id3) { + ck_assert_msg(id_closest(comp_client_id, test_id1, test_id3) == 1, "Id has been removed but is closer to than another one"); + ck_assert_msg(id_closest(comp_client_id, test_id2, test_id3) == 1, "Id has been removed but is closer to than another one"); + } +} + +void test_addto_lists_good(DHT *dht, + Client_data *list, + uint32_t length, + IP_Port *ip_port, + const uint8_t *comp_client_id) +{ + uint8_t client_id[CLIENT_ID_SIZE]; + uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0; + + mark_all_good(list, length, ipv6); + + // check "good" client id replacement + do { + randombytes(client_id, sizeof(client_id)); + } while (is_furthest(comp_client_id, list, length, client_id)); + ip_port->port += 1; + addto_lists(dht, *ip_port, client_id); + ck_assert_msg(client_in_list(list, length, client_id) >= 0, "Good client id is not in the list"); + + // check "good" client id skip + do { + randombytes(client_id, sizeof(client_id)); + } while (!is_furthest(comp_client_id, list, length, client_id)); + + ip_port->port += 1; + addto_lists(dht, *ip_port, client_id); + ck_assert_msg(client_in_list(list, length, client_id) == -1, "Good client id is in the list"); +} + +void test_addto_lists(IP ip) +{ + Networking_Core* net = new_networking(ip, TOX_PORT_DEFAULT); + ck_assert_msg(net != 0, "Failed to create Networking_Core"); + + DHT* dht = new_DHT(net); + ck_assert_msg(dht != 0, "Failed to create DHT"); + + IP_Port ip_port = { .ip = ip, .port = TOX_PORT_DEFAULT }; + uint8_t client_id[CLIENT_ID_SIZE]; + int i; + + // check lists filling + for (i = 0; i < MAX(LCLIENT_LIST, MAX_FRIEND_CLIENTS); ++i) { + randombytes(client_id, sizeof(client_id)); + int used = addto_lists(dht, ip_port, client_id); + ck_assert_msg(used == dht->num_friends + 1, "Wrong number of added clients with existing ip_port"); + } + + for (i = 0; i < MAX(LCLIENT_LIST, MAX_FRIEND_CLIENTS); ++i) { + ip_port.port += 1; + int used = addto_lists(dht, ip_port, client_id); + ck_assert_msg(used == dht->num_friends + 1, "Wrong number of added clients with existing client_id"); + } + + for (i = 0; i < MAX(LCLIENT_LIST, MAX_FRIEND_CLIENTS); ++i) { + ip_port.port += 1; + randombytes(client_id, sizeof(client_id)); + int used = addto_lists(dht, ip_port, client_id); + ck_assert_msg(used >= 1, "Wrong number of added clients"); + } + + /*check: Current behavior if there are two clients with the same id is + * to replace the first ip by the second. */ + test_addto_lists_update(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port); + for (i = 0; i < dht->num_friends; ++i) + test_addto_lists_update(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port); + + // check "bad" entries + test_addto_lists_bad(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port); + for (i = 0; i < dht->num_friends; ++i) + test_addto_lists_bad(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port); + + // check "possibly bad" entries + test_addto_lists_possible_bad(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port, dht->self_public_key); + // for (i = 0; i < dht->num_friends; ++i) + // test_addto_lists_possible_bad(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port, dht->friends_list[i].client_id); + + // check "good" entries + test_addto_lists_good(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port, dht->self_public_key); + for (i = 0; i < dht->num_friends; ++i) + test_addto_lists_good(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port, dht->friends_list[i].client_id); + + kill_DHT(dht); + kill_networking(net); +} + +START_TEST(test_addto_lists_ipv4) +{ + IP ip; + ip_init(&ip, 0); + test_addto_lists(ip); + +} +END_TEST + +START_TEST(test_addto_lists_ipv6) +{ + IP ip; + ip_init(&ip, 1); + test_addto_lists(ip); + +} +END_TEST + +Suite *dht_suite(void) +{ + Suite *s = suite_create("DHT"); + + DEFTESTCASE(addto_lists_ipv4); + DEFTESTCASE(addto_lists_ipv6); + return s; +} + +int main(int argc, char *argv[]) +{ + srand((unsigned int) time(NULL)); + + Suite *dht = dht_suite(); + SRunner *test_runner = srunner_create(dht); + + int number_failed = 0; + srunner_run_all(test_runner, CK_NORMAL); + number_failed = srunner_ntests_failed(test_runner); + + srunner_free(test_runner); + + return number_failed; +} From bdf1c972735f4a6518f24d3770729fab142020fe Mon Sep 17 00:00:00 2001 From: pyruvate Date: Sat, 9 Aug 2014 10:50:43 +0300 Subject: [PATCH 3/4] Refactoring of node replacements in addto_lists function An index for replacement candidate is searched in one lookup cycle for all types (bad, possibly bad, good). Sorting of items has been removed (sorting logic can be substituted by a maximum search). --- auto_tests/dht_test.c | 4 +- toxcore/DHT.c | 251 ++++++++---------------------------------- 2 files changed, 50 insertions(+), 205 deletions(-) diff --git a/auto_tests/dht_test.c b/auto_tests/dht_test.c index a5a97233..091b50fb 100644 --- a/auto_tests/dht_test.c +++ b/auto_tests/dht_test.c @@ -293,8 +293,8 @@ void test_addto_lists(IP ip) // check "possibly bad" entries test_addto_lists_possible_bad(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port, dht->self_public_key); - // for (i = 0; i < dht->num_friends; ++i) - // test_addto_lists_possible_bad(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port, dht->friends_list[i].client_id); + for (i = 0; i < dht->num_friends; ++i) + test_addto_lists_possible_bad(dht, dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, &ip_port, dht->friends_list[i].client_id); // check "good" entries test_addto_lists_good(dht, dht->close_clientlist, LCLIENT_LIST, &ip_port, dht->self_public_key); diff --git a/toxcore/DHT.c b/toxcore/DHT.c index 77d126c1..20f4fb82 100644 --- a/toxcore/DHT.c +++ b/toxcore/DHT.c @@ -65,17 +65,6 @@ /* Number of get node requests to send to quickly find close nodes. */ #define MAX_BOOTSTRAP_TIMES 10 -/* Used in the comparison function for sorting lists of Client_data. */ -typedef struct { - Client_data c1; - Client_data c2; -} ClientPair; - -/* Create the declaration for a quick sort for ClientPair structures. */ -declare_quick_sort(ClientPair); -/* Create the quicksort function. See misc_tools.h for the definition. */ -make_quick_sort(ClientPair); - Client_data *DHT_get_close_list(DHT *dht) { return dht->close_clientlist; @@ -107,19 +96,6 @@ int id_closest(const uint8_t *id, const uint8_t *id1, const uint8_t *id2) return 0; } -/* Turns the result of id_closest into something quick_sort can use. - * Assumes p1->c1 == p2->c1. - */ -static int client_id_cmp(const ClientPair p1, const ClientPair p2) -{ - int c = id_closest(p1.c1.client_id, p1.c2.client_id, p2.c2.client_id); - - if (c == 2) - return -1; - - return c; -} - /* Shared key generations are costly, it is therefor smart to store commonly used * ones so that they can re used later without being computed again. * @@ -629,142 +605,20 @@ int get_close_nodes(const DHT *dht, const uint8_t *client_id, Node_format *nodes #endif } -/* Replace first bad (or empty) node with this one. +/* Replace a first bad (or empty) node with this one + * or replace a possibly bad node (tests failed or not done yet) + * that is further than any other in the list + * from the comp_client_id + * or replace a good node that is further + * than any other in the list from the comp_client_id + * and further than client_id. * - * return 0 if successful. - * return 1 if not (list contains no bad nodes). - */ -static int replace_bad( Client_data *list, - uint32_t length, - const uint8_t *client_id, - IP_Port ip_port ) -{ - if ((ip_port.ip.family != AF_INET) && (ip_port.ip.family != AF_INET6)) - return 1; - - uint32_t i; - - for (i = 0; i < length; ++i) { - /* If node is bad */ - Client_data *client = &list[i]; - - if (is_timeout(client->assoc4.timestamp, BAD_NODE_TIMEOUT) && - is_timeout(client->assoc6.timestamp, BAD_NODE_TIMEOUT)) { - - IPPTsPng *ipptp_write = NULL; - IPPTsPng *ipptp_clear = NULL; - - if (ip_port.ip.family == AF_INET) { - ipptp_write = &client->assoc4; - ipptp_clear = &client->assoc6; - } else { - ipptp_write = &client->assoc6; - ipptp_clear = &client->assoc4; - } - - memcpy(client->client_id, client_id, CLIENT_ID_SIZE); - ipptp_write->ip_port = ip_port; - ipptp_write->timestamp = unix_time(); - - ip_reset(&ipptp_write->ret_ip_port.ip); - ipptp_write->ret_ip_port.port = 0; - ipptp_write->ret_timestamp = 0; - - /* zero out other address */ - memset(ipptp_clear, 0, sizeof(*ipptp_clear)); - - return 0; - } - } - - return 1; -} - - -/* Sort the list. It will be sorted from furthest to closest. - * Turns list into data that quick sort can use and reverts it back. - */ -static void sort_list(Client_data *list, uint32_t length, const uint8_t *comp_client_id) -{ - Client_data cd; - ClientPair pairs[length]; - uint32_t i; - - memcpy(cd.client_id, comp_client_id, CLIENT_ID_SIZE); - - for (i = 0; i < length; ++i) { - pairs[i].c1 = cd; - pairs[i].c2 = list[i]; - } - - ClientPair_quick_sort(pairs, length, client_id_cmp); - - for (i = 0; i < length; ++i) - list[i] = pairs[i].c2; -} - -/* Replace first node that is possibly bad (tests failed or not done yet.) with this one. + * Do not replace any node if the list has no bad or possibly bad nodes + * and all nodes in the list are closer to comp_client_id + * than client_id. * - * return 0 if successful. - * return 1 if not (list contains no bad nodes). - */ -static int replace_possible_bad( Client_data *list, - uint32_t length, - const uint8_t *client_id, - IP_Port ip_port, - const uint8_t *comp_client_id ) -{ - if ((ip_port.ip.family != AF_INET) && (ip_port.ip.family != AF_INET6)) - return 1; - - sort_list(list, length, comp_client_id); - - /* TODO: decide if the following lines should stay commented or not. - if (id_closest(comp_client_id, list[0].client_id, client_id) == 1) - return 0;*/ - - uint32_t i; - - for (i = 0; i < length; ++i) { - /* If node is bad */ - Client_data *client = &list[i]; - - if (hardening_correct(&client->assoc4.hardening) != HARDENING_ALL_OK && - hardening_correct(&client->assoc6.hardening) != HARDENING_ALL_OK) { - - IPPTsPng *ipptp_write = NULL; - IPPTsPng *ipptp_clear = NULL; - - if (ip_port.ip.family == AF_INET) { - ipptp_write = &client->assoc4; - ipptp_clear = &client->assoc6; - } else { - ipptp_write = &client->assoc6; - ipptp_clear = &client->assoc4; - } - - memcpy(client->client_id, client_id, CLIENT_ID_SIZE); - ipptp_write->ip_port = ip_port; - ipptp_write->timestamp = unix_time(); - - ip_reset(&ipptp_write->ret_ip_port.ip); - ipptp_write->ret_ip_port.port = 0; - ipptp_write->ret_timestamp = 0; - - /* zero out other address */ - memset(ipptp_clear, 0, sizeof(*ipptp_clear)); - - return 0; - } - } - - return 1; -} - -/* Replace the first good node that is further to the comp_client_id than that of the client_id in the list - * - * returns 0 when the item was stored, 1 otherwise */ -static int replace_good( Client_data *list, + * returns True(1) when the item was stored, False(0) otherwise */ +static int replace_all( Client_data *list, uint32_t length, const uint8_t *client_id, IP_Port ip_port, @@ -773,28 +627,39 @@ static int replace_good( Client_data *list, if ((ip_port.ip.family != AF_INET) && (ip_port.ip.family != AF_INET6)) return 1; - /* TODO: eventually remove this.*/ - if (length != LCLIENT_LIST) - sort_list(list, length, comp_client_id); + uint32_t i, replace = ~0, bad = ~0, possibly_bad = ~0, good = ~0; - int8_t replace = -1; + for (i = 0; i < length; ++i) { - /* Because the list is sorted, we can simply check the client_id at the - * border, either it is closer, then every other one is as well, or it is - * further, then it gets pushed out in favor of the new address, which - * will with the next sort() move to its "rightful" position - * - * CAVEAT: weirdly enough, the list is sorted DESCENDING in distance - * so the furthest element is the first, NOT the last (at least that's - * what the comment above sort_list() claims) - */ - if (id_closest(comp_client_id, list[0].client_id, client_id) == 2) - replace = 0; + Client_data *client = &list[i]; - if (replace != -1) { -#ifdef DEBUG - assert(replace >= 0 && replace < length); -#endif + if (is_timeout(client->assoc4.timestamp, BAD_NODE_TIMEOUT) && + is_timeout(client->assoc6.timestamp, BAD_NODE_TIMEOUT)) { + // "bad" node + bad = i; + break; + } else if (hardening_correct(&client->assoc4.hardening) != HARDENING_ALL_OK && + hardening_correct(&client->assoc6.hardening) != HARDENING_ALL_OK) { + // "possibly bad" node + if (possibly_bad == ~0 || + id_closest(comp_client_id, list[possibly_bad].client_id, list[i].client_id) == 1) + possibly_bad = i; + } else { + // "good" node + if (good == ~0 || + id_closest(comp_client_id, list[good].client_id, list[i].client_id) == 1) + good = i; + } + } + + if (bad != ~0) + replace = bad; + else if (possibly_bad != ~0) + replace = possibly_bad; + else if (good != ~0 && id_closest(comp_client_id, list[good].client_id, client_id) == 2) + replace = good; + + if (replace != ~0) { Client_data *client = &list[replace]; IPPTsPng *ipptp_write = NULL; IPPTsPng *ipptp_clear = NULL; @@ -807,7 +672,7 @@ static int replace_good( Client_data *list, ipptp_clear = &client->assoc4; } - memcpy(client->client_id, client_id, CLIENT_ID_SIZE); + id_copy(client->client_id, client_id); ipptp_write->ip_port = ip_port; ipptp_write->timestamp = unix_time(); @@ -818,10 +683,10 @@ static int replace_good( Client_data *list, /* zero out other address */ memset(ipptp_clear, 0, sizeof(*ipptp_clear)); - return 0; + return 1; } - return 1; + return 0; } /* Attempt to add client with ip_port and client_id to the friends client list @@ -843,16 +708,7 @@ int addto_lists(DHT *dht, IP_Port ip_port, const uint8_t *client_id) * to replace the first ip by the second. */ if (!client_or_ip_port_in_list(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port)) { - if (replace_bad(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port)) { - if (replace_possible_bad(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port, - dht->self_public_key)) { - /* If we can't replace bad nodes we try replacing good ones. */ - if (!replace_good(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port, - dht->self_public_key)) - used++; - } else - used++; - } else + if (replace_all(dht->close_clientlist, LCLIENT_LIST, client_id, ip_port, dht->self_public_key)) used++; } else used++; @@ -860,19 +716,8 @@ int addto_lists(DHT *dht, IP_Port ip_port, const uint8_t *client_id) for (i = 0; i < dht->num_friends; ++i) { if (!client_or_ip_port_in_list(dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, client_id, ip_port)) { - - if (replace_bad(dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, - client_id, ip_port)) { - /*if (replace_possible_bad(dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, - client_id, ip_port, dht->friends_list[i].client_id)) {*/ - /* If we can't replace bad nodes we try replacing good ones. */ - if (!replace_good(dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, - client_id, ip_port, dht->friends_list[i].client_id)) - used++; - - /*} else - used++;*/ - } else + if (replace_all(dht->friends_list[i].client_list, MAX_FRIEND_CLIENTS, + client_id, ip_port, dht->friends_list[i].client_id)) used++; } else used++; From 354f08ec3b3bb8bfe86ff2183d01f13f51ba1bbd Mon Sep 17 00:00:00 2001 From: pyruvate Date: Sat, 9 Aug 2014 13:23:59 +0300 Subject: [PATCH 4/4] Tests fix for an original behavior for duplicates --- auto_tests/dht_test.c | 47 +++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/auto_tests/dht_test.c b/auto_tests/dht_test.c index 091b50fb..6d7e70ad 100644 --- a/auto_tests/dht_test.c +++ b/auto_tests/dht_test.c @@ -44,7 +44,7 @@ void mark_good(IPPTsPng *ipptp) void mark_all_good(Client_data *list, uint32_t length, uint8_t ipv6) { - int i; + uint32_t i; for (i = 0; i < length; ++i) { if (ipv6) mark_good(&list[i].assoc6); @@ -57,17 +57,17 @@ void mark_all_good(Client_data *list, uint32_t length, uint8_t ipv6) than all client_id's in the list */ uint8_t is_furthest(const uint8_t *comp_client_id, Client_data *list, uint32_t length, const uint8_t *client_id) { - int i; + uint32_t i; for (i = 0; i < length; ++i) if (id_closest(comp_client_id, client_id, list[i].client_id) == 1) return 0; return 1; } -int client_in_list(Client_data *list, uint32_t length, uint8_t *client_id) +int client_in_list(Client_data *list, uint32_t length, const uint8_t *client_id) { int i; - for (i = 0; i < length; ++i) + for (i = 0; i < (int)length; ++i) if (id_equal(client_id, list[i].client_id)) return i; return -1; @@ -78,28 +78,38 @@ void test_addto_lists_update(DHT *dht, uint32_t length, IP_Port *ip_port) { - int used, test = rand() % length, test1 = rand() % (length / 2), test2 = rand() % (length / 2) + length / 2; - uint8_t test_id[CLIENT_ID_SIZE]; + int used, test, test1, test2, found; IP_Port test_ipp; + uint8_t test_id[CLIENT_ID_SIZE]; uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0; + // check id update for existing ip_port + test = rand() % length; ipport_copy(&test_ipp, ipv6 ? &list[test].assoc6.ip_port : &list[test].assoc4.ip_port); - // check id update for existing ip_port randombytes(test_id, sizeof(test_id)); used = addto_lists(dht, test_ipp, test_id); ck_assert_msg(used >= 1, "Wrong number of added clients"); - ck_assert_msg(client_in_list(list, length, test_id) == test, "Client id is not in the list"); - ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test].assoc6.ip_port : &list[test].assoc4.ip_port), "Client IP_Port is incorrect"); + // it is possible to have ip_port duplicates in the list, so ip_port @ found not always equal to ip_port @ test + found = client_in_list(list, length, test_id); + ck_assert_msg(found >= 0, "Client id is not in the list"); + ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[found].assoc6.ip_port : &list[found].assoc4.ip_port), "Client IP_Port is incorrect"); // check ip_port update for existing id + test = rand() % length; test_ipp.port = rand() % TOX_PORT_DEFAULT; + id_copy(test_id, list[test].client_id); + used = addto_lists(dht, test_ipp, test_id); ck_assert_msg(used >= 1, "Wrong number of added clients"); + // it is not possible to have id duplicates in the list, so id @ found must be equal id @ test ck_assert_msg(client_in_list(list, length, test_id) == test, "Client id is not in the list"); ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test].assoc6.ip_port : &list[test].assoc4.ip_port), "Client IP_Port is incorrect"); // check ip_port update for existing id and ip_port (... port ... id ...) + test1 = rand() % (length / 2); + test2 = rand() % (length / 2) + length / 2; + ipport_copy(&test_ipp, ipv6 ? &list[test1].assoc6.ip_port : &list[test1].assoc4.ip_port); id_copy(test_id, list[test2].client_id); if (ipv6) list[test2].assoc6.ip_port.port = -1; else list[test2].assoc4.ip_port.port = -1; @@ -109,6 +119,9 @@ void test_addto_lists_update(DHT *dht, ck_assert_msg(ipport_equal(&test_ipp, ipv6 ? &list[test2].assoc6.ip_port : &list[test2].assoc4.ip_port), "Client IP_Port is incorrect"); // check ip_port update for existing id and ip_port (... id ... port ...) + test1 = rand() % (length / 2); + test2 = rand() % (length / 2) + length / 2; + ipport_copy(&test_ipp, ipv6 ? &list[test2].assoc6.ip_port : &list[test2].assoc4.ip_port); id_copy(test_id, list[test1].client_id); if (ipv6) list[test1].assoc6.ip_port.port = -1; else list[test1].assoc4.ip_port.port = -1; @@ -124,8 +137,8 @@ void test_addto_lists_bad(DHT *dht, IP_Port *ip_port) { // check "bad" clients replacement + int used, test1, test2, test3; uint8_t client_id[CLIENT_ID_SIZE], test_id1[CLIENT_ID_SIZE], test_id2[CLIENT_ID_SIZE], test_id3[CLIENT_ID_SIZE]; - int test1, test2, test3; uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0; randombytes(client_id, sizeof(client_id)); @@ -152,7 +165,7 @@ void test_addto_lists_bad(DHT *dht, } ip_port->port += 1; - int used = addto_lists(dht, *ip_port, client_id); + used = addto_lists(dht, *ip_port, client_id); ck_assert_msg(used >= 1, "Wrong number of added clients"); ck_assert_msg(client_in_list(list, length, client_id) >= 0, "Client id is not in the list"); @@ -168,8 +181,8 @@ void test_addto_lists_possible_bad(DHT *dht, const uint8_t *comp_client_id) { // check "possibly bad" clients replacement + int used, test1, test2, test3; uint8_t client_id[CLIENT_ID_SIZE], test_id1[CLIENT_ID_SIZE], test_id2[CLIENT_ID_SIZE], test_id3[CLIENT_ID_SIZE]; - int test1, test2, test3; uint8_t ipv6 = ip_port->ip.family == AF_INET6 ? 1 : 0; randombytes(client_id, sizeof(client_id)); @@ -196,7 +209,7 @@ void test_addto_lists_possible_bad(DHT *dht, } ip_port->port += 1; - int used = addto_lists(dht, *ip_port, client_id); + used = addto_lists(dht, *ip_port, client_id); ck_assert_msg(used >= 1, "Wrong number of added clients"); ck_assert_msg(client_in_list(list, length, client_id) >= 0, "Client id is not in the list"); @@ -258,25 +271,25 @@ void test_addto_lists(IP ip) IP_Port ip_port = { .ip = ip, .port = TOX_PORT_DEFAULT }; uint8_t client_id[CLIENT_ID_SIZE]; - int i; + int i, used; // check lists filling for (i = 0; i < MAX(LCLIENT_LIST, MAX_FRIEND_CLIENTS); ++i) { randombytes(client_id, sizeof(client_id)); - int used = addto_lists(dht, ip_port, client_id); + used = addto_lists(dht, ip_port, client_id); ck_assert_msg(used == dht->num_friends + 1, "Wrong number of added clients with existing ip_port"); } for (i = 0; i < MAX(LCLIENT_LIST, MAX_FRIEND_CLIENTS); ++i) { ip_port.port += 1; - int used = addto_lists(dht, ip_port, client_id); + used = addto_lists(dht, ip_port, client_id); ck_assert_msg(used == dht->num_friends + 1, "Wrong number of added clients with existing client_id"); } for (i = 0; i < MAX(LCLIENT_LIST, MAX_FRIEND_CLIENTS); ++i) { ip_port.port += 1; randombytes(client_id, sizeof(client_id)); - int used = addto_lists(dht, ip_port, client_id); + used = addto_lists(dht, ip_port, client_id); ck_assert_msg(used >= 1, "Wrong number of added clients"); }