diff --git a/daemon/redis.c b/daemon/redis.c index 022e616bf..3cfdd509c 100644 --- a/daemon/redis.c +++ b/daemon/redis.c @@ -1259,6 +1259,8 @@ static int redis_hash_get_endpoint(struct endpoint *out, const struct redis_hash return 0; } +define_get_type_format(endpoint, struct endpoint); + static int redis_hash_get_stats(struct stream_stats *out, const struct redis_hash *h, const char *k) { if (redis_hash_get_a64_f(&out->packets, h, "%s-packets", k)) return -1; @@ -2154,9 +2156,7 @@ static int redis_restore_checkpoints(call_t *c, const struct redis_hash *call, for (int64_t i = 0; i < num; i++) { struct redis_hash rh; - char key[64]; - snprintf(key, sizeof(key), "checkpoint-%lld", (long long) i); - if (json_get_hash(&rh, key, -1, root)) + if (json_get_hash(&rh, "checkpoint", (unsigned int) i, root)) return -1; int64_t offerer = -1, answerer = -1, pending = 0; @@ -3108,7 +3108,8 @@ static str redis_encode_json(ng_parser_ctx_t *ctx, call_t *c, void **to_free, bo str redis_snapshot_encode(call_t *c) { ng_parser_ctx_t ctx; bencode_buffer_t bbuf; - redis_format_parsers[rtpe_config.redis_format]->init(&ctx, &bbuf); + // never leaves the daemon, so the format is ours to pick + ng_parser_native.init(&ctx, &bbuf); void *to_free = NULL; str encoded = redis_encode_json(&ctx, c, &to_free, true); @@ -3209,10 +3210,10 @@ static void snapshot_apply_stream(call_t *c, struct packet_stream *ps, redis_hash_get_endpoint(&ps->last_local_endpoint, rh, "last_local_endpoint"); for (unsigned int i = 0; i < G_N_ELEMENTS(ps->detected_endpoints); i++) { - char key[32]; - snprintf(key, sizeof(key), "detected_endpoint-%u", i); - if (!redis_hash_get_str(&s, rh, key) && s.len) - redis_hash_get_endpoint(&ps->detected_endpoints[i], rh, key); + /* an absent endpoint is an empty string, which would parse as 0.0.0.0:0 */ + if (!redis_hash_get_str_f(&s, rh, "detected_endpoint-%u", i) && s.len) + redis_hash_get_endpoint_f(&ps->detected_endpoints[i], rh, + "detected_endpoint-%u", i); else ZERO(ps->detected_endpoints[i]); } @@ -3248,8 +3249,8 @@ static void snapshot_apply_media_codecs(struct call_media *m, parser_arg root) { const char *key; size_t offset; } stores[] = { - { "payload_types-%u", offsetof(struct call_media, codecs) }, - { "offered_payload_types-%u", offsetof(struct call_media, offered_codecs) }, + { "payload_types-%u", G_STRUCT_OFFSET(struct call_media, codecs) }, + { "offered_payload_types-%u", G_STRUCT_OFFSET(struct call_media, offered_codecs) }, }; for (unsigned int i = 0; i < G_N_ELEMENTS(stores); i++) { @@ -3258,7 +3259,7 @@ static void snapshot_apply_media_codecs(struct call_media *m, parser_arg root) { parser_arg list = redis_parser->dict_get_expect(root, key, BENCODE_LIST); if (!list.gen) continue; - struct codec_store *cs = (void *) ((char *) m + stores[i].offset); + struct codec_store *cs = &G_STRUCT_MEMBER(struct codec_store, m, stores[i].offset); codec_store_cleanup(cs); codec_store_init(cs, m); redis_decode_codec_store(redis_parser, list, cs); diff --git a/t/auto-daemon-tests-rollback-redis.pl b/t/auto-daemon-tests-rollback-redis.pl index ecc24dee0..c8c669cc8 100644 --- a/t/auto-daemon-tests-rollback-redis.pl +++ b/t/auto-daemon-tests-rollback-redis.pl @@ -244,6 +244,9 @@ sub inspect_checkpoint { # than shape: what it must contain is asserted by restoring from it. ok(!$expected_pending || length(field($checkpoint->{snapshot}) // ''), "$redis_format pending checkpoint carries a snapshot"); + # A snapshot never leaves the daemon, so it is bencode whatever the record is. + ok(!$expected_pending || (field($checkpoint->{snapshot}) // '') =~ /^d/, + "$redis_format snapshot is bencode"); } # What rollback restored, judged against the record rather than against `query`. @@ -253,7 +256,20 @@ sub inspect_checkpoint { # carries all of it: the committed state must serialise the same before and after. sub durable_state { my ($record) = @_; - my $decoded = decode_record($record); + return durable_fields(decode_record($record)); +} + +# A snapshot is a call record too, so it is filtered the same way. It is always +# bencode, whatever the record around it is. +sub checkpoint_snapshot { + my ($record) = @_; + my $checkpoint = decode_record($record)->{'checkpoint-0'} or return undef; + my $snapshot = field($checkpoint->{snapshot}) or return undef; + return durable_fields(Bencode::bdecode($snapshot, 1)); +} + +sub durable_fields { + my ($decoded) = @_; my %out; for my $key (keys %$decoded) { # Checkpoint entries describe the pending exchange, not the committed @@ -483,5 +499,53 @@ is_deeply(durable_state(redis_record_after($before_up)), durable_state($plain_record), 'rollback removes DTLS state the rejected offer introduced'); +# --- the snapshot-only state has to survive a rollback too --- +# +# ICE credentials and candidates, endpoint learning, offered codecs, tls_id, the +# preferred hash function and the endpoint map are written into snapshots only, +# so a record comparison cannot see them: a rollback could stop restoring any of +# them with every assertion above still green. Comparing the snapshot taken +# before the rejected offer against one taken after the rollback covers all of +# them at once, because a snapshot is taken before its offer is applied and so +# describes the state the rollback was supposed to reproduce. +new_call; +my ($rt_call, $rt_from, $rt_to) = (cid(), ft(), tt()); +redis_rtpe_req(1, 'offer', 'round trip: offer', { + 'from-tag' => $rt_from, flags => ['track-state'], + sdp => sdp('198.51.100.98', 15030, 'roundTripUfrag', 'roundTripPassword0123456', + 'Ai0RVBUpx3FYuJEyv1oOTQVHrfXEIQGRxWLXQBvR', 'sendrecv'), +}); +redis_rtpe_req(0, 'answer', 'round trip: answer', { + 'from-tag' => $rt_from, 'to-tag' => $rt_to, + sdp => sdp('198.51.100.99', 15040, 'roundTripAnswer', 'roundTripAnswerPwd012345', + 'HHf1TXWnpZlfXHBw5Q3xTNTIhFvbEHIYnmSMDGqR', 'sendrecv'), +}); + +redis_rtpe_req(1, 'offer', 'round trip: rejected offer', { + 'from-tag' => $rt_from, 'to-tag' => $rt_to, + sdp => sdp('198.51.100.100', 15050, 'roundTripReject', 'roundTripRejectPwd01234', + 'GHi1TXWnpZlfXHBw5Q3xTNTIhFvbEHIYnmSMDGqQ', 'sendrecv'), +}); +my $snapshot_before = checkpoint_snapshot($last_record); +ok($snapshot_before, 'round trip: committed snapshot captured'); + +my $rt_rollback = rtpe_req('rollback', 'round trip: rollback', { + 'call-id' => $rt_call, 'from-tag' => $rt_from, 'to-tag' => $rt_to, +}); +is($rt_rollback->{'rolled-back'}, 1, 'round trip: rollback applied'); + +# The snapshot for this offer is taken before it is applied, so it describes the +# state the rollback restored. +redis_rtpe_req(1, 'offer', 'round trip: offer after rollback', { + 'from-tag' => $rt_from, 'to-tag' => $rt_to, + sdp => sdp('198.51.100.101', 15060, 'roundTripAfter', 'roundTripAfterPwd012345', + 'JKl1TXWnpZlfXHBw5Q3xTNTIhFvbEHIYnmSMDGqP', 'sendrecv'), +}); +my $snapshot_after = checkpoint_snapshot($last_record); +ok($snapshot_after, 'round trip: post-rollback snapshot captured'); + +is_deeply($snapshot_after, $snapshot_before, + 'rollback restores the snapshot-only state as well'); + NGCP::Rtpengine::AutoTest::shut_rtpe(); done_testing;