Re: [quicwg/base-drafts] Add note about stale ACK frames breaking ECN (#2879)

Martin Thomson <notifications@github.com> Tue, 09 July 2019 04:28 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 3FB331200FD for <quic-issues@ietfa.amsl.com>; Mon, 8 Jul 2019 21:28:16 -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 qcGndzUjSXl5 for <quic-issues@ietfa.amsl.com>; Mon, 8 Jul 2019 21:28:13 -0700 (PDT)
Received: from out-7.smtp.github.com (out-7.smtp.github.com [192.30.252.198]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id AC00112023F for <quic-issues@ietf.org>; Mon, 8 Jul 2019 21:28:13 -0700 (PDT)
Date: Mon, 08 Jul 2019 21:28:12 -0700
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1562646492; bh=2HxrNiEpXBPUcXfRWibCiljGKnR5CcgADr9pZ3Z9F1A=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=aoC9oPvGxEJa1RzBMl4tuxkdD/lQ3dmXOfz/AXZffEy2MJDuK3HCZa2ageS4KRchS FTNoczDMQHREFBbXWgOognSAm8/LiFF2IQO7xU1ruKK58JlBgXM7t+Ov7YbFptuCyO M1kX/7YHjkoQcMqg3apTs2QfoSGBwOBr6LyEbBTI=
From: Martin Thomson <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJK3CT3XDMIRYAHBHKF53GFFFZEVBNHHBXP7DOI@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/2879/review/259262482@github.com>
In-Reply-To: <quicwg/base-drafts/pull/2879@github.com>
References: <quicwg/base-drafts/pull/2879@github.com>
Subject: Re: [quicwg/base-drafts] Add note about stale ACK frames breaking ECN (#2879)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5d2417dc58350_32f3fd988acd964107321e"; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: martinthomson
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/Y84SwxML2MwVt7Ez63ituuSIbro>
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, 09 Jul 2019 04:28:17 -0000

martinthomson commented on this pull request.

I don't think that the conditions are quite right here.

> @@ -3154,11 +3154,13 @@ to be greater than the number of packets acknowledged in an ACK frame.  When
 this happens, and if verification succeeds, the local reference counts MUST be
 increased to match the counts in the ACK frame.
 
-Processing counts out of order can result in verification failure.  An endpoint
-SHOULD NOT perform this verification if the ACK frame is received in a packet
-with packet number lower than a previously received ACK frame.  Verifying based
-on ACK frames that arrive out of order can result in disabling ECN
-unnecessarily.
+Processing counts out of order can result in verification failure.  This can
+happen when packets containing ACK frames are reordered in the network, or if a
+sender retransmits an ACK frame with stale information.  An endpoint SHOULD NOT
+perform this verification if the ACK frame is received in a packet with packet
+number lower than a previously received ACK frame or if the ACK frame does not
+acknowledge any new packets. ACK frames that arrive out of order can result in

```suggestion
acknowledge any new packets. Validating ACK frames in those conditions can result in
```

> @@ -3154,11 +3154,13 @@ to be greater than the number of packets acknowledged in an ACK frame.  When
 this happens, and if verification succeeds, the local reference counts MUST be
 increased to match the counts in the ACK frame.
 
-Processing counts out of order can result in verification failure.  An endpoint
-SHOULD NOT perform this verification if the ACK frame is received in a packet
-with packet number lower than a previously received ACK frame.  Verifying based
-on ACK frames that arrive out of order can result in disabling ECN
-unnecessarily.
+Processing counts out of order can result in verification failure.  This can
+happen when packets containing ACK frames are reordered in the network, or if a
+sender retransmits an ACK frame with stale information.  An endpoint SHOULD NOT
+perform this verification if the ACK frame is received in a packet with packet
+number lower than a previously received ACK frame or if the ACK frame does not
+acknowledge any new packets. ACK frames that arrive out of order can result in

Question: is it possible to skip validation based on just one of these conditions?  I think that only the packet number condition is really necessary.

Rationale: Say I ACK 1-4 then later drop that range (for any reason) and ACK 5-9 in another packet.  If those packets get reordered, then when the first eventually arrives, it will have new acknowledgments, but it will still have bad ECN counts.  So you can't say that new acknowledgments is necessary.

The packet number is clearly the thing to use - and sufficient on its own - but would it be enough to look for increases in largest acknowledged?  ACK processors should not be expected to have access to packet numbers.  Allowing implementations to use largest acknowledged would depend on us not allowing that to regress, but that seems like it follows from existing advice.

-- 
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/2879#pullrequestreview-259262482