From e016aeddeb47921a28ba5613ed7e0e0ed720c2df Mon Sep 17 00:00:00 2001
From: "Sijie.Sun" <sunsijie@buaa.edu.cn>
Date: Tue, 7 Jan 2025 22:42:57 +0800
Subject: [PATCH] optimize pingpong conn close condition (#549)

if received some packets from peer, only close conn after enough
rounds of pingpong
---
 easytier/src/peers/peer_conn.rs      | 10 ++-----
 easytier/src/peers/peer_conn_ping.rs | 42 ++++++++++++++--------------
 2 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/easytier/src/peers/peer_conn.rs b/easytier/src/peers/peer_conn.rs
index adbca90..5650fbc 100644
--- a/easytier/src/peers/peer_conn.rs
+++ b/easytier/src/peers/peer_conn.rs
@@ -484,13 +484,6 @@ mod tests {
         let c_recorder = Arc::new(DropSendTunnelFilter::new(drop_start, drop_end));
         let c = TunnelWithFilter::new(c, c_recorder.clone());
 
-        let s = if drop_both {
-            let s_recorder = Arc::new(DropSendTunnelFilter::new(drop_start, drop_end));
-            Box::new(TunnelWithFilter::new(s, s_recorder.clone()))
-        } else {
-            s
-        };
-
         let c_peer_id = new_peer_id();
         let s_peer_id = new_peer_id();
 
@@ -506,6 +499,7 @@ mod tests {
         s_peer
             .start_recv_loop(tokio::sync::mpsc::channel(200).0)
             .await;
+        // do not start ping for s, s only reponde to ping from c
 
         assert!(c_ret.is_ok());
         assert!(s_ret.is_ok());
@@ -547,7 +541,7 @@ mod tests {
 
     #[tokio::test]
     async fn peer_conn_pingpong_bothside_timeout() {
-        peer_conn_pingpong_test_common(4, 12, true, true).await;
+        peer_conn_pingpong_test_common(3, 14, true, true).await;
     }
 
     #[tokio::test]
diff --git a/easytier/src/peers/peer_conn_ping.rs b/easytier/src/peers/peer_conn_ping.rs
index 5192549..47ed249 100644
--- a/easytier/src/peers/peer_conn_ping.rs
+++ b/easytier/src/peers/peer_conn_ping.rs
@@ -289,29 +289,29 @@ impl PeerConnPinger {
                 "pingpong task recv pingpong_once result"
             );
 
-            if (counter > 5 && loss_rate_20 > 0.74) || (counter > 150 && loss_rate_1 > 0.20) {
-                let current_rx_packets = throughput.rx_packets();
-                let need_close = if last_rx_packets != current_rx_packets {
-                    // if we receive some packet from peers, we should relax the condition
-                    counter > 50 && loss_rate_1 > 0.5
+            let current_rx_packets = throughput.rx_packets();
+            if last_rx_packets != current_rx_packets {
+                // if we receive some packet from peers, reset the counter to avoid conn close.
+                // conn will close only if we have 5 continous round pingpong loss after no packet received.
+                counter = 0;
+            }
 
-                    // TODO: wait more time to see if the loss rate is still high after no rx
-                } else {
-                    true
-                };
+            tracing::debug!(
+                "counter: {}, loss_rate_1: {}, loss_rate_20: {}, cur_rx_packets: {}, last_rx: {}, node_id: {}",
+                counter, loss_rate_1, loss_rate_20, current_rx_packets, last_rx_packets, my_node_id
+            );
 
-                if need_close {
-                    tracing::warn!(
-                        ?ret,
-                        ?self,
-                        ?loss_rate_1,
-                        ?loss_rate_20,
-                        ?last_rx_packets,
-                        ?current_rx_packets,
-                        "pingpong loss rate too high, closing"
-                    );
-                    break;
-                }
+            if (counter > 5 && loss_rate_20 > 0.74) || (counter > 100 && loss_rate_1 > 0.35) {
+                tracing::warn!(
+                    ?ret,
+                    ?self,
+                    ?loss_rate_1,
+                    ?loss_rate_20,
+                    ?last_rx_packets,
+                    ?current_rx_packets,
+                    "pingpong loss rate too high, closing"
+                );
+                break;
             }
 
             last_rx_packets = throughput.rx_packets();