Return-Path: 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 4963D1311DB for ; Wed, 26 Dec 2018 10:41:42 -0800 (PST) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -7.086 X-Spam-Level: X-Spam-Status: No, score=-7.086 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.065, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FROM_EXCESS_BASE64=0.979, HTML_MESSAGE=0.001, MAILING_LIST_MULTI=-1, RCVD_IN_DNSWL_HI=-5, 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 qy4hdGsA8abj for ; Wed, 26 Dec 2018 10:41:38 -0800 (PST) Received: from out-6.smtp.github.com (out-6.smtp.github.com [192.30.252.197]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C64791311D4 for ; Wed, 26 Dec 2018 10:41:36 -0800 (PST) Date: Wed, 26 Dec 2018 10:41:35 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1545849695; bh=TgxMZVUQqDYJqDtuG1Rg80VbagpRmRWk3lfxNtMKhVw=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=EGcfrACLWpTFSE5zVZ0I2fRRYEasWgSHYdn6/yI6m2s8UkJgyZWFaHzUBEo5zRn1g s2Drlbedf6rT0QUGTAT/+6Z1NIQTOyyggkdOjUW1974W8Z5WBn3obIlF/cypcypzA0 buftq8qAxFOq97InKQqUW2qmNh2+Lh0UcH8lB9eE= From: =?UTF-8?B?QmVuY2UgQsOpa3k=?= Reply-To: quicwg/base-drafts To: quicwg/base-drafts Cc: Subscribed Message-ID: In-Reply-To: References: Subject: Re: [quicwg/base-drafts] Use "Insert Count" rather than "Largest Reference" (#2111) Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="--==_mimepart_5c23cb5f7a6de_58633f9e318d45b4365688"; 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 Archived-At: X-BeenThere: quic-issues@ietf.org X-Mailman-Version: 2.1.29 List-Id: Notification list for GitHub issues related to the QUIC WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Dec 2018 18:41:42 -0000 ----==_mimepart_5c23cb5f7a6de_58633f9e318d45b4365688 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit bencebeky requested changes on this pull request. I enthusiastically support this change for the following reasons: * The Required Insert Count concept inherently is zero if there are no dynamic table references ("the lowest possible value of insert count with which this header block can be decoded"). No need for awkward language like what I am proposing at issue #2251 (which I'm happy to abandon in favor of this PR). Sure one can add a clarifying note about the zero case, but this flows much better to me. * This PR silently adds clarity by using distinct terms for the abstract value "Required Insert Count" and its encoding "EncodedInsertCount". Me having proposed #2250 is a testimony that I feel strongly about this. (And I'm happy to abandon #2250 in favor of this.) * While I understand that this is purely editorial, I also feel strongly that dynamic table absolute indexing should be zero based, to match static table indexing, and indexing schemes in the majority of programming languages. I never voiced this, because issue #1650 was shut down, in large part because 0 for Largest Reference is used to indicate no dynamic table references, making it convenient to start absolute indices with 1. I support this change because it gets rid of this fairly artificial restriction, unblocking the discussion to change absolute indexing to more natural zero-based. Couple of humble suggestions below. > -index in the dynamic table becomes greater than or equal to the Largest -Reference for all header blocks the decoder has started reading from the stream. -If the decoder encounters a header block where the actual largest reference is -not equal to the Largest Reference declared in the prefix, it MAY treat this as -a stream error of type HTTP_QPACK_DECOMPRESSION_FAILED. +Each header block contains the total number of entries added to the dynamic +table - the Required Insert Count - which identifies the table state necessary +for decoding (see {{header-prefix}}). If the Required Insert Count is greater +than the number of dynamic table entries received, the stream is considered +"blocked." While blocked, header field data SHOULD remain in the blocked +stream's flow control window. When the Required Insert Count is zero, the frame +contains no references to the dynamic table and can always be processed +immediately. A stream becomes unblocked when the number of entries inserted in +the dynamic table becomes greater than or equal to the Required Insert Count for +all header blocks the decoder has started reading from the stream. If the +decoder encounters a header block where the largest Absolute Index used not s/not equal to/is not equal to/ or s/not equal to/does not equal/ > -index in the dynamic table becomes greater than or equal to the Largest -Reference for all header blocks the decoder has started reading from the stream. -If the decoder encounters a header block where the actual largest reference is -not equal to the Largest Reference declared in the prefix, it MAY treat this as -a stream error of type HTTP_QPACK_DECOMPRESSION_FAILED. +Each header block contains the total number of entries added to the dynamic +table - the Required Insert Count - which identifies the table state necessary +for decoding (see {{header-prefix}}). If the Required Insert Count is greater +than the number of dynamic table entries received, the stream is considered +"blocked." While blocked, header field data SHOULD remain in the blocked +stream's flow control window. When the Required Insert Count is zero, the frame +contains no references to the dynamic table and can always be processed +immediately. A stream becomes unblocked when the number of entries inserted in +the dynamic table becomes greater than or equal to the Required Insert Count for +all header blocks the decoder has started reading from the stream. If the +decoder encounters a header block where the largest Absolute Index used not "where the Required Insert Count does not match the definition in {{conventions-and-definitions}}, it MAY" to avoid having to talk explicitly about the case of no dynamic table references. > -Reference for all header blocks the decoder has started reading from the stream. -If the decoder encounters a header block where the actual largest reference is -not equal to the Largest Reference declared in the prefix, it MAY treat this as -a stream error of type HTTP_QPACK_DECOMPRESSION_FAILED. +Each header block contains the total number of entries added to the dynamic +table - the Required Insert Count - which identifies the table state necessary +for decoding (see {{header-prefix}}). If the Required Insert Count is greater +than the number of dynamic table entries received, the stream is considered +"blocked." While blocked, header field data SHOULD remain in the blocked +stream's flow control window. When the Required Insert Count is zero, the frame +contains no references to the dynamic table and can always be processed +immediately. A stream becomes unblocked when the number of entries inserted in +the dynamic table becomes greater than or equal to the Required Insert Count for +all header blocks the decoder has started reading from the stream. If the +decoder encounters a header block where the largest Absolute Index used not +equal to the largest value permitted by the Required Insert Count, it MAY treat s/permitted/communicated/ or s/permitted/specified/ > +---+---------------------------+ | Compressed Headers ... +-------------------------------+ ~~~~~~~~~~ {:#fig-base-index title="Frame Payload"} -#### Largest Reference +#### Insert Count s/Insert Count/Required Insert Count/ here and below throughout. > -`Largest Reference` identifies the largest absolute dynamic index referenced in -the block. Blocking decoders use the Largest Reference to determine when it is -safe to process the rest of the block. If Largest Reference is greater than -zero, the encoder transforms it as follows before encoding: +`Insert Count` identifies the state of the dynamic table needed to process the +header block successfully. Blocking decoders use the Insert Count to determine +when it is safe to process the rest of the block. If Insert Count is greater Explicitly include the case of RequiredInsertCount == 0. (Currently EncodedInsertCount is undefined in that case.) > ~~~~~~~~~~ drawing 0 1 2 3 4 5 6 7 +---+---+---+---+---+---+---+---+ -| Largest Reference (8+) | +| Required Insert Count (8+) | s/Required Insert Count/EncodedInsertCount/ > @@ -784,29 +798,29 @@ streams emit the headers for an HTTP request or response. ### Header Data Prefix {#header-prefix} -Header data is prefixed with two integers, `Largest Reference` and `Base Index`. +Header data is prefixed with two integers, `Required Insert Count` and `Base`. In wire format sections I usually see an explicit textual description of all fields. Consider adding some text similar to https://github.com/quicwg/base-drafts/pull/2250/files#diff-c3ed24528f71fade042cdefbeadbd7b9R787. > @@ -249,18 +250,19 @@ Because QUIC does not guarantee order between data on different streams, a header block might reference an entry in the dynamic table that has not yet been received. -Each header block contains a Largest Reference ({{header-prefix}}) which -identifies the table state necessary for decoding. If the greatest absolute -index in the dynamic table is less than the value of the Largest Reference, the -stream is considered "blocked." While blocked, header field data SHOULD remain -in the blocked stream's flow control window. When the Largest Reference is -zero, the frame contains no references to the dynamic table and can always be -processed immediately. A stream becomes unblocked when the greatest absolute -index in the dynamic table becomes greater than or equal to the Largest -Reference for all header blocks the decoder has started reading from the stream. -If the decoder encounters a header block where the actual largest reference is -not equal to the Largest Reference declared in the prefix, it MAY treat this as -a stream error of type HTTP_QPACK_DECOMPRESSION_FAILED. +Each header block contains the total number of entries added to the dynamic Maybe "Each header block contains a Required Insert Count, the lowest possible value for the Insert Count with which the header block can be decoded. For a header block with no references to the dynamic table, the Required Insert Count is zero."? > n = count of entries inserted d = count of entries dropped ~~~~~ {: title="Example Dynamic Table Indexing - Control Stream"} Because frames from request streams can be delivered out of order with -instructions on the encoder stream, relative indices are relative to the Base -Index at the beginning of the header block (see {{header-prefix}}). The Base -Index is an absolute index. When interpreting the rest of the frame, the entry -identified by Base Index has a relative index of zero. The relative indices of -entries do not change while interpreting headers on a request or push stream. +instructions on the encoder stream, relative indices are relative to the Base at +the beginning of the header block (see {{header-prefix}}). The Base is encoded "Unlike on the encoder stream, relative indices on push and request streams are relative to the Base at the beginning of the header block (see bla). This allows for the mapping from relative indices to absolute indices to not be affected by dynamic table insertions on the encoder stream, regardless of what order the decoder processes request stream and encoder stream instructions.", and scratch the paragraph "Though new entries..." below. Processing order encompasses on-the-wire reordering as well as the parallel processing case, and specifically talking about "mapping" makes it unnecessary to mention blocking in this sentence. > @@ -923,10 +948,9 @@ header field name matches the header field name of an entry stored in the static table or the dynamic table. If the entry is in the static table, or in the dynamic table with an absolute -index less than or equal to Base Index, this representation starts with the '01' -two-bit pattern. If the entry is in the dynamic table with an absolute index -greater than Base Index, the representation starts with the '0000' four-bit -pattern. +index less than the Base, this representation starts with the '01' two-bit Are you changing one-based absolute indexing to zero-based in this PR? Then update 3.2.4 Absolute Indexing, #indexing. Or maybe do this in a separate PR? > ### Literal Header Field With Post-Base Name Reference -For entries in the dynamic table with an absolute index greater than Base Index, +For entries in the dynamic table with an absolute index greater than the Base, If you change "less than or equal to" to "less than" above, maybe "greate than" here should be changed to "greater than or equal to"? -- 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/2111#pullrequestreview-187935117 ----==_mimepart_5c23cb5f7a6de_58633f9e318d45b4365688 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

@bencebeky requested changes on this pull request.

I enthusiastically support this change for the following reasons:

  • The Required Insert Count concept inherently is zero if there are no = dynamic table references ("the lowest possible value of insert count with= which this header block can be decoded"). No need for awkward language = like what I am proposing at issue #2251 (which I'm happy to= abandon in favor of this PR). Sure one can add a clarifying note about = the zero case, but this flows much better to me.
  • This PR silently adds clarity by using distinct terms for the abstrac= t value "Required Insert Count" and its encoding "EncodedInsertCount". M= e having proposed #2250 is a testimony that I feel strongly a= bout this. (And I'm happy to abandon #2250 in favor of thi= s.)
  • While I understand that this is purely editorial, I also feel strongl= y that dynamic table absolute indexing should be zero based, to match sta= tic table indexing, and indexing schemes in the majority of programming l= anguages. I never voiced this, because issue #1650 was shu= t down, in large part because 0 for Largest Reference is used to indicate= no dynamic table references, making it convenient to start absolute indi= ces with 1. I support this change because it gets rid of this fairly art= ificial restriction, unblocking the discussion to change absolute indexin= g to more natural zero-based.

Couple of humble suggestions below.

In draft-ietf-quic-qpack.md:

> -index in the dynamic table becomes greate=
r than or equal to the Largest
-Reference for all header blocks the decoder has started reading from the=
 stream.
-If the decoder encounters a header block where the actual largest refere=
nce is
-not equal to the Largest Reference declared in the prefix, it MAY treat =
this as
-a stream error of type HTTP_QPACK_DECOMPRESSION_FAILED.
+Each header block contains the total number of entries added to the dyna=
mic
+table - the Required Insert Count - which identifies the table state nec=
essary
+for decoding (see {{header-prefix}}). If the Required Insert Count is gr=
eater
+than the number of dynamic table entries received, the stream is conside=
red
+"blocked."  While blocked, header field data SHOULD remain in =
the blocked
+stream's flow control window.  When the Required Insert Count is zer=
o, the frame
+contains no references to the dynamic table and can always be processed
+immediately. A stream becomes unblocked when the number of entries inser=
ted in
+the dynamic table becomes greater than or equal to the Required Insert C=
ount for
+all header blocks the decoder has started reading from the stream.  If t=
he
+decoder encounters a header block where the largest Absolute Index used =
not

s/not equal to/is not equal to/ or s/not equal to/does not equal/

In draft-ietf-quic-qpack.md:

> -index in the dynamic table becomes greate=
r than or equal to the Largest
-Reference for all header blocks the decoder has started reading from the=
 stream.
-If the decoder encounters a header block where the actual largest refere=
nce is
-not equal to the Largest Reference declared in the prefix, it MAY treat =
this as
-a stream error of type HTTP_QPACK_DECOMPRESSION_FAILED.
+Each header block contains the total number of entries added to the dyna=
mic
+table - the Required Insert Count - which identifies the table state nec=
essary
+for decoding (see {{header-prefix}}). If the Required Insert Count is gr=
eater
+than the number of dynamic table entries received, the stream is conside=
red
+"blocked."  While blocked, header field data SHOULD remain in =
the blocked
+stream's flow control window.  When the Required Insert Count is zer=
o, the frame
+contains no references to the dynamic table and can always be processed
+immediately. A stream becomes unblocked when the number of entries inser=
ted in
+the dynamic table becomes greater than or equal to the Required Insert C=
ount for
+all header blocks the decoder has started reading from the stream.  If t=
he
+decoder encounters a header block where the largest Absolute Index used =
not

"where the Required Insert Count does not match the definition in {{co= nventions-and-definitions}}, it MAY" to avoid having to talk explicitly a= bout the case of no dynamic table references.

In draft-ietf-quic-qpack.md:

> -Reference for all header blocks the decod=
er has started reading from the stream.
-If the decoder encounters a header block where the actual largest refere=
nce is
-not equal to the Largest Reference declared in the prefix, it MAY treat =
this as
-a stream error of type HTTP_QPACK_DECOMPRESSION_FAILED.
+Each header block contains the total number of entries added to the dyna=
mic
+table - the Required Insert Count - which identifies the table state nec=
essary
+for decoding (see {{header-prefix}}). If the Required Insert Count is gr=
eater
+than the number of dynamic table entries received, the stream is conside=
red
+"blocked."  While blocked, header field data SHOULD remain in =
the blocked
+stream's flow control window.  When the Required Insert Count is zer=
o, the frame
+contains no references to the dynamic table and can always be processed
+immediately. A stream becomes unblocked when the number of entries inser=
ted in
+the dynamic table becomes greater than or equal to the Required Insert C=
ount for
+all header blocks the decoder has started reading from the stream.  If t=
he
+decoder encounters a header block where the largest Absolute Index used =
not
+equal to the largest value permitted by the Required Insert Count, it MA=
Y treat

s/permitted/communicated/ or s/permitted/specified/

In draft-ietf-quic-qpack.md:

>  +---+---------------------------+
 |      Compressed Headers     ...
 +-------------------------------+
 ~~~~~~~~~~
 {:#fig-base-index title=3D"Frame Payload"}
 =

-#### Largest Reference
+#### Insert Count

s/Insert Count/Required Insert Count/ here and below throughout.

In draft-ietf-quic-qpack.md:

>  =

-`Largest Reference` identifies the largest absolute dynamic index refere=
nced in
-the block.  Blocking decoders use the Largest Reference to determine whe=
n it is
-safe to process the rest of the block.  If Largest Reference is greater =
than
-zero, the encoder transforms it as follows before encoding:
+`Insert Count` identifies the state of the dynamic table needed to proce=
ss the
+header block successfully.  Blocking decoders use the Insert Count to de=
termine
+when it is safe to process the rest of the block.  If Insert Count is gr=
eater

Explicitly include the case of RequiredInsertCount =3D=3D 0. (Current= ly EncodedInsertCount is undefined in that case.)

In draft-ietf-quic-qpack.md:

>  =

 ~~~~~~~~~~  drawing
   0   1   2   3   4   5   6   7
 +---+---+---+---+---+---+---+---+
-|     Largest Reference (8+)    |
+|   Required Insert Count (8+)  |

s/Required Insert Count/EncodedInsertCount/

In draft-ietf-quic-qpack.md:

> @@ -784,29 +798,29 @@ streams emit the hea=
ders for an HTTP request or response.
 =

 ### Header Data Prefix {#header-prefix}
 =

-Header data is prefixed with two integers, `Largest Reference` and `Base=
 Index`.
+Header data is prefixed with two integers, `Required Insert Count` and `=
Base`.

In wire format sections I usually see an explicit textual description = of all fields. Consider adding some text similar to https://github.com/quicwg/base-drafts/pull/2250/files#diff= -c3ed24528f71fade042cdefbeadbd7b9R787.

In draft-ietf-quic-qpack.md:

> @@ -249,18 +250,19 @@ Because QUIC does no=
t guarantee order between data on different streams, a
 header block might reference an entry in the dynamic table that has not =
yet been
 received.
 =

-Each header block contains a Largest Reference ({{header-prefix}}) which=

-identifies the table state necessary for decoding. If the greatest absol=
ute
-index in the dynamic table is less than the value of the Largest Referen=
ce, the
-stream is considered "blocked."  While blocked, header field d=
ata SHOULD remain
-in the blocked stream's flow control window.  When the Largest Refer=
ence is
-zero, the frame contains no references to the dynamic table and can alwa=
ys be
-processed immediately. A stream becomes unblocked when the greatest abso=
lute
-index in the dynamic table becomes greater than or equal to the Largest
-Reference for all header blocks the decoder has started reading from the=
 stream.
-If the decoder encounters a header block where the actual largest refere=
nce is
-not equal to the Largest Reference declared in the prefix, it MAY treat =
this as
-a stream error of type HTTP_QPACK_DECOMPRESSION_FAILED.
+Each header block contains the total number of entries added to the dyna=
mic

Maybe "Each header block contains a Required Insert Count, the lowest = possible value for the Insert Count with which the header block can be de= coded. For a header block with no references to the dynamic table, the R= equired Insert Count is zero."?

In draft-ietf-quic-qpack.md:

>  =

 n =3D count of entries inserted
 d =3D count of entries dropped
 ~~~~~
 {: title=3D"Example Dynamic Table Indexing - Control Stream"}
 =

 Because frames from request streams can be delivered out of order with
-instructions on the encoder stream, relative indices are relative to the=
 Base
-Index at the beginning of the header block (see {{header-prefix}}). The =
Base
-Index is an absolute index. When interpreting the rest of the frame, the=
 entry
-identified by Base Index has a relative index of zero.  The relative ind=
ices of
-entries do not change while interpreting headers on a request or push st=
ream.
+instructions on the encoder stream, relative indices are relative to the=
 Base at
+the beginning of the header block (see {{header-prefix}}). The Base is e=
ncoded

"Unlike on the encoder stream, relative indices on push and request st= reams are relative to the Base at the beginning of the header block (see = bla). This allows for the mapping from relative indices to absolute indi= ces to not be affected by dynamic table insertions on the encoder stream,= regardless of what order the decoder processes request stream and encode= r stream instructions.", and scratch the paragraph "Though new entries...= " below. Processing order encompasses on-the-wire reordering as well as = the parallel processing case, and specifically talking about "mapping" ma= kes it unnecessary to mention blocking in this sentence.

In draft-ietf-quic-qpack.md:

> @@ -923,10 +948,9 @@ header field name mat=
ches the header field name of an entry stored in the static
 table or the dynamic table.
 =

 If the entry is in the static table, or in the dynamic table with an abs=
olute
-index less than or equal to Base Index, this representation starts with =
the '01'
-two-bit pattern.  If the entry is in the dynamic table with an absolute =
index
-greater than Base Index, the representation starts with the '0000=
9; four-bit
-pattern.
+index less than the Base, this representation starts with the '01=
9; two-bit

Are you changing one-based absolute indexing to zero-based in this PR?= Then update 3.2.4 Absolute Indexing, #indexing. Or maybe do this in a = separate PR?

In draft-ietf-quic-qpack.md:

>  =

 ### Literal Header Field With Post-Base Name Reference
 =

-For entries in the dynamic table with an absolute index greater than Bas=
e Index,
+For entries in the dynamic table with an absolute index greater than the=
 Base,

If you change "less than or equal to" to "less than" above, maybe "gre= ate than" here should be changed to "greater than or equal to"?

&m= dash;
You are receiving this because you are subscribed to this thre= ad.
Reply to this email directly, view it on GitHub, or mute the thread.

= = ----==_mimepart_5c23cb5f7a6de_58633f9e318d45b4365688--