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

The integrated handshake feature (aka HSv5) with some refactoring #76

Merged
merged 14 commits into from
Sep 6, 2017

Conversation

ethouris
Copy link
Collaborator

@ethouris ethouris commented Aug 2, 2017

NOTE: this should be merged NOT to a master branch, but to a feature branch, with a name proposed as the source branch: dev-integrated-handshake.

This is the "integrated handshake" feature, aka HSv5, which makes the SRT portion of the handshake, including encryption, done in the same process as the UDT handshake.

The old way for exchanging the SRT specific information was the following:

  1. The usual UDT connection is established.
  2. The side that is set the SRTO_SENDER option starts sending messages based on UMSG_EXT with SRT_CMD_HSREQ, just before sending any data at the first call to UDT::sendmsg. Then it awaits a response with SRT_CMD_HSRSP the same way.
  3. If encryption is on, then after HSRSP is received, the sender side sends next UMSG_EXT messages with SRT_CMD_KMREQ, and so it awaits SRT_CMD_KMRSP.

The problem with the old way was that the whole process completes after several messages with data have been sent already, and if the data were encrypted, the receiver was unable to decrypt them until the KMREQ message arrives. Also due to an "important" bug in the UDT code it took quite a time to establish the connection (sometimes 1-3 seconds).

In HSv5 the HSREQ/HSRSP and KMREQ/KMRSP are attached to the handshake messages of type "conclusion" (in caller-listener it's the second part, after "induction" did the cookie exchange, in rendezvous it's either the very first message or the response to the initial "waveahand" message). Because of that the new term was introduced, which is "handshake side": INITIATOR and RESPONDER, which is also used for HSv4. INITIATOR should send HSREQ/KMREQ information to RESPONDER, which should respond with HSRSP/KMRSP.

For HSv4 it's simple - INITIATOR is the party that is set SRTO_SENDER, the other one is RESPONDER. This is regardless of the connection method and side.

For HSv5:

  • when caller-listener, then caller is INITIATOR, listener is RESPONDER
  • when rendezvous, they exchange the cookie value. By comparing them as integer numbers, the "winner" with the bigger cookie becomes an INITIATOR and the other party is RESPONDER

Note that in result in HSv5 has more chances to fail during the connection, however once the connection is established with the handshake process, it's completely configured for latency and encryption since the first sent byte, and handshake process - at least for the blocking mode - takes a fraction of a second (I didn't measure it exactly, but I predict it should be not more than 4*RTT).

Note also that since HSv5 SRT is bidirectional by default (there's even no "direction" term at all), so SRTO_TWOWAYDATA has been removed and SRTO_SENDER option is not necessary if you don't plan to communicate with the old client.

Secondary changes done here include:

  • The CCC class and its dependency has been completely removed. This can be restored later, but this time it should be done with marking all the places where any activity regarding congestion control is done in the CUDT class, which need not be the same as it was previously in UDT.
  • The CSRTCC class has been voided of any congestion control code, which has been moved to CUDT class. The CSRTCC class remained with all the encryption-related stuff, and encrypt/decrypt functions from CPacket class have been moved into this class, so it was finally renamed to CCryptoControl.
  • The bidirectional support according to the old handshake (known now as HSv4) has been removed, together with SRTO_TWOWAYDATA option. The only bidirectional support is with HSv5, the new handshake and it's bidirectional by default.
  • Due to bidirectional support, note that the latency can be set separately for sending and receiving for particular party. Setting a "peer" latency sets a "proposed" latency for the peer, and the effective latency is the maximum of the agent's latency and the "peer latency" set by the peer. Due to that there are two new options introduced: SRTO_RCVLATENCY and SRTO_PEERLATENCY. The old option SRTO_LATENCY sets both these latter options to the same value. Note that for one direction this will effectively work the same way as before (because the sender will effectively ignore SRTO_RCVLATENCY setting as well as SRTO_PEERLATENCY received from the peer receiver, as the sender isn't receiving).
  • Removed some conditionals, such as SRT_ENABLE_TSBPD, SRT_ENABLE_BSTATS and HAI_PATCH and the alternative code. I hope you'll feel more convenient with this.
  • New option: SRTO_MINVERSION. The value's syntax is for 32-bit integer 0x00JJNNPP, where JJ NN and PP mean Major, Minor and Patch, so 1.2.3 version will be 0x00010203. This is the minimum version required for the peer, meaning that when the peer's SRT version is less than this value, the connection will be rejected. This is useful if you, for example, require a bidirectional connection for your application to work correctly. Note though that this feature works only since a version that supports HSv5, so any HSv4 peer will fail to satisfy ANY minimum version requirement - leave this value with 0 to prevent any minimum version check. There's also a symbol introduced SRT_VERSION_FEAT_HSv5 set to 1.3.0 (this) version - you can use that value to request that the peer support HSv5 or check if it does (by SRTO_PEERVERSION).
  • New option: SRTO_STREAMID, plus two functions to get/set the ID, as the C interface for these options is a little bit clumsy. This sets the text-based identifier (maximum 512 bytes) to a socket that will be then connecting. When the listener side accepts the connection, the same identifier will be set to a socket returned by accept() and can be retrieved using this option. You can treat this as a kinda "port number extension" so that the application may serve multiple various streams using one port, where the stream can be selected by identifier. There's also a new application called siplex that demonstrates this feature, together with another existing feature since always - a possibility to use one outgoing port (and therefore one UDP socket) for multiple SRT sockets.
  • The rendezvous connection process has been COMPLETELY REWRITTEN for HSv5, although the old process for rendezvous with HSv4 is still working the old way. In contradiction to the old process, this now works with a special extra state machine that changes the states according to the types of incoming handshake messages. This was required because transmitting the handshake extensions with also side selection was too complicated to be simply injected into the old HSv4 code.

