[core] Fixed wrong max socket ID value formula

[ethouris]

Changes:

1. Removed check if socket ID provided in the handshake fits in some range. This check is useless, sockets are allowed to have any value, even value in the group range if used against pre-1.4.2 version. Just not 0 (although this is a special value used in the handshake).
2. Definition of MAX_SOCKET_VAL fixed to be the same as SOCKGROUP_MASK, that is 0x40000000. 1 << 29 was wrong (0x20000000).
3. Fixed formula for translating random value into a random socket ID value. The old formula didn't take into account, though little probable, theoretically possible case when rand() returns exactly RAND_MAX.

[maxsharabayko]

Let's split this PR into two.
I agree with the removal of this check for socket ID value.
It might be still worth checking the value, but given the check was added recently in #1781 and result in a connection error - can be deleted.

But I am not yet sure about changing MAX_SOCKET_VAL and changing random Socket ID generation.

[ethouris]

Ok, #1817 replaces.

[ethouris]

The value of SRTGROUP_MASK is 1 << 30, that is 0x4000000. The special values:

• 0 - used in the handshake for connection request
• -1 - invalid socket value or error marker

are considered excluded. Therefore the last 2 bits are reserved (values 0x8000'0000 and 0x4000'0000).

The current version of MAX_SOCKET_VAL is 1 << 29, that is, 0x2000'0000, which is wrong because it excludes also the range between 0x2000'0000 and 0x3FFF'FFFF. The goal of this constant is to exclude only the range between 0x4000'000 and 0x7FFF'FFFF, which are reserved for group IDs. This is required in the socket ID generator that should, in case when decreased to 0, roll back to 0x3FFF'FFFF. But with the current value of 1 << 29 it actually rolls back to 0x1FFF'FFFF. Not that it causes problems, but it contradicts the definitions.

It's not a problem for getting proper IDs for sockets and groups because generation of the group ID relies on generating a regular socket ID and set it the SRTGROUP_MASK flag. So, it can potentially generate 0x3FFF'FFFF and as a group ID it will return the value of 0x7FFF'FFFF.

[maxsharabayko]

SRT v1.4.0, v1.4.1

m_SocketIDGenerator = 1 + (int)((1 << 30) * (double(rand()) / RAND_MAX));

SRT v1.4.2

static const int32_t MAX_SOCKET_VAL = 1 << 29;    // maximum value for a regular socket
m_SocketIDGenerator = 1 + int(MAX_SOCKET_VAL * rand1_0);

The issue was introduced in PR #1076.
Affected SRT version: v1.4.2.

[maxsharabayko]

Ranges of Socket ID values.

• Value 0. Reserved for handshake procedure. If the destination Socket ID is 0 (destination Socket ID unknown) the packet will be sent to the listening socket or to a socket that is in the rendezvous connection phase.

• [1; 2^30). Single socket ID range.

• (2^30; 2^31). Group socket ID range. Effectively any positive number from [1; 2^30) with bit 30 set to 1.

The most significant bit 31 (sign bit) is left unused so that checking for a value <= 0 identifies an invalid socket ID.

Also, the value 2^30 is an invalid group socket ID (to confirm). <- @ethouris

[maxsharabayko]

@ethouris So what happens if SRT v1.4.1 and earlier tries to establish a single connection to e.g. v1.4.2, with a Socket ID in the range reserved for groups: (2^30; 2^31)?

[ethouris]

The socket will be accepted. There's no problem with it. The peer can provide a socket value of any kind, just not 0 and not -1. It simply fits in 32-bit number and it's used in all these operations and the agent's application will never see it. Of course, if there's some method to provide it to the user application it may have the condition sock & SRTGROUP_MASK != 0 satisfied, but fortunately it will never have a chance.

The agent that supports groups must only make sure that a value for a local socket (both from srt_create_socket() and srt_accept()) is never in the range for groups. But this is what group-supporting version do anyway in the first place.

[maxsharabayko]

I feel in this case a lot of conditions inside the library like if (u & SRTGROUP_MASK) will result to true. E.g. locating a socket in CUDTUnited::connect(..).
Although it is for local Socket IDs, not remote...

[ethouris]

This will never happen in a version that supports groups for the LOCAL (agent's) sockets because a value like that will never be generated. When a group ID value is required it still generates the socket ID value and sets the SRTGROUP_MASK flag.

[ethouris]

The only potential problem I could see with the implementation of some special kinds of groups, whose IDs could be sent with the intention of being group IDs in the SRT protocol. That's why we need to take care of that such things use only new syntax of the protocol, as the socket ID in the "destination" field in the header and the data in the handshake packets can always contain a socket ID that is in the group-reserved range.

[maxsharabayko]

I think MAX_SOCKET_VAL must be (1 << 30) - 1 for convenience.

static const int32_t MAX_SOCKET_VAL = 1 << 30 - 1; // maximum socket value

Of course, changing all usages like

    if (sockval <= 0)
    {
        // A rollover on the socket value.
        m_SocketIDGenerator = MAX_SOCKET_VAL;
    }

[maxsharabayko]

Looks good.
We should however consider using C++11 std::uniform_int_distribution for a better randomness.

Further related work: #1825, #1823.