Subject: DetectAndRemoveAckedPackets

From: Timo Völker <[email-masked]>
Date: Tue, 24 Nov 2020 07:08:28 +0000

Hi all,

The pseudo code in the recovery draft uses the function DetectAndRemoveAckedPackets. Is my assumption correct that this function should return a list of all newly acked packets (including these that do not count as in flight)?

If so, it seems there is a bug in the code, because this list is passed to the congestion controller, which uses *all* the packets in the list to decrease its bytes_in_flight.

Timo

---

From: Ian Swett <[email-masked]>
Date: Tue, 24 Nov 2020 13:19:17 -0500

It's a bit subtle, but the current pseudocode does the right thing,
because sent_bytes is only set to a non-0 value in OnPacketSent() if the
packet is in_flight:

sent_packets[pn_space][packet_number].sent_bytes =
      sent_bytes

Given the way it's used, maybe it makes sense to rename sent_bytes to
inflight_bytes? Though that could be confused with byte_in_flight?

Ian

On Tue, Nov 24, 2020 at 2:09 AM Timo Völker <[email-masked]>
wrote:

> Hi all,
>
> The pseudo code in the recovery draft uses the function
> DetectAndRemoveAckedPackets. Is my assumption correct that this function
> should return a list of all newly acked packets (including these that do
> not count as in flight)?
>
> If so, it seems there is a bug in the code, because this list is passed to
> the congestion controller, which uses *all* the packets in the list to
> decrease its bytes_in_flight.
>
> Timo

---

From: Lucas Pardue <[email-masked]>
Date: Wed, 25 Nov 2020 00:13:36 +0000

Ian or Jana,

If you believe Timo's comment is valid can you please create an issue?

Cheers
Lucas

On Tue, Nov 24, 2020 at 6:19 PM Ian Swett <ianswett=
[email-masked]> wrote:

>> If so, it seems there is a bug in the code, because this list is passed
>> to the congestion controller, which uses *all* the packets in the list to

---

From: Ian Swett <[email-masked]>
Date: Tue, 24 Nov 2020 21:12:13 -0500

Filed #4408 <https://github.com/quicwg/base-drafts/issues/4408> and PR
#4409 sent out to improve clarity in the best way I can think of.

---

From: Lucas Pardue <[email-masked]>
Date: Wed, 25 Nov 2020 02:13:59 +0000

Thanks!

---

From: Timo Völker <[email-masked]>
Date: Wed, 25 Nov 2020 06:13:26 +0000

Ian,

Thank you for the clarification. I missed that. With your proposed changes, the code is easier to understand (at least for me).

https://github.com/quicwg/base-drafts/pull/4409

Timo

> On 24. Nov 2020, at 19:19, Ian Swett <[email-masked]> wrote:
> 
> It's a bit subtle, but the current pseudocode does the right thing, because sent_bytes is only set to a non-0 value in OnPacketSent() if the packet is in_flight:
> Given the way it's used, maybe it makes sense to rename sent_bytes to inflight_bytes? Though that could be confused with byte_in_flight?
> 
> Ian
> 
> On Tue, Nov 24, 2020 at 2:09 AM Timo Völker <[email-masked]> wrote:
> Hi all,
>

---

From: Ian Swett <[email-masked]>
Date: Wed, 25 Nov 2020 09:47:39 -0500

Thanks, I'm glad it helps.

On Wed, Nov 25, 2020 at 1:13 AM Timo Völker <[email-masked]>
wrote:

> Ian,
>
> Thank you for the clarification. I missed that. With your proposed
> changes, the code is easier to understand (at least for me).
>