ackhandler: don’t generate an immediate ACK for the first packet (#5447)

Instead apply the regular RFC 9000 logic and send an acknowledgment
after receiving two ack-eliciting packets.

diff --git a/internal/ackhandler/received_packet_tracker.go b/internal/ackhandler/received_packet_tracker.go
index 80f154ba86a00506c6e4c0efeb41ac9cd10db6d9..64092cccf8314c3bba454fc76959d81693747a54 100644 (file)
--- a/internal/ackhandler/received_packet_tracker.go
+++ b/internal/ackhandler/received_packet_tracker.go
@@ -155,6 +155,9 @@ func (h *appDataReceivedPacketTracker) isMissing(p protocol.PacketNumber) bool {
 }
 
 func (h *appDataReceivedPacketTracker) hasNewMissingPackets() bool {
+       if h.lastAck == nil {
+               return false
+       }
        if h.largestObserved < reorderingThreshold {
                return false
        }
@@ -170,12 +173,6 @@ func (h *appDataReceivedPacketTracker) hasNewMissingPackets() bool {
 }
 
 func (h *appDataReceivedPacketTracker) shouldQueueACK(pn protocol.PacketNumber, ecn protocol.ECN, wasMissing bool) bool {
-       // always acknowledge the first packet
-       if h.lastAck == nil {
-               h.logger.Debugf("\tQueueing ACK because the first packet should be acknowledged.")
-               return true
-       }
-
        // Send an ACK if this packet was reported missing in an ACK sent before.
        // Ack decimation with reordering relies on the timer to send an ACK, but if
        // missing packets we reported in the previous ACK, send an ACK immediately.

diff --git a/internal/ackhandler/received_packet_tracker_test.go b/internal/ackhandler/received_packet_tracker_test.go
index 76fc4751374cc48e5c6fe42c5dd602b6ba11b255..119a38c42035af2dc4f5df9a4f4c83ca26438ce5 100644 (file)
--- a/internal/ackhandler/received_packet_tracker_test.go
+++ b/internal/ackhandler/received_packet_tracker_test.go
@@ -71,9 +71,8 @@ func TestAppDataReceivedPacketTrackerECN(t *testing.T) {
 
 func TestAppDataReceivedPacketTrackerAckEverySecondPacket(t *testing.T) {
        tr := newAppDataReceivedPacketTracker(utils.DefaultLogger)
-       // the first packet is always acknowledged
-       require.NoError(t, tr.ReceivedPacket(0, protocol.ECNNon, monotime.Now(), true))
-       require.NotNil(t, tr.GetAckFrame(monotime.Now(), true))
+       require.Nil(t, tr.GetAckFrame(monotime.Now(), true))
+
        for p := protocol.PacketNumber(1); p <= 20; p++ {
                require.NoError(t, tr.ReceivedPacket(p, protocol.ECNNon, monotime.Now(), true))
                switch p % 2 {
@@ -88,10 +87,6 @@ func TestAppDataReceivedPacketTrackerAckEverySecondPacket(t *testing.T) {
 func TestAppDataReceivedPacketTrackerAlarmTimeout(t *testing.T) {
        tr := newAppDataReceivedPacketTracker(utils.DefaultLogger)
 
-       // the first packet is always acknowledged
-       require.NoError(t, tr.ReceivedPacket(0, protocol.ECNNon, monotime.Now(), true))
-       require.NotNil(t, tr.GetAckFrame(monotime.Now(), true))
-
        now := monotime.Now()
        require.NoError(t, tr.ReceivedPacket(1, protocol.ECNNon, now, false))
        require.Nil(t, tr.GetAckFrame(monotime.Now(), true))
@@ -110,10 +105,6 @@ func TestAppDataReceivedPacketTrackerAlarmTimeout(t *testing.T) {
 func TestAppDataReceivedPacketTrackerQueuesECNCE(t *testing.T) {
        tr := newAppDataReceivedPacketTracker(utils.DefaultLogger)
 
-       // the first packet is always acknowledged
-       require.NoError(t, tr.ReceivedPacket(0, protocol.ECNNon, monotime.Now(), true))
-       require.NotNil(t, tr.GetAckFrame(monotime.Now(), true))
-
        require.NoError(t, tr.ReceivedPacket(1, protocol.ECNCE, monotime.Now(), true))
        ack := tr.GetAckFrame(monotime.Now(), true)
        require.NotNil(t, ack)
@@ -125,9 +116,8 @@ func TestAppDataReceivedPacketTrackerMissingPackets(t *testing.T) {
        tr := newAppDataReceivedPacketTracker(utils.DefaultLogger)
 
        now := monotime.Now()
-       // the first packet is always acknowledged
        require.NoError(t, tr.ReceivedPacket(0, protocol.ECNNon, now, true))
-       require.NotNil(t, tr.GetAckFrame(now, true))
+       require.Nil(t, tr.GetAckFrame(now, true))
 
        require.NoError(t, tr.ReceivedPacket(5, protocol.ECNNon, now, true))
        ack := tr.GetAckFrame(now, true) // ACK: 0 and 5, missing: 1, 2, 3, 4