Subject: Re: [quicwg/base-drafts] Add EXTENSION frame (#1068)

From: Martin Thomson <[email-masked]>
Date: Wed, 04 Apr 2018 08:13:56 +0000 (UTC)

I've just updated this PR based on #1072.  It now uses @vasilvv's design, with - I hope - the comments on both that PR and this addressed.

-- 
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/1068#issuecomment-378518233

---

From: Dmitri Tikhonov <[email-masked]>
Date: Wed, 04 Apr 2018 12:27:43 +0000 (UTC)

dtikhonov commented on this pull request.

> +| 0x02        | CONNECTION_CLOSE   | {{frame-connection-close}}  |
+| 0x03        | APPLICATION_CLOSE  | {{frame-application-close}} |
+| 0x04        | MAX_DATA           | {{frame-max-data}}          |
+| 0x05        | MAX_STREAM_DATA    | {{frame-max-stream-data}}   |
+| 0x06        | MAX_STREAM_ID      | {{frame-max-stream-id}}     |
+| 0x07        | PING               | {{frame-ping}}              |
+| 0x08        | BLOCKED            | {{frame-blocked}}           |
+| 0x09        | STREAM_BLOCKED     | {{frame-stream-blocked}}    |
+| 0x0a        | STREAM_ID_BLOCKED  | {{frame-stream-id-blocked}} |
+| 0x0b        | NEW_CONNECTION_ID  | {{frame-new-connection-id}} |
+| 0x0c        | STOP_SENDING       | {{frame-stop-sending}}      |
+| 0x0d        | ACK                | {{frame-ack}}               |
+| 0x0e        | PATH_CHALLENGE     | {{frame-path-challenge}}    |
+| 0x0f        | PATH_RESPONSE      | {{frame-path-response}}     |
+| 0x10 - 0x17 | STREAM             | {{frame-stream}}            |
+| 0x80 - 0xff | (extension frames) | {{frame-extension}}         |

Maybe save `0xFF` for a "super-extension" to allow for more than 128 extended frames?

-- 
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/1068#pullrequestreview-109302831

---

From: ekr <[email-masked]>
Date: Wed, 04 Apr 2018 13:17:35 +0000 (UTC)

I'm not a huge fan of this design, for two reasons:

1. The mapping seems complex, and really only serves to save a few bytes. As I think I observed in Melbourne, given that QUIC already has variable length integers and we are only presently defining a very small number of frame types, why don't simply declare the frame type to be a varint. Then we will have an unlimited number of extension frame types, and we can assign low (hence short) frame types to anything that becomes a standard (thus not paying any bytes there) and non-standard frame types can just use longer numbers, without any mapping.

2. Because this design couples frame type negotiation and its code point, that means that if you want to negotiate parameters for the use of extension frames, you then need a separate transport parameters negotiation for *that*

I'm certainly sympathetic for people's desire to not have a bunch of transport parameters messages whose only purpose is to signal "I support feature X" (this is kind of an annoyance in TLS) but I think that can actually be dealt with in a cleaner way: Simply have TP contain a list of numbers (varints) which define the features that you are advertising. This wouldn't be the only way to do things and so if you had a feature that needed negotiation, you could still do it that way, but simple features could be indicated this way.

-- 
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/1068#issuecomment-378596154

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 04 Apr 2018 09:33:28 -0700

I do not have a strong opinion on if we should use TP for negotiating extensions, however I agree with @ekr that doing some negotiation in TP and some by using frames seems awkward to me. It might make sense to do all in TP or do all in frames (as we have done in HTTP/2).

I have two other concerns about the proposed approach.

The first concern is the approach expecting an extension to always introduce a new frame type. I wonder if that assumption would always be true. Consider the case when we would have a different PNE mechanism. Use of the mechanism needs to be negotiated. However, it would not introduce a new frame; since it just changes how the packet is formatted. Therefore, I'd prefer defining a mapping that maps a feature to *zero* or more frame IDs, rather than a one-to-one mapping.

The second concern is frame_type getting ossified. Consider the case when we adopt the first extension named Foo. Implementations that only support Foo will likely assign 0x80 as the frame_type (endpoints can unilaterally declare the mapping). The same frame_type (0x80) will be used to designate the same extension, when sending *and* receiving. My fear is that I doubt if we can be sure that all the implementations will work correctly even in case different frame_types are assigned for sending and receiving the extension in such scenario. However, unless all implementations succeed in implementing it correctly, the frame_type will become ossified. Therefore, I'd prefer greasing the frame_type.

-- 
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/1068#issuecomment-378663899

---

From: Christian Huitema <[email-masked]>
Date: Wed, 04 Apr 2018 19:15:00 +0000 (UTC)

Now that I read EKR's comment, I believe he might be correct. Or maybe not. Defining the frame type as a varint would remove the need for dynamic mapping. On the other hand, there will still be a need for a negotiation, to check whether the option is actually supported by the peer. The implementation in C would be:

1) With the current mapping definition, use a table of function pointers to map the extension frame type to the type specific coding/processing functions. The mapping table is filled up when processing the TP parameter. Unused values point to some error code.

2) With the varint coding of frame type, add some tests for varint decoding. Use switch statements with fixed values to point to the implementation-supported extensions. Use a table of booleans to indicate which implementation-supported extensions are valid for the connection. The boolean table is filled up when processing the TP parameter. Unsupported values and non negotiated values point to some error code.

This seems pretty similar in term of complexity. I am not enthusiastic about varint decoding of frame types, and I think if would only be needed if some application required more that 128 extension frames simultaneously for a given connection. All things considered, I like the currently proposed design.

-- 
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/1068#issuecomment-378713814

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Apr 2018 02:52:30 +0000 (UTC)

Just to clear something up: this establishes what frames mean.  It is NOT feature negotiation.  If, as in @kazuho's example, we want to add a new packet number encryption scheme, we could do that with a new transport parameter.  It makes no sense to do this.

If we want to add simple feature negotiation, then we would do that with empty extensions, or as @ekr suggests with a single "feature list" extension.  Again, it makes no sense to use this mechanism for that unless those features also use frames.

And there I can see the potential for duplication and overlap: if a feature needs to negotiate use of frames, then it can use this mechanism and it probably doesn't need a separate negotiation mechanism.  If the feature doesn't use frames, then it can use its own mechanism (or a feature list extension if we added that).  But if the new feature also has parameters, it needs to use both this mechanism and a separate transport parameter.  This mechanism does in a sense overlap with a feature's own negotiation, or a feature list mechanism if you think of it that way.  I don't think that's such a big problem though.

I'm undecided on the varint idea.  It seems to critically depend on also having a length field on every frame.  I suspect that this is what @ekr wants, but I have heard pretty strong opposition to that notion from others.

-- 
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/1068#issuecomment-378805572

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Apr 2018 02:53:35 +0000 (UTC)

martinthomson commented on this pull request.

As opposed to...say 0x7f, which remains unassigned?  I'd say that if we haven't changed the QUIC version number by that point, something went (badly) wrong.

-- 
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/1068#discussion_r179339162

---

From: ekr <[email-masked]>
Date: Thu, 05 Apr 2018 03:12:16 +0000 (UTC)

On Wed, Apr 4, 2018 at 7:52 PM, Martin Thomson <[email-masked]>
wrote:

> Just to clear something up: this establishes what frames mean. It is NOT
> feature negotiation. If, as in @kazuho <https://github.com/kazuho>'s
> example, we want to add a new packet number encryption scheme, we could do
> that with a new transport parameter. It makes no sense to do this.
>
Well, as you acknowledge below, it is feature negotiation for features that
need new frames.

I'm undecided on the varint idea. It seems to critically depend on also
> having a length field on every frame. I suspect that this is what @ekr
> <https://github.com/ekr> wants, but I have heard pretty strong opposition
> to that notion from others.
>
Why does it depend on having a length field for the frame? You would still
be forbidden from sending non-negotiated frame types. The varint is just a
hack to keep the size of the type byte down.

-- 
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/1068#issuecomment-378808162

---

From: Rui Paulo <[email-masked]>
Date: Thu, 05 Apr 2018 22:00:21 +0000 (UTC)

Negotiating extensions as part of the handshake is important, but do we need the {0,0xffff} -> {0x80, 0xff} mapping?  Are we expecting this mapping to be maintained for multiple QUIC versions?

-- 
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/1068#issuecomment-379089941

---

From: Mike Bishop <[email-masked]>
Date: Thu, 05 Apr 2018 23:24:44 +0000 (UTC)

MikeBishop commented on this pull request.

>        (65535)
    } TransportParameterId;
 
+   struct {
+      uint8 frame_type;
+      uint16 extension_frame_identifier;
+   } ExtensionFrameMappings<3..2^16-1>;

If I understand the TLS presentation language correctly, this makes a length-prefixed array of structs.  However, the transport parameter itself is also a length-prefixed sequence of bytes and the units within this array are fixed-length.  Do we need the length-prefix inside the length-prefixed sequence, or can we just say `optionLength / 3`?

-- 
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/1068#pullrequestreview-109421385

---

From: Mike Bishop <[email-masked]>
Date: Thu, 05 Apr 2018 16:34:46 -0700

The Private Use area of only 256 values makes me a little nervous, given that a collision means authenticated packets will fail to parse properly.  I'd be more inclined to go straight to provisional registration as soon as someone wants to interop beyond their own clients.

