Module: kamailio Branch: master Commit: ba921b2112e87625fba5789d1b049161bb611073 URL: https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789d1b049161...
Author: S-P Chan shihping.chan@gmail.com Committer: S-P Chan shihping.chan@gmail.com Date: 2024-02-06T10:17:53+08:00
core/rthread.h: add prototype for db queries
---
Modified: src/core/rthreads.h
---
Diff: https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789d1b049161... Patch: https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789d1b049161...
---
diff --git a/src/core/rthreads.h b/src/core/rthreads.h index a5ad7670dae..a416ad2ca50 100644 --- a/src/core/rthreads.h +++ b/src/core/rthreads.h @@ -98,3 +98,60 @@ static void run_threadV(_thread_protoV fn) pthread_join(tid, NULL); } #endif + +/* + * prototype: int fn(void *, void *) { ... } + */ +#ifdef KSR_RTHREAD_NEED_4PP +typedef int (*_thread_proto4PP)(void *, void *); +struct _thread_args4PP +{ + _thread_proto4PP fn; + void *arg1; + void *arg2; + int *ret; +}; +static void *run_thread_wrap4PP(struct _thread_args4PP *args) +{ + *args->ret = (*args->fn)(args->arg1, args->arg2); + return NULL; +} + +static int run_thread4PP(_thread_proto4PP fn, void *arg1, void *arg2) +{ + pthread_t tid; + int ret; + + pthread_create(&tid, NULL, (_thread_proto)run_thread_wrap4PP, + &(struct _thread_args4PP){fn, arg1, arg2, &ret}); + pthread_join(tid, NULL); + + return ret; +} +#endif + +/* + * prototype: void fn(void *) { ... } + */ +#ifdef KSR_RTHREAD_NEED_0P +typedef void (*_thread_proto0P)(void *); +struct _thread_args0P +{ + _thread_proto0P fn; + void *arg1; +}; +static void *run_thread_wrap0P(struct _thread_args0P *args) +{ + (*args->fn)(args->arg1); + return NULL; +} + +static void run_thread0P(_thread_proto0P fn, void *arg1) +{ + pthread_t tid; + + pthread_create(&tid, NULL, (_thread_proto)run_thread_wrap0P, + &(struct _thread_args0P){fn, arg1}); + pthread_join(tid, NULL); +} +#endif
Hello,
by latest commits, I notice that more functions need to be executed in another thread when having to deal libssl3. I wonder if it is not an alternative to run only the tls module functions in other threads?
If not, maybe we can introduce a global parameter (or per module) that makes this wrapper functions to execute directly or through another thread. There are cases when it is no need to run through different threads.
Cheers, Daniel
On 06.02.24 03:40, S-P Chan via sr-dev wrote:
Module: kamailio Branch: master Commit: ba921b2112e87625fba5789d1b049161bb611073 URL: https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789d1b049161...
Author: S-P Chan shihping.chan@gmail.com Committer: S-P Chan shihping.chan@gmail.com Date: 2024-02-06T10:17:53+08:00
core/rthread.h: add prototype for db queries
Modified: src/core/rthreads.h
Diff: https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789d1b049161... Patch: https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789d1b049161...
diff --git a/src/core/rthreads.h b/src/core/rthreads.h index a5ad7670dae..a416ad2ca50 100644 --- a/src/core/rthreads.h +++ b/src/core/rthreads.h @@ -98,3 +98,60 @@ static void run_threadV(_thread_protoV fn) pthread_join(tid, NULL); } #endif
+/*
- prototype: int fn(void *, void *) { ... }
- */
+#ifdef KSR_RTHREAD_NEED_4PP +typedef int (*_thread_proto4PP)(void *, void *); +struct _thread_args4PP +{
- _thread_proto4PP fn;
- void *arg1;
- void *arg2;
- int *ret;
+}; +static void *run_thread_wrap4PP(struct _thread_args4PP *args) +{
- *args->ret = (*args->fn)(args->arg1, args->arg2);
- return NULL;
+}
+static int run_thread4PP(_thread_proto4PP fn, void *arg1, void *arg2) +{
- pthread_t tid;
- int ret;
- pthread_create(&tid, NULL, (_thread_proto)run_thread_wrap4PP,
&(struct _thread_args4PP){fn, arg1, arg2, &ret});- pthread_join(tid, NULL);
- return ret;
+} +#endif
+/*
- prototype: void fn(void *) { ... }
- */
+#ifdef KSR_RTHREAD_NEED_0P +typedef void (*_thread_proto0P)(void *); +struct _thread_args0P +{
- _thread_proto0P fn;
- void *arg1;
+}; +static void *run_thread_wrap0P(struct _thread_args0P *args) +{
- (*args->fn)(args->arg1);
- return NULL;
+}
+static void run_thread0P(_thread_proto0P fn, void *arg1) +{
- pthread_t tid;
- pthread_create(&tid, NULL, (_thread_proto)run_thread_wrap0P,
&(struct _thread_args0P){fn, arg1});- pthread_join(tid, NULL);
+} +#endif
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
Hello,
yes, good idea. It would be also probably a good idea to have a bit more time to review these commits in git master before merging them directly to stable branch, as done some minutes ago.
Cheers,
Henning
-----Original Message----- From: Daniel-Constantin Mierla via sr-dev sr-dev@lists.kamailio.org Sent: Dienstag, 6. Februar 2024 08:40 To: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org Cc: Daniel-Constantin Mierla miconda@gmail.com Subject: [sr-dev] Re: git:master:ba921b21: core/rthread.h: add prototype for db queries
Hello,
by latest commits, I notice that more functions need to be executed in another thread when having to deal libssl3. I wonder if it is not an alternative to run only the tls module functions in other threads?
If not, maybe we can introduce a global parameter (or per module) that makes this wrapper functions to execute directly or through another thread. There are cases when it is no need to run through different threads.
Cheers, Daniel
On 06.02.24 03:40, S-P Chan via sr-dev wrote:
Module: kamailio Branch: master Commit: ba921b2112e87625fba5789d1b049161bb611073 URL:
https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789 d1b
049161bb611073
Author: S-P Chan shihping.chan@gmail.com Committer: S-P Chan shihping.chan@gmail.com Date: 2024-02-06T10:17:53+08:00
core/rthread.h: add prototype for db queries
Modified: src/core/rthreads.h
Diff:
https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789 d1b
049161bb611073.diff Patch:
https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789 d1b
049161bb611073.patch
diff --git a/src/core/rthreads.h b/src/core/rthreads.h index a5ad7670dae..a416ad2ca50 100644 --- a/src/core/rthreads.h +++ b/src/core/rthreads.h @@ -98,3 +98,60 @@ static void run_threadV(_thread_protoV fn) pthread_join(tid, NULL); } #endif
+/*
- prototype: int fn(void *, void *) { ... } */ #ifdef
+KSR_RTHREAD_NEED_4PP typedef int (*_thread_proto4PP)(void *, void *); +struct _thread_args4PP {
- _thread_proto4PP fn;
- void *arg1;
- void *arg2;
- int *ret;
+}; +static void *run_thread_wrap4PP(struct _thread_args4PP *args) {
- *args->ret = (*args->fn)(args->arg1, args->arg2);
- return NULL;
+}
+static int run_thread4PP(_thread_proto4PP fn, void *arg1, void *arg2) +{
- pthread_t tid;
- int ret;
- pthread_create(&tid, NULL, (_thread_proto)run_thread_wrap4PP,
&(struct _thread_args4PP){fn, arg1, arg2, &ret});- pthread_join(tid, NULL);
- return ret;
+} +#endif
+/*
- prototype: void fn(void *) { ... } */ #ifdef KSR_RTHREAD_NEED_0P
+typedef void (*_thread_proto0P)(void *); struct _thread_args0P {
- _thread_proto0P fn;
- void *arg1;
+}; +static void *run_thread_wrap0P(struct _thread_args0P *args) {
- (*args->fn)(args->arg1);
- return NULL;
+}
+static void run_thread0P(_thread_proto0P fn, void *arg1) {
- pthread_t tid;
- pthread_create(&tid, NULL, (_thread_proto)run_thread_wrap0P,
&(struct _thread_args0P){fn, arg1});- pthread_join(tid, NULL);
+} +#endif
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
-- Daniel-Constantin Mierla (@ asipto.com) twitter.com/miconda -- linkedin.com/in/miconda Kamailio Consultancy, Training and Development Services -- asipto.com Kamailio Advanced Training, February 20-22, 2024 -- asipto.com Kamailio World Conference, April 18-19, 2024, Berlin -- kamailioworld.com
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr- dev-leave@lists.kamailio.org
For 5.7 stable I have a draft PR https://github.com/kamailio/kamailio/pull/3744 that minimises thread execution: it will run functions in a thread only if process_no = 0. This restores unchanged behaviour to all workers. FYC.
On Tue, 6 Feb 2024 at 16:02, Henning Westerholt via sr-dev < sr-dev@lists.kamailio.org> wrote:
Hello,
yes, good idea. It would be also probably a good idea to have a bit more time to review these commits in git master before merging them directly to stable branch, as done some minutes ago.
Cheers,
Henning
-----Original Message----- From: Daniel-Constantin Mierla via sr-dev sr-dev@lists.kamailio.org Sent: Dienstag, 6. Februar 2024 08:40 To: Kamailio (SER) - Development Mailing List <sr-dev@lists.kamailio.org
Cc: Daniel-Constantin Mierla miconda@gmail.com Subject: [sr-dev] Re: git:master:ba921b21: core/rthread.h: add prototype
for
db queries
Hello,
by latest commits, I notice that more functions need to be executed in
another
thread when having to deal libssl3. I wonder if it is not an alternative
to run
only the tls module functions in other threads?
If not, maybe we can introduce a global parameter (or per module) that
makes
this wrapper functions to execute directly or through another thread.
There
are cases when it is no need to run through different threads.
Cheers, Daniel
On 06.02.24 03:40, S-P Chan via sr-dev wrote:
Module: kamailio Branch: master Commit: ba921b2112e87625fba5789d1b049161bb611073 URL:
https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789 d1b
049161bb611073
Author: S-P Chan shihping.chan@gmail.com Committer: S-P Chan shihping.chan@gmail.com Date: 2024-02-06T10:17:53+08:00
core/rthread.h: add prototype for db queries
Modified: src/core/rthreads.h
Diff:
https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789 d1b
049161bb611073.diff Patch:
https://github.com/kamailio/kamailio/commit/ba921b2112e87625fba5789 d1b
049161bb611073.patch
diff --git a/src/core/rthreads.h b/src/core/rthreads.h index a5ad7670dae..a416ad2ca50 100644 --- a/src/core/rthreads.h +++ b/src/core/rthreads.h @@ -98,3 +98,60 @@ static void run_threadV(_thread_protoV fn) pthread_join(tid, NULL); } #endif
+/*
- prototype: int fn(void *, void *) { ... } */ #ifdef
+KSR_RTHREAD_NEED_4PP typedef int (*_thread_proto4PP)(void *, void *); +struct _thread_args4PP {
- _thread_proto4PP fn;
- void *arg1;
- void *arg2;
- int *ret;
+}; +static void *run_thread_wrap4PP(struct _thread_args4PP *args) {
- *args->ret = (*args->fn)(args->arg1, args->arg2);
- return NULL;
+}
+static int run_thread4PP(_thread_proto4PP fn, void *arg1, void *arg2) +{
- pthread_t tid;
- int ret;
- pthread_create(&tid, NULL, (_thread_proto)run_thread_wrap4PP,
&(struct _thread_args4PP){fn, arg1, arg2, &ret});- pthread_join(tid, NULL);
- return ret;
+} +#endif
+/*
- prototype: void fn(void *) { ... } */ #ifdef KSR_RTHREAD_NEED_0P
+typedef void (*_thread_proto0P)(void *); struct _thread_args0P {
- _thread_proto0P fn;
- void *arg1;
+}; +static void *run_thread_wrap0P(struct _thread_args0P *args) {
- (*args->fn)(args->arg1);
- return NULL;
+}
+static void run_thread0P(_thread_proto0P fn, void *arg1) {
- pthread_t tid;
- pthread_create(&tid, NULL, (_thread_proto)run_thread_wrap0P,
&(struct _thread_args0P){fn, arg1});- pthread_join(tid, NULL);
+} +#endif
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
-- Daniel-Constantin Mierla (@ asipto.com) twitter.com/miconda -- linkedin.com/in/miconda Kamailio Consultancy, Training and Development Services -- asipto.com Kamailio Advanced
Training,
February 20-22, 2024 -- asipto.com Kamailio World Conference, April
18-19,
2024, Berlin -- kamailioworld.com
Kamailio (SER) - Development Mailing List To unsubscribe send an email
to sr-
dev-leave@lists.kamailio.org
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
Hi Daniel / Henning,
I would like to propose a global config to restore the non-threaded default:
enable_tls = no|yes #(EXISTING) boolean enable_tls_threads = 0 | 1 | 2 #(NEW) int
0: disable thread-wrappers (restores kamailio behaviour) - default when enable_tls = no
1: thread-wrapper only for process_no = 0 (main process) - default when enable_tls = yes
2: thread-wrapper on for all processes
Now the behaviour for the thread wrappers can be
/* pseudo-code * fn is the wrapped function */ run_threadXXXX (fn, ...) { int flag = cfg_get_tls_threads(); if (likely(flag == 0 || (flag == 1 && process_no != 0))) { return fn(...) ; // execute wrapped function directly - no thread } else { /* flag == 2 ||( flag == 1 && process_no == 0) */ /* ** run fn in thread */ }
I am not familiar with the bison grammar or parsing of the global config file — I would need your help (or another developer familiar with the core parsing) to set this up. When this cfg flag is available I can change all the thread-runners to check the global config.
With respect to 5.7 - stable branch - unfortunately due to the changes to OpenSSL 3 it is broken - #3635 - with more load there will be double-free errors; #3727 - cannot load tls and db module (even if the db module does not use TLS it may initialize OpenSSL).
The changes while more intrusive than usual are the minimal viable set of changes. With the commits on 5.7 you can have a TLS-enabled /etc/kamailio.cfg using OpenSSL 3 and load a db module (with or without TLS). To reiterate - even a pure in-memory TLS proxy without database is subject to double free corruption.
To make the changes less intrusive: backport the global enable_tls_threads config to 5.7.5+ or make the thread wrappers check for process_no = 0. The latter (and more minimal) change would mean that all Kamailio workers will have the existing behaviour and only process_no = 0 tries to run thread wrappers.
Options: A 5.8-pre:. add a global config enable_tls_threads to 5.8-pre (need help on this part - the thread wrappers I would be able to fix) B. 5.7.5+: backport A to 5.7 OR check for process_no = 0 in thread wrappers(only change in parent process, no change to worker processes)
Let me know what you think - thanks for the comments.
Cheers Richard
Hello Richard,
I added the global parameter tls_threads_mode, I consider to reflect better the purpose than the proposed enable_tls_threads. In the code it is the global variable ksr_tls_threads_mode which is exposed via core/globals.h -- you can see commit:
- https://github.com/kamailio/kamailio/commit/4d6e37fa048a1aaa2d2fc6655985b4bc...
Cheers, Daniel
On 06.02.24 12:20, Richard Chan wrote:
Hi Daniel / Henning,
I would like to propose a global config to restore the non-threaded default:
enable_tls = no|yes #(EXISTING) boolean enable_tls_threads = 0 | 1 | 2 #(NEW) int
0: disable thread-wrappers (restores kamailio behaviour) - default when enable_tls = no
1: thread-wrapper only for process_no = 0 (main process) - default when enable_tls = yes
2: thread-wrapper on for all processes
Now the behaviour for the thread wrappers can be
/* pseudo-code * fn is the wrapped function */ run_threadXXXX (fn, ...) { int flag = cfg_get_tls_threads(); if (likely(flag == 0 || (flag == 1 && process_no != 0))) { return fn(...) ; // execute wrapped function directly - no thread } else { /* flag == 2 ||( flag == 1 && process_no == 0) */ /* ** run fn in thread */ }
I am not familiar with the bison grammar or parsing of the global config file — I would need your help (or another developer familiar with the core parsing) to set this up. When this cfg flag is available I can change all the thread-runners to check the global config.
With respect to 5.7 - stable branch - unfortunately due to the changes to OpenSSL 3 it is broken - #3635 - with more load there will be double-free errors; #3727 - cannot load tls and db module (even if the db module does not use TLS it may initialize OpenSSL).
The changes while more intrusive than usual are the minimal viable set of changes. With the commits on 5.7 you can have a TLS-enabled /etc/kamailio.cfg using OpenSSL 3 and load a db module (with or without TLS). To reiterate - even a pure in-memory TLS proxy without database is subject to double free corruption.
To make the changes less intrusive: backport the global enable_tls_threads config to 5.7.5+ or make the thread wrappers check for process_no = 0. The latter (and more minimal) change would mean that all Kamailio workers will have the existing behaviour and only process_no = 0 tries to run thread wrappers.
Options: A 5.8-pre:. add a global config enable_tls_threads to 5.8-pre (need help on this part - the thread wrappers I would be able to fix) B. 5.7.5+: backport A to 5.7 OR check for process_no = 0 in thread wrappers(only change in parent process, no change to worker processes)
Let me know what you think - thanks for the comments.
Cheers Richard
Hello Richard,
thanks for working on this topic and providing a way to control this new protection mechanism depending on the requirements and age of systems etc..
Cheers,
Henning
From: Richard Chan via sr-dev sr-dev@lists.kamailio.org Sent: Dienstag, 6. Februar 2024 12:20 To: miconda@gmail.com Cc: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org; Richard Chan shihping.chan@gmail.com Subject: [sr-dev] Re: git:master:ba921b21: core/rthread.h: add prototype for db queries
Hi Daniel / Henning,
I would like to propose a global config to restore the non-threaded default:
enable_tls = no|yes #(EXISTING) boolean enable_tls_threads = 0 | 1 | 2 #(NEW) int
0: disable thread-wrappers (restores kamailio behaviour) - default when enable_tls = no
1: thread-wrapper only for process_no = 0 (main process) - default when enable_tls = yes
2: thread-wrapper on for all processes
Now the behaviour for the thread wrappers can be
/* pseudo-code * fn is the wrapped function */ run_threadXXXX (fn, ...) { int flag = cfg_get_tls_threads(); if (likely(flag == 0 || (flag == 1 && process_no != 0))) { return fn(...) ; // execute wrapped function directly - no thread } else { /* flag == 2 ||( flag == 1 && process_no == 0) */ /* ** run fn in thread */ }
I am not familiar with the bison grammar or parsing of the global config file — I would need your help (or another developer familiar with the core parsing) to set this up. When this cfg flag is available I can change all the thread-runners to check the global config. With respect to 5.7 - stable branch - unfortunately due to the changes to OpenSSL 3 it is broken - #3635 - with more load there will be double-free errors; #3727 - cannot load tls and db module (even if the db module does not use TLS it may initialize OpenSSL).
The changes while more intrusive than usual are the minimal viable set of changes. With the commits on 5.7 you can have a TLS-enabled /etc/kamailio.cfg using OpenSSL 3 and load a db module (with or without TLS). To reiterate - even a pure in-memory TLS proxy without database is subject to double free corruption.
To make the changes less intrusive: backport the global enable_tls_threads config to 5.7.5+ or make the thread wrappers check for process_no = 0. The latter (and more minimal) change would mean that all Kamailio workers will have the existing behaviour and only process_no = 0 tries to run thread wrappers.
Options: A 5.8-pre:. add a global config enable_tls_threads to 5.8-pre (need help on this part - the thread wrappers I would be able to fix) B. 5.7.5+: backport A to 5.7 OR check for process_no = 0 in thread wrappers(only change in parent process, no change to worker processes)
Let me know what you think - thanks for the comments.
Cheers Richard
-
Daniel-Constantin Mierla -
Henning Westerholt -
Richard Chan -
S-P Chan