RTCSctpCapabilities maxMessageSize issues #626

Open
lgrahl opened this Issue Dec 17, 2016 · 13 comments

Projects

None yet

4 participants

@lgrahl
lgrahl commented Dec 17, 2016

I see two problems with the maxMessageSize attribute of RTCSctpCapabilities:

  1. An unsigned short provides insufficient space. I'm not entirely sure what the maximum message size of SCTP effectively is, maybe @tuexen can add a statement to that. However, streaming APIs should be able to handle messages of arbitrary size (although the W3C API is not a streaming API, other implementations may provide one).
  2. We should clarify that 0 indicates that the other peer can handle messages of any size. This is particularly important once SCTP ndata will be deployed.

For reference, here is the SDP section relevant for WebRTC.

@aboba aboba self-assigned this Dec 17, 2016
@aboba
Contributor
aboba commented Dec 17, 2016

In WebRTC 1.0, the maxMessageSize attribute is an unsigned long, so there is a potential compatibility issue:
https://rawgit.com/w3c/webrtc-pc/master/webrtc.html#rtcsctptransport-interface

@lgrahl
lgrahl commented Dec 17, 2016 edited

unsigned long means uint64_t right? That should be sufficient. :)

WebRTC 1.0 also does not clarify the 0 case. Maybe we should ping them about that as well.

@tuexen
tuexen commented Dec 17, 2016
@lgrahl
lgrahl commented Dec 17, 2016

Thanks Michael. Taking into account that implementations can set maxMessageSize to 0 to indicate support for messages of arbitrary size, having a 64 bit unsigned integer should be absolutely fine. Implementations that choose to limit message sizes will usually choose a much lower amount than 2^64 anyway (but it's good to have the full range from 1 to 2^64).

@aboba
Contributor
aboba commented Dec 22, 2016

Proposed resolution: change to unsigned long

@lgrahl
lgrahl commented Dec 22, 2016

And maybe adding something like this to the description:

A value of 0 indicates that messages of arbitrary size can be handled.

@aboba aboba added the PR exists label Jan 5, 2017
@aboba aboba added a commit that referenced this issue Jan 5, 2017
@aboba aboba maxMessageSize update
Fix for Issue #626
6e7ef7c
@robin-raymond
Contributor
robin-raymond commented Jan 11, 2017 edited

@lgrahl if EOR is not supported, maxMessageSize is limited to the max SCTP packet size and arbitrary sizes cannot be supported. Even if this limitation was fixed, message sizes are still not unlimited due to other constraints (e.g. memory). Even TCP has max message sizes, but it gets around this by sends of some of the buffer rather than all of the buffer, but we can't do that here because the buffers are all or nothing. Thus I prefer to have a max size without a "0" to a) not require an additional "if (0 == maxMessageSize)" checks and b) there's always realistically going to be a maxMessageSize. I'm okay with a large possible value though up to 2^64.

@lgrahl
lgrahl commented Jan 11, 2017 edited

I do understand that this is a limitation of ORTC's design as we do not have a streaming API. Yes, I agree that 2^64 is more than enough, don't get me wrong. But 0 is definitely not a valid maxMessageSize as SCTP cannot handle empty messages, so it needs to be handled differently anyway, right? So, I don't understand your point that additional checks would be required.

0 would be a crystal clear indication for fragmentation/reassembly projects such as https://github.com/saltyrtc/chunked-dc-js that no chunking is needed at all. While 2^64 can be interpreted as unlimited, everything below that is a question mark: Is it large enough for my use case or do I need to turn on fragmentation/reassembly?

Furthermore, I probably should have mentioned earlier that draft-ietf-mmusic-sctp-sdp does handle the 0 case.

Edit: Sorry for all the editing.

@aboba
Contributor
aboba commented Jan 11, 2017

@lgrahl @pthatcher @robin-raymond In looking at the usage of maxMessageSize within the start method and example code, some questions arise. As defined in WebRTC 1.0, maxMessageSize represents a limitation on the use of the send() method, for the application developer to take into account. However, in Example 22, maxMessageSize is exchanged in signaling and the start method is called with remoteCaps as an argument. Passing the remote maxMessageSize to the start method would seem to imply that the local implementation will do something with this information, but it isn't clear to me what this should be.

@aboba
Contributor
aboba commented Jan 11, 2017

@lgrahl @pthatcher While draft-ietf-mmusic-sctp-sdp does handle 0 as unlimited, is this something that implementations support? Given that Binary Partial and String Partial are both deprecated within the SCTP parameters registry (see http://www.iana.org/assignments/sctp-parameters/sctp-parameters.xhtml#sctp-parameters-25), I don't see how this is expected to work.

@lgrahl
lgrahl commented Jan 11, 2017

@aboba I'm not sure if I understand your question correctly, but basically this value needs to be stored and checked in send calls. This also means that RTCDataChannel.send is not available before the SCTP transport is in the connecting state. But this isn't a problem as the data channels wouldn't be open at that point.

Regarding your second question: Yes, implementations can support messages of arbitrary size by using SCTP's explicit EOR mode when sending and checking for the EOR flag when receiving. In fact, our implementation already supports messages of arbitrary size. Sadly, this hasn't been implemented in browsers as both Firefox and Chromium ignore the EOR flag completely which is a bug and an open issue @tuexen and I have discussed several times. Basically, @tuexen didn't want to open an issue before we have a patch. This bug is also the reason why we cannot send more than 64 KiB reliably (16 KiB in FF - Chromium interop, I've written about this in detail here) until it has been resolved.

@lgrahl
lgrahl commented Jan 30, 2017

@robin-raymond I overlooked your point about no EOR support. It's vital that we know whether we talk about EOR when receiving or EOR when sending because one is required and one is not. Every SCTP-based data channel implementation must look at the EOR flag when receiving messages or it will violate the message-oriented principle of SCTP and data channels. This is exactly what pretty much every implementation out there currently does (violate). You've mentioned that TCP supports arbitrary sized messages by sending only parts of the buffer. It's the same for SCTP which will also send only parts of the buffer and not all or nothing. That's why the EOR flag exists and why looking at it is so essential even if an implementation doesn't use the explicit EOR mode for sending.

Back to the main topic: Granted, by allowing maxMessageSize to be 0, there's one further if (remote.maxMessageSize == 0) conditional required in the send method and potentially in the receive/reassembly function. Honestly, I don't think that's a big deal. We'd trade that for the conditional required when validating the RTCSctpCapabilities.maxMessageSize because if 0 is not unlimited, then 0 is not acceptable as SCTP is not capable of sending messages of size 0. Furthermore, let's not forget that ORTC is currently incompatible to draft-ietf-mmusic-sctp-sdp. A WebRTC-ORTC shim will have to compensate if we leave it as is.

@lgrahl
lgrahl commented Feb 20, 2017

Ping! Any new thoughts/comments?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment