Re: [quicwg/base-drafts] QPACK [editorial] Misc minor editorial changes. (#2942)

Bence Béky <notifications@github.com> Wed, 16 October 2019 14:19 UTC

Return-Path: <bounces+848413-a050-quic-issues=ietf.org@sgmail.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 CFA081200F4 for <quic-issues@ietfa.amsl.com>; Wed, 16 Oct 2019 07:19:05 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -3
X-Spam-Level:
X-Spam-Status: No, score=-3 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_NONE=-0.0001, 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 X2aVFUmJ4iUH for <quic-issues@ietfa.amsl.com>; Wed, 16 Oct 2019 07:19:03 -0700 (PDT)
Received: from o3.sgmail.github.com (o3.sgmail.github.com [192.254.112.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 2BF52120026 for <quic-issues@ietf.org>; Wed, 16 Oct 2019 07:19:03 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=github.com; h=from:reply-to:to:cc:in-reply-to:references:subject:mime-version:content-type:content-transfer-encoding:list-id:list-archive:list-post:list-unsubscribe; s=s20150108; bh=6GZYff7plez+jRgXwkQ7gpexPxw=; b=qjJWvNzwo3RTpsJ8 39ycvzRvWU/3jgHWL73sVE4hVH26isMtSypM3MAjUmVDvatZoIDORm7/OPXMnkCn J3KgIGMUdoUmmd2xWa+vhqALKZ5YY+wJryVBWuI99ebSgoBPeDkmEm43zWYW673w p4ldXkwdHaBuB5cfD2PB3CHGc4s=
Received: by filter1525p1mdw1.sendgrid.net with SMTP id filter1525p1mdw1-3783-5DA726D3-61 2019-10-16 14:18:59.760698417 +0000 UTC m=+483849.431685613
Received: from github-lowworker-5825cd4.ac4-iad.github.net (unknown [140.82.115.70]) by ismtpd0023p1iad2.sendgrid.net (SG) with ESMTP id Wjg_NDoHQ8OUU9V3znAbDg for <quic-issues@ietf.org>; Wed, 16 Oct 2019 14:18:59.662 +0000 (UTC)
Received: from github.com (localhost [127.0.0.1]) by github-lowworker-5825cd4.ac4-iad.github.net (Postfix) with ESMTP id 9D0F03A80057 for <quic-issues@ietf.org>; Wed, 16 Oct 2019 07:18:59 -0700 (PDT)
Date: Wed, 16 Oct 2019 14:18:59 +0000 (UTC)
From: =?UTF-8?B?QmVuY2UgQsOpa3k=?= <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJKZDSHP42QWJQ6WDT353WRTWHEVBNHHBYU6MYI@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/2942/review/302591474@github.com>
In-Reply-To: <quicwg/base-drafts/pull/2942@github.com>
References: <quicwg/base-drafts/pull/2942@github.com>
Subject: Re: [quicwg/base-drafts] QPACK [editorial] Misc minor editorial changes. (#2942)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5da726d39bd21_2c043fd1b78cd96c227056"; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: bencebeky
X-GitHub-Recipient: quic-issues
X-GitHub-Reason: subscribed
X-Auto-Response-Suppress: All
X-GitHub-Recipient-Address: quic-issues@ietf.org
X-SG-EID: l64QuQ2uJCcEyUykJbxN122A6QRmEpucztpreh3Pak3ddJ7uWYiUgXVEW/OviFxFU8/PfBd6VwjgAD d3vdk6dt2w3ldrYxHmqkLpsu2cQjwbXXGdxpluYvss2WMrbb9e26rfBoH3EzD3SE9qM8dTM/7Lo5Z4 qGzn7suoTmmc9FdaWGMkQdwBnU3VSmC4KaPearSo3ETFEpifAA5aF3crYWEBSdZK9+S9kiIAUrGNjy Q=
Archived-At: <https://mailarchive.ietf.org/arch/msg/quic-issues/mHvr9UGoTn6ExmPsZGVn1Wc_iaU>
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: Wed, 16 Oct 2019 14:19:06 -0000

bencebeky commented on this pull request.



> @@ -952,10 +954,8 @@ entry.
 
 ### Indexed Header Field
 
-An indexed header field representation identifies an entry in either the static
-table or the dynamic table and causes that header field to be added to the
-decoded header list, as described in Section 3.2 of [RFC7541].
-<!-- is the 7541 reference still helpful here -->
+An indexed header field instruction identifies an entry in either the static

Sorry about "instruction".  I had not realized the distinction between instructions and representations when I started working on this PR, and even though later I reverted most of my related changes, I missed this one.  I'm changing it back to "representation".

As far as "add" versus "emit" is concerned, 4.5.2 has "causes to be added", 4.5.3 does not say anything explicitly, 4.5.4 and 4.5.5 has "represents", and 4.5.6 has "encodes".  Let me address this in another PR.  Until then I might as well revert to "causes to be added".

>  
 A header block that does not reference the dynamic table can use any value for
-the Base; setting Delta Base to zero is the most efficient encoding.
+the Base; setting Delta Base to zero is one of the most efficient encodings.

My motivation for the change to this sentence is that I consider the statement "setting Delta Base to zero is the most efficient encoding" to be incorrect.  I agree that it is nice to encourage 0/0, and I think this is still the case after my proposed change.  As far as requiring it, I see no advantage to that, but in any case that change might not be editorial, so I think it should be done in a separate PR.

> @@ -603,7 +607,8 @@ is used without modification.
 This document expands the definition of string literals and permits them to
 begin other than on a byte boundary.  An "N-bit prefix string literal" begins
 with the same Huffman flag, followed by the length encoded as an (N-1)-bit
-prefix integer.  The remainder of the string literal is unmodified.
+prefix integer.  The prefix size, N, can have a value between 2 and 8.  The

For comparison, RFC7541 5.1 specificly allows the prefix length to be between 1 and 8, inclusive, even though prefix lengths 1, 2, and 8 are never used in HPACK.  And that's great because now QPACK does not have to call out that the prefix can be 8 bits (for example, for Encoder Required Insert Count).  So I suggest to be as permissive here as reasonable.

If we allow N to be 1, then after the Huffman bit, the length is encoded as an 8-bit prefix integer, and not an N-1 = 0 bit prefix integer.  Is this worth adding here given that it's not used?

> +including the new entry does not exceed the table capacity.  This can be
+achieved by adding the new entry and then removing entries from the end until
+the table size is less than or equal to the table capacity.  Another option is
+to first evict entries until the size of the dynamic table is less than or equal
+to (table capacity - size of new entry), then add the new entry.  Note that a

It's a good point that this section is not normative.  Let me just revert most of my changes to this paragraph for sake of simplicity.

-- 
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/2942#discussion_r335487671