Subject: [quicwg/base-drafts] "ACK-only" confusion in -recovery (#1932)

From: Lars Eggert <[email-masked]>
Date: Tue, 30 Oct 2018 07:34:59 +0000 (UTC)

There are some inconsistencies - at least in my reading - in how "ACK-only" packets are discussed. I think there are really two distinct definitions here, namely, "ACK-without-PADDING-only", and "ACK-plus-optionally-PADDING-only". It would be helpful if the document clearly distinguished which it means whenever it casually says "ACK-only" at the moment.

Details:

Section 2 says:
```
   ACK-only:  Any packet containing only an ACK frame.
```
and that is pretty clear. (Except that I think multiple ACK frames but no other frames are also allowed. Reword?)

Section 3 then muddles this a bit, by saying that there is actually "ACKs-without-PADDING-only", and "ACKs-plus-optionally-PADDING-only". So that sounds like these two concepts should be more clearly introduced.
```
   o  Packets that contain only ACK frames do not count toward
      congestion control limits and are not considered in-flight.  Note
      that this means PADDING frames cause packets to contribute toward
      bytes in flight without directly causing an acknowledgment to be
      sent.
```

However, Section 4.5.4 then defines the `ack_only` argument to `OnPacketSent ` as
```
   o  ack_only: A boolean that indicates whether a packet contains only
      ACK or PADDING frame(s).
```
First, that definition is imprecise, since a packet with only PADDING frames is probably not supposed to count as `ack_only`.  Suggest you refer back to whatever the definition in Section 2 ends up being. Second, calling it `ack_only` when PADDING is also allowed is misleading.

Section 4.5.2 defines the variables of interest, which include `sent_packets`, which includes `
a boolean indicating whether the packet is ack-only`. It's not clear if that boolean is of the "PADDING allowed" variety or not.

Also, since the definition of `sent_packets` doesn't actually give names for the fields it defines, it remains unclear in the pseudo code which field refers to this definition. In `OnPacketSent`, the field `ack_only` is used, but in `OnPacketAcked` and `DetectLostPackets`, a field called `is_ack_only` is used. Are they the same? `is_ack_only` is not defined anywhere.

Finally, I think this text in Section 3
```
   o  All packets are acknowledged, though packets that contain only ACK
      and PADDING frames are not acknowledged immediately.
```
sounds lke packets with only ACK and not PADDING *are* in fact ACKed immediately.
Might be better stated as "...that contain only ACK and (optionally also) PADDING frames..."

-- 
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/issues/1932