From ee023f4f84eb60ccb77c033af0225706d974e4e8 Mon Sep 17 00:00:00 2001 From: Asias He Date: Wed, 12 Nov 2014 18:09:22 +0800 Subject: [PATCH] tcp: Fix delayed ack When doing tcp rx testing, I saw a lot of retransmission because of the delayed ACK. Our current delayed ACK algorithm does not comply with what RFC 1122 suggests. As described in RFC 1122, a host may delay sending an ACK response by up to 500 ms. Additionally, with a stream of full-sized incoming segments, ACK responses must be sent for every second segment. === Before === [asias@hjpc pingpong]$ go run client-rxrx.go Bytes Sent(MiB): 100 Total Time(Secs): 322.620879376 Bandwidth(MiB/Sec): 0.30996133974160595 78 2.412385 192.168.66.100 -> 192.168.66.123 TCP 32174 37672 > 10000 [ACK] Seq=2149425323 Ack=1000001 Win=229 Len=32120 79 2.612985 192.168.66.100 -> 192.168.66.123 TCP 1514 [TCP Retransmission] 37672 > 10000 [ACK] Seq=2149425323 Ack=1000001 Win=229 Len=1460 80 2.613131 192.168.66.123 -> 192.168.66.100 TCP 54 10000 > 37672 [ACK] Seq=1000001 Ack=2149457443 Win=29200 Len=0 === After === [asias@hjpc pingpong]$ go run client-rxrx.go Bytes Sent(MiB): 100 Total Time(Secs): 0.244951095 Bandwidth(MiB/Sec): 408.2447559583271 No retransmission is seen. --- net/tcp.hh | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/net/tcp.hh b/net/tcp.hh index 3d941730f4..dd501a34bb 100644 --- a/net/tcp.hh +++ b/net/tcp.hh @@ -207,6 +207,7 @@ private: std::chrono::milliseconds _retransmit_timeout{3000}; static constexpr uint16_t _max_nr_retransmit{5}; timer _retransmit; + uint16_t _nr_full_seg_received = 0; public: tcb(tcp& t, connid id); void input(tcp_hdr* th, packet p); @@ -226,7 +227,7 @@ private: void merge_out_of_order(); void insert_out_of_order(tcp_seq seq, packet p); void trim_receive_data_after_window(); - bool should_send_ack(); + bool should_send_ack(uint16_t seg_len); void clear_delayed_ack(); packet get_transmit_packet(); void start_retransmit_timer() { _retransmit.rearm(clock_type::now() + _retransmit_timeout); }; @@ -390,7 +391,7 @@ tcp::tcb::tcb(tcp& t, connid id) , _foreign_ip(id.foreign_ip) , _local_port(id.local_port) , _foreign_port(id.foreign_port) { - _delayed_ack.set_callback([this] { output(); }); + _delayed_ack.set_callback([this] { _nr_full_seg_received = 0; output(); }); _retransmit.set_callback([this] { retransmit(); }); } @@ -508,7 +509,7 @@ void tcp::tcb::input(tcp_hdr* th, packet p) { } } // Send to ack data only when data is present in this packet - do_output = should_send_ack(); + do_output = should_send_ack(seg_len); } } if (th->f_fin) { @@ -616,6 +617,8 @@ void tcp::tcb::input(tcp_hdr* th, packet p) { } // send some stuff if (do_output || (do_output_data && can_send())) { + // Since we will do output, we can canncel scheduled delayed ACK. + clear_delayed_ack(); output(); } } @@ -795,13 +798,31 @@ void tcp::tcb::close() { } template -bool tcp::tcb::should_send_ack() { - if (_delayed_ack.cancel()) { +bool tcp::tcb::should_send_ack(uint16_t seg_len) { + // We've received a TSO packet, do ack immediately + if (seg_len > _rcv.mss) { + _nr_full_seg_received = 0; + _delayed_ack.cancel(); return true; - } else { - _delayed_ack.arm(400ms); + } + + // We've received a full sized segment, ack for every second full sized segment + if (seg_len == _rcv.mss) { + if (_nr_full_seg_received++ >= 1) { + _nr_full_seg_received = 0; + _delayed_ack.cancel(); + return true; + } + } + + // If the timer is armed and its callback hasn't been run. + if (_delayed_ack.armed()) { return false; } + + // If the timer is not armed, schedule a delayed ACK. + _delayed_ack.arm(500ms); + return false; } template