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

set keep-alive defaults for new connections #128

Merged
merged 3 commits into from
Nov 9, 2021

Conversation

MisterHW
Copy link
Contributor

@MisterHW MisterHW commented Nov 6, 2021

Configure scpi-specific keep-alive settings for new connections. Without these, a connection lost on the client side blocks the server baiscally indefinitely, requiring a reset or some means to recover via another port or interface.

scpi server connection timeout is chosen such that when idle, zero window probes are sent periodically, and the connection is closed when they fail SCPI_KEEP_CNT times.

To change the defaults for all connections, add these values (lwipopts.h):

#define  TCP_KEEPIDLE_DEFAULT   15000UL /* Default KEEPALIVE timer in milliseconds */
#define  TCP_KEEPINTVL_DEFAULT   5000UL /* Default Time between KEEPALIVE probes in milliseconds */
#define  TCP_KEEPCNT_DEFAULT        3U  /* Default Counter for KEEPALIVE probes */

For keep-alive settings to be used, keep-alive needs to be enabled, and LwIP timers need to be used.

in lwipopts.h:

#define LWIP_TCP_KEEPALIVE 1

To allow LwIP timer operation, set (in opt.h or lwipopts.h):

#define LWIP_TIMERS 1 

(also requires FreeRTOS USE_TIMERS 1 or, for non-OS implementations, calling sys_check_timeouts() in the main loop).

tcp_priv.h tcp_tmr(); is called every 250 ms to keep track of timeouts, which is handled internally.


Also note: when other connections are used (e.g. http server listening socket), revisit memory pool sizes (lwipopts.h):

#define MEMP_NUM_NETCONN 10

(default is 4, which can be exhausted by scpi server when two connections are active on top of the two listening socket protocol control block (pcb) structs which stay allocated.

Configure scpi-specific keep-alive settings for new connections. Without these, a connection lost on the client side blocks the server baiscally indefinitely, requiring a reset or some means to recover via another port or interface.

scpi server connection timeout is chosen such that when idle, zero window probes are sent periodically, and the connection is closed when they fail SCPI_KEEP_CNT times.

also requires (lwipopts.h):
set #define LWIP_TCP_KEEPALIVE 1

main.c:
add #include "tcp_priv.h"
call tcp_tmr(); every 250 ms (e.g. via xTimerCreate(), xTimerStart() in callback).

To change the defaults for all connections, add these values (lwipopts.h):

#define  TCP_KEEPIDLE_DEFAULT   15000UL /* Default KEEPALIVE timer in milliseconds */
#define  TCP_KEEPINTVL_DEFAULT   5000UL /* Default Time between KEEPALIVE probes in milliseconds */
#define  TCP_KEEPCNT_DEFAULT        3U  /* Default Counter for KEEPALIVE probes */
@j123b567
Copy link
Owner

j123b567 commented Nov 8, 2021

The PR is probably ok, but you have something wrong with your LwIP configuration.

In the first place, from FreeRTOS, you should never ever call raw api functions directly (like tcp_tmr).

You should probably just define LWIP_TIMERS in the lwipopts.h and it will just start working without separate thread.

When not using RTOS, one should call sys_check_timeouts in the main loop, which will call other timers and also the tcp one. When RTOS (NO_SYS=0) is used, it is done by tcpip thread itself (needs LWIP_TIMERS to be enabled).

@MisterHW
Copy link
Contributor Author

MisterHW commented Nov 8, 2021

Thank for for the hint. Both USE_TIMERS and LWIP_TIMERS are enabled, and it appears that changes got lost between modifying defines directly, and regenerating them via STM32CubeMX - leading me to believe additionally calling tcp_tmr() would be necessary.
I verified it's working as per your description (I removed the redundant call to tcp_tmr() ). PR description updated accordingly.

examples/test-LwIP-netconn/scpi_server.c Outdated Show resolved Hide resolved
@j123b567 j123b567 merged commit 224f7ce into j123b567:master Nov 9, 2021
@MisterHW MisterHW deleted the keep-alive branch November 9, 2021 13:38
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.

2 participants