Re: [quicwg/base-drafts] Define AreAllPacketsLost() (#3290)

ianswett <notifications@github.com> Fri, 06 December 2019 18:40 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 689941200F7 for <quic-issues@ietfa.amsl.com>; Fri, 6 Dec 2019 10:40:50 -0800 (PST)
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 kC3Me78oykN6 for <quic-issues@ietfa.amsl.com>; Fri, 6 Dec 2019 10:40:48 -0800 (PST)
Received: from out-1.smtp.github.com (out-1.smtp.github.com [192.30.252.192]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 84D3412006D for <quic-issues@ietf.org>; Fri, 6 Dec 2019 10:40:48 -0800 (PST)
Date: Fri, 06 Dec 2019 10:40:47 -0800
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1575657647; bh=4U9GAe4Iq+IE8H5xX/T6OQN9+XU0N9l3/K62hZ5oidk=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=rqXzPQF8v0amJBvylcIaRDwYY1HsnurovMy2CW6jh7yKZqUOThleHtBcykco1xwLy 0b1PQCSGqy/g799zN3yGKHdQSCn5sllIRPwCWw04eDMzMQnIEAGq9k6EuqExd9YQIJ cl9vJJRYEqn7BRFbv9alCgmxDzswQjjRWO1o10F0=
From: ianswett <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJKY4J2DY4WD2RJ2QJX5367JS7EVBNHHB7VVU2M@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/3290/review/328385993@github.com>
In-Reply-To: <quicwg/base-drafts/pull/3290@github.com>
References: <quicwg/base-drafts/pull/3290@github.com>
Subject: Re: [quicwg/base-drafts] Define AreAllPacketsLost() (#3290)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5deaa0af743fd_6f1d3f97f72cd96c6282b"; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: ianswett
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/OvIerSG2G5yFI6yquXWpEiWI3Jg>
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: Fri, 06 Dec 2019 18:40:50 -0000

ianswett commented on this pull request.



>       pto = smoothed_rtt + max(4 * rttvar, kGranularity) +
        max_ack_delay
      congestion_period = pto * kPersistentCongestionThreshold
      // Determine if all packets in the time period before the
      // newest lost packet, including the edges, are marked
      // lost
-     return AreAllPacketsLost(largest_lost_packet,
-                              congestion_period)
+     return congestion_period >
+       largest_lost_packet.time_sent - earliest_lost_packet.time_sent

Good point, that is a problem.  I think that was why we resorted to a helper method to begin with.

For a packet between smallest and largest to be acknowledged AND smallest not previously declared lost, I believe it must occur in the ACK frame that caused smallest and largest to be lost.  Otherwise the time threshold would have kicked in long before persistent congestion.

Therefore, if the largest lost packet is smaller than the earliest newly acknowledged packet, we can guarantee the lost packets are contiguous.  However, then we do not trigger persistent congestion if a packet sent at the very beginning of the congestion period is acknowledged in a long delayed ACK, and at the same time a much later packet is ACKed.  I'm not sure we care about capturing that case, but it's worth considering.

If we were ok with more complex code, we could loop over all newly acked packets and ensure none were between the smallest and largest lost packets.

-- 
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/3290#discussion_r354975276