From 7c3951a10fc6dbff42412c310ae6f0014a82b131 Mon Sep 17 00:00:00 2001 From: Logan oos Even <46396513+Logan007@users.noreply.github.com> Date: Fri, 26 Mar 2021 20:45:31 +0545 Subject: [PATCH] revised tcp implementation (#674) --- include/n2n.h | 4 ++-- include/n2n_typedefs.h | 3 ++- src/n2n.c | 10 +++++----- src/sn_utils.c | 34 +++++++++++++++++++++++----------- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/include/n2n.h b/include/n2n.h index 8c3e4e5..7a38da3 100644 --- a/include/n2n.h +++ b/include/n2n.h @@ -229,14 +229,14 @@ int time_stamp_verify_and_update (uint64_t stamp, uint64_t * previous_stamp, int /* Operations on peer_info lists. */ size_t purge_peer_list (struct peer_info ** peer_list, SOCKET socket_not_to_close, - n2n_tcp_connection_t *tcp_connections, + n2n_tcp_connection_t **tcp_connections, time_t purge_before); size_t clear_peer_list (struct peer_info ** peer_list); size_t purge_expired_nodes (struct peer_info **peer_list, SOCKET socket_not_to_close, - n2n_tcp_connection_t *tcp_connections, + n2n_tcp_connection_t **tcp_connections, time_t *p_last_purge, int frequency, int timeout); diff --git a/include/n2n_typedefs.h b/include/n2n_typedefs.h index 388dd27..306d338 100644 --- a/include/n2n_typedefs.h +++ b/include/n2n_typedefs.h @@ -711,8 +711,9 @@ typedef struct n2n_tcp_connection { struct sockaddr sock; /* network order socket */ uint16_t expected; /* number of bytes expected to be read */ - uint16_t position; /* current position in the buffer*/ + uint16_t position; /* current position in the buffer */ uint8_t buffer[N2N_PKT_BUF_SIZE + sizeof(uint16_t)]; /* buffer for data collected from tcp socket incl. prepended length */ + uint8_t inactive; /* connection not be handled if set, already closed and to be deleted soon */ UT_hash_handle hh; /* makes this structure hashable */ } n2n_tcp_connection_t; diff --git a/src/n2n.c b/src/n2n.c index aee2280..24eacd7 100644 --- a/src/n2n.c +++ b/src/n2n.c @@ -440,7 +440,7 @@ void print_n2n_version () { size_t purge_expired_nodes (struct peer_info **peer_list, SOCKET socket_not_to_close, - n2n_tcp_connection_t *tcp_connections, + n2n_tcp_connection_t **tcp_connections, time_t *p_last_purge, int frequency, int timeout) { @@ -463,9 +463,9 @@ size_t purge_expired_nodes (struct peer_info **peer_list, /** Purge old items from the peer_list, eventually close the related socket, and * return the number of items that were removed. */ -size_t purge_peer_list (struct peer_info ** peer_list, +size_t purge_peer_list (struct peer_info **peer_list, SOCKET socket_not_to_close, - n2n_tcp_connection_t *tcp_connections, + n2n_tcp_connection_t **tcp_connections, time_t purge_before) { struct peer_info *scan, *tmp; @@ -476,9 +476,9 @@ size_t purge_peer_list (struct peer_info ** peer_list, if((scan->purgeable == SN_PURGEABLE) && (scan->last_seen < purge_before)) { if((scan->socket_fd >=0) && (scan->socket_fd != socket_not_to_close)) { if(tcp_connections) { - HASH_FIND_INT(tcp_connections, &scan->socket_fd, conn); + HASH_FIND_INT(*tcp_connections, &scan->socket_fd, conn); if(conn) { - HASH_DEL(tcp_connections, conn); + HASH_DEL(*tcp_connections, conn); free(conn); } shutdown(scan->socket_fd, SHUT_RDWR); diff --git a/src/sn_utils.c b/src/sn_utils.c index f2a5b32..ef77d47 100644 --- a/src/sn_utils.c +++ b/src/sn_utils.c @@ -91,9 +91,8 @@ static void close_tcp_connection(n2n_sn_t *sss, n2n_tcp_connection_t *conn) { // close the connection shutdown(conn->socket_fd, SHUT_RDWR); closesocket(conn->socket_fd); - // forget about the connection - HASH_DEL(sss->tcp_connections, conn); - free(conn); + // forget about the connection, will be deleted later + conn->inactive = 1; } @@ -862,7 +861,7 @@ static int re_register_and_purge_supernodes (n2n_sn_t *sss, struct sn_community } // purge long-time-not-seen supernodes - purge_expired_nodes(&(comm->edges), sss->sock, sss->tcp_connections, p_last_re_reg_and_purge, + purge_expired_nodes(&(comm->edges), sss->sock, &sss->tcp_connections, p_last_re_reg_and_purge, RE_REG_AND_PURGE_FREQUENCY, LAST_SEEN_SN_INACTIVE); if(comm != NULL) { @@ -943,7 +942,7 @@ static int purge_expired_communities (n2n_sn_t *sss, continue; // purge the community's local peers - num_reg += purge_peer_list(&comm->edges, sss->sock, sss->tcp_connections, now - REGISTRATION_TIMEOUT); + num_reg += purge_peer_list(&comm->edges, sss->sock, &sss->tcp_connections, now - REGISTRATION_TIMEOUT); // purge the community's associated peers (connected to other supernodes) HASH_ITER(hh, comm->assoc, assoc, tmp_assoc) { @@ -2136,12 +2135,16 @@ int run_sn_loop (n2n_sn_t *sss, int *keep_running) { #ifdef N2N_HAVE_TCP // the so far known tcp connections - // do NOT use 'HASH_ITER(hh, sss->tcp_connections, conn, tmp_conn) {' to iterate because - // deletion of OTHER connections (that can happen if forwarding to another edge node fails) - // may result in seg faults if using HASH ITER which is only safe to use if deleting current item - for(conn = sss->tcp_connections; conn != NULL; conn = next) { - // conn might not be available next iteration due to its deletion so we store 'next' upfront - next = (n2n_tcp_connection_t*)(conn->hh.next); + + // beware: current conn and other items of the connection list may be found + // due for deletion while processing packets. Even OTHER connections, e.g. if + // forwarding to another edge node fails. connections due for deletion will + // not immediately be deleted but marked 'inactive' for later deletion + HASH_ITER(hh, sss->tcp_connections, conn, tmp_conn) { + // do not process entries that have been marked inactive, those will be deleted + // immediately after this loop + if(conn->inactive) + continue; if(FD_ISSET(conn->socket_fd, &socket_mask)) { @@ -2191,6 +2194,14 @@ int run_sn_loop (n2n_sn_t *sss, int *keep_running) { } } + // remove inactive / already closed tcp connections from list + HASH_ITER(hh, sss->tcp_connections, conn, tmp_conn) { + if(conn->inactive) { + HASH_DEL(sss->tcp_connections, conn); + free(conn); + } + } + // accept new incoming tcp connection if(FD_ISSET(sss->tcp_sock, &socket_mask)) { struct sockaddr_in sender_sock; @@ -2204,6 +2215,7 @@ int run_sn_loop (n2n_sn_t *sss, int *keep_running) { if(conn) { conn->socket_fd = tmp_sock; memcpy(&(conn->sock), &sender_sock, sizeof(struct sockaddr_in)); + conn->inactive = 0; conn->expected = sizeof(uint16_t); conn->position = 0; HASH_ADD_INT(sss->tcp_connections, socket_fd, conn);