Subject: too much call to SetLossDetectionTimer()?

From: newbie quic <[email-masked]>
Date: Thu, 10 Sep 2020 16:09:49 +0200

Hello,

I have a question about the pseudo-code found in
draft-ietf-quic-recovery-30.txt.

Especially the part where at least one ack-eliciting packet is sent from
OnLossDetectionTimeout(). In every cases, we finally set the loss
detection/PTO timer calling SetLossDetectionTimer():

     if (bytes_in_flight > 0):
         // PTO. Send new data if available, else retransmit old data.
         // If neither is available, send a single PING frame.
         _, pn_space = GetPtoTimeAndSpace()
        SendOneOrTwoAckElicitingPackets(pn_space)
     else:
         assert(!PeerCompletedAddressValidation())
         // Client sends an anti-deadlock packet: Initial is padded
         // to earn more anti-amplification credit,
         // a Handshake packet proves address ownership.
         if (has Handshake keys):
              SendOneAckElicitingHandshakePacket()
         else:
              SendOneAckElicitingPaddedInitialPacket()

     pto_count++
     SetLossDetectionTimer()

Then as a sent ack-eliciting packet must be considered as in flight,
OnSentPacket() will also make a call to SetLossDetectionTimer().

Regards.

---

From: Ian Swett <[email-masked]>
Date: Thu, 10 Sep 2020 11:26:31 -0400

Good point, this is typically unnecessary.  However, I think it also may be
clearer to call it here than not?  Alternatively, we could add a comment
and an assert.

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 10 Sep 2020 20:28:39 -0700

This is fine, I'd argue... it's unnecessary, but it doesn't break anything.
And the pseudo code is meant to clarify the text, rather than simply be
used verbatim in implementations anyways.

On Thu, Sep 10, 2020 at 8:27 AM Ian Swett <ianswett=
[email-masked]> wrote:

> Good point, this is typically unnecessary.  However, I think it also may
> be clearer to call it here than not?  Alternatively, we could add a comment
> and an assert.
>

---

From: newbie quic <[email-masked]>
Date: Fri, 11 Sep 2020 14:03:31 +0200

I am not sure at all this does not break anything because pto_count is
incremented before calling SetLossDetectionTimer() which potentially makes
usage of this variable to set the timer value. Furthermore I agree there
are few chances an implementation will use this pseudo-code as is. But
there are chances the logic will be used.

For example, perhaps I have missed something but it seems this pseudo-code
logic has been implemented in ngtcp2, isn't?

https://github.com/ngtcp2/ngtcp2/blob/master/lib/ngtcp2_conn.c#L9748
https://github.com/ngtcp2/ngtcp2/blob/master/lib/ngtcp2_conn.c#L1378

Regards.

On Fri, Sep 11, 2020 at 5:28 AM Jana Iyengar <[email-masked]> wrote:

> This is fine, I'd argue... it's unnecessary, but it doesn't break
> anything. And the pseudo code is meant to clarify the text, rather than
> simply be used verbatim in implementations anyways.
>

---

From: Lars Eggert <[email-masked]>
Date: Fri, 11 Sep 2020 15:27:30 +0300

Hi "newbie quic",

anonymous IETF participation isn't really possible. Please see https://github.com/quicwg/base-drafts/blob/master/CONTRIBUTING.md and referenced documents.

Thanks,
Lars

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 14 Sep 2020 16:11:33 -0700

Good point -- though the pseudo-code should work fine, even if redundant.
Yes, the pto_count is incremented before SetLossDetectionTimer() is called,
and that will push the timer to run for the longer, correct timeout.

My point was not to say that implementers cannot use the code as is -- they
can, since the pseudo code is expected to be correct -- just that they
might include some redundancies that were introduced here for clarity.

On Fri, Sep 11, 2020 at 5:27 AM Lars Eggert <[email-masked]> wrote:

> Hi "newbie quic",
>
> anonymous IETF participation isn't really possible. Please see
> https://github.com/quicwg/base-drafts/blob/master/CONTRIBUTING.md and
> referenced documents.
>
> Thanks,
> Lars
>