Re: [quicwg/base-drafts] Add details of marking packets lost on PTO (#2624)

[ianswett <[e-mail]>]

ianswett requested changes on this pull request.

LG overall, but I think the example needs to be fixed, unfortunately.

> @@ -623,10 +623,21 @@ time.
 
 ### Sending Probe Packets
 
-When a PTO timer expires, the sender MUST send one ack-eliciting packet as a
-probe, unless there is nothing to send. A sender MAY send up to two
-ack-eliciting packets, to avoid an expensive consecutive PTO expiration due
-to a single packet loss.
+When a PTO timer expires, a sender MUST send at least one ack-eliciting packet
+as a probe, unless there is no data available to send.  An endpoint MAY send up
+to two ack-eliciting packets, to avoid an expensive consecutive PTO expiration
+due to a single packet loss.
+
+It is possible that the sender has no new or previously-sent data to send.  For
+example, this can happen when there is no new application data to send and all
+streams with data in flight are reset (see Section 13.2 of {{QUIC-TRANSPORT}}).
+Under these conditions, the sender SHOULD send PING or other ack-eliciting
+frames in up to two packets, re-arming the PTO timer.

Why send two packets in the case when you're just trying to establish loss?  I'd specifically say 1 packet for this case.

> -When a PTO timer expires, the sender MUST send one ack-eliciting packet as a
-probe, unless there is nothing to send. A sender MAY send up to two
-ack-eliciting packets, to avoid an expensive consecutive PTO expiration due
-to a single packet loss.
+When a PTO timer expires, a sender MUST send at least one ack-eliciting packet
+as a probe, unless there is no data available to send.  An endpoint MAY send up
+to two ack-eliciting packets, to avoid an expensive consecutive PTO expiration
+due to a single packet loss.
+
+It is possible that the sender has no new or previously-sent data to send.  For
+example, this can happen when there is no new application data to send and all
+streams with data in flight are reset (see Section 13.2 of {{QUIC-TRANSPORT}}).
+Under these conditions, the sender SHOULD send PING or other ack-eliciting
+frames in up to two packets, re-arming the PTO timer.
+
+When there is no data to send, the sender MAY mark any packets still in flight

I'd like to be clear that this is an alternative, so maybe say "Alternatively, when there is..."?

> @@ -1242,7 +1246,8 @@ OnLossDetectionTimeout():
        SendOnePaddedInitialPacket()
     crypto_count++
   else:
-    // PTO
+    // PTO. Send new data if available, else retransmit old data.
+    // If neither is available, send PING frames.

```suggestion
    // If neither is available, send a PING frame.
```

> @@ -1455,8 +1460,7 @@ Invoked when an ACK frame with an ECN section is received from the peer.
 
 ## On Packets Lost
 
-Invoked by loss detection from DetectLostPackets when new packets
-are detected lost.
+Invoked when packets are deemed lost.

Is it worth specifying "from DetectLostPackets" still?

> @@ -623,10 +623,21 @@ time.
 
 ### Sending Probe Packets
 
-When a PTO timer expires, the sender MUST send one ack-eliciting packet as a
-probe, unless there is nothing to send. A sender MAY send up to two
-ack-eliciting packets, to avoid an expensive consecutive PTO expiration due
-to a single packet loss.
+When a PTO timer expires, a sender MUST send at least one ack-eliciting packet
+as a probe, unless there is no data available to send.  An endpoint MAY send up
+to two ack-eliciting packets, to avoid an expensive consecutive PTO expiration
+due to a single packet loss.
+
+It is possible that the sender has no new or previously-sent data to send.  For
+example, this can happen when there is no new application data to send and all
+streams with data in flight are reset (see Section 13.2 of {{QUIC-TRANSPORT}}).

Because of the reset issue, I think another example, such as Nick's, would be best.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2624#pullrequestreview-227249612