@@ -166,7 +166,12 @@ SocketOption srt_options [] {
{ "tsbpddelay", 0, SRTO_TSBPDDELAY, SocketOption::INT, SocketOption::PRE },
{ "tlpktdrop", 0, SRTO_TLPKTDROP, SocketOption::BOOL, SocketOption::PRE },
{ "nakreport", 0, SRTO_NAKREPORT, SocketOption::BOOL, SocketOption::PRE },
{ "conntimeo", 0, SRTO_CONNTIMEO, SocketOption::INT, SocketOption::PRE }
{ "conntimeo", 0, SRTO_CONNTIMEO, SocketOption::INT, SocketOption::PRE },
{ "lossmaxttl", 0, SRTO_LOSSMAXTTL, SocketOption::INT, SocketOption::PRE },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw no desctiption of this in the pull request comment

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a minor change, this option was there since a long time (at least since the github version) and I forgot to expose it through socketoptions.

int err = NET_ERROR;
if ( err != EAGAIN ) // For EAGAIN, this isn't an error, just a useless call.
if ( err == EAGAIN || err == EINTR ) // For EAGAIN, this isn't an error, just a useless call.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come is EINTR a retry later. Most probably an application terminating I may have found why my CLI test app do not terminate as before on Ctrl-C.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EINTR is "interrupted system call". It can also happen when an application gets SIGUSR1 sent from some other process because the application was designed to work with this. This signal is theoretically unrelated to SRT, but still it's a signal and it will interrupt a system call, like every signal does. This is generally a problem for libraries, and a library must be as transparent as possible for signals. Not sure if EINTR is even possible in nonblocking mode, but at least I can imagine such a situation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EINTR is fatal for most normal applications and library should have means of indicating this condition to the main application code. Please do not treat it as "try again".

Copy link
Collaborator Author

@ethouris ethouris Aug 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not discuss such details here. This is a long story, I don't want to spread large comments here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @ethouris , I don't think Jean and me are looking for a story but rather for explanation why we should be masking EINTR in this particular case. Also I believe Jean thinks this is already breaking his app so if this change is to blame for CTRL-C not working it is a bug.

@ethouris
Copy link
Collaborator Author

Have no idea what happened to this threadname.h in srtcore directory. It must remain there until this branch is merged into upstream, I'll delete it somehow later.

@rndi rndi changed the base branch from master to dev September 6, 2017 15:41
@rndi rndi merged commit c91d5f7 into Haivision:dev Sep 6, 2017
@ethouris ethouris deleted the dev-integrated-handshake branch January 17, 2018 10:30
@maxsharabayko maxsharabayko mentioned this pull request Sep 4, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants