TCP-server keepalive

[Eric-1dev]

Hardware

Hardware: ESP-12
Core Version: 2.4.0-rc2

Hello. It may be useful for someone to be able to enable keepalive packets for a TCP server. For example, like this:

In file WifiClient.cpp:

void WiFiClient::keepalive(bool enable, uint32_t idle, uint32_t interval, uint32_t count)
{
	keep_enabled = enable;
	keep_idle = idle;
	keep_interval = interval;
	keep_count = count;
}

int WiFiClient::connect(IPAddress ip, uint16_t port)
{
    ip_addr_t addr;
    addr.addr = ip;

    if (_client)
        stop();

    // if the default interface is down, tcp_connect exits early without
    // ever calling tcp_err
    // http://lists.gnu.org/archive/html/lwip-devel/2010-05/msg00001.html
#if LWIP_VERSION_MAJOR == 1
    netif* interface = ip_route(&addr);
    if (!interface) {
        DEBUGV("no route to host\r\n");
        return 0;
    }
#endif

    tcp_pcb* pcb = tcp_new();
    if (!pcb)
        return 0;

    if (_localPort > 0) {
        pcb->local_port = _localPort++;
    }

> 	if ( keep_enabled )
> 	{
> 		pcb->so_options |= SOF_KEEPALIVE;
> 		pcb->keep_idle = keep_idle;
> 		pcb->keep_intvl = keep_interval;
> 		pcb->keep_cnt = keep_count;
> 	}
> 	else
> 		pcb->so_options &= ~SOF_KEEPALIVE;

    _client = new ClientContext(pcb, nullptr, nullptr);
    _client->ref();
    _client->setTimeout(_timeout);
    int res = _client->connect(&addr, port);
    if (res == 0) {
        _client->unref();
        _client = nullptr;
        return 0;
    }

    return 1;
}

In file WifiClient.h:

public:
 void keepalive(bool enable, uint32_t idle, uint32_t interval, uint32_t count);
private:
  bool keep_enabled;
  uint32_t keep_idle;
  uint32_t keep_interval;
  uint32_t keep_count;

Can it be worth adding this feature to the project? Thank you. Sorry for my english.

[devyte]

@d-a-v what do you think of this? At first glance, it looks sound to me.

[devyte]

My only comment would be: what happens when all three are specified as 0? Also, maybe split the keepalive(),ethod into 2: one to set the values, and also enable keepalive, and a different method, or maybe an overload, to disable keepalive.

[devyte]

void keepAlive(uint32_t idle, uint32_t interval, uint32_t count)
{
  _kaEnabled = true;
  _kaIdle = idle;
 _kaInterval = interval;
 _kaCount = count;
}
void keepAliveDisable()
{
  _kaEnabled = false;
  _kaIdle = 0;
 _kaInterval = 0;
 _kaCount = 0;
}

[d-a-v]

I personaly don't use this so I don't know. But it surely can be added so it can be tested, in the form of WiFiClient::keepAlive(idle,interval_count) that would manipulate the pcb by setting it inside ClientContext.h so that it can be enabled for any started tcp client (the ones by hand, or the ones got from a tcp server).
Should I make a PR for that ?

[kentaylor]

Keepalives, once enabled, should be on for every TCP connection regardless of what traffic is being sent over that socket. Therefore I'd argue against the suggested implementation, it should be done lower down the stack.

Keepalives operate below the TCP layer and do not interfere with the message flow in TCP so the only reason not to use keepalive is to reduce data transmissions. According to this good reference "Keepalive is non-invasive, and in most cases, if you're in doubt, you can turn it on without the risk of doing something wrong."

I added more to this response in the last two paragraphs of this comment.

[d-a-v]

@kentaylor I'm not sure lwIP API propose automatic keepalive for every new tcp connection. It needs to be checked but anyway It can be done by hand in ClientContext.h since it is the esp8266/arduino entry point for every tcp connection.
I agree with you, once enabled, keep-alive is silently handled by the tcp stack and do not interfere with anything. It is part of the protocol. It can be also seen as a continuous ping replacement that seems to be needed in #2330.
Another important point in your reference link for #2330 if the following:

Preventing disconnection due to network inactivity

[kentaylor]

My reading of the code is that keepalive is turned on globally by this #define rather than via an API. Once turned on it will be used on every TCP connection. Is that wrong? It is the behaviour I would have expected.

[Eric-1dev]

Hello. The keepalive packets were not sent before I wrote the functions from my first message. I watched this with tcpdump by connecting to the raspberry pi. I use this so that the node knows that it has disconnected from the server.

[kentaylor]

Thanks @EnricoElCartmanez, my reading of the code was wrong. There is also a test for a socket option here that is set for each socket.

[kentaylor]

Looking again at @EnricoElCartmanez initial suggestion in light of my new understanding that keepalives are not enabled by default. Can I suggest that the @EnricoElCartmanez suggestion be modified to enable keepalives by default with the default keep_idle, keep_interval and keep_count values also left as default?

According to this reference "Keepalive is non-invasive, and in most cases, if you're in doubt, you can turn it on without the risk of doing something wrong." The only downside is a little extra data traffic.

A TCP connection is created and destroyed in accordance with this state diagram
and when that process is followed "the node knows that it has disconnected from the server".

Sometimes though, packet loss means the Close - Responder Sequence in the bottom right of the diagram is not followed. This is normally rare but it is then that the keepalives provide the functionality that "that the node knows that it has disconnected from the server". Additionally keep alives will maintain mapping through a NAT which is required when a TCP connection is maintained for a long time and most IOT devices are behind NATs.

As the cost is low and the benefit high of avoiding an occasional incorrect assumption that a TCP socket exists when it doesn't, keepalives should be enabled for all TCP connections by default. Methods should exist to turn it off but it should be on by default. I doubt the options to change from default values for default keep_idle, keep_interval and keep_count values are even very useful.

[bill-orange]

I am a bit confused about how to use your code. For he keepalive function to work do you have to add anything like
WiFiClient.keepalive = true;

to your sketch's setup or is there no change to the affected sketch necessary?

[Eric-1dev]

Hello Bill. If you need to activate keepalive on your client, you should call keepalive() function in your sketch before calling connect() or do not call if you not need keepalive. Like this:

WiFiClient client;

void setup (void) {
  client.keepalive(true, 10000, 10000, 2);
  client.connect(ip, port);
}

void loop (void) {
  ...
}

I'm not a professional programmer. I just needed this function and it worked. I will be pleased if someone is useful.

[devyte]

@d-a-v is there a RAM difference when keepalive is enabled in lwip? I expect something like 12 bytes per pcb or along those lines.

[d-a-v]

Since keepalive code is already enabled by #define, there is no impact using it on lwip side (neither ram or flash).
The involved code is there

[devyte]

Already implemented in #3937 .