If we feel we must have unstructured experiments, let *those* use a defined TLV format with a nice, long type field.

> @@ -1197,6 +1204,16 @@ ack_delay_exponent (0x0007):
   value is also used for ACK frames that are sent in Initial, Handshake, and
   Retry packets.  Values above 20 are invalid.
 
+extension_frames (0x0009):
+
+: A list of extension frame mappings, encoded using the ExtensionFrameMappings
+  struct.  That is, a 2 octet length field, followed by 1 or more repetitions of
+  a one octet frame type and a two octet identifiers.  Each entry defines a
+  mapping of frame type to the 16-bit extension frame types defined in
+  {{frame-extension}}.  Valid frame types MUST be from 0x80 to 0xff, other

Comma splice; either use a `;` or make these separate sentences.

> +The `extension_frames` transport parameter establishes a mapping from the 16-bit
+frame extension identifier to a frame type octet in the range from 0x80 to 0xff
+inclusive.  Each endpoint establishes their own preferred mapping; this
+determines the mapping for frames that they receive.  An endpoint translates the
+16-bit extension frame identifier into a frame type using the `extension_frames`
+transport parameter advertised by their peer.
+
+This design allows the same extension frame to use a different frame type in
+each direction.  It also allows for extension frames to be used in one direction
+only.
+
+An endpoint MUST NOT use extension frames in packets protected with handshake
+keys.  A client can use extension frames in 0-RTT based on the value of the
+server's transport parameters used in a previous connection.
+
+The definition of a extension frame identifier MUST include the format of the

an

> +determines the mapping for frames that they receive.  An endpoint translates the
+16-bit extension frame identifier into a frame type using the `extension_frames`
+transport parameter advertised by their peer.
+
+This design allows the same extension frame to use a different frame type in
+each direction.  It also allows for extension frames to be used in one direction
+only.
+
+An endpoint MUST NOT use extension frames in packets protected with handshake
+keys.  A client can use extension frames in 0-RTT based on the value of the
+server's transport parameters used in a previous connection.
+
+The definition of a extension frame identifier MUST include the format of the
+extension frame and its semantics.  QUIC frames are not length-delimited, so
+knowledge of the format of frames is necessary for decoding packets.  Each
+extension frame identifier definition MUST include the versions of QUIC that the

"identifier" is redundant.

-- 
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/1068#pullrequestreview-109909386

---

From: Martin Thomson <[email-masked]>
Date: Fri, 06 Apr 2018 00:05:00 +0000 (UTC)

@martinthomson pushed 1 commit.

9f5b9ca  Editorial tweaks

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1068/files/951c3fbf77fba6377951b887a1a131612c138e09..9f5b9ca24457080d1e864f4970367ee5fe0fed57

---

From: Martin Thomson <[email-masked]>
Date: Fri, 06 Apr 2018 00:04:56 +0000 (UTC)

} TransportParameterId;
 
+   struct {
+      uint8 frame_type;

This is idiomatic in TLS.  Take a look at RFC 7301 and you will see the same repetition.

-- 
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/1068#discussion_r179632139

---

From: Martin Thomson <[email-masked]>
Date: Fri, 06 Apr 2018 00:07:02 +0000 (UTC)

> Negotiating extensions as part of the handshake is important, but do we need the {0,0xffff} -> {0x80, 0xff} mapping? Are we expecting this mapping to be maintained for multiple QUIC versions?

I think that we're building much of QUICv1 with the expectation that QUICv2+ won't change it.  I think of it more from the perspective of future simplicity: we wouldn't want to rebuild all QUICv1 extensions and applications just because we added (for example) multipath in a later version.

-- 
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/1068#issuecomment-379112092

---

From: Rui Paulo <[email-masked]>
Date: Fri, 06 Apr 2018 02:38:29 +0000 (UTC)

@martinthomson I was asking because we're essentially trying to map 2**16 into 2**7.  Why would the TLS parameter for this extension be so large when we're not going to use it for anything else (i.e., for extensions that don't require frames) ?

-- 
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/1068#issuecomment-379132420

---

From: Martin Thomson <[email-masked]>
Date: Thu, 05 Apr 2018 20:59:23 -0700

The idea is that you can concurrently use `2**7`, selected from a set of `2**16` with arbitrary (bijective) mapping from one to the other.

-- 
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/1068#issuecomment-379141748

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 07 Apr 2018 03:15:40 +0000 (UTC)

@martinthomson Thank you for pointing out that it is necessary to use TP for negotiating some features, and also for moving the negotiation there.

@ekr 
> Well, as you acknowledge below, it is feature negotiation for features that need new frames.

I share this model, and see two issues with the proposed approach:
* The proposed approach has two ways of extending QUIC, one by assigning a new TransportParameterId and one by assigning one or more `extension_frame_identifier`. I think that we should better unify the two schemes.
* We might want to pass along some parameters for the extension being negotiated. However, the proposal does not allow an `ExtensionFrameMappings` to carry such parameters.

Considering the two, how about defining the frame types to be used for each extension as a direct member of `TransportParameter`, like below?

```
struct {
  TransportParameterId parameter;
  opaque value<0..2^16-1>;
  uint8 frame_types<0..128>;
} TransportParameter;
```

Then, we can simplify the procedure of adding new extensions, regardless of if it needs frame types or not. An extension would always have a new TransportParameterId, can have any argument passed by using `value`, and have any number of frame types being assigned. We will only need one IANA registry for TransportParameterIds.

-- 
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/1068#issuecomment-379428145

---

From: Martin Thomson <[email-masked]>
Date: Mon, 09 Apr 2018 01:31:58 +0000 (UTC)

I wouldn't be supportive of that sort of change.  Frame types aren't 1:1 with transport parameters.

There doesn't need to be a higher level of formality around the process by which the transport is extended.  The basic tool we have - transport parameters - is actually sufficient.  That said, though it was already possible to use transport parameters to define new frame types, it wasn't possible to do so without creating a coordination disaster, so we end up 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/1068#issuecomment-379602175

---

From: ekr <[email-masked]>
Date: Mon, 09 Apr 2018 02:19:19 +0000 (UTC)

"That said, though it was already possible to use transport parameters to define new frame types, it wasn't possible to do so without creating a coordination disaster, so we end up here."

I'm not sure where this claim comes from. What's the coordination disaster?

-- 
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/1068#issuecomment-379608300

---

From: Martin Thomson <[email-masked]>
Date: Mon, 09 Apr 2018 04:34:31 +0000 (UTC)

It would be technically possible to use transport parameters like TLS extensions, which can (and do) change the protocol from the handshake onwards.

The coordination disaster arises from the fact that we don't have any way to manage collisions between different extensions that add the use of new frames.

-- 
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/1068#issuecomment-379628361

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 05:48:53 +0000 (UTC)

@martinthomson Thank you for your comments.

> I wouldn't be supportive of that sort of change. Frame types aren't 1:1 with transport parameters.

That's correct. Hence variable number of frame types (`uint8 frame_types<0..128>`).

Re coordination, if we consider that as a disaster, I think the correct path would be to let the client declare the extension IDs only in TP, and let the server to choose the frame types for which extensions that the server wants to support.

Doing so solves two issues: removes assignment of frame types that would never be used (it is useless to assign a id for a frame that the peer cannot sent), removes the possibility of having different IDs being assigned by both peers for the same extension (which would be a source of confusion).

-- 
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/1068#issuecomment-379639925

---

From: MikkelFJ <[email-masked]>
Date: Mon, 09 Apr 2018 06:38:39 +0000 (UTC)

I'm with @kazuho  here - I was almost about to suggest something similar.
The only thing I'm a bit tentative about is that you can imagine having some extensions getting quite popular which you want to use, but still include some less common extensions. This cannot be done with a single extension ID. On the ID I suggest a 128-bit uuid because it won't collide.

-- 
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/1068#issuecomment-379649672

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 09 Apr 2018 07:05:46 +0000 (UTC)

> The only thing I'm a bit tentative about is that you can imagine having some extensions getting quite popular which you want to use, but still include some less common extensions. This cannot be do…

I do not think that we need identifiers sent on the wire assuming that each "feature" is designated by a unique TransportParameterId.

A feature that assigns some frame types can send them as a _list_ in TransportParameter, and designate them in the specification as "the Nth frame type" to give them the names.

If some frame types are optional, `0` can be sent as a element of the list to indicate that it will not be used.

-- 
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/1068#issuecomment-379655182

---

From: ekr <[email-masked]>
Date: Mon, 09 Apr 2018 11:53:43 +0000 (UTC)

On Sun, Apr 8, 2018 at 9:34 PM, Martin Thomson <[email-masked]>
wrote:

> It would be technically possible to use transport parameters like TLS
> extensions, which can (and do) change the protocol from the handshake
> onwards.
>
> The coordination disaster arises from the fact that we don't have any way
> to manage collisions between different extensions that add the use of new
> frames.
>

I don't see why this is a problem, except for collisions of the frame type
IDs. And that can be solved by making that space very large. What am I
missing?

-- 
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/1068#issuecomment-379725981

---

From: Martin Thomson <[email-masked]>
Date: Fri, 13 Apr 2018 02:30:13 +0000 (UTC)

It sounds like we're deep in bikeshed territory and we have three options.  I'll take this to the list to see if we can get any better insight into what to do.

-- 
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/1068#issuecomment-381002485