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

Adding support for DATAGRAM #77

Closed
pthatcherg opened this issue Nov 7, 2018 · 13 comments
Closed

Adding support for DATAGRAM #77

pthatcherg opened this issue Nov 7, 2018 · 13 comments
Assignees
Labels
PR exists A pull request has been submitted

Comments

@pthatcherg
Copy link
Collaborator

Is DATAGRAM mature enough to propose an API? I found that it's already implemented (I think....) in Chromium's QUIC impl.

An API could look like this:

partial interface QuicTransportBase {
// If you hit backpressure, returns false; if too big, throws exception
bool sendMessage(Uint8Array message);
// If you don't read fast enough, applies back pressure (maybe...)
Promise receiveMessage();
}

Or, in WHATWG stream style:

partial interface QuicTransportBase {
// chunks == messages
WritableStream sendMessages();
ReadableStream receiveMessages();
}

@aboba
Copy link
Collaborator

aboba commented Nov 7, 2018

Currently, there are multiple proposals within the QUIC WG relating to partial reliability or datagram support.

IETF drafts:
datagram: https://tools.ietf.org/html/draft-pauly-quic-datagram
R-QUIC: https://mailarchive.ietf.org/arch/msg/quic/tUMYPmNce6XDkingYKkRiTkUdN4/2
partial reliability: https://tools.ietf.org/html/draft-lubashev-quic-partial-reliability

Presentations at IETF 103:
Datagram: https://datatracker.ietf.org/meeting/103/materials/slides-103-quic-unreliable-datagram-extension-00
RTP over QUIC: https://datatracker.ietf.org/meeting/103/materials/slides-103-avtcore-rtp-over-quic

A vigorous discussion is underway on the QUIC WG mailing list:
https://www.ietf.org/mail-archive/web/quic/current/msg05012.html

@eomara
Copy link

eomara commented Nov 13, 2018

I personally thing Datagram proposal makes the most sense but anyway partial reliability is explicitly not goal for the QUIC working group, so any of the above will be in QUIC V1.

@pthatcherg
Copy link
Collaborator Author

Note to self: add these methods to the spec because we want to get developer's feedback

@aboba
Copy link
Collaborator

aboba commented Nov 14, 2018

You might want to reference draft-pauly-quic-datagram. There is already a note in Section 4.1 saying that both the API and protocol documents are preliminary,, but you might consider adding a cautionary note to the DATAGRAM section saying that this is especially true of that section.

@aboba
Copy link
Collaborator

aboba commented Nov 22, 2018

@eomara DATAGRAM support is proposed as a transport parameter, so it does not require version negotiation.

@aboba
Copy link
Collaborator

aboba commented Dec 3, 2018

From @johnwason:

Also, having an option to directly send and receive messages using the "streams as messages" concept without creating QuicTransportStreams would be helpful. For something like an online game that sends large numbers of time sensitive packets, using the current proposed design would cause significant overhead creating and destroying streams.

@johnwason
Copy link

@aboba my use case requires out-of-order message delivery and partial reliability similar to SCTP. This seems to be possible using streams-as-messages and draft-lubashev-quic-partial-reliability-03, but probably not easily done with a raw datagram transport.

@aboba
Copy link
Collaborator

aboba commented Dec 3, 2018

@johnwason Would a partially reliable stream meet your needs better? I have filed an Issue for this:
https://github.com/w3c/webrtc-quic/issues/92

@johnwason
Copy link

@ababo no, I don't think so because the messages are likely to be significantly larger than MTU. The individual messages will still need to be reassembled once received, and either delivered out-of-order or dropped as a message rather than a packet. (The use case involves robotics and real-time communication with isochronous messages passed between devices.)

@pthatcherg
Copy link
Collaborator Author

pthatcherg commented Dec 5, 2018

@johnwason, We expect many people will use the "stream as a message" approach with a large number of streams/messages. We know that works with QUIC as a protocol (we already know of uses that do so and it works fine). The question is whether the API can handle it (from a JS object standpoint). We believe that it can. But if it can't, we'll tweak the API until it can. One option is perhaps to have .sendStream method on QuicTransport. Another is to use WHATWG streams (and streams of streams).

I think "a message is a stream" approach is the right one and we'll make sure it works.

That said, a lot of people have asked for DATAGRAM as well, so that's why we're adding it. It's only slightly more efficient on the wire than "a message is a stream".

@johnwason
Copy link

@pthatcherg after studying the QUIC standard a bit more, streams-as-message definitely seems like the best option for my use case, and I agree that there will need to be an object to represent each active stream to handle buffering properly. (Trying to deliver the entire stream at once would require very large buffers.) The JS API will need to be very lightweight to allow the rapid creating and destruction of stream objects for each message. WHATWG and streams of streams would probably just cause more complexity. My use case will often run either in embedded hardware or real-time loops so simplicity is very important. The base QUIC protocol streams is pretty lightweight so it should work. I am going to tinker with picoquic over the next few months and see how well it works.

I can definitely see where a DATAGRAM packet would be useful for something like RTP tunneling, but it doesn't match my use case.

@pthatcherg
Copy link
Collaborator Author

I'm glad to hear it will work well for your use case. I'll be interested to hear how picoquic does with many streams. I know the impl of QUIC in Chromium handles it fine.

@shampson shampson added PR exists A pull request has been submitted and removed Ready for PR labels Dec 18, 2018
@pthatcherg
Copy link
Collaborator Author

It's been merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR exists A pull request has been submitted
Projects
None yet
Development

No branches or pull requests

5 participants