From fb1ad3f0cf5f43065c490d3cd663e1680733918a Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Tue, 21 Jan 2025 15:28:48 -0400 Subject: [PATCH] MT#55283 properly support empty keyspace list If keyspace notifications are used at all, the respective objects and threads must be created during startup. This requires at least some keyspace to be configured. To support usage without any keyspaces initially (and add them during runtime), add a special case (set keyspace to -1). Convert all keyspace variables to signed ints. Ignore negative keyspace numbers where appropriate. Support Redis endpoint addresses without database number. Fixes #1902 Change-Id: I45a3c87bc515f9b14e64ec1ec0906dde271b5f8d --- daemon/cli.c | 52 +++++++++++++++++++++++++---------------------- daemon/main.c | 49 +++++++++++++++++++++++++------------------- daemon/redis.c | 5 ++++- docs/rtpengine.md | 9 +++++++- 4 files changed, 68 insertions(+), 47 deletions(-) diff --git a/daemon/cli.c b/daemon/cli.c index 95c5cf2e4..0902f085b 100644 --- a/daemon/cli.c +++ b/daemon/cli.c @@ -427,7 +427,7 @@ RTPE_CONFIG_UINT64_PARAMS } count=0; for (__auto_type s = initial_rtpe_config.redis_subscribed_keyspaces.head; s ; s = s->next) { - cw->cw_printf(cw, "keyspace[%d] = %d\n", count, GPOINTER_TO_UINT(s->data)); + cw->cw_printf(cw, "keyspace[%d] = %d\n", count, GPOINTER_TO_INT(s->data)); ++count; } @@ -479,7 +479,7 @@ RTPE_CONFIG_UINT64_PARAMS } count=0; for (__auto_type c = rtpe_config.redis_subscribed_keyspaces.head; c ; c = c->next) { - cw->cw_printf(cw, "keyspace[%d] = %d\n", count, GPOINTER_TO_UINT(c->data)); + cw->cw_printf(cw, "keyspace[%d] = %d\n", count, GPOINTER_TO_INT(c->data)); ++count; } @@ -1147,7 +1147,7 @@ static void cli_incoming_terminate(str *instr, struct cli_writer *cw, const cli_ } static void cli_incoming_ksadd(str *instr, struct cli_writer *cw, const cli_handler_t *handler) { - unsigned long uint_keyspace_db; + long int_keyspace_db; char *endptr; if (!rtpe_redis_notify) { @@ -1161,20 +1161,22 @@ static void cli_incoming_ksadd(str *instr, struct cli_writer *cw, const cli_hand } errno = 0; - uint_keyspace_db = strtoul(instr->s, &endptr, 10); + int_keyspace_db = strtol(instr->s, &endptr, 10); - if ((errno == ERANGE && (uint_keyspace_db == ULONG_MAX)) || (errno != 0 && uint_keyspace_db == 0)) { - cw->cw_printf(cw, "Fail adding keyspace %s to redis notifications; errno=%d\n", instr->s, errno); + if ((errno == ERANGE && (int_keyspace_db == ULONG_MAX)) || int_keyspace_db >= INT_MAX + || int_keyspace_db < 0 + || (errno != 0 && int_keyspace_db == 0)) { + cw->cw_printf(cw, "Fail adding keyspace " STR_FORMAT " to redis notifications; errno=%d\n", STR_FMT(instr), errno); } else if (endptr == instr->s) { - cw->cw_printf(cw, "Fail adding keyspace %s to redis notifications; no digits found\n", instr->s); + cw->cw_printf(cw, "Fail adding keyspace " STR_FORMAT " to redis notifications; no digits found\n", STR_FMT(instr)); } else { rwlock_lock_w(&rtpe_config.keyspaces_lock); - if (!g_queue_find(&rtpe_config.redis_subscribed_keyspaces, GUINT_TO_POINTER(uint_keyspace_db))) { - g_queue_push_tail(&rtpe_config.redis_subscribed_keyspaces, GUINT_TO_POINTER(uint_keyspace_db)); - redis_notify_subscribe_action(rtpe_redis_notify, SUBSCRIBE_KEYSPACE, uint_keyspace_db); - cw->cw_printf(cw, "Success adding keyspace %lu to redis notifications.\n", uint_keyspace_db); + if (!g_queue_find(&rtpe_config.redis_subscribed_keyspaces, GINT_TO_POINTER(int_keyspace_db))) { + g_queue_push_tail(&rtpe_config.redis_subscribed_keyspaces, GINT_TO_POINTER(int_keyspace_db)); + redis_notify_subscribe_action(rtpe_redis_notify, SUBSCRIBE_KEYSPACE, int_keyspace_db); + cw->cw_printf(cw, "Success adding keyspace %ld to redis notifications.\n", int_keyspace_db); } else { - cw->cw_printf(cw, "Keyspace %lu is already among redis notifications.\n", uint_keyspace_db); + cw->cw_printf(cw, "Keyspace %ld is already among redis notifications.\n", int_keyspace_db); } rwlock_unlock_w(&rtpe_config.keyspaces_lock); } @@ -1182,7 +1184,7 @@ static void cli_incoming_ksadd(str *instr, struct cli_writer *cw, const cli_hand static void cli_incoming_ksrm(str *instr, struct cli_writer *cw, const cli_handler_t *handler) { GList *l; - unsigned long uint_keyspace_db; + long int_keyspace_db; char *endptr; if (!rtpe_redis_notify) { @@ -1196,26 +1198,28 @@ static void cli_incoming_ksrm(str *instr, struct cli_writer *cw, const cli_handl } errno = 0; - uint_keyspace_db = strtoul(instr->s, &endptr, 10); + int_keyspace_db = strtol(instr->s, &endptr, 10); rwlock_lock_w(&rtpe_config.keyspaces_lock); - if ((errno == ERANGE && (uint_keyspace_db == ULONG_MAX)) || (errno != 0 && uint_keyspace_db == 0)) { - cw->cw_printf(cw, "Fail removing keyspace %s to redis notifications; errno=%d\n", instr->s, errno); + if ((errno == ERANGE && (int_keyspace_db == ULONG_MAX)) || int_keyspace_db >= INT_MAX + || int_keyspace_db < 0 + || (errno != 0 && int_keyspace_db == 0)) { + cw->cw_printf(cw, "Fail removing keyspace " STR_FORMAT " to redis notifications; errno=%d\n", STR_FMT(instr), errno); } else if (endptr == instr->s) { - cw->cw_printf(cw, "Fail removing keyspace %s to redis notifications; no digits found\n", instr->s); - } else if ((l = g_queue_find(&rtpe_config.redis_subscribed_keyspaces, GUINT_TO_POINTER(uint_keyspace_db)))) { + cw->cw_printf(cw, "Fail removing keyspace " STR_FORMAT " to redis notifications; no digits found\n", STR_FMT(instr)); + } else if ((l = g_queue_find(&rtpe_config.redis_subscribed_keyspaces, GINT_TO_POINTER(int_keyspace_db)))) { // remove this keyspace - redis_notify_subscribe_action(rtpe_redis_notify, UNSUBSCRIBE_KEYSPACE, uint_keyspace_db); + redis_notify_subscribe_action(rtpe_redis_notify, UNSUBSCRIBE_KEYSPACE, int_keyspace_db); g_queue_remove(&rtpe_config.redis_subscribed_keyspaces, l->data); - cw->cw_printf(cw, "Successfully unsubscribed from keyspace %lu.\n", uint_keyspace_db); + cw->cw_printf(cw, "Successfully unsubscribed from keyspace %lu.\n", int_keyspace_db); // destroy foreign calls for this keyspace - destroy_keyspace_foreign_calls(uint_keyspace_db); + destroy_keyspace_foreign_calls(int_keyspace_db); // update cli - cw->cw_printf(cw, "Successfully removed all foreign calls for keyspace %lu.\n", uint_keyspace_db); + cw->cw_printf(cw, "Successfully removed all foreign calls for keyspace %ld.\n", int_keyspace_db); } else { - cw->cw_printf(cw, "Keyspace %lu is not among redis notifications.\n", uint_keyspace_db); + cw->cw_printf(cw, "Keyspace %ld is not among redis notifications.\n", int_keyspace_db); } rwlock_unlock_w(&rtpe_config.keyspaces_lock); @@ -1233,7 +1237,7 @@ static void cli_incoming_kslist(str *instr, struct cli_writer *cw, const cli_han rwlock_lock_r(&rtpe_config.keyspaces_lock); for (l = rtpe_config.redis_subscribed_keyspaces.head; l; l = l->next) { - cw->cw_printf(cw, "%u ", GPOINTER_TO_UINT(l->data)); + cw->cw_printf(cw, "%d ", GPOINTER_TO_INT(l->data)); } rwlock_unlock_r(&rtpe_config.keyspaces_lock); diff --git a/daemon/main.c b/daemon/main.c index 651512fa3..21881aae6 100644 --- a/daemon/main.c +++ b/daemon/main.c @@ -382,19 +382,21 @@ static int redis_ep_parse(endpoint_t *ep, int *db, char **hostname, char **auth, else if ((sl = getenv(auth_env))) *auth = g_strdup(sl); - sl = strchr(s, '/'); - if (!sl) - return -1; - *sl = 0; - sl++; - if (!*sl) - return -1; - l = strtol(sl, &sl, 10); - if (*sl != 0) - return -1; - if (l < 0) - return -1; - *db = l; + if (db) { + sl = strchr(s, '/'); + if (!sl) + return -1; + *sl = 0; + sl++; + if (!*sl) + return -1; + l = strtol(sl, &sl, 10); + if (*sl != 0) + return -1; + if (l < 0) + return -1; + *db = l; + } /* copy for the case with re-resolve during re-connections */ sp = strrchr(s, ':'); /* make sure to not take port into the value of hostname */ @@ -493,7 +495,7 @@ static void release_listeners(GQueue *q) { static void options(int *argc, char ***argv, GHashTable *templates) { g_autoptr(char_p) if_a = NULL; g_autoptr(char_p) ks_a = NULL; - unsigned long uint_keyspace_db; + long int_keyspace_db; str str_keyspace_db; char **iter; g_autoptr(char_p) listenps = NULL; @@ -577,7 +579,7 @@ static void options(int *argc, char ***argv, GHashTable *templates) { { "port-max", 'M', 0, G_OPTION_ARG_INT, &rtpe_config.port_max, "Highest port to use for RTP", "INT" }, { "redis", 'r', 0, G_OPTION_ARG_STRING, &redisps, "Connect to Redis database", "[PW@]IP:PORT/INT" }, { "redis-write",'w', 0, G_OPTION_ARG_STRING, &redisps_write, "Connect to Redis write database", "[PW@]IP:PORT/INT" }, - { "redis-subscribe", 0, 0, G_OPTION_ARG_STRING, &redisps_subscribe, "Connect to Redis subscribe database", "[PW@]IP:PORT/INT" }, + { "redis-subscribe", 0, 0, G_OPTION_ARG_STRING, &redisps_subscribe, "Connect to Redis subscribe database", "[PW@]IP:PORT[/INT]" }, { "redis-resolve-on-reconnect", 0,0, G_OPTION_ARG_NONE, &rtpe_config.redis_resolve_on_reconnect, "Re-resolve given FQDN on each re-connect to the redis server.", NULL }, { "redis-num-threads", 0, 0, G_OPTION_ARG_INT, &rtpe_config.redis_num_threads, "Number of Redis restore threads", "INT" }, { "redis-expires", 0, 0, G_OPTION_ARG_INT, &rtpe_config.redis_expires_secs, "Expire time in seconds for redis keys", "INT" }, @@ -816,15 +818,15 @@ static void options(int *argc, char ***argv, GHashTable *templates) { if (ks_a) { for (iter = ks_a; *iter; iter++) { str_keyspace_db = STR(*iter); - uint_keyspace_db = strtoul(str_keyspace_db.s, &endptr, 10); + int_keyspace_db = strtol(str_keyspace_db.s, &endptr, 10); - if ((errno == ERANGE && (uint_keyspace_db == ULONG_MAX)) || - (errno != 0 && uint_keyspace_db == 0)) { + if ((errno == ERANGE && (int_keyspace_db == ULONG_MAX)) || int_keyspace_db >= INT_MAX || + (errno != 0 && int_keyspace_db == 0)) { ilog(LOG_ERR, "Fail adding keyspace '" STR_FORMAT "' to redis notifications; errno=%d\n", STR_FMT(&str_keyspace_db), errno); } else if (endptr == str_keyspace_db.s) { ilog(LOG_ERR, "Fail adding keyspace '" STR_FORMAT "' to redis notifications; no digits found\n", STR_FMT(&str_keyspace_db)); } else { - g_queue_push_tail(&rtpe_config.redis_subscribed_keyspaces, GUINT_TO_POINTER(uint_keyspace_db)); + g_queue_push_tail(&rtpe_config.redis_subscribed_keyspaces, GINT_TO_POINTER(int_keyspace_db)); } } } @@ -923,7 +925,10 @@ static void options(int *argc, char ***argv, GHashTable *templates) { if (redis_ep_parse(&rtpe_config.redis_subscribe_ep, &rtpe_config.redis_subscribe_db, &rtpe_config.redis_subscribe_hostname, &rtpe_config.redis_subscribe_auth,"RTPENGINE_REDIS_SUBSCRIBE_AUTH_PW", redisps_subscribe)) { - die("Invalid Redis endpoint [IP:PORT/INT] '%s' (--redis-subscribe)", redisps_subscribe); + rtpe_config.redis_subscribe_db = -1; + if (redis_ep_parse(&rtpe_config.redis_subscribe_ep, NULL, &rtpe_config.redis_subscribe_hostname, + &rtpe_config.redis_subscribe_auth,"RTPENGINE_REDIS_SUBSCRIBE_AUTH_PW", redisps_subscribe)) + die("Invalid Redis endpoint [IP:PORT/INT] '%s' (--redis-subscribe)", redisps_subscribe); } if (rtpe_config.fmt > 2) @@ -1175,7 +1180,7 @@ static void fill_initial_rtpe_cfg(struct rtpengine_config* ini_rtpe_cfg) { for (__auto_type l = rtpe_config.redis_subscribed_keyspaces.head; l ; l = l->next) { // l->data has been assigned to a variable before being given into the queue structure not to get a shallow copy - unsigned int num = GPOINTER_TO_UINT(l->data); + int num = GPOINTER_TO_INT(l->data); g_queue_push_tail(&ini_rtpe_cfg->redis_subscribed_keyspaces, GINT_TO_POINTER(num)); } @@ -1529,6 +1534,8 @@ static void do_redis_restore(void) { for (GList *l = rtpe_config.redis_subscribed_keyspaces.head; l; l = l->next) { int db = GPOINTER_TO_INT(l->data); + if (db < 0) + continue; if (redis_restore(r, true, db)) ilog(LOG_WARN, "Unable to restore calls from the active-active peer"); } diff --git a/daemon/redis.c b/daemon/redis.c index 4e18a812a..cae51fb59 100644 --- a/daemon/redis.c +++ b/daemon/redis.c @@ -776,7 +776,10 @@ static int redis_notify(struct redis *r) { // subscribe to the values in the configured keyspaces rwlock_lock_r(&rtpe_config.keyspaces_lock); for (l = rtpe_config.redis_subscribed_keyspaces.head; l; l = l->next) { - redis_notify_subscribe_action(r, SUBSCRIBE_KEYSPACE, GPOINTER_TO_INT(l->data)); + int id = GPOINTER_TO_INT(l->data); + if (id < 0) + continue; + redis_notify_subscribe_action(r, SUBSCRIBE_KEYSPACE, id); } rwlock_unlock_r(&rtpe_config.keyspaces_lock); diff --git a/docs/rtpengine.md b/docs/rtpengine.md index 693d3567c..092492e86 100644 --- a/docs/rtpengine.md +++ b/docs/rtpengine.md @@ -535,12 +535,13 @@ call to inject-DTMF won't be sent to __\-\-dtmf-log-dest=__ or __\-\-listen-tcp- When both options are given, __rtpengine__ will start and use the Redis database regardless of the database's role (master or slave). -- __\-\-redis-subscribe=__\[*PW*@\]*IP*:*PORT*/*INT* +- __\-\-redis-subscribe=__\[*PW*@\]*IP*:*PORT*\[/*INT*\] Configures a Redis database for subscribing and receiving notifications. This option takes precedence over __\-\-redis__, if configured. When __\-\-subscribe-keyspace__ is also configured, the keyspace part of __\-\-redis-subscribe=__ is not used, the former takes precedence. + The keyspace number can also be omitted altogether. For password protected Redis servers, the environment variable for the password is __RTPENGINE\_REDIS\_SUBSCRIBE\_AUTH\_PW__. @@ -555,6 +556,12 @@ call to inject-DTMF won't be sent to __\-\-dtmf-log-dest=__ or __\-\-listen-tcp- Further subscriptions could be added/removed via __rtpengine-ctl ksadd/ksrm__. This may lead to enabling/disabling of the redis keyspace notification feature. + The list of keyspace subscriptions can initially be left empty, but if any + keyspaces are to be added later during runtime, the feature must still be + configured at *rtpengine* startup. This can be achieved by either setting + __\-\-redis-subscribe=__ to a valid address, or by listing the single value + __-1__ under __\-\-subscribe-keyspace=__. + - __\-\-redis-num-threads=__*INT* How many redis restore threads to create.