Repository navigation
Conversation
Call updates (SET) and deletions (DEL) are pipelined, and redis_consume() freed their replies unread: a write refused with READONLY or NOREPLICAS, as during a Sentinel failover, left no trace in rtpengine. Check every reply. An error reply is counted in a new statistic, rediswriteerrors, and logged at most once a second per connection; the connection is kept, since a reconnect would get the same answer. A read error or timeout on the replies drops the connection at once instead of at the next PING. An encoding failure no longer drops the connection. Tested by auto-daemon-tests-redis-errors.pl, with a new fake Redis module, NGCP::Rtpengine::FakeRedis.
On a role mismatch, the reconnect in redis_check_conn() sleeps one second, asks INFO again and sleeps once more: two seconds under r->lock, which every Redis update holds before the ng reply (sipwise#1865). After a failover, with --redis on the old master's address, every update did this, and ng requests timed out. Make one attempt, with no sleep. After a role failure, or a failure that took --redis-reconnect-slow (100 ms) or more, skip Redis operations on that connection for --redis-reconnect-pause (1000 ms), through restore_tick_us as --redis-allowed-errors does. A quick failure (connection refused) is retried at once, as before. As with a failed attempt, a skipped operation is not written; skips are logged once per pause. The startup connection keeps its eleven attempts. Tested by auto-daemon-tests-redis-errors.pl.
cli_handle() takes a str, but most "set" handlers parsed their value
with strtol(), strtod() or atoi() on instr->s and printed it with "%s",
reading up to the next NUL. The ng "cli" command passes a slice of the
request: "set maxsessions 1000" set 10007, "set rediscmdtimeout 0" set
7 ms, and error messages quoted the rest of the request.
Parse with the str_to_X helpers, plus a new str_to_d for maxcpu and
maxload, and print with STR_FORMAT. As with the str_to_X family, a
value with trailing characters ("10x") is refused, and so is a
non-number in the seven handlers that read it as 0. "terminate" now
matches all, own and foreign exactly (str_cmp()), not as a prefix.
Tested by auto-daemon-tests-redis-errors.pl and the new t/test-str.c.
The timer that rewrites never-stored SRTP calls can hold a call while it waits for r->lock. If the call is deleted meanwhile, the deletion, deciding without the lock that the call was never stored, deletes nothing, and the timer then writes it: the call is back at the next restore. Any update racing with a deletion can do the same. Under r->lock, redis_update_onekey() writes a call only while it is still in the call table, and redis_do_delete() decides "never stored", so that a write already past that check is followed by the DEL. Tested by auto-daemon-tests-redis-destroyed.pl.
Behind Redis Sentinel, rtpengine stays on the address it was given: after a failover its writes go to the old master, which refuses them until it is demoted, then serves them as a replica. New options --redis-sentinel, --redis-sentinel-master and --redis-sentinel-interval; the keyword "sentinel" replaces IP:PORT in --redis, --redis-write or --redis-subscribe. A "redis sentinel" thread asks every Sentinel for the master, takes the highest config epoch (never an older one than in use), checks the node with ROLE and publishes it with a generation number. It also runs at once when a connection meets a replica. redis_check_conn() compares the generation and reconnects on a change; these connections require the master role. Without "sentinel", none of this runs. Tested by auto-daemon-tests-redis-sentinel.pl and -sentinel-start.pl.
A subscription made through "sentinel" stayed on the old master after a failover, and missed the notifications of the new one. With Sentinel, the notification event loop gets a one-second timer: when the master changed, it closes the subscription, and redis_notify_loop() subscribes again on the new master. The subscription's context also gets a connect callback, since hiredis frees a context that fails to connect without calling the disconnect callback. Notifications missed during the switch are not replayed. Tested by auto-daemon-tests-redis-sentinel-subscribe.pl.
After a failover the new master refuses writes with NOREPLICAS until it has a replica. Those writes are counted and logged, but never made again: a call created then is missing at the next restore, and a call deleted then comes back. With --redis-repair-writes (off by default), an own call whose write was not stored (an error reply, a connection lost before the reply, a skipped write) is marked, and a stored write clears the mark. A "redis repair" thread writes the marked calls again, one at a time, through redis_write_call(), now shared with redis_update_onekey(): a live call gets its current state, a call that is gone a DEL, unless a new call uses its ID. Signalling writes waiting for r->lock go first, for 100 ms at most. A failed write is retried after a second; at most 100000 calls are kept. New statistic: redisrepairedwrites. A repaired DEL assumes one writer per database. Tested by auto-daemon-tests-redis-repair.pl.
Replication is asynchronous, so a master that is replaced takes with it the writes it acknowledged but had not replicated: a call created then is missing on the new master, and a call deleted then comes back. --redis-resync-on-master-change (off by default) keeps the master_replid of the INFO that redis_connect() already sends. When the write connection reconnects to another ID, every own call stored at least once, and the own DELs of the last --redis-resync-delete-window (at most --redis-resync-delete-max), are queued for the repair thread of --redis-repair-writes. The write connection must require the master role. No key is scanned. New statistics: redisresyncs, redisresynccalls and redisresyncdeletes. Tested by auto-daemon-tests-redis-resync.pl.
There is no way to reach a Redis server that only accepts TLS (managed services, or tls-port with port 0); asked on the mailing list in 2022: https://groups.google.com/g/rtpengine/c/gO2hWMKxCNA/m/DNH37Z54BQAJ With --redis-cafile or --redis-capath, every Redis connection uses TLS, and --redis-certfile with --redis-keyfile adds a client certificate. As for MQTT, the options exist only in a build with hiredis_ssl. hiredis checks only the chain, so the name is checked here, in the subjectAltName: the host as configured, the IP address, or --redis-tls-server-name. The handshake is bounded by the command timeout, or by the connect timeout when there is none. Tested by auto-daemon-tests-redis-tls.pl (libio-socket-ssl-perl added to Build-Depends).
With the TLS options, the Sentinel thread still spoke plain TCP to the Sentinels and for the role check of the master they name, so a Sentinel set with tls-port and port 0 could not be used. sentinel_connect() now starts TLS with the same context and options. A Sentinel's certificate must match its host as given in --redis-sentinel; a master's must match the address the Sentinels gave, or --redis-tls-server-name. The handshake happens in the thread, never under r->lock. Tested by auto-daemon-tests-redis-tls.pl.
A reconnect attempt runs under r->lock, which every ng request that writes a call waits for (sipwise#1865). Against a server that drops packets it takes the full connect timeout, so signalling stalls for up to that long at each attempt, for as long as the outage lasts. Make the attempt without the lock: the thread that reconnects sets r->reconnecting and owns r->ctx until it clears the flag under the lock. Other threads wait for it for up to --redis-reconnect-slow, then skip their operation, as during the pause. After the lock was released, redis_update_onekey() checks again that the call is live, and redis_do_delete() leaves the key to a new call with the same ID. The CLI, the DEL after a failed restore and the notification thread leave a connection being made alone. A command on an established connection still holds the lock. Tested by auto-daemon-tests-redis-reconnect.pl.
redis, redis-json and redis-subscribe compared raw bytes on their own sockets, and redis-srtp-index and rollback-redis each carried the same forked server. They now use NGCP::Rtpengine::FakeRedis, and the forked server moves into it as redis_serve(). The checks, their names and their counts are unchanged.
--redis-write accepts any role, so after a failover that demotes its node, rtpengine reconnects to the same address, accepts the replica, and every later write is refused with READONLY. --redis-require-master (off by default) makes the connections that write call data require the master role at connect, and drop the connection on a write refused with READONLY or MASTERDOWN, so that the next operation reconnects and checks the role. Other refusals keep the connection. Tested by auto-daemon-tests-redis-require-master.pl.
kbinetruy
force-pushed
the
redis-tls
branch
2 times, most recently
from
October 20, 2026 09:52
ff6160c to
f15fc2d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A Redis server that only accepts TLS (a managed service, or
tls-portwithport 0) cannot be used today. This was asked on the mailing list in 2022: https://groups.google.com/g/rtpengine/c/gO2hWMKxCNA/m/DNH37Z54BQAJThis PR depends on #2212, which depends on #2211. The branch has 13 commits; only the last two are new:
MT#55283 redis: optional TLS for the Redis connections(12 files, +550 −5)MT#55283 redis: TLS for the Redis Sentinel connections(4 files, +87 −8)The two commits on their own: kbinetruy/rtpengine@redis-sentinel...redis-tls
1. With
--redis-cafileor--redis-capath, as for the MQTT options, every Redis connection uses TLS:--redis,--redis-write,--redis-subscribe, the restore copies, and the async connections.--redis-certfileand--redis-keyfileadd a client certificate. OneSSL_CTXis made at startup. The handshake follows the command timeout. On a reconnect it runs in the thread that makes the attempt, withoutr->lock. A refused certificate or a handshake timeout is a failed connect, logged with the reason.Released hiredis (1.2.0 to 1.4.1) checks only the chain, so the name is checked here, in the subjectAltName only. An address needs an IP entry, and no SNI is sent. A host name is checked with
X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS, and sent as SNI.--redis-tls-server-namereplaces the expected name of every data node.2. The Sentinel thread uses the same context: a Sentinel's certificate must match its host as given in
--redis-sentinel. TheROLEcheck of a master must match the address that the Sentinels gave, or--redis-tls-server-name.--redis-cafile=FILE,--redis-capath=PATH--redis-certfile=FILE,--redis-keyfile=FILE--redis-tls-server-name=NAMETLS 1.2 is the minimum. Files are PEM, and password-protected keys are not supported, as for MQTT.
utils/gen-common-flagsdefinesHAVE_HIREDIS_SSLwhenpkg-configfindshiredis_ssl. On Debian trixie and Ubuntu noble,libhiredis-devships it. The TLS options exist only in a build withhiredis_ssl. On other builds (Debian bookworm) they are unknown command-line options, as the MQTT ones, and aredis-cafilekey in the configuration file is ignored.Lab. Everything runs over TLS, with Sentinel, repair and resync and client certificates: failover ×3 and frozen master ×3. Each run had 0 lost, 0 deleted back, and 156 calls restored by a takeover. A reconnect to the new master took 3.4–4.6 ms over TLS, against 4.3 ms over plain TCP.
Testing.
t/auto-daemon-tests-redis-tls.pl(126 checks, about 7 s) runs fake Redis and fake Sentinels overIO::Socket::SSL, with certificates made byopensslat test time (libio-socket-ssl-perladded to Build-Depends). It covers every connection type, every refusal (another CA, another address or name, partial wildcard, CN only), SNI, a required client certificate, a silent peer and the startup checks. It fails on the parent commit, and catches mutants of the checks (one equivalent mutant survives).make asan-checkon master3865f896: exit 0 on Debian trixie and Ubuntu noble, 9503ok, 0not ok. The patches apply withgit amon3865f896.Questions:
--redis-cafile(as for MQTT), or by an explicit--redis-tlsswitch?tls-*directives), or have their own?Lab method, per-run figures and test details: https://github.com/kbinetruy/rtpengine/blob/redis-notes/redis-sentinel-notes.md