Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composability of QUIC Extensions #3332

Closed
marten-seemann opened this issue Jan 10, 2020 · 15 comments
Closed

Composability of QUIC Extensions #3332

marten-seemann opened this issue Jan 10, 2020 · 15 comments
Labels
-transport design has-consensus

Comments

@marten-seemann
Copy link
Contributor

@marten-seemann marten-seemann commented Jan 10, 2020

@LPardue just raised an interesting point on the mailing list, which is currently not covered in the documents:

In draft-deconinck-quic-multipath-03 [1] it states that because there are new path-related IDs, some frames need to be modified and then lists them: NEW_CONNECTION_ID, RETIRE_CONNECTION_ID and ACK. IIUC this would keep the type value but modify the frame contents. My model of thinking has been to assume that frame definitions are immutable, and that extensions that require tweaks to frames would define new ones.

[...]

So my observation is not about the proposals themselves but about the composability of extensions. Say for example someone wanted to combine mpquic with 1wd, would they modify the ACK frame or define a new one?

I find the composability argument convincing, and was wondering if it makes sense to add something along those lines to the transport document:

Extensions MUST NOT modify existing frame types.

@dtikhonov
Copy link
Contributor

@dtikhonov dtikhonov commented Jan 10, 2020

This is an important observation. Proscribing frame modification by extensions does make sense -- I can't come up with any counterpoints. The 1wd extension could specify a new timestamp frame type and simply send it together with the ACK frame.

@LPardue
Copy link
Member

@LPardue LPardue commented Jan 10, 2020

I didn't dig into the 1wd spec too much so I'm not sure what @huitema's rationale was for incorporating the timestamp into a new frame.

Reflecting on my immutable thinking model, this is probably influenced by have more familiarity with the HTTP/3 layer, which does not require a-priori negotiation of frames, and so meddling with the layouts of frames is problematic. QUIC extension works slightly different, but I'd still err on the side of not meddling with frames. Using types allow for failing early and explicitly e.g. "extension foo adds frames X and Z, it deprecates A and B. Reception of A or B is connection error"

@MikeBishop
Copy link
Contributor

@MikeBishop MikeBishop commented Jan 10, 2020

The recipient must be able to identify and parse the frames it receives, obviously. HTTP/3 already recommends redefining, rather than modifying, frames. This is because frames can be sent before receiving SETTINGS, and coordinating the point in the connection at which things change is... challenging.

QUIC doesn't have this problem directly, because the negotiation takes place before any frames are sent. (0-RTT implies support for everything that was supported last time.) So for single extensions that modify frame definitions, this is fine.

The insight here is that multiple extensions which all redefine the same frame will stomp on each other. This is potentially true with redefinitions as well. If extension 1 deprecates A and replaces it with X, but extension 2 deprecates A and replaces it with Y, you're either repeating the information (and effects) of the base frame by sending both X and Y, or you're only supplying information to one of the two extensions each time you would have sent A.

It's probably better that extensions define separate frames whenever possible and leave the base ones totally alone, even if that means using some kind of reference to the base frame to correlate them. I don't know that we can go so far as mandating that, though. Perhaps simply a caution that extensions which modify the core protocol have the potential to interact poorly with other extensions modifying the same component unless that interaction is defined.

@MikeBishop
Copy link
Contributor

@MikeBishop MikeBishop commented Jan 10, 2020

Or, as @mirjak suggests, if you have to modify a core frame type then you're defining a new version rather than an extension.

@LPardue
Copy link
Member

@LPardue LPardue commented Jan 10, 2020

I think it would be helpful to capture some caution or guidance. QUIC transport has a section on Extension Frames which focuses on some specific issues. A higher-level section on extensibility wouldn't hurt, such as was written in HTTP/2.

@ianswett
Copy link
Contributor

@ianswett ianswett commented Jan 10, 2020

Defining new frame types doesn't really solve the problem in general. I think extensions which change the same frame type and want to coexist need to call out how that works or declare themselves as mutually exclusive with all other extensions that change that frame type.

For example, if we didn't have ECN in the core of QUIC and an extension defined that, and there was also an extension to add timestamps to the ACK frame, then having them coexist would be important. Making a receiver choose whether to send an ACK frame with ECN or an ACK frame with a timestamp is not the desired behavior.

@dtikhonov
Copy link
Contributor

@dtikhonov dtikhonov commented Jan 10, 2020

Making a receiver choose whether to send an ACK frame with ECN or an ACK frame with a timestamp is not the desired behavior.

The solution is to make additional frames: the (theoretical) ECN extension would define an auxiliary frame that carries ECN counts and the 1wd extension would define an auxiliary timestamp frame. Both extensions would require these frames to be packaged in the same packet as the ACK frame they correspond to.

@huitema
Copy link
Contributor

@huitema huitema commented Jan 10, 2020

In the case of 1WD, I did consider coding that as a "prefix frame": encode a timestamp frame, and then encode the ACK. That could work too, but it requires making assumptions about the presence and order of frames in packets.

My current code is simple: go through the packet, parse and process the frames as they come. Each frame contains all the information required for processing it. In the case of the ACK frame, the processing involves passing information to the congestion control algorithm about packets acknowledged and RTT measured. I wanted to pass the information about one-way delay at the same time as the measured RTT. Having separate frames would require storing information in a "packet context". It is doable if I can assume that the Timestamp frame always precedes the ACK frame, but becomes fishy if the Timestamp frame follows the ACK.

Other problems appear if we start having a per packet context. Is it OK to have several timestamps in a packet? What about several ACK frames?

@DavidSchinazi
Copy link
Contributor

@DavidSchinazi DavidSchinazi commented Jan 10, 2020

I really don't think we should ban extensions from modifying core frames. I think this restricts our extensibility without providing any benefits that I can see.

The original example is a good one: if extension A wants to modify ACK frames to add field ACK_FIELD_A at the end of the ACK frame and extension B does the same with the ACK_FIELD_B field, then we have a problem when both extensions are in use: we don't know how to parse ACK frames (is ACK_FIELD_B before or after ACK_FIELD_A?).

However, saying that extensions should have their own separate frame types does not solve this problem. If extension A defines custom frame type ACK_A which contains a regular ACK plus ACK_FIELD_A, and B does the same with ACK_B, we end up having to send double the ACK data to get ACK_FIELD_A and ACK_FIELD_B across, and in my opinion that's a deal-breaker.

Therefore we'll need to define how to compose extensions A and B, no matter what. Banning changes to existing core frames didn't solve this problem.

If and when we do end up having this problem, we can deal with it then.

@LPardue
Copy link
Member

@LPardue LPardue commented Jan 10, 2020

My initial thoughts were that composing A and B could result in the explicit definition of ACK_A_B, defined in some document that says only ACK_A_B is allowed, with reception of ACK_A or ACK_B being a simple explicit error.

I see that is an over simplistic view. I agree we shouldn't overthink the problem before it is at hand.

@ekr
Copy link
Collaborator

@ekr ekr commented Jan 11, 2020

@igorlord
Copy link
Contributor

@igorlord igorlord commented Jan 13, 2020

I think there are two types of extensions we are discussing: extensions that add new fields and extensions that replace fields.

Extensions that add new fields can coexist easily by all extensions agreeing on a disambiguation principle. For example, new fields associated with an extension with a smaller Transport Parameter come first. It would be useful to agree on this and have all such extensions include a statement to this effect.

Extension that replace fields can be logically split into removing an existing field and adding new fields. They can co-exist by removing existing fields and shifting the rest of the frame to the left, and adding new fields using the above-mentioned principle (in TP number order).

@kazuho
Copy link
Member

@kazuho kazuho commented Jan 14, 2020

I second @DavidSchinazi and @ekr for postponing the decision until later moment.

Extensions are composable only when those extensions do not change the frame encoding, nor change the existing semantics of the frames. But as @huitema states in the 1WD case, the benefit of reusing the current ACK frame format is debatable even in such case.

I am not necessarily opposed to having composability when possible, but there's uncertainty, and we do not need a resolution now.

@mnot mnot added this to Triage in Late Stage Processing Jan 15, 2020
@LPardue LPardue added the design label Feb 4, 2020
@project-bot project-bot bot moved this from Triage to Design Issues in Late Stage Processing Feb 4, 2020
@LPardue
Copy link
Member

@LPardue LPardue commented Feb 4, 2020

Discussed in editors meeting - this feels like a design issue.

@larseggert
Copy link
Member

@larseggert larseggert commented Feb 5, 2020

Discussed in ZRH. Proposed resolution is to close with no action.

@larseggert larseggert added the proposal-ready label Feb 5, 2020
@project-bot project-bot bot moved this from Design Issues to Consensus Emerging in Late Stage Processing Feb 5, 2020
@LPardue LPardue moved this from Consensus Emerging to Consensus Call issued in Late Stage Processing Feb 19, 2020
@LPardue LPardue removed the proposal-ready label Feb 19, 2020
@LPardue LPardue added the call-issued label Feb 26, 2020
@LPardue LPardue added has-consensus and removed call-issued labels Mar 4, 2020
@project-bot project-bot bot moved this from Consensus Call issued to Consensus Declared in Late Stage Processing Mar 4, 2020
Late Stage Processing automation moved this from Consensus Declared to Text Incorporated Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design has-consensus
Projects
Late Stage Processing
  
Issue Handled
Development

No branches or pull requests