LLAPI - Big issue? Is / Can we have the IncomingMessageQueue be kept separate per connection?

[aabramychev]

Its not meant to prevent ddos, its just meant to prevent people from joining the game when they are not meant to.
For example…
-Someone joins the game and send a connect request to everyone.
-Everyone accepts the request.
-Later in the game everyone decides to kick this player due to cheating.
-Player send connection request and everyone blindly accepts…
-Client then realizes player is meant to have the player kicked and automatically re-kicks the player.
-Player keeps sending connect requests and rejoining, potentially blocking other players from joining.

I think it is the game related behavior. For example, player was not kicked off but banned for 20 second? Another example, you want to implement filter for course language, should it be solved on transport layer or user layer?

I think you misunderstood. What I was saying was that there is a ReactorMaximumSentMessages and a MaxSentMessageQueueSize, and those 2 work together to put limits on the total max send messages in the single send queue all connections share, as well as the max send messages a single connection can queue in that shared single queue for all connections.

understood now

Or do you mean we would bypass the single queue all together and messages will directly go to their separate queues per connection

this one. I can implement this. But by choosing that kind of configuration user MUST to pope events fro connections by himself…

If I do not understand you or want to argue we will always discuss this

[HiddenMonk]

@aabramychev

I think it is the game related behavior. For example, player was not kicked off but banned for 20 second? Another example, you want to implement filter for course language, should it be solved on transport layer or user layer?

Kicking / Banning / Filtering can all be done on the user level, however, there would need to be the ability to not allow a connection before a connection is made.
The 2 main reasons I want this ability is
1 - If I Kick / Ban a player, I do not want to allow them to reconnect until I decide otherwise.
2 - When working with Steam and Steamworks, I only want to allow players to join if they are actually in the same steam lobby as me.
Both of these would be handled through user code, but we would need at least the ability to check connection requests and decide if we want to accept them.
The steamworks api has something like this. A user sends a request, the other side gets a request callback and can then call a “Accept” method to accept the request if desired, or just ignore it.

@aabramychev

I can implement this. But by choosing that kind of configuration user MUST to pope events fro connections by himself…

That would be great and is exactly what we are looking for =), however, what do you mean by “user MUST to pope events fro connections by himself”. Arent we already poping events from connections ourselves? We make NetworkTransport.ReceiveFromHost calls which gives a Networking.NetworkEventType that we check and take action based on. How would it be done with the new configuration?

[aabramychev]

Kicking / Banning / Filtering can all be done on the user level, however, there would need to be the ability to not allow a connection before a connection is made.

correct, but it has done on user level (in main thread) it means that connect + info should be enough to make decision like:

if(event == connect)
   OnConnect(connectId, info)

...
OnConnect()
    if(info == wrongInfo)
         Disconnect(connectId);

=========
Right now user call ReceiveFromHost(... out connectionId, ...)
if we implement connection queue function will be

ReceiveFromConnection (... connectioId ...)

if you miss to receive from some connection, this connection will closed as its queue will overfull and all new packets for that connection will drop…
Other words, if user decide to sort out messages per connection he should

1. Use only ReceiveFromConnection() (+ RecieveFromHost to be notified about other events but not DataEvents)
2. Use this function for all connected connections:

ConnectDisconnect event = ReceiveDataFromHost();
for(int i = 0; i < MAxConnection; ++ i)
   if(connection == connected)
      ReceiveDataFromConnection (..., connection.connectionId, ...);

[HiddenMonk]

@aabramychev

correct, but it has done on user level (in main thread) it means that connect + info should be enough to make decision like:

if(event == connect)
   OnConnect(connectId, info)
...
OnConnect()
    if(info == wrongInfo)
         Disconnect(connectId);

The problem with that is that by the time we got the Connect event, the connection was already successfully made (at least from my test on localhost). Is there a way we can have it so we can decide this before the connection was made? In other words, I dont want to be sending packets to people I am going to be refusing a connection to later on anyways.
-If there are 9 people in the room with a max of 10 players
-Someone joins and become the 10th player, name is “joe”
-We kick that player “joe” due to cheating
-Player “joe” re-sends connection request
-LLAPI blindly accepts connect request and sends out a connect accept to “joe”
-NetworkTransport.Receive connect event has not been called yet
-A new good player named “sam” tries to join game, but game is seen as full due to connection made with “joe”
-Main thread runs and NetworkTransport.Receive connect event is called
-We detect “joe” should be kicked and call Disconnect
-Player “joe” re-sends connection request…
etc…

Maybe my thought process is wrong?

@aabramychev

if you miss to receive from some connection, this connection will closed as its queue will overfull and all new packets for that connection will drop…

Does the connection need to be dropped right away? Cant we just drop new received packets if the queue is full like how it is currently with NetworkTransport.Receive, and have the connection be dropped after a certain time? At least I think thats how things currently are.

Is this how the code will be like?

List<int> connectionIDs;
int maxEventsPerFramePerConnection = 50;

void Update()
{
    int outHostId;
    int outConnectionId;
    int outChannelId;

    int receivedSize;
    byte error;

    do
    {
        NetworkEventType eventType = NetworkTransport.ReceiveFromHost(hostId, out outConnectionId, out outChannelId, buffer, buffer.Length, out receivedSize, out error);

        switch (eventType)
        {
            case NetworkEventType.ConnectEvent:
            {
                OnConnect(outConnectionId, (NetworkError)error);
                break;
            }
            case NetworkEventType.DisconnectEvent:
            {
                OnDisconnect(outConnectionId, (NetworkError)error);
                break;
            }
        }
    }while(eventType != NetworkEventType.Nothing);

    foreach(int connectionID in connectionIDs)
    {
        int eventCount = 0;

//Example 1
        //ReceiveDataFromHostFromConnection returns true if there is data, false if there isnt
        while(eventCount++ < maxEventsPerFramePerConnection &&
              NetworkTransport.ReceiveDataFromHostFromConnection(hostID, connectionID, out outChannelId, buffer, buffer.Length, out receivedSize, out error))
        {
            OnData(connectionID, buffer, receivedSize);
        }

//Example 2
        ////Or it can return a NetworkEventType, doesnt really matter.
        //do
        //{
        //  NetworkEventType eventType = NetworkTransport.ReceiveDataFromHostFromConnection(hostID, connectionID, out outChannelId, buffer, buffer.Length, out receivedSize, out error);

        //  if(eventType == NetworkEventType.Data)
        //  {
        //      OnData(connectionID, buffer, receivedSize);
        //  }
        //}while(eventCount++ < maxEventsPerFramePerConnection &&
        //       eventType != NetworkEventType.Nothing);
    }
}

If so then that should be fine =)

[TwoTen]

@HiddenMonk

The problem with that is that by the time we got the Connect event, the connection was already successfully made (at least from my test on localhost). Is there a way we can have it so we can decide this before the connection was made? In other words, I dont want to be sending packets to people I am going to be refusing a connection to later on anyways.
-If there are 9 people in the room with a max of 10 players
-Someone joins and become the 10th player, name is “joe”
-We kick that player “joe” due to cheating
-Player “joe” re-sends connection request
-LLAPI blindly accepts connect request and sends out a connect accept to “joe”
-NetworkTransport.Receive connect event has not been called yet
-A new good player named “sam” tries to join game, but game is seen as full due to connection made with “joe”
-Main thread runs and NetworkTransport.Receive connect event is called
-We detect “joe” should be kicked and call Disconnect
-Player “joe” re-sends connection request…
etc…

Maybe my thought process is wrong?

Does the connection need to be dropped right away? Cant we just drop new received packets if the queue is full like how it is currently with NetworkTransport.Receive, and have the connection be dropped after a certain time? At least I think thats how things currently are.

Is this how the code will be like?

List<int> connectionIDs;

void Update()
{
    int outHostId;
    int outConnectionId;
    int outChannelId;

    int receivedSize;
    byte error;

    do
    {
        NetworkEventType eventType = NetworkTransport.ReceiveFromHost(hostId, out outConnectionId, out outChannelId, buffer, buffer.Length, out receivedSize, out error);

        switch (eventType)
        {
            case NetworkEventType.ConnectEvent:
            {
                OnConnect(outConnectionId, (NetworkError)error);
                break;
            }
            case NetworkEventType.DisconnectEvent:
            {
                OnDisconnect(outConnectionId, (NetworkError)error);
                break;
            }
        }
    }while(eventType != NetworkEventType.Nothing);

    foreach(int connectionID in connectionIDs)
    {
        //ReceiveDataFromHostFromConnection returns true if there is data, false if there isnt
        while(NetworkTransport.ReceiveDataFromHostFromConnection(hostID, connectionID, out outChannelId, buffer, buffer.Length, out receivedSize, out error))
        {
            OnData(connectionID, buffer, receivedSize);
        }

        ////Or it can return a NetworkEventType, doesnt really matter.
        //do
        //{
        //  NetworkEventType eventType = NetworkTransport.ReceiveDataFromHostFromConnection(hostID, connectionID, out outChannelId, buffer, buffer.Length, out receivedSize, out error);

        //  if(eventType == NetworkEventType.Data)
        //  {
        //      OnData(connectionID, buffer, receivedSize);
        //  }
        //}while(eventType != NetworkEventType.Nothing);
    }
}

If so then that should be fine =)

@HiddenMonk I suggest a design for aabra on this thread:
https://forum.unity3d.com/threads/is-there-a-built-in-authentication-system-in-unet.343217/#post-3166995
Would be nice if you could add to that / agree and/or disagree with it. Would partially solve some of the problems in this post (Even tho a per connection queue would be super sweet, atleast to optionally turn on)

[HiddenMonk]

@aabramychev

2 c more. @HiddenMonk , could you generate feature request? (do not have free resources for proper design right now) It will be much appreciated

@aabramychev

2 c more. @HiddenMonk , could you generate feature request? (do not have free resources for proper design right now) It will be much appreciated

I have written up the feature requests on the feedback site.

Feedback for having a MaxReceivedMessageQueueSize.
https://feedback.unity3d.com/suggestions/llapi-add-a-maxreceivedmessagequeuesize-in-connectionconfig-class

Feedback for having a serparate incoming message queue per connection.
https://feedback.unity3d.com/suggestions/llapi-option-to-have-the-incomingmessagequeue-be-kept-separate-per-connection

Feedback for authenticating connection requests
https://feedback.unity3d.com/suggestions/llapi-ability-to-authenticate-connection-requests