Subject: [quicwg/base-drafts] Modify QPACK steam and instruction terminology (#2440)

From: Lucas Pardue <[email-masked]>
Date: Fri, 08 Feb 2019 09:44:40 -0800

This PR is more controversial than I first anticipated. It is intended to address #2339.

QPACK uses the term instruction extensively section 4 goes into great detail about all the different types. However, the instructions have been presented in terms of what streams they appear on. Which makes things a little unclear, and verbose, sometimes. I think it also conflates the idea of the true QPACK unframed encoder and decoder streams, with header block messages (sets of instructions).

So this change explicitly creates the terms encoder instruction, decoder instruction and header block instruction. It also removes a lot of the reference to request and push streams.

I'm not 100% happy with the net outcome of these changes but posting the PR for feedback.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/2440

-- Commit Summary --

  * Focus terminology on QPACK instructions rather than stream types
  * Few more tweaks

-- File Changes --

    M draft-ietf-quic-qpack.md (146)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2440.patch
https://github.com/quicwg/base-drafts/pull/2440.diff

-- 
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/2440

---

From: Mike Bishop <[email-masked]>
Date: Fri, 08 Feb 2019 13:54:03 -0800

MikeBishop approved this pull request.

In general, I think this is fine.

-The decoder stream carries information used to ensure consistency of the dynamic
-table. Information is sent from the decoder to the encoder; that is, the server
-informs the client about the processing of the client's header blocks and table
-updates, and the client informs the server about the processing of the server's
-header blocks and table updates.
+Decoder instructions provide information used to ensure consistency of the
+dynamic table. They are sent from the decoder to the encoder on a decoder stream;

Need to rewrap this paragraph -- your build break is an overlong line here.

-- 
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/2440#pullrequestreview-201801599

---

From: Martin Thomson <[email-masked]>
Date: Sun, 10 Feb 2019 15:14:07 -0800

martinthomson commented on this pull request.

-QPACK instructions occur in three locations, each of which uses a separate
-instruction space:
+There are three separate QPACK instruction spaces. Encoder instructions
+({{encoder-instructions}}) carry table updates, decoder instructions
+({{decoder-instructions}}) carry acknowledgments of table modifications and
+header processing, and header block instructions ({{header-block-instructions}})
+reference the QPACK table state.

Isn't the main purpose of header block instructions to cause header fields to be emitted?

-## Encoder Stream {#encoder-stream}
+<!-- s/exactly/no more than/  ? -->
+HTTP/3 endpoints contain a QPACK encoder and decoder. Each endpoint MUST
+initiate a single encoder stream and decoder stream. Receipt of a second
+instance of either stream type MUST treat it as a connection error of type

Check grammar: "An endpoint that receives a second" or "MUST be treated as"

> @@ -811,11 +819,13 @@ An encoder cannot infer from this instruction that any updates to the dynamic
 table have been received.

-## Request and Push Streams
+## Header Block Instructions
+
+HTTP/3 endpoints convert header lists to headers blocks and exchange them inside
+HEADERS and PUSH_PROMISE frames. Header block instructions reference the static
+table, or dynamic table in a particular state without modifying it.

Same comment as before.  The primary purpose is to convey the header field values.  That they reference various tables to do so is secondary.

-- 
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/2440#pullrequestreview-201928026

---

From: Lucas Pardue <[email-masked]>
Date: Sun, 10 Feb 2019 16:07:56 -0800

LPardue commented on this pull request.

Hmm. I think I agree with your position, which is why I have some struggle with the text as it currently stand. The changes here try to carry the existing QPACK position and text where possible. I'll take editor(s) cue on whether to broaden the changes in this PR.

AFAICT the QPACK approach was to discuss instructions and how they relate to the dynamic table. In that sense, this paragraph provides a clear statement that these instructions do not mutate.

-- 
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/2440#discussion_r255364068

---

From: Lucas Pardue <[email-masked]>
Date: Sun, 10 Feb 2019 16:08:22 -0800

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/2440#discussion_r255364095

---

From: Martin Thomson <[email-masked]>
Date: Sun, 10 Feb 2019 17:02:50 -0800

I'm OK with that aspect of this.  However, I would first say what these are for, then explain that they don't refer to the tables, but don't modify them.

-- 
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/2440#discussion_r255366771

---

From: Mike Bishop <[email-masked]>
Date: Mon, 11 Feb 2019 16:27:02 -0800

MikeBishop commented on this pull request.

+compress a header list by referring to the QPACK table state.

Well, they don't really *compress* the header list, they are the compression output.  Perhaps "reconstruct" a header list?

-- 
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/2440#pullrequestreview-202417440

---

From: Mike Bishop <[email-masked]>
Date: Mon, 11 Feb 2019 16:29:06 -0800

> @@ -1128,7 +1141,7 @@ The entries in the following table are registered by this document.
 
 ## Stream Type Registration
 
-This document creates two new settings in the "HTTP/3 Stream Type" registry
+This document creates two new streams in the "HTTP/3 Stream Type" registry

Perhaps "types" or "entries"?

-- 
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/2440#pullrequestreview-202417944

---

From: Lucas Pardue <[email-masked]>
Date: Mon, 11 Feb 2019 16:40:31 -0800

The change addressed a bug in the spec. I'm happy to use entries but would do so consistently through the document (settings, errors, stream)

-- 
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/2440#discussion_r255756823

---

From: Lucas Pardue <[email-masked]>
Date: Mon, 11 Feb 2019 16:47:18 -0800

Well, an encoder view of the header block is not to reconstruct the list. 

The initial text simply stated that these instructions refer to tables. Adding flavour seems to complicate the matter, unless we think spelling out verbatim is useful. 

Would something like "header block instructions convey an encoded representation of a header list by referring to the QPACK table state" suffice?

-- 
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/2440#discussion_r255758199

---

From: Mike Bishop <[email-masked]>
Date: Mon, 11 Feb 2019 16:56:25 -0800

WFM.

-- 
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/2440#discussion_r255760003

---

From: Mike Bishop <[email-masked]>
Date: Mon, 11 Feb 2019 16:58:11 -0800

Well, arguably, the other two actually are creating new settings and error types.  But yes, if you want to fix the bug and remain consistent, this should probably be "types".

-- 
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/2440#discussion_r255760308

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 12 Feb 2019 03:17:31 -0800

@LPardue pushed 1 commit.

5897a2005cba410e5a39ce9ac2744fe499cd0e47  Mike's feedback

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2440/files/ac2cb093b7996bc18d375754c9b3e453426c6e35..5897a2005cba410e5a39ce9ac2744fe499cd0e47

---

From: afrind <[email-masked]>
Date: Tue, 12 Feb 2019 22:26:13 +0000 (UTC)

afrind approved this pull request.

-This section describes the instructions which are possible on each stream type.
+ - A decoder stream is a unidirectional stream of type `0x68` (ASCII 'h').
+   It carries an unframed sequence of decoder instructions from decoder
+   to encoder.
+
+<!-- s/exactly/no more than/  ? -->
+HTTP/3 endpoints contain a QPACK encoder and decoder. Each endpoint MUST

I think I'll change this to MAY in a subsequent PR

I'm OK with the text here.

-- 
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/2440#pullrequestreview-202938360

---

From: afrind <[email-masked]>
Date: Tue, 12 Feb 2019 14:28:56 -0800

@afrind pushed 1 commit.

123d04f35532324f616bd8ccf248575eedc70f67  Merge branch 'master' into qpack-instructions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2440/files/5897a2005cba410e5a39ce9ac2744fe499cd0e47..123d04f35532324f616bd8ccf248575eedc70f67

---

From: afrind <[email-masked]>
Date: Tue, 12 Feb 2019 15:07:34 -0800

Merged #2440 into master.

-- 
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/2440#event-2135213847