Reduce number of packets sent by ROWP This CL changes our default ROWP behavior to start the exponential backoff after 10 packets instead of 200, and also adds a connection-lifetime cap on ROWP to disable ROWP after 1000 ROWPs have been sent. PiperOrigin-RevId: 338078574 Change-Id: If025717bf5b509c6bbbfb3dec34a928dfdeec7d5
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index fa02801..1e59d4f 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -285,6 +285,7 @@ ping_timeout_(QuicTime::Delta::FromSeconds(kPingTimeoutSecs)), initial_retransmittable_on_wire_timeout_(QuicTime::Delta::Infinite()), consecutive_retransmittable_on_wire_ping_count_(0), + retransmittable_on_wire_ping_count_(0), arena_(), ack_alarm_(alarm_factory_->CreateAlarm(arena_.New<AckAlarmDelegate>(this), &arena_)), @@ -4054,7 +4055,9 @@ return; } if (initial_retransmittable_on_wire_timeout_.IsInfinite() || - sent_packet_manager_.HasInFlightPackets()) { + sent_packet_manager_.HasInFlightPackets() || + retransmittable_on_wire_ping_count_ > + GetQuicFlag(FLAGS_quic_max_retransmittable_on_wire_ping_count)) { // Extend the ping alarm. ping_alarm_->Update(clock_->ApproximateNow() + ping_timeout_, QuicTime::Delta::FromSeconds(1)); @@ -4090,6 +4093,7 @@ if (max_aggressive_retransmittable_on_wire_ping_count != 0) { consecutive_retransmittable_on_wire_ping_count_++; } + retransmittable_on_wire_ping_count_++; return; }
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 4be1b96..59fa80d 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -526,6 +526,10 @@ QuicConnectionStats& mutable_stats() { return stats_; } + int retransmittable_on_wire_ping_count() const { + return retransmittable_on_wire_ping_count_; + } + // Returns statistics tracked for this connection. const QuicConnectionStats& GetStats(); @@ -1651,6 +1655,9 @@ // receiving any new data in between. int consecutive_retransmittable_on_wire_ping_count_; + // Indicates how many retransmittable-on-wire pings have been emitted. + int retransmittable_on_wire_ping_count_; + // Arena to store class implementations within the QuicConnection. QuicConnectionArena arena_;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 433ba74..7d736b3 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -8251,6 +8251,75 @@ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); } +// Make sure that we never send more retransmissible on the wire pings than +// the limit in FLAGS_quic_max_retransmittable_on_wire_ping_count. +TEST_P(QuicConnectionTest, RetransmittableOnWirePingLimit) { + static constexpr int kMaxRetransmittableOnWirePingCount = 3; + SetQuicFlag(FLAGS_quic_max_retransmittable_on_wire_ping_count, + kMaxRetransmittableOnWirePingCount); + static constexpr QuicTime::Delta initial_retransmittable_on_wire_timeout = + QuicTime::Delta::FromMilliseconds(200); + static constexpr QuicTime::Delta short_delay = + QuicTime::Delta::FromMilliseconds(5); + ASSERT_LT(short_delay * 10, initial_retransmittable_on_wire_timeout); + connection_.set_initial_retransmittable_on_wire_timeout( + initial_retransmittable_on_wire_timeout); + + EXPECT_TRUE(connection_.connected()); + EXPECT_CALL(visitor_, ShouldKeepConnectionAlive()) + .WillRepeatedly(Return(true)); + + const char data[] = "data"; + // Advance 5ms, send a retransmittable data packet to the peer. + clock_.AdvanceTime(short_delay); + EXPECT_FALSE(connection_.GetPingAlarm()->IsSet()); + connection_.SendStreamDataWithString(1, data, 0, NO_FIN); + EXPECT_TRUE(connection_.sent_packet_manager().HasInFlightPackets()); + // The ping alarm is set for the ping timeout, not the shorter + // retransmittable_on_wire_timeout. + EXPECT_TRUE(connection_.GetPingAlarm()->IsSet()); + EXPECT_EQ(connection_.ping_timeout(), + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); + + EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)).Times(AnyNumber()); + EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)) + .Times(AnyNumber()); + + // Verify that the first few consecutive retransmittable on wire pings are + // sent with aggressive timeout. + for (int i = 0; i <= kMaxRetransmittableOnWirePingCount; i++) { + // Receive an ACK of the previous packet. This should set the ping alarm + // with the initial retransmittable-on-wire timeout. + clock_.AdvanceTime(short_delay); + QuicPacketNumber ack_num = creator_->packet_number(); + QuicAckFrame frame = InitAckFrame( + {{QuicPacketNumber(ack_num), QuicPacketNumber(ack_num + 1)}}); + ProcessAckPacket(&frame); + EXPECT_TRUE(connection_.GetPingAlarm()->IsSet()); + EXPECT_EQ(initial_retransmittable_on_wire_timeout, + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); + // Simulate the alarm firing and check that a PING is sent. + writer_->Reset(); + if (!GetQuicReloadableFlag(quic_let_connection_handle_pings)) { + EXPECT_CALL(visitor_, SendPing()).WillOnce(Invoke([this]() { + SendPing(); + })); + } + clock_.AdvanceTime(initial_retransmittable_on_wire_timeout); + connection_.GetPingAlarm()->Fire(); + } + + // Receive an ACK of the previous packet. This should set the ping alarm + // but this time with the default ping timeout. + QuicPacketNumber ack_num = creator_->packet_number(); + QuicAckFrame frame = InitAckFrame( + {{QuicPacketNumber(ack_num), QuicPacketNumber(ack_num + 1)}}); + ProcessAckPacket(&frame); + EXPECT_TRUE(connection_.GetPingAlarm()->IsSet()); + EXPECT_EQ(connection_.ping_timeout(), + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); +} + TEST_P(QuicConnectionTest, ValidStatelessResetToken) { const QuicUint128 kTestToken = 1010101; const QuicUint128 kWrongTestToken = 1010100;