Datachannel: Port Numbers #227

Closed
aboba opened this Issue Aug 23, 2015 · 21 comments

Projects

None yet

4 participants

@aboba
Contributor
aboba commented Aug 23, 2015

From Robin Raymond:

We have an issue with the DataChannel API. There is we have no way to specify the SCTP port number associated with the data channel (but we do have a stream identifier) It is possible to make arbitrary assumptions about port numbers, such as assuming 5000, 5001,… based on “signalling”, i.e. SDP, and then the stream identifier is assigned to the port number to use (where multiple streams can be muxed into a single port).

We have two options:

  1. programmer must create an SCTP transport PER port then data channels are assigned to SCTP transport [thus DTLS transport would be a 1 to many SCTP transport relationship]… OR

  2. we add an optional “port” number to the data channel so the data channel can go over a specified SCTP port number [and if not specified, then the system assigns one].

The trouble with option #2 is that the data channel API will include an additional param vs WebRTC 1.0 (where the port number is inside the SDP).

@aboba aboba added 1.1 1.0 labels Aug 23, 2015
@aboba aboba changed the title from Datachennel: Port Numbers to Datachannel: Port Numbers Aug 23, 2015
@pthatcherg

Good point. Without specifying the port, it wouldn't be possible to attach
more than one SctpTransport to a single DtlsTransport. I'm not sure how
valuable that is, but there was some interest in such a thing at the last
IETF.

The port is per-SctpTransport, not per-DataChannel, so there is no conflict
with 1.0. It could go on either the constructor or in Start. Since it
acts as sort of an ID of the SctpTransport, I'd vote for the
constructor/readonly-attribute. If you don't specify one, it's chosen by
the browser.

On Sun, Aug 23, 2015 at 10:16 AM, aboba notifications@github.com wrote:

From Robin Raymond:

We have an issue with the DataChannel API. There is we have no way to
specify the SCTP port number associated with the data channel (but we do
have a stream identifier) It is possible to make arbitrary assumptions
about port numbers, such as assuming 5000, 5001,… based on “signalling”,
i.e. SDP, and then the stream identifier is assigned to the port number to
use (where multiple streams can be muxed into a single port).

We have two options:

  1. programmer must create an SCTP transport PER port then data channels
    are assigned to SCTP transport [thus DTLS transport would be a 1 to many
    SCTP transport relationship]… OR

  2. we add an optional “port” number to the data channel so the data
    channel can go over a specified SCTP port number [and if not specified,
    then the system assigns one].

The trouble with option #2 #2 is
that the data channel API will include an additional param vs WebRTC 1.0
(where the port number is inside the SDP).


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

@robin-raymond
Contributor

That implies that both local and remote sides must create an SctpTransport with the same port number matching. The trouble will be that one side must initiate the communication (i.e. socket connect) and the other side must respond (i.e. socket listen + accept). In this model there is not a clear indication of who is initiating the connection and who is responding (unless we tie it to the DTLS role and I'm not sure that is a WebRTC 1.0 compatible assumption). Also any data channel that gets created by a local party before the remote side creates a SctpTransport might get dropped as there is no remote SctpTransport ready to go when the data channel is created, so the assumption must also be that both parties have pre-created all SctpTransports ready to go.

@pthatcherg

It's up to the application to choose the ports and create the transports.
It just has to be the same number on both sides, which is just like
createDataChannel({negotiated: true}). If an application wants two or
three SctpTransports, it could simply choose 3 arbitrary numbers apriori
and use them. Or it can let the browser choose, signal the port number
chosen and pass it to the other side. Or it can create one on both sides,
signal in both directions, and pass them down on both sides, ending up with
duplicates (some on one side and some on the other). All of these should
work, and I don't see a problem.

But most apps will only want one. We need to make sure the default for the
first one is always the same (as it is now). Otherwise, the simple case
wouldn't work.

On Mon, Aug 24, 2015 at 10:09 AM, Robin Raymond notifications@github.com
wrote:

That implies that both local and remote sides must create an SctpTransport
with the same port number matching. The trouble will be that one side must
initiate the communication (i.e. socket connect) and the other side must
respond (i.e. socket listen + accept). In this model there is not a clear
indication of who is initiating the connection and who is responding
(unless we tie it to the DTLS role and I'm not sure that is a WebRTC 1.0
compatible assumption). Also any data channel that gets created by a local
party before the remote side creates a SctpTransport might get dropped as
there is no remote SctpTransport ready to go when the data channel is
created, so the assumption must also be that both parties have pre-created
all SctpTransports ready to go.


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

@robin-raymond
Contributor

@pthatcher yes, but there is still a matter of who initiates the SCTP socket "connect" vs "listen/accept" for each SctpTransport. The connector vs connectee is unclear (unless, as I said it's based upon the DTLS client / server role). As for picking the port numbers automatically, the port value can be a property of the SCTP transport post construction (thus the value is known if not specified) and the starting port can also be something like "5000" so the very simple case will just work even without retrieving that value.

@pthatcherg

On Mon, Aug 24, 2015 at 11:01 AM, Robin Raymond notifications@github.com
wrote:

@pthatcher https://github.com/pthatcher yes, but there is still a
matter of who initiates the SCTP socket "connect" vs "listen/accept" for
each SctpTransport.

The connector vs connectee is unclear (unless, as I said it's based upon
the DTLS client / server role).

​​
I'm not really sure what this is about. I wrote much of the SCTP data
channel in Chrome, and I just re-read it, and I don't see anything about
setting a particular role on an SctpTransport. In WebRTC 1.0, we don't do
anything with roles as far as I can tell, and it works. Am I missing
something? Is there a particular function of usrsctplib you could point me
at?
​ ​

As for picking the port numbers automatically, the port value can be a
property of the SCTP transport post construction (thus the value is known
if not specified) and the starting port can also be something like "5000"
so the very simple case will just work even without retrieving that value.

​That's exactly what I was suggesting.​


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

@robin-raymond
Contributor

In the code here:
https://chromium.googlesource.com/external/webrtc/+/master/talk/media/sctp/sctpdataengine.cc

Chrome has both sides do a "connect()" request with usrsctplib with some magic timing and packet dropping (e.g. read comments on line 603) so that the bidirectional INIT vs INIT-ACK sent by SCTP does not cause conflict and both to mutually connect to the other so there is no connector vs connectee. However, Mozilla does a "connect()" and "listen/accept" https://dxr.mozilla.org/mozilla-central/source/netwerk/sctp/datachannel/DataChannel.cpp?offset=0 in their code. IMHO the google code is slightly wrong doing the mutual "connect" procedure instead of a "connect" / "accept" but that depends if you see SCTP more like TCP or more like UDP. The chrome method feels like a slight hack to me given SCTP does to a TCP like handshake (i.e. SCTP INIT/INIT-ACK methodology) and I personally think Mozilla's approach is actually more proper despite that chrome's code does trick usrsctp into working with a mutual connect.

@pthatcherg

So how does Mozilla choose the role?

On Mon, Aug 24, 2015 at 12:30 PM, Robin Raymond notifications@github.com
wrote:

In the code here:

https://chromium.googlesource.com/external/webrtc/+/master/talk/media/sctp/sctpdataengine.cc

Chrome has both sides do a "connect()" request with usrsctplib with some
magic timing and packet dropping (e.g. read comments on line 603) so that
the bidirectional INIT vs INIT-ACK sent by SCTP does not cause conflict and
both to mutually connect to the other so there is no connector vs
connectee. However, Mozilla does a "connect()" and "listen/accept"
https://dxr.mozilla.org/mozilla-central/source/netwerk/sctp/datachannel/DataChannel.cpp?offset=0
in their code. IMHO the google code is slightly wrong doing the mutual
"connect" procedure instead of a "connect" / "accept" but that depends if
you see SCTP more like TCP or more like UDP. The chrome method feels like a
slight hack to me given SCTP does to a TCP like handshake (i.e. SCTP
INIT/INIT-ACK methodology) and I personally think Mozilla's approach is
actually more proper despite that chrome's code does trick us rsctp in to
working with a mutual connect.


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

@robin-raymond
Contributor

I'm not 100% sure. I am still investigating. usrsctplib with mutual "connect()" trick does work. But SCTP really the protocol does demand that one side issues and the other side accepts for SCTP INIT/INIT-ACK. I'm personally fine if we define client vs server DTLS roles to indicate the direction, just like even/odd is used for the SCTP session ID...

@pthatcherg

I'm pretty sure it works if that's all we've doing all along.

TCP connect/connect works as well, and isn't a hack. Receiving a SYN when
you send a SYN is just fine in the TCP state machine.

On Mon, Aug 24, 2015 at 1:11 PM, Robin Raymond notifications@github.com
wrote:

I'm not 100% sure. I am still investigating. usrsctplib with mutual
"connect()" trick does work. But SCTP really the protocol does demand that
one side issues and the other side accepts for SCTP INIT/INIT-ACK. I'm
personally fine if we define client vs server DTLS roles to indicate the
direction, just like even/odd is used for the SCTP session ID...


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

@pthatcherg

On Mon, Aug 24, 2015 at 2:48 PM, Peter Thatcher pthatcher@google.com
wrote:

I'm pretty sure it works if that's all we've doing all along.

TCP connect/connect works as well, and isn't a hack. Receiving a SYN when
you send a SYN is just fine in the TCP state machine.

On Mon, Aug 24, 2015 at 1:11 PM, Robin Raymond notifications@github.com
wrote:

I'm not 100% sure. I am still investigating. usrsctplib with mutual
"connect()" trick does work. But SCTP really the protocol does demand that
one side issues and the other side accepts for SCTP INIT/INIT-ACK. I'm
personally fine if we define client vs server DTLS roles to indicate the
direction, just like even/odd is used for the SCTP session ID...

​I'd prefer to avoid having SCTP roles in the API. It's one less thing to
worry about (or one more if we added it).​

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

@robin-raymond
Contributor

@pthatcher In either case (mutual connect vs listen/accept) the ORTC API doesn't need to add roles as that can be assumed based on the DTLS client/server roles (like even vs odd SID) so I'm less concerned about that... If you want to define mutual SCTP connect as proper then it needs to be documented as such at an IETF level to make it absolutely clear that behaviour is the proper expected behaviour (and possibly it is already but I've not seen it, i.e. TLDR). Otherwise, I had assumed that DTLS client would assume the SCTP "connect" role and DTLS server would assume the SCTP "listen/accept" role...

@robin-raymond
Contributor

FYI - Alternatively, it's technically it's possible to emit SCTP transports as events when incoming SCTP connections happen (just like DataChannels) based upon incoming SCTP messages on unused ports. That way you don't need to have both sides pre-setup SCTP transports with fixed port numbers in advance and the initiating side need only create a new SCTP transport for the receiving side emit an SCTP transport. To avoid port conflicts, the even/odd port rule can apply for auto-port selection. Also this clears up the connect/accept role since the client role only depends on the initiator. If both sides initiate a connect then it's an SCTP simultaneous open (just as it's possible to do TCP simultaneous open).

Whatever solution chosen, the google code will remain compatible as it always does simultaneous open...

Just want to lay out all options on the table...

@pthatcherg

That's a good idea to make sure aware of all the options.

I think having more than one SctpTransport is a sufficiently advanced use
case that if it's slightly harder (you have to pick a port number in your
app), it isn't a big deal. I don't think we should add any complexity to
help such an edge case. In other words, let's do the simplest thing
possible to support the use case but no more. And I think allowing the app
to set the port is all we need.

On Tue, Aug 25, 2015 at 7:03 AM, Robin Raymond notifications@github.com
wrote:

FYI - Alternatively, it's technically it's possible to "emit" SCTP
transports as events when incoming SCTP connections happen (just like
DataChannels) based upon incoming SCTP messages on unused ports. That way
you don't need to have both sides pre-setup SCTP transports with fixed port
numbers in advance and the initiating side need only create a new SCTP
transport for the receiving side emit an SCTP transport. To avoid port
conflicts, the even/odd port rule can apply for auto-port selection. Also
this clears up the connect/accept role since the client role only depends
on the initiator. If both sides initiate a connect then it's an SCTP
simultaneous open (just as it's possible to do TCP simultaneous open).

Whatever solution chosen, the google code will remain compatible as it
always does simultaneous open...

Just want to lay out all options on the table...


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

@pthatcherg

On Tue, Aug 25, 2015 at 6:28 AM, Robin Raymond notifications@github.com
wrote:

@pthatcher https://github.com/pthatcher In either case (mutual connect
vs listen/accept) the ORTC API doesn't need to add roles as that can be
assumed based on the DTLS client/server roles (like even vs odd SID) so I'm
less concerned about that.

If you want to define mutual SCTP connect as proper then it needs to be
documented as such at an IETF level to make it absolutely clear that
behaviour is the proper expected behaviour (and possibly it is already but
I've not seen it, i.e. TLDR). Otherwise, I had assumed that DTLS client
would assume the SCTP "connect" role and DTLS server would assume the SCTP
"listen/accept" role...

If we don't add SCTP roles to the API, then the real question is: do we
standardize the connect/listen behavior, or leave it up to the
implementation to decide? Since symmetric connect works and is simple and
already implemented, I'd need a really strong argument to convince me to
change the code to do something else, especially to something more complex.


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

@robin-raymond
Contributor

@pthatcher Hmm... Since simultaneous open vs listen / accept are mutually compatible for usrsctplib which everyone is using and the SCTP state machine does allow for simultaneous open there might not need to be a strong reason to standardize. Certainly the chrome code does not need to change regardless as it will remain compatible in either scenario. The critical point is that constructing an ORTC SctpTransport must cause connection to be established to the remote party.

I agree on not needing to cover this edge case right now re: emitting SctpTransport objects as an incoming transport event.

@juberti
juberti commented Aug 25, 2015

simultaneous open is a feature of SCTP - I think we should explicitly force this for ORTC, as it's one less thing you have to negotiate.

@pthatcherg

You mean put in the standard that an implementation must use simultaneous
open? If we do that, should we do that for WebRTC as well? I'd be in
favor of both having it.

On Tue, Aug 25, 2015 at 1:27 PM, Justin Uberti notifications@github.com
wrote:

simultaneous open is a feature of SCTP - I think we should explicitly
force this for ORTC, as it's one less thing you have to negotiate.


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

@juberti
juberti commented Aug 25, 2015

Yes, at least for the former.

@robin-raymond
Contributor

I'm fine with simultaneous open for ORTC.

A statement like:

Creation of an SctpTransport causes an SCTP INIT request to be issued over the DtlsTransport from the local SctpTransport to the remote SctpTransport where the remote SctpTransport responds with an SCTP INIT-ACK. Since both local and remote parties must mutually create an SctpTransport, SCTP SO [Simultaneous Open] is used to establish a connection over SCTP.

@aboba
Contributor
aboba commented Aug 26, 2015

It seems like a good idea to specify this (and the single port/DTLS restriction if that is agreed to) for WEBRTC 1.0 as well, to keep ORTC and WEBRTC 1.0 in sync.

On Aug 25, 2015, at 14:22, pthatcherg notifications@github.com wrote:

You mean put in the standard that an implementation must use simultaneous
open? If we do that, should we do that for WebRTC as well? I'd be in
favor of both having it.

On Tue, Aug 25, 2015 at 1:27 PM, Justin Uberti notifications@github.com
wrote:

simultaneous open is a feature of SCTP - I think we should explicitly
force this for ORTC, as it's one less thing you have to negotiate.


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


Reply to this email directly or view it on GitHub.

@aboba
Contributor
aboba commented Sep 8, 2015

Here is a proposal for resolving this:

12.2 Operation

An RTCSctpTransport is constructed from an RTCDtlsTransport object, and optionally a port number (with a default of 5000, or the next unused port). If a port already in use is provided in the constructor, throw an InvalidParameters exception. Creation of an RTCSctpTransport causes an SCTP INIT request to be issued over the RTCDtlsTransport from the local RTCSctpTransport to the remote RTCSctpTransport where the remote RTCSctpTransport responds with an SCTP INIT-ACK. Since both local and remote parties must mutually create an RTCSctpTransport, SCTP SO (Simultaneous Open) is used to establish a connection over SCTP.

12.3 Interface Definition

[Constructor(RTCDtlsTransport transport, unsigned short? port)]
partial interface RTCSctpTransport : RTCDataTransport {
readonly attribute unsigned short port;
};

@robin-raymond robin-raymond pushed a commit that referenced this issue Sep 8, 2015
Robin Raymond - Added the "failed" state to RTCDtlsTransportState, as noted in:
#219

- Added support for WebRTC 1.0 RTCIceCredentialType, as noted in:
#222

- Clarified behavior of createAssociatedGatherer(), as noted in:
#223

- Fixed SCTP port numbers, as noted in:
#227
2e5238d
@robin-raymond robin-raymond pushed a commit that referenced this issue Sep 21, 2015
Robin Raymond Added support for the WebRTC 1.0 certificate management API, as noted…
… 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
674f156
@aboba aboba closed this Oct 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment