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

DataChannel: negotiated behaviour appears to be not possible to implement #233

Closed
robin-raymond opened this issue Sep 7, 2015 · 8 comments

Comments

@robin-raymond
Copy link
Contributor

The first way is to construct an RTCDataChannel at one of the peers with the RTCDataChannelParameters.negotiated attribute unset or set to its default value false. This will announce the new channel in-band and trigger an ondatachannel event with the corresponding RTCDataChannel object at the other peer. The second way is to let the application negotiate the RTCDataChannel. To do this, create an RTCDataChannel object with the RTCDataChannelParameters.negotiated dictionary member set to true, and signal out-of-band (e.g. via a web server) to the other side that it should create a corresponding RTCDataChannel with the RTCDataChannelParameters.negotiated dictionary member set to true and the same id.

The trouble is that constructing a DataChannel with negotiated = true appears to have a race condition. If the local side constructs with negotiated as true, the DATA_CHANNEL_OPEN message (http://tools.ietf.org/html/draft-ietf-rtcweb-data-protocol-09#section-5.1) is sent to the remote side immediately. This will cause an "ondatachannel" with a new DataChannel event to happen remotely if the remote side did not pre-create a DataChannel with the same ID in advanced to receiving the in-band signalling. The remote side has no way to tell if this newly incoming channel had the negotiated flag set to true or not (as the negotiated field is not carried in the DATA_CHANNEL_OPEN message to know). If the remote side pre-creates the object, then it will also send the DATA_CHANNEL_OPEN and then the local side doesn't know in advance that this is a negotiated channel. Thus both sides must create the DataChannel while the DATA_CHANNEL_OPEN is still shuttling over the wire or the ondatachannel event will occur. There appears to be no way to suppress this behaviour.

@pthatcherg
Copy link

It's my understanding that if {negotiated: true}, then
the DATA_CHANNEL_OPEN will not be sent. That's the whole reason we added
the negotiated flag to begin with (to suppress in the in-band signalling),
and that's how the data channels are currently implemented in Chrome:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjingle/source/talk/app/webrtc/datachannel.cc&q=webrtc::datachannel%20negotiated&sq=package:chromium&dr=CSs&l=149

On Mon, Sep 7, 2015 at 7:30 AM, Robin Raymond notifications@github.com
wrote:

The first way is to construct an RTCDataChannel at one of the peers with
the RTCDataChannelParameters.negotiated attribute unset or set to its
default value false. This will announce the new channel in-band and trigger
an ondatachannel event with the corresponding RTCDataChannel object at the
other peer. The second way is to let the application negotiate the
RTCDataChannel. To do this, create an RTCDataChannel object with the
RTCDataChannelParameters.negotiated dictionary member set to true, and
signal out-of-band (e.g. via a web server) to the other side that it should
create a corresponding RTCDataChannel with the
RTCDataChannelParameters.negotiated dictionary member set to true and the
same id.

The trouble is that constructing a DataChannel with negotiated = true
appears to have a race condition. If the local side constructs with
negotiated as true, the DATA_CHANNEL_OPEN message (
http://tools.ietf.org/html/draft-ietf-rtcweb-data-protocol-09#section-5.1)
is sent to the remote side immediately. This will cause an "ondatachannel"
with a new DataChannel event to happen remotely if the remote side did not
pre-create a DataChannel with the same ID in advanced to receiving the
in-band signalling. The remote side has no way to tell if this newly
incoming channel had the negotiated flag set to true or not (as the
negotiated field is not carried in the DATA_CHANNEL_OPEN message to
know). If the remote side pre-creates the object, then it will also send
the DATA_CHANNEL_OPEN and then t he local side doesn't know in advance
that this is a negotiated channel. Thus both sides must create the
DataChannel while the DATA_CHANNEL_OPEN is still shuttling over the wire
or the ondatachannel event will occur. There appears to be no way to
suppress this behaviour.


Reply to this email directly or view it on GitHub
#233.

@aboba
Copy link
Contributor

aboba commented Sep 15, 2015

Proposed fix is to change the description of "negotiated" in Section 11.6.1 to the following:

negotiated of type boolean, , defaulting to false
The negotiated attribute returns true if this RTCDataChannel was negotiated by the application, or false otherwise. The attribute must be initialized to false by default and must return the value to which it was set when the RTCDataChannel was constructed. If set to true, the application developer must signal to the remote peer to construct an RTCDataChannel object with the same id for the data channel to be open. As noted in [DATA-PROT], DATA_CHANNEL_OPEN is not sent to the remote peer nor is DATA_CHANNEL_ACK expected in return. If set to false, the remote party will receive an ondatachannel event with a system constructed RTCDataChannel object.

@robin-raymond
Copy link
Contributor Author

What happens if Alice creates DataChannel with ID 5 with negotiated = false and Bob created a DataChannel with ID 5 with negotiated = true?

Normally Alice's DataChannel would get evented to Bob as ondatachannel event. However, if Bob has already created the DataChannel in advance there would be no ondatachannel event. Unless, Bob did NOT create the DataChannel in advance, and Alice's incoming DATA_CHANNEL_OPEN to Bob caused ondatachannel to event to fire a new DataChannel (which Bob's app layer has not process yet); meanwhile Bob attempts to construct a DataChannel with ID 5 and negotiated = true. Should the timing of when Bob creates the DataChannel with 5 cause one case to succeed and setup a DataChannel but another case to fail?

In other words, should Bob's engine disallow creation of a DataChannel when it already exists incoming (even if Bob's app layer is unaware of this yet)? But if Bob managed to open the DataChannel before the DataChannel was incoming there would be no error (or rejection of the incoming DataChannel)?

@robin-raymond
Copy link
Contributor Author

Variation: Alice creates a DatatChannel and sets negotiated to true and immediately starts sending data (as Alice is ready by her side immediately). Data arrives on Bob's side without a DATA_CHANNEL_OPEN (which can happen anyway because of out of order to DATA_CHANNEL_OPEN). Does this cause Bob's side to create a DataChannel for the incoming data (even if ondatachannel is not evented in this case)? Would Bob then constructing a DataChannel be legal in this case? Does that conflict with mismatch negotiated logic from previous comment?

@pthatcherg
Copy link

On Tue, Sep 15, 2015 at 4:24 PM, Robin Raymond notifications@github.com
wrote:

What happens if Alice creates DataChannel with ID 5 with negotiated =
false and Bob created a DataChannel with ID 5 with negotiated = true?

Normally Alice's DataChannel would get evented to Bob as ondatachannel
event. However, if Bob has already created the DataChannel in advance
there would be no ondatachannel event. Unless, Bob did NOT create the
DataChannel in advance, and Alice's incoming DATA_CHANNEL_OPEN to Bob
caused ondatachannel to event to fire a new DataChannel (which Bob's app
layer has not process yet); meanwhile Bob attempts to construct a
DataChannel with ID 5 and negotiated = true. Should the timing of when
Bob creates the DataChannel with 5 cause one case to succeed and setup a
DataChannel but another case to fail?

In other words, should Bob's engine disallow creation of a DataChannel
when it already exists incoming (even if Bob's app layer is unaware of this
yet)? But if Bob managed to open the DataChannel before the DataChannel
was incoming there would be no error (or rejection of the incoming
DataChannel)?

​If a DataChannel already exists with ID = 5 and an OPEN message comes in
with ID = 5, it should either be acked or ignored, but it shouldn't create
a new data channel or fire an event. But our behavior here should be
consistent with WebRTC 1.0, so we should just go check there. ​

This is definitely in the territory of "doctor, it hurts when I do this",
however.


Reply to this email directly or view it on GitHub
#233 (comment).

@pthatcherg
Copy link

On Tue, Sep 15, 2015 at 5:01 PM, Robin Raymond notifications@github.com
wrote:

Variation: Alice creates a DatatChannel and sets negotiated to true and
immediately starts sending data (as Alice is ready by her side
immediately). Data arrives on Bob's side which can happen out of order to
DATA_CHANNEL_OPEN. Does this cause Bob's side to create a DataChannel for
the incoming data (even if ondatachannel is not evented in this case)?
Would Bob then constructing a DataChannel be legal in this case? Does
that conflict with mismatch negotiated logic from previous comment?

​All data sent between when the OPEN is sent and the ACK is received must
be sent reliably. That's what the WebRTC standard says.​


Reply to this email directly or view it on GitHub
#233 (comment).

@aboba
Copy link
Contributor

aboba commented Sep 18, 2015

draft-ietf-rtcweb-data-protocol Section 6 says:

"All Data Channel Establishment Protocol messages MUST be sent using ordered delivery and reliable transmission."

[BA] So even though the data channel might have been created on Alice's side as unreliable and unordered, on Bob's side the SCTP transport will not provide data before the DATA_CHANNEL_OPEN.

Robin said:

"What happens if Alice creates DataChannel with ID 5 with negotiated =
false and Bob created a DataChannel with ID 5 with negotiated = true?"

Collision avoidance is addressed in draft-ietf-rtcweb-data-protocol Section 4:

To avoid collisions where both sides try to open a Data Channel with
the same Stream Identifiers, each side MUST use Streams with either
even or odd Stream Identifiers when sending a DATA_CHANNEL_OPEN
message. When using SCTP over DTLS
[I-D.ietf-tsvwg-sctp-dtls-encaps], the method used to determine which
side uses odd or even is based on the underlying DTLS connection
role: the side acting as the DTLS client MUST use Streams with even
Stream Identifiers, the side acting as the DTLS server MUST use
Streams with odd Stream Identifiers.

@robin-raymond
Copy link
Contributor Author

Assuming the DataChannal params.id is set to the same value on both local and remote sides:

When DataChannel params.negotiated is set to false and an existing DataChannel exists, throw an exception. When DataChannel params.negotiated is set to true and an existing DataChannel exists as created by a remote party then connect the remote DataChannel to the local DataChannel, unless DATA_CHANNEL_OPEN arrived previously from the report party in which case throw an exception.

Attempting to create a new DataChannel where the params.id is equal to a previously still open DataChannel created by the local party attached to the same SctpTransport, throw an exception.

robin-raymond pushed a commit that referenced this issue Sep 21, 2015
… in: Issue #195

Added certificate argument to the RTCDtlsTransport constructor, as noted in: Issue #218
Added the "failed" state to RTCDtlsTransportState, as noted in: Issue #219
Changed getNominatedCandidatePair to getSelectedCandidatePair, as noted in: Issue #220
Added support for WebRTC 1.0 RTCIceCredentialType, as noted in: Issue #222
Clarified behavior of createAssociatedGatherer(), as noted in: Issue #223
Changed spelling from "iceservers" to "iceServers" for consistency with WebRTC 1.0, as noted in: Issue #225
Added support for SCTP port numbers, as noted in: Issue #227
Changed "outbound-rtp" to "outboundrtp" within the Statistics API, as noted in: Issue #229
Changed maxPacketLifetime and maxRetransmits from unsigned short to unsigned long, as noted in: Issue #231
Clarified DataChannel negotiation, as noted in: Issue #233
Added getContributingSources() method, as noted in: Issue #236
Fixes to Examples 5 and 6, as noted in: Issue 237 and Issue #239
Fixed cut and paste errors in Example 11, as noted in: Issue #241
@aboba aboba closed this as completed Oct 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants