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

No procedure for the ICE failed state #2004

Closed
lgrahl opened this issue Oct 6, 2018 · 11 comments
Closed

No procedure for the ICE failed state #2004

lgrahl opened this issue Oct 6, 2018 · 11 comments
Assignees

Comments

@lgrahl
Copy link
Contributor

lgrahl commented Oct 6, 2018

Do we need a procedure for the ICE failed state?

It should at least trigger closing all data channels abruptly (similar to RTCPeerConnection.close). I'm pretty sure there's also something to be done for media tracks. What can survive an ICE restart before we dive into discussing a procedure?

  • Could a data channel survive an ICE restart? From my perspective: Yes, if the SCTP association does not time out. So, data channels can be left open until the SCTP association is dead (or the user closes the channel or the peer connection). But there is no guarantee that the SCTP association will survive.
  • Can a media track survive an ICE restart? I'm guessing yes?

If it turns out that we don't need to do anything in case we move into the failed state, should we at least mention that the failed state is recoverable and all associated data transports and media may survive it but it depends on the individual transport?

@fippo
Copy link
Contributor

fippo commented Oct 6, 2018

failed is recoverable by an ICE restart, closing data channels or stopping media seems wrong

@lgrahl
Copy link
Contributor Author

lgrahl commented Oct 6, 2018

@fippo Agree. Edited my initial posting as there are some open questions.

We also may want to add tests for that case. At least for data channels, I don't remember having seen any for the failed state.

@lgrahl
Copy link
Contributor Author

lgrahl commented Apr 20, 2019

Bumping this one. At least for data channels, it's currently undefined whether or not the SCTP association is supposed to survive which makes ICE restarts unreliable for data channel applications.

@aboba
Copy link
Contributor

aboba commented Apr 21, 2019

An SCTP association (or a TCP or QUIC connection, for that matter) should not terminate immediately when ICE enters the "failed" state, since an ICE restart could restore the ICE virtual connection. By default, retransmission timers will fire and the RTO estimate will increase until potentially the association limits will be reached. When ICE enters the "connected" state again, if the selected pair changes, an implementation might want to reset the RTT estimate, but that is a potential implementation decision.

@lgrahl
Copy link
Contributor Author

lgrahl commented Apr 22, 2019

To not ABORT due to ICE failed is logical (although not well-defined). But the question is what happens when ICE recovers but the SCTP association already failed.

@aboba
Copy link
Contributor

aboba commented Apr 22, 2019

If the SCTP association fails, then RTCSctpTransport.state will transition to closed and a statechange event will be fired. I would also expect a close event to fire on the RTCDataChannel. If the ICE restart subsequently succeeds, then it would be necessary to establish another SCTP association and bring up another data channel.

Would calling pc.createDataChannel() cause that to happen?

@lgrahl
Copy link
Contributor Author

lgrahl commented Apr 22, 2019

JSEP sec 5.11:

  *  If no valid SCTP association exists, prepare to initiate a SCTP
     association over the associated ICE component and DTLS
     connection, using the local SCTP port value from the local
     description, and the remote SCTP port value from the remote
     description, as described in [I-D.ietf-mmusic-sctp-sdp],
     Section 10.2.

Not sure the terminology applies 100% but I'm somewhat certain we can interpret the case that the SCTP association closed as no SCTP association exists.

Since all data channels are closed, I assume the application would have to create at least one new data channel because otherwise SCTP would not be negotiated for the ICE restart. So, yes, I think your assumption is correct.

But this brings us back to the original topic. I think we should clarify these things in an ICE failed procedure. This would define what should happen in such a case and thus automatically clarifies what should not happen (e.g. abort the SCTP association or close any data channels).

@juberti
Copy link
Contributor

juberti commented Apr 23, 2019

I don't think anything should happen automatically in an ICE failed situation. The app can stop things if it wants to, but given that an ICE restart might bring things back to life, we shouldn't do anything automatically.

@aboba
Copy link
Contributor

aboba commented Apr 24, 2019

@juberti Agreed.

@lgrahl Have you seen implementations that do things automatically (like closing SCTP associations) when ICE fails? This is really more of a protocol implementation issue, but just trying to understand whether this is an interop issue.

@lgrahl
Copy link
Contributor Author

lgrahl commented Apr 24, 2019

Have you seen implementations that do things automatically (like closing SCTP associations) when ICE fails?

Haven't seen any browser that closes in case of ICE failed but I haven't tested whether anyone (re)starts the SCTP association. So, perhaps this is not an issue at all. I believe a clarification in the form of "ICE moving into the failed state does not affect any of the upper layer transports" would be helpful since I can't imagine being the only person wondering.

@fippo
Copy link
Contributor

fippo commented Apr 24, 2019

for media: do we need something in jsep or rtp-usage saying there are no timeouts such as the ones described in https://tools.ietf.org/html/rfc3550#section-6.3.5?

aboba added a commit that referenced this issue Aug 13, 2019
@aboba aboba closed this as completed Aug 15, 2019
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

5 participants