Skip to content

Commit

Permalink
For efficiency, pick up and send an undersized utp packet if there's …
Browse files Browse the repository at this point in the history 
…room in the congestion window when 'forcing' a packet
  • Loading branch information
@thrnz @arvidn
thrnz authored and arvidn committed Sep 9, 2022
1 parent 239f7e6 commit a692732
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 21 deletions.
38 changes: 19 additions & 19 deletions simulation/test_utp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -213,18 +213,18 @@ TORRENT_TEST(utp_straw)

std::vector<std::int64_t> 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);
Expand Down
13 changes: 11 additions & 2 deletions src/utp_stream.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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<void*>(this));
#endif

// pick up the nagle packet and keep adding bytes to it
p = std::move(m_nagle_packet);
m_nagle_packet.reset();
Expand Down

0 comments on commit a692732

Please sign in to comment.