Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

dtls server quick connection fix #38

Merged
merged 4 commits into from
Jul 7, 2022

Conversation

slabajo
Copy link
Member

@slabajo slabajo commented Jun 27, 2022

What is the current behavior you want to change?

This change is related to a previous PR in #37
More specifically this change covers a use case not covered in the previous PR. This is as follows:

  • When Kurento is set as DTLS server it may happen that the remote peer of the WebRTC connection reaches to ICE connected state before Kurento does.
  • The problem appears when not only the remote peer gets ICE connected before but when the peer sends the DTLS Client Hello and it arrives before the WebRTCEndpoint in Kurento has reached the ICE connected state.
  • If this happens, the DTLS Client hello is received in the nicesrc element from the Webrtctransportsrc and it is relayed to the dtlssrtpdec element. This in turns starts DTLS handshake and sends back a DTLS Server hello to the nicesink element. But, and this is the real problem, the nicesink finds the ICE connection not yet established and silently drops the DTLS server hello.
  • When the DTLS timeout is up (1 second this first time, but doubling each time), the endpoint will resend again the DTLS server hello packet, and hopefully this time it will find the ICE connection established and the DTLS server hello will be delivered to the remote peer. If not, it will try again on the next timeout.
  • The problem is that each timeout introduces some additional delay in the WebRTC connection establishment.
  • In many cases that we have observed, the DTLS Client hello just arrives a few milliseconds before the ICE gets CONNECTED in WebRTCEndpoint. But the impact is that those few miliseconds will make the connection to delay around 1 second.

This problem has a lesser impact than the previous PR, but in the end it is worth improving it.

What is the new behavior provided by this change?

  • This change only takes effect when the WebRTCEndpoint is configured as DTLS server (waiter for DTLS handshake)
  • It just captures all packets coming from the nicesrc element to the dtlssrpdec and stores them until the ICE gets connected.
  • A signal is connected so that when ICE connection changes state, and the new state is connected, it gets the temporary stored buffers and delivers them to the dtlssrtpdec element, thus ensuring the DTLS handshake process only takes place when the ICE connection is established.

According to our observations there is only one possible buffer stored that corresponds to the DTLS Client hello received from remote peer. Also, we noticed that if in the same call that notifies ICE connection we delivered the stored DTLS client hello, the answer (DTLS server hello) still finds the ICE connection not usable, so to avoid that, we just make that delivery asynchronously in a separate thread 10 ms later.

How has this been tested?

IT has been tested with automatic tests that cannot yet be used in this Kurento version as they need GStreamer 1.18 to work.
We have also tested it in real environmentes capturing network traffic to see that the change causes the desired behaviour.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature / enhancement (non-breaking change which improves the project)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • My change requires a change to the documentation
  • My change requires a change in other repository

Checklist

  • I have read the Contribution Guidelines
  • I have added an explanation of what the changes do and why they should be included
  • I have written new tests for the changes, as applicable, and have successfully run them locally

@j1elo j1elo self-assigned this Jul 4, 2022
(cherry picked from commit acadb81a575c3f531297fce98b634a31c84ff2ea)
@j1elo j1elo merged commit dd2d019 into Kurento:master Jul 7, 2022
j1elo added a commit that referenced this pull request Jul 7, 2022
j1elo added a commit that referenced this pull request Jul 7, 2022
Reverts #38

Reason: Code compiles but linker fails:

```
[ 40%] Linking C shared library libkmswebrtcendpointlib.so
CMakeFiles/kmswebrtcendpointlib.dir/kmswebrtctransportsink.c.o: In function `gst_bin_iterate_all_by_element_factory_name':
./src/gst-plugins/webrtcendpoint/./src/gst-plugins/webrtcendpoint/kmswebrtctransportsink.c:59: multiple definition of `gst_bin_iterate_all_by_element_factory_name'
CMakeFiles/kmswebrtcendpointlib.dir/kmswebrtctransportsrc.c.o:./src/gst-plugins/webrtcendpoint/./src/gst-plugins/webrtcendpoint/kmswebrtctransportsrc.c:57: first defined here
collect2: error: ld returned 1 exit status
```
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants