From a692732082cb1ec653692a2a32afbc58309a1fda Mon Sep 17 00:00:00 2001 From: thrnz Date: Tue, 6 Sep 2022 13:56:08 +1200 Subject: [PATCH] For efficiency, pick up and send an undersized utp packet if there's room in the congestion window when 'forcing' a packet --- simulation/test_utp.cpp | 38 +++++++++++++++++++------------------- src/utp_stream.cpp | 13 +++++++++++-- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/simulation/test_utp.cpp b/simulation/test_utp.cpp index 33fedec3acc..03485701fd8 100644 --- a/simulation/test_utp.cpp +++ b/simulation/test_utp.cpp @@ -136,10 +136,10 @@ TORRENT_TEST(utp_pmtud) // TODO: 3 This timeout happens at shutdown. It's not very clean TEST_EQUAL(metric(cnt, "utp.utp_timeout"), 1); - TEST_EQUAL(metric(cnt, "utp.utp_packets_in"), 611); - TEST_EQUAL(metric(cnt, "utp.utp_payload_pkts_in"), 23); + TEST_EQUAL(metric(cnt, "utp.utp_packets_in"), 593); + TEST_EQUAL(metric(cnt, "utp.utp_payload_pkts_in"), 66); - TEST_EQUAL(metric(cnt, "utp.utp_packets_out"), 612); + TEST_EQUAL(metric(cnt, "utp.utp_packets_out"), 602); // we don't expect any invalid packets, since we're talking to ourself TEST_EQUAL(metric(cnt, "utp.utp_invalid_pkts_in"), 0); @@ -162,10 +162,10 @@ TORRENT_TEST(utp_plain) TEST_EQUAL(metric(cnt, "utp.utp_fast_retransmit"), 0); TEST_EQUAL(metric(cnt, "utp.utp_packet_resend"), 0); - TEST_EQUAL(metric(cnt, "utp.utp_packets_in"), 609); - TEST_EQUAL(metric(cnt, "utp.utp_payload_pkts_in"), 23); + TEST_EQUAL(metric(cnt, "utp.utp_packets_in"), 590); + TEST_EQUAL(metric(cnt, "utp.utp_payload_pkts_in"), 76); - TEST_EQUAL(metric(cnt, "utp.utp_packets_out"), 608); + TEST_EQUAL(metric(cnt, "utp.utp_packets_out"), 596); // we don't expect any invalid packets, since we're talking to ourself TEST_EQUAL(metric(cnt, "utp.utp_invalid_pkts_in"), 0); @@ -188,13 +188,13 @@ TORRENT_TEST(utp_buffer_bloat) TEST_EQUAL(metric(cnt, "utp.utp_fast_retransmit"), 0); TEST_EQUAL(metric(cnt, "utp.utp_packet_resend"), 0); - TEST_EQUAL(metric(cnt, "utp.utp_samples_above_target"), 425); - TEST_EQUAL(metric(cnt, "utp.utp_samples_below_target"), 156); + TEST_EQUAL(metric(cnt, "utp.utp_samples_above_target"), 428); + TEST_EQUAL(metric(cnt, "utp.utp_samples_below_target"), 153); - TEST_EQUAL(metric(cnt, "utp.utp_packets_in"), 646); - TEST_EQUAL(metric(cnt, "utp.utp_payload_pkts_in"), 62); + TEST_EQUAL(metric(cnt, "utp.utp_packets_in"), 633); + TEST_EQUAL(metric(cnt, "utp.utp_payload_pkts_in"), 84); - TEST_EQUAL(metric(cnt, "utp.utp_packets_out"), 645); + TEST_EQUAL(metric(cnt, "utp.utp_packets_out"), 633); // we don't expect any invalid packets, since we're talking to ourself TEST_EQUAL(metric(cnt, "utp.utp_invalid_pkts_in"), 0); @@ -213,18 +213,18 @@ TORRENT_TEST(utp_straw) std::vector cnt = utp_test(cfg); - TEST_EQUAL(metric(cnt, "utp.utp_packet_loss"), 69); - TEST_EQUAL(metric(cnt, "utp.utp_timeout"), 29); - TEST_EQUAL(metric(cnt, "utp.utp_fast_retransmit"), 72); - TEST_EQUAL(metric(cnt, "utp.utp_packet_resend"), 133); + TEST_EQUAL(metric(cnt, "utp.utp_packet_loss"), 64); + TEST_EQUAL(metric(cnt, "utp.utp_timeout"), 32); + TEST_EQUAL(metric(cnt, "utp.utp_fast_retransmit"), 67); + TEST_EQUAL(metric(cnt, "utp.utp_packet_resend"), 130); TEST_EQUAL(metric(cnt, "utp.utp_samples_above_target"), 0); - TEST_EQUAL(metric(cnt, "utp.utp_samples_below_target"), 277); + TEST_EQUAL(metric(cnt, "utp.utp_samples_below_target"), 269); - TEST_EQUAL(metric(cnt, "utp.utp_packets_in"), 429); - TEST_EQUAL(metric(cnt, "utp.utp_payload_pkts_in"), 55); + TEST_EQUAL(metric(cnt, "utp.utp_packets_in"), 394); + TEST_EQUAL(metric(cnt, "utp.utp_payload_pkts_in"), 53); - TEST_EQUAL(metric(cnt, "utp.utp_packets_out"), 563); + TEST_EQUAL(metric(cnt, "utp.utp_packets_out"), 531); // we don't expect any invalid packets, since we're talking to ourself TEST_EQUAL(metric(cnt, "utp.utp_invalid_pkts_in"), 0); diff --git a/src/utp_stream.cpp b/src/utp_stream.cpp index 6a4d2da2795..0d735d87468 100644 --- a/src/utp_stream.cpp +++ b/src/utp_stream.cpp @@ -1435,8 +1435,11 @@ bool utp_socket_impl::send_pkt(int const flags) utp_header* h = nullptr; // payload size being zero means we're just sending - // an force. We should not pick up the nagle packet - if (!m_nagle_packet || (payload_size == 0 && force)) + // an force. For efficiency, pick up the nagle packet + // if there's room + if (!m_nagle_packet || (payload_size == 0 && force + && m_bytes_in_flight + m_nagle_packet->size + > std::min(int(m_cwnd >> 16), int(m_adv_wnd)))) { p = acquire_packet(effective_mtu); @@ -1471,6 +1474,12 @@ bool utp_socket_impl::send_pkt(int const flags) } else { +#if TORRENT_UTP_LOG + if (payload_size == 0 && force) + UTP_LOGV("%8p: Picking up Nagled packet due to forced send\n" + , static_cast(this)); +#endif + // pick up the nagle packet and keep adding bytes to it p = std::move(m_nagle_packet); m_nagle_packet.reset();