eclipse / paho.mqtt-sn.embedded-c

Paho C MQTT-SN gateway and libraries for embedded systems. Paho is an Eclipse IoT project.
https://eclipse.org/paho
Other
313 stars 179 forks source link

Known client unable to connect #234

Closed saumilsdk closed 2 years ago

saumilsdk commented 3 years ago

I am getting below error when client name is valid and showing up but still not able to connect.

Error: Client(117.254.177.XXX:36XXX) was rejected. CONNECT message has been discarded.

ty4tw commented 3 years ago

Hi, Could you attach gateway.conf and clients.conf?

Krolken commented 3 years ago

Maybe I can hijack this issue. I am seeing the same thing on an increasing number of our devices.

I have forked the project to up the device limit to 10 000 devices as the current hardcoded max was set at 1000 and that caused problems for us when we got over 1000 devices.

We are running in transparent mode, as we right now don't have a simple way to automate a new deploy with an updated client-list everytime we get new devices.

As I understad it in the code the dropping of the CONNECT message is due to a nullptr for client. But I can't see any reason for the client to be nullptr

Could it have anything to do with transparent mode and the number of clients? ~1200 as of now and will increase to around 9000.

Or is there an error in the internal client list that is saving a client as nullprt and is returning this on getClient resulting in dropping of CONNECT?

My gateway conf:

# config file of MQTT-SN Gateway
#

BrokerName=172.16.43.151
BrokerPortNo=1883
BrokerSecurePortNo=8883

#
# When AggregatingGateway=YES or ClientAuthentication=YES,
# All clients must be specified by the ClientList File
#

ClientAuthentication=NO
AggregatingGateway=NO
QoS-1=NO
Forwarder=NO

#ClientsList=/path/to/your_clients.conf

PredefinedTopic=NO
#PredefinedTopicList=/path/to/your_predefinedTopic.conf

#RootCAfile=/etc/ssl/certs/ca-certificates.crt
#RootCApath=/etc/ssl/certs/
#CertsFile=/path/to/certKey.pem
#PrivateKey=/path/to/privateKey.pem

GatewayID=1
GatewayName=MDM-MQTT-SN
KeepAlive=900
#LoginID=your_ID
#Password=your_Password

# UDP
GatewayPortNo=2883
MulticastIP=225.1.1.1
MulticastPortNo=1883

# LOG
ShearedMemory=NO;

We are using RabbitMQ as broker and it has lots of free file-descriptors so I don't think it is the brokers problem.

ty4tw commented 3 years ago

Hi, SELECT macro is used for a multiplex io. so, socket must be less than 1024.

2021年6月24日(木) 19:34 Henrik Wahlgren @.***>:

Maybe I can hijack this issue. I am seeing the same thing on an increasing number of our devices.

I have forked the project to up the device limit to 10 000 devices as the current hardcoded max was set at 1000 and that caused problems for us when we got over 1000 devices.

We are running in transparent mode, as we right now don't have a simple way to automate a new deploy with an updated client-list everytime we get new devices.

As I understad it in the code the dropping of the CONNECT message is due to a nullptr for client. But I can't see any reason for the client to be nullptr

Could it have anything to do with transparent mode and the number of clients? ~1200 as of now and will increase to around 9000.

Or is there an error in the internal client list that is saving a client as nullprt and is returning this on getClient resulting in dropping of CONNECT?

My gateway conf:

config file of MQTT-SN Gateway

#

BrokerName=172.16.43.151 BrokerPortNo=1883 BrokerSecurePortNo=8883

#

When AggregatingGateway=YES or ClientAuthentication=YES,

All clients must be specified by the ClientList File

#

ClientAuthentication=NO AggregatingGateway=NO QoS-1=NO Forwarder=NO

ClientsList=/path/to/your_clients.conf

PredefinedTopic=NO

PredefinedTopicList=/path/to/your_predefinedTopic.conf

RootCAfile=/etc/ssl/certs/ca-certificates.crt

RootCApath=/etc/ssl/certs/

CertsFile=/path/to/certKey.pem

PrivateKey=/path/to/privateKey.pem

GatewayID=1 GatewayName=MDM-MQTT-SN KeepAlive=900

LoginID=your_ID

Password=your_Password

UDP

GatewayPortNo=2883 MulticastIP=225.1.1.1 MulticastPortNo=1883

LOG

ShearedMemory=NO;

We are using RabbitMQ as broker and it has lots of free file-descriptors so I don't think it is the brokers problem.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eclipse/paho.mqtt-sn.embedded-c/issues/234#issuecomment-867529123, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3X2BESUD2ZDBBTWTDRIKTTUMC5FANCNFSM4ZWVOYWA .

Krolken commented 3 years ago

Thanks for the response. I didnt know that the select was the limiting factor. Now the max device of 1000 makes more sense :) I guess it started working with the 1000 we had and some more until we got over 1024 then.

Is this on the normal MQTT connection to the broker? Since the MQTT-SN is over UDP. Could I get around it if I make the setup aggregating. I might have to spend some time adding devices in the device list but that should be fine if I can still make it work. We will end up with around 9000 devices when all are rolled out.

ty4tw commented 3 years ago

Aggregating Gateway might be a solution. or Change select() to poll().

Check MQTTSNGWBrokerRecvTask.cpp to change it.

Krolken commented 3 years ago

Thanks for the response :)

Looking into aggregating gateway I understand it as I need to add the sending port for each client. Is that correct?

#Client List
 *     ClientId1,192.168.10.10:11200

The problem is that the sending port for out devices are random. Is it possible to use a wildcard for the port and just use IP?

Krolken commented 3 years ago

Another thought. Is there a function to clear stale clients? I couldn't find any uses for the erase function on the clientList. If they are cleated we could load balance incoming UDP traffic with sticky sessions to focus on one backend. But if the clients aren't cleared from the clientList except on restart we would eventually fill upp the clientList.

ty4tw commented 3 years ago

How do you decide that a client is stale?

Looking into aggregating gateway I understand it as I need to add the sending port for each client. Is that correct?

If you don't need connecting to a broker over TLS or Clients are not QoS-1, Adding Client address and portNo are not required.

Krolken commented 3 years ago

Thanks for the tip. I ended up implementing an async python Gateway instead as we hade some other requirements that was not easy to add in this project. No need to keep the issue open for me.

saumilsdk commented 3 years ago

@Krolken Sorry for responding late but you are right. It was the issue with MAX client supported which was hardcoded to 100. We have modified it to 64K and seems working fine now. Along with that it require change in max inflight messages as below.

_packetEventQue.setMaxSize(_params.maxInflightMsgs * _params.maxClients);

Below code need to be changed to support more clients.

/*  anonimous clients */
    if ( _clientCnt > MAX_CLIENTS )
    {
        return 0;  // full of clients
    }
ty4tw commented 3 years ago

I could find a bug because of your info. The bugfix is as follows:

Client* ClientList::createPredefinedTopic(MQTTSNString* clientId, string topicName, uint16_t topicId, bool aggregate)
{
    if (topicId == 0)
    {
        WRITELOG("Invalid TopicId. Predefined Topic %s,  TopicId is 0. \n", topicName.c_str());
        return nullptr;
    }

    if (strcmp(clientId->cstring, common_topic) == 0)
    {
        _gateway->getTopics()->add((const char*) topicName.c_str(), topicId);
        return nullptr;
    }
    else
    {
        Client* client = getClient(clientId);

        if (_authorize && client == nullptr)
        {
            return nullptr;
        }

        client = createClient(NULL, clientId, aggregate);    //  <=== BUGFIX
        if (client == nullptr)
        {
            return nullptr;
        }

        // create Topic & Add it
        client->getTopics()->add((const char*) topicName.c_str(), topicId);
        client->_hasPredefTopic = true;
        return client;
    }
}
nikhil30081995 commented 2 years ago

It was the issue with MAX client supported which was hardcoded to 100. We have modified it to 64K and seems working fine now. Along with that it require change in max inflight messages as below.

_packetEventQue.setMaxSize(_params.maxInflightMsgs * _params.maxClients);

Hello @saumilsdk what could be the max clients we can define ? As per your suggestion if i keep clients number to 64K, do i have to change "max inflight messages" or the commit took care of that ?

saumilsdk commented 2 years ago

@nikhil30081995 you have to take care of removing stale client.

nikhil30081995 commented 2 years ago

take care of removing stale client

Can you explain a little what i need to change in which file ?

saumilsdk commented 2 years ago

Just make sure MQTTSNGWClientList.cpp maxClients checks are correct and when client DISCONNECT it should remove the client. May be you can ignore this as of now and go ahead with your current changes when you hit this issue then you can come back and check this.

ty4tw commented 2 years ago

Hi,

In the latest version of gateway, you can assign the maximum number of clients with the MaxNumberOfClients parameter in gateway.conf.

Tomoaki YAMAGUCHI

2021年10月5日(火) 18:23 Saumil Kapadia @.***>:

Just make sure MQTTSNGWClientList.cpp maxClients checks are correct and when client DISCONNECT it should remove the client. May be you can ignore this as of now and go ahead with your current changes when you hit this issue then you can come back and check this.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/eclipse/paho.mqtt-sn.embedded-c/issues/234#issuecomment-934229820, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3X2BC63EFPW4HSWLERHYTUFK72VANCNFSM4ZWVOYWA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

nikhil30081995 commented 2 years ago

In the latest version of gateway, you can assign the maximum number of clients with the MaxNumberOfClients parameter in gateway.conf.

And what's the max limit(value) of this parameter ?

ty4tw commented 2 years ago

Hi nikhil30081995,

It depends on available heap size and number of sockets of your system.

Tomoaki YAMAGUCHI

2021年10月6日(水) 12:17 nikhil30081995 @.***>:

In the latest version of gateway, you can assign the maximum number of clients with the MaxNumberOfClients parameter in gateway.conf.

And what's the max limit of this parameter ?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/eclipse/paho.mqtt-sn.embedded-c/issues/234#issuecomment-935341490, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3X2BBK4JSAN5HSO2DRLWDUFO5VZANCNFSM4ZWVOYWA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

nikhil30081995 commented 2 years ago

Hey @ty4tw

It depends on available heap size and number of sockets of your system. Okay i have kept it to 1000.

Just make sure MQTTSNGWClientList.cpp maxClients checks are correct and when client DISCONNECT it should remove the client. Do i have to do this or this is taken care of in the commit number f2dcda3 (bug fix of this post) ?

Thanks in advance.