Re: [quicwg/base-drafts] Fix Recovery Pseudocode (#2907)

Jana Iyengar <notifications@github.com> Tue, 27 August 2019 22:34 UTC

Return-Path: <noreply@github.com>
X-Original-To: quic-issues@ietfa.amsl.com
Delivered-To: quic-issues@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 9264C12011F for <quic-issues@ietfa.amsl.com>; Tue, 27 Aug 2019 15:34:54 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -8
X-Spam-Level:
X-Spam-Status: No, score=-8 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, MAILING_LIST_MULTI=-1, RCVD_IN_DNSWL_HI=-5, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=github.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lE0txnpHYGTy for <quic-issues@ietfa.amsl.com>; Tue, 27 Aug 2019 15:34:52 -0700 (PDT)
Received: from out-24.smtp.github.com (out-24.smtp.github.com [192.30.252.207]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C810F12004D for <quic-issues@ietf.org>; Tue, 27 Aug 2019 15:34:52 -0700 (PDT)
Date: Tue, 27 Aug 2019 15:34:51 -0700
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1566945291; bh=Mw+cTTS8G+3zm7OlEQUI4aIfg8SPwe4e6BmX7RQdo/Q=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=COvJtb890Jl77e6WE09WcqXtbImeSLk1cb6lhYb4oBoTuD26bnCBb+JGDFggOKM/5 aRtfVpZy66UFjyvSLYGJon8Ds4AYa46E4B0HXXlh++IGU5ysPHzrXrqEk45v3Uh6g2 XnLvm66m0pAeddxN1dqvRS3sDAfpkTVJuGDlm3ZQ=
From: Jana Iyengar <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJK3B2WFIZ2KPKUH72VF3OLYJXEVBNHHBX66EHQ@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/2907/review/280506163@github.com>
In-Reply-To: <quicwg/base-drafts/pull/2907@github.com>
References: <quicwg/base-drafts/pull/2907@github.com>
Subject: Re: [quicwg/base-drafts] Fix Recovery Pseudocode (#2907)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5d65b00bcd698_61373fae270cd9643102e8"; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: janaiyengar
X-GitHub-Recipient: quic-issues
X-GitHub-Reason: subscribed
X-Auto-Response-Suppress: All
X-GitHub-Recipient-Address: quic-issues@ietf.org
Archived-At: <https://mailarchive.ietf.org/arch/msg/quic-issues/xsGXcyb4a8haTUeFAnBk-vdIBw0>
X-BeenThere: quic-issues@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Notification list for GitHub issues related to the QUIC WG <quic-issues.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/quic-issues/>
List-Post: <mailto:quic-issues@ietf.org>
List-Help: <mailto:quic-issues-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 27 Aug 2019 22:34:55 -0000

janaiyengar requested changes on this pull request.



> @@ -580,14 +580,17 @@ data at Handshake encryption MUST be retransmitted before any ApplicationData
 data.  If no data can be sent, then the PTO alarm MUST NOT be armed until
 data has been received from the client.
 
-Because the server could be blocked until more packets are received, the client
-MUST ensure that the retransmission timer is set if the client does not yet
-have 1-RTT keys.  If the probe timer expires before the client has 1-RTT keys,
-it is possible that the client may not have any crypto data to retransmit.
-However, the client MUST send a new packet to allow the server to continue
-sending data.  At this stage in the connection, when few to none RTT samples
-have been generated, it is possible that the probe timer expiration is due to
-an incorrect RTT estimate at the client. To allow the client to improve its RTT
+Since the server could be blocked until more packets are received from the
+client, it is the client's responsibility to send packets to unblock the server
+until it is certain that the server has finished its address validation
+(see Section 8 of {{QUIC-TRANSPORT}}).  That is, the client MUST set the
+probe timer if one of the following conditions is true: the client
+does not yet have 1-RTT keys, or the client has not received an acknowledgement

```suggestion
does not yet have handshake confirmation, or the client has not received an acknowledgement
```

> @@ -1190,9 +1193,13 @@ SetLossDetectionTimer():
     return
 
   // Don't arm timer if there are no ack-eliciting packets
-  // in flight and the handshake is complete.
-  if (endpoint is client with 1-RTT keys &&
-      no ack-eliciting packets in flight):
+  // in flight and the endpoint is a server. Arm the
+  // timer until the client has received a Handshake ACK
+  // or has completed the handshake.
+  if (no ack-eliciting packets in flight &&
+      (endpoint is server ||
+       has 1-RTT keys ||
+       has received Handshake ACK)):

Chatting about this at the editors meeting, @martinthomson pointed out that this isn't quite right. Here's my reformulation that I believe is the right condition:
"endpoint is server || until the handshake is confirmed || has received Handshake ACK"

-- 
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/2907#pullrequestreview-280506163