From 4071d74cc9b8a4d01e6be01d6f5c413724ca1aa6 Mon Sep 17 00:00:00 2001 From: Green Sky Date: Sat, 15 Mar 2025 22:33:39 +0100 Subject: [PATCH] fix(ngc): dont double every message, if we are not directly connected but we and the other peer would support direct. --- toxcore/group_chats.c | 8 ++++---- toxcore/group_common.h | 1 + toxcore/group_connection.c | 20 ++++++++++++++++---- toxcore/group_connection.h | 8 ++++++-- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/toxcore/group_chats.c b/toxcore/group_chats.c index 22f60bf3a3..43ed7e8272 100644 --- a/toxcore/group_chats.c +++ b/toxcore/group_chats.c @@ -1626,7 +1626,7 @@ int group_packet_wrap( * Returns true on success. */ non_null() -static bool send_lossy_group_packet(const GC_Chat *chat, const GC_Connection *gconn, const uint8_t *data, +static bool send_lossy_group_packet(const GC_Chat *chat, GC_Connection *gconn, const uint8_t *data, uint16_t length, uint8_t packet_type) { assert(length <= MAX_GC_CUSTOM_LOSSY_PACKET_SIZE); @@ -2236,7 +2236,7 @@ static int handle_gc_invite_response_reject(const GC_Session *c, GC_Chat *chat, * Return true on success. */ non_null() -static bool send_gc_invite_response_reject(const GC_Chat *chat, const GC_Connection *gconn, uint8_t type) +static bool send_gc_invite_response_reject(const GC_Chat *chat, GC_Connection *gconn, uint8_t type) { if (type >= GJ_INVALID) { type = GJ_INVITE_FAILED; @@ -2353,7 +2353,7 @@ static bool send_gc_lossy_packet_all_peers(const GC_Chat *chat, const uint8_t *d uint32_t confirmed_peers = 0; for (uint32_t i = 1; i < chat->numpeers; ++i) { - const GC_Connection *gconn = get_gc_connection(chat, i); + GC_Connection *gconn = get_gc_connection(chat, i); assert(gconn != nullptr); @@ -7072,7 +7072,7 @@ static void do_peer_delete(const GC_Session *c, GC_Chat *chat, void *userdata) * Return true on success. */ non_null() -static bool ping_peer(const GC_Chat *chat, const GC_Connection *gconn) +static bool ping_peer(const GC_Chat *chat, GC_Connection *gconn) { const uint16_t buf_size = GC_PING_PACKET_MIN_DATA_SIZE + sizeof(IP_Port); uint8_t *data = (uint8_t *)mem_balloc(chat->mem, buf_size); diff --git a/toxcore/group_common.h b/toxcore/group_common.h index 9be3377be0..19217706a7 100644 --- a/toxcore/group_common.h +++ b/toxcore/group_common.h @@ -115,6 +115,7 @@ typedef struct GC_Connection { uint64_t last_sent_tcp_relays_time; /* the last time we attempted to send this peer our tcp relays */ uint16_t tcp_relay_share_index; uint64_t last_received_direct_time; /* the last time we received a direct UDP packet from this connection */ + uint64_t last_sent_direct_try_time; /* the last time we tried sending a direct UDP packet */ uint64_t last_sent_ip_time; /* the last time we sent our ip info to this peer in a ping packet */ Node_format connected_tcp_relays[MAX_FRIEND_TCP_CONNECTIONS]; diff --git a/toxcore/group_connection.c b/toxcore/group_connection.c index 79c982959f..dd6d21fd7c 100644 --- a/toxcore/group_connection.c +++ b/toxcore/group_connection.c @@ -29,6 +29,9 @@ /** Seconds since last direct UDP packet was received before the connection is considered dead */ #define GCC_UDP_DIRECT_TIMEOUT (GC_PING_TIMEOUT + 4) +/** Seconds since last direct UDP packet was sent before we can try again. Cheap NAT hole punch */ +#define GCC_UDP_DIRECT_RETRY 1 + /** Returns true if array entry does not contain an active packet. */ non_null() static bool array_entry_is_empty(const GC_Message_Array_Entry *array_entry) @@ -595,7 +598,7 @@ void gcc_resend_packets(const GC_Chat *chat, GC_Connection *gconn) } } -bool gcc_send_packet(const GC_Chat *chat, const GC_Connection *gconn, const uint8_t *packet, uint16_t length) +bool gcc_send_packet(const GC_Chat *chat, GC_Connection *gconn, const uint8_t *packet, uint16_t length) { if (packet == nullptr || length == 0) { return false; @@ -608,8 +611,12 @@ bool gcc_send_packet(const GC_Chat *chat, const GC_Connection *gconn, const uint return (uint16_t) sendpacket(chat->net, &gconn->addr.ip_port, packet, length) == length; } - if ((uint16_t) sendpacket(chat->net, &gconn->addr.ip_port, packet, length) == length) { - direct_send_attempt = true; + if (gcc_conn_should_try_direct(chat->mono_time, gconn)) { + gconn->last_sent_direct_try_time = mono_time_get(chat->mono_time); + + if ((uint16_t) sendpacket(chat->net, &gconn->addr.ip_port, packet, length) == length) { + direct_send_attempt = true; + } } } @@ -617,7 +624,7 @@ bool gcc_send_packet(const GC_Chat *chat, const GC_Connection *gconn, const uint return ret == 0 || direct_send_attempt; } -int gcc_encrypt_and_send_lossless_packet(const GC_Chat *chat, const GC_Connection *gconn, const uint8_t *data, +int gcc_encrypt_and_send_lossless_packet(const GC_Chat *chat, GC_Connection *gconn, const uint8_t *data, uint16_t length, uint64_t message_id, uint8_t packet_type) { const uint16_t packet_size = gc_get_wrapped_packet_size(length, NET_PACKET_GC_LOSSLESS); @@ -659,6 +666,11 @@ bool gcc_conn_is_direct(const Mono_Time *mono_time, const GC_Connection *gconn) return GCC_UDP_DIRECT_TIMEOUT + gconn->last_received_direct_time > mono_time_get(mono_time); } +bool gcc_conn_should_try_direct(const Mono_Time *mono_time, const GC_Connection *gconn) +{ + return mono_time_is_timeout(mono_time, gconn->last_sent_direct_try_time, GCC_UDP_DIRECT_RETRY); +} + bool gcc_direct_conn_is_possible(const GC_Chat *chat, const GC_Connection *gconn) { return !net_family_is_unspec(gconn->addr.ip_port.ip.family) && !net_family_is_unspec(net_family(chat->net)); diff --git a/toxcore/group_connection.h b/toxcore/group_connection.h index e165ac577c..7122f7a12d 100644 --- a/toxcore/group_connection.h +++ b/toxcore/group_connection.h @@ -135,6 +135,10 @@ void gcc_make_session_shared_key(GC_Connection *gconn, const uint8_t *sender_pk) non_null() bool gcc_conn_is_direct(const Mono_Time *mono_time, const GC_Connection *gconn); +/** @brief Return true if we can try a direct connection with `gconn` again. */ +non_null() +bool gcc_conn_should_try_direct(const Mono_Time *mono_time, const GC_Connection *gconn); + /** @brief Return true if a direct UDP connection is possible with `gconn`. */ non_null() bool gcc_direct_conn_is_possible(const GC_Chat *chat, const GC_Connection *gconn); @@ -146,7 +150,7 @@ bool gcc_direct_conn_is_possible(const GC_Chat *chat, const GC_Connection *gconn * Return true on success. */ non_null() -bool gcc_send_packet(const GC_Chat *chat, const GC_Connection *gconn, const uint8_t *packet, uint16_t length); +bool gcc_send_packet(const GC_Chat *chat, GC_Connection *gconn, const uint8_t *packet, uint16_t length); /** @brief Sends a lossless packet to `gconn` comprised of `data` of size `length`. * @@ -184,7 +188,7 @@ bool gcc_send_lossless_packet_fragments(const GC_Chat *chat, GC_Connection *gcon * Return -2 if the packet fails to send. */ non_null(1, 2) nullable(3) -int gcc_encrypt_and_send_lossless_packet(const GC_Chat *chat, const GC_Connection *gconn, const uint8_t *data, +int gcc_encrypt_and_send_lossless_packet(const GC_Chat *chat, GC_Connection *gconn, const uint8_t *data, uint16_t length, uint64_t message_id, uint8_t packet_type); /** @brief Called when a peer leaves the group. */