Add support of QMux#5984
Conversation
guhetier
left a comment
There was a problem hiding this comment.
Thanks for opening this PR.
I added a few very high level comments.
Also, as an early warning, we have work planned that will impact the same area of the code and concepts (see everything tied to RDMA), so please expect conflicts and redesigns.
| _IRQL_requires_max_(PASSIVE_LEVEL) | ||
| QUIC_STATUS | ||
| QUIC_API | ||
| MsQuicListenerQmuxOpen( |
There was a problem hiding this comment.
I think we will want to avoid these API change.
QMux should be useable by the app transparently, with some initial configuration to enable it, but using the usual APIs for everything else.
We currently have some work that also involve underlying transport configuration and should define how we want this config to look like.
There was a problem hiding this comment.
I largely agree with the direction of avoiding API changes and keeping QMux transparent to applications, with only minimal initial configuration.
However, I think there are some cases where we need to decide in advance whether we are using regular QUIC or QMux. In particular, for cryptographic operations, regular QUIC relies on the built-in QUIC crypto mechanisms, whereas QMux would handle these aspects differently within its own layer.
This could become problematic when configuration must be done prior to connection start. For example, settings related to resumption tickets or other crypto-related parameters may depend on whether the underlying transport is standard QUIC or QMux. If that distinction is not known early enough, it may be unclear how or where such configurations should be applied.
So while I agree with keeping the APIs unified, we may need to consider how to handle these pre-start configurations in a way that accounts for the transport differences.
There was a problem hiding this comment.
Yes, that's the challenge. Our current thinking is to add some more configuration at the registration level to decide which underlying transport to use. We are looking for something flexible enough to allow for a variety of underlying transport configurations. But that is early thinking.
| _IRQL_requires_max_(PASSIVE_LEVEL) | ||
| _Success_(return != FALSE) | ||
| BOOLEAN | ||
| QuicPacketBuilderQMuxPrepare( |
There was a problem hiding this comment.
I think that ideally, we would want to avoid or drastically simplify packetization when using QMux.
The underlying connection is assumed to be a secure, reliable stream (typically TCP + TLS).
My first instinct is that we should embrace that layering and not try to reimplement a standard TLS pipe here.
Description
This pull request introduces support for a new "QMux" connection type in the core library. It adds new APIs and internal logic to allocate, initialize, and manage QMux connections, which are handled differently from standard QUIC connections. The changes include new source files, build system updates, and significant modifications to connection lifecycle management to accommodate QMux-specific behavior.
QMux Connection Support
qmux.cand included it in the build system (CMakeLists.txtandclog.inputs). [1] [2]MsQuicConnectionQmuxOpen,MsQuicConnectionQmuxOpenInPartition, andMsQuicListenerQmuxOpeninapi.hand implemented them inapi.c. [1] [2] [3]QuicConnQMuxAllocandQuicConnQMuxFree. These handle initialization and resource management for QMux connections. [1] [2]Connection Lifecycle and Behavior Adjustments
Protocol and Event Handling
These changes collectively enable the core library to support a new class of connections with QMux semantics, while maintaining compatibility with existing QUIC connection handling.
Testing
Do any existing tests cover this change? Are new tests needed?
TBD
Documentation
Is there any documentation impact for this change?
TBD