Module: kamailio Branch: master Commit: 1acede64041307b783ed90736ca114917bafbc14 URL: https://github.com/kamailio/kamailio/commit/1acede64041307b783ed90736ca11491...
Author: Daniel-Constantin Mierla miconda@gmail.com Committer: Daniel-Constantin Mierla miconda@gmail.com Date: 2023-10-16T08:35:09+02:00
core: added tcp_check_timer parameter
- set the check interval (in seconds) for tcp connections - default 10
---
Modified: src/core/cfg.lex Modified: src/core/cfg.y Modified: src/core/globals.h Modified: src/main.c
---
Diff: https://github.com/kamailio/kamailio/commit/1acede64041307b783ed90736ca11491... Patch: https://github.com/kamailio/kamailio/commit/1acede64041307b783ed90736ca11491...
---
diff --git a/src/core/cfg.lex b/src/core/cfg.lex index c8548c4eb6a..44c62279b7d 100644 --- a/src/core/cfg.lex +++ b/src/core/cfg.lex @@ -367,6 +367,7 @@ MAXSNDBUFFER maxsndbuffer SQL_BUFFER_SIZE sql_buffer_size MSG_RECV_MAX_SIZE msg_recv_max_size TCP_MSG_READ_TIMEOUT tcp_msg_read_timeout +TCP_CHECK_TIMER tcp_check_timer CHILDREN children SOCKET socket BIND bind @@ -854,6 +855,7 @@ IMPORTFILE "import_file" <INITIAL>{SQL_BUFFER_SIZE} { count(); yylval.strval=yytext; return SQL_BUFFER_SIZE; } <INITIAL>{MSG_RECV_MAX_SIZE} { count(); yylval.strval=yytext; return MSG_RECV_MAX_SIZE; } <INITIAL>{TCP_MSG_READ_TIMEOUT} { count(); yylval.strval=yytext; return TCP_MSG_READ_TIMEOUT; } +<INITIAL>{TCP_CHECK_TIMER} { count(); yylval.strval=yytext; return TCP_CHECK_TIMER; } <INITIAL>{CHILDREN} { count(); yylval.strval=yytext; return CHILDREN; } <INITIAL>{SOCKET} { count(); yylval.strval=yytext; return SOCKET; } <INITIAL>{BIND} { count(); yylval.strval=yytext; return BIND; } diff --git a/src/core/cfg.y b/src/core/cfg.y index 9c7efd83f92..a5e16e259e1 100644 --- a/src/core/cfg.y +++ b/src/core/cfg.y @@ -423,6 +423,7 @@ extern char *default_routename; %token SQL_BUFFER_SIZE %token MSG_RECV_MAX_SIZE %token TCP_MSG_READ_TIMEOUT +%token TCP_CHECK_TIMER %token USER %token GROUP %token CHROOT @@ -1019,6 +1020,8 @@ assign_stm: | MSG_RECV_MAX_SIZE EQUAL error { yyerror("number expected"); } | TCP_MSG_READ_TIMEOUT EQUAL NUMBER { ksr_tcp_msg_read_timeout=$3; } | TCP_MSG_READ_TIMEOUT EQUAL error { yyerror("number expected"); } + | TCP_CHECK_TIMER EQUAL NUMBER { ksr_tcp_check_timer=$3; } + | TCP_CHECK_TIMER EQUAL error { yyerror("number expected"); } | CHILDREN EQUAL NUMBER { children_no=$3; } | CHILDREN EQUAL error { yyerror("number expected"); } | STATS_NAMESEP EQUAL STRING { ksr_stats_namesep=$3; } diff --git a/src/core/globals.h b/src/core/globals.h index 91c97ae9a4a..aac82563d51 100644 --- a/src/core/globals.h +++ b/src/core/globals.h @@ -240,6 +240,7 @@ extern int ksr_rpc_exec_delta;
extern int ksr_msg_recv_max_size; extern int ksr_tcp_msg_read_timeout; +extern int ksr_tcp_check_timer;
#ifdef USE_DNS_CACHE extern int diff --git a/src/main.c b/src/main.c index 719debba099..a949d930407 100644 --- a/src/main.c +++ b/src/main.c @@ -532,6 +532,7 @@ char *pgid_file = 0;
int ksr_msg_recv_max_size = 32767; /* 2^15 - 1 */ int ksr_tcp_msg_read_timeout = KSR_TCP_MSGREAD_TIMEOUT; +int ksr_tcp_check_timer = 10; /* seconds to check tcp connections */
/* memory manager */ #define SR_MEMMNG_DEFAULT "qm" @@ -1722,8 +1723,10 @@ int main_loop(void) cfg_main_reset_local();
#ifdef USE_TCP - if(!tcp_disable) { - if(sr_wtimer_add(tcp_timer_check_connections, NULL, 10) < 0) { + if(!tcp_disable && ksr_tcp_check_timer > 0) { + if(sr_wtimer_add( + tcp_timer_check_connections, NULL, ksr_tcp_check_timer) + < 0) { LM_CRIT("cannot add timer for tcp connection checks\n"); goto error; }
If so, how about defaulting to half of tcp_msg_read_timeout or tcp_msg_data_timeout whichever is smallest?
On 16.10.23 10:25, Juha Heinanen wrote:
Daniel-Constantin Mierla via sr-dev writes:
core: added tcp_check_timer parameter
- set the check interval (in seconds) for tcp connections
- default 10
Does this need to be smaller than tcp_msg_read_timeout and tcp_msg_data_timeout in order for them to have effect?
They will have anyhow effect, but can be later -- you have to consider always the value of tcp_check_timeras a possible delay.
Smaller is recommended for better accuracy.
Cheers, Daniel
On 16.10.23 12:27, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
Smaller is recommended for better accuracy.
In order to make configuration simpler, how about having a dynamic default as I suggested?
Default is half (hardcoded), but its own parameter gives more flexibility for granularity -- one extra parameter is not adding much complexity, imo, and case by case one may want more often checks to clean up those that end up in timeout.
Cheers, Daniel
Daniel-Constantin Mierla writes:
In order to make configuration simpler, how about having a dynamic default as I suggested?
Default is half (hardcoded), but its own parameter gives more flexibility for granularity -- one extra parameter is not adding much complexity, imo, and case by case one may want more often checks to clean up those that end up in timeout.
I didn't suggest to remove the parameter, but change its default value if the parameter is not given.
On 16.10.23 12:44, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
In order to make configuration simpler, how about having a dynamic default as I suggested?
Default is half (hardcoded), but its own parameter gives more flexibility for granularity -- one extra parameter is not adding much complexity, imo, and case by case one may want more often checks to clean up those that end up in timeout.
I didn't suggest to remove the parameter, but change its default value if the parameter is not given.
You can add such behaviour if you want.
Cheers, Daniel
How about the diff below?
Also, is there plan to backport ksr_tcp_msg_data_timeout, ksr_tcp_msg_read_timeout, and ksr_tcp_check_timer to 5.7, since they can help in protecting from DoS attacks that we have seen in the wild.
-- Juha
diff --git a/src/main.c b/src/main.c index 0fa2da6ec2..f3cddf8bad 100644 --- a/src/main.c +++ b/src/main.c @@ -535,7 +535,7 @@ int ksr_tcp_msg_read_timeout = 20; /* timeout (secs) to read SIP message */ int ksr_tcp_msg_data_timeout = 20; /* timeout (secs) to receive first msg data */ int ksr_tcp_accept_iplimit = 1024; /* limit of accepted connections per IP */ -int ksr_tcp_check_timer = 10; /* seconds to check tcp connections */ +int ksr_tcp_check_timer = -1; /* seconds to check tcp connections */
/* memory manager */ #define SR_MEMMNG_DEFAULT "qm" @@ -1726,12 +1726,22 @@ int main_loop(void) cfg_main_reset_local();
#ifdef USE_TCP - if(!tcp_disable && ksr_tcp_check_timer > 0) { - if(sr_wtimer_add( + if(!tcp_disable) { + if(ksr_tcp_check_timer == -1) { + if(ksr_tcp_msg_data_timeout > 0 && ksr_tcp_msg_read_timeout > 0) + ksr_tcp_check_timer = + MIN(ksr_tcp_msg_data_timeout, ksr_tcp_msg_read_timeout) / 2; + else + ksr_tcp_check_timer = ksr_tcp_msg_data_timeout > 0 ? + ksr_tcp_msg_data_timeout / 2 : ksr_tcp_msg_read_timeout / 2; + } + if(ksr_tcp_check_timer > 0) { + if(sr_wtimer_add( tcp_timer_check_connections, NULL, ksr_tcp_check_timer) - < 0) { - LM_CRIT("cannot add timer for tcp connection checks\n"); - goto error; + < 0) { + LM_CRIT("cannot add timer for tcp connection checks\n"); + goto error; + } } } #endif
Looks ok for me. If you push it, then you have to update the wiki core books section for it as well.
Cheers, Daniel
On 19.10.23 09:14, Juha Heinanen via sr-dev wrote:
How about the diff below?
Also, is there plan to backport ksr_tcp_msg_data_timeout, ksr_tcp_msg_read_timeout, and ksr_tcp_check_timer to 5.7, since they can help in protecting from DoS attacks that we have seen in the wild.
-- Juha
diff --git a/src/main.c b/src/main.c index 0fa2da6ec2..f3cddf8bad 100644 --- a/src/main.c +++ b/src/main.c @@ -535,7 +535,7 @@ int ksr_tcp_msg_read_timeout = 20; /* timeout (secs) to read SIP message */ int ksr_tcp_msg_data_timeout = 20; /* timeout (secs) to receive first msg data */ int ksr_tcp_accept_iplimit = 1024; /* limit of accepted connections per IP */ -int ksr_tcp_check_timer = 10; /* seconds to check tcp connections */ +int ksr_tcp_check_timer = -1; /* seconds to check tcp connections */
/* memory manager */ #define SR_MEMMNG_DEFAULT "qm" @@ -1726,12 +1726,22 @@ int main_loop(void) cfg_main_reset_local();
#ifdef USE_TCP
if(!tcp_disable && ksr_tcp_check_timer > 0) {if(sr_wtimer_add(
if(!tcp_disable) {if(ksr_tcp_check_timer == -1) {if(ksr_tcp_msg_data_timeout > 0 && ksr_tcp_msg_read_timeout > 0)ksr_tcp_check_timer =MIN(ksr_tcp_msg_data_timeout, ksr_tcp_msg_read_timeout) / 2;elseksr_tcp_check_timer = ksr_tcp_msg_data_timeout > 0 ?ksr_tcp_msg_data_timeout / 2 : ksr_tcp_msg_read_timeout / 2;}if(ksr_tcp_check_timer > 0) {if(sr_wtimer_add( tcp_timer_check_connections, NULL, ksr_tcp_check_timer)
< 0) {LM_CRIT("cannot add timer for tcp connection checks\n");goto error;
< 0) {LM_CRIT("cannot add timer for tcp connection checks\n");goto error;} }#endif _______________________________________________ Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
-
Daniel-Constantin Mierla -
Juha Heinanen