pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39221?usp=email )
Change subject: mgw: Simplify and redo code around ssrc patch feature ......................................................................
mgw: Simplify and redo code around ssrc patch feature
The previous logic to configure SSRC patching was over complex and confusing, with even some broken logic like "patch only once", which is not really needed. Hence, simplify the internal logic to either patch SSRC always or don't do it, based on VTY configuration.
The "force SSRC" feature was added in db2d431697609d473de433b7028f81ce499a02c0 and further changed in e2292f3aa1e9ae1faf5521a9cb108775f1ac0540, due to the need for nanoBTS to always keep the same SSRC on received packets. However, it makes no sense that is actually needed only once: if remote end changed several times, then we should keep patching in that case.
This change allows getting rid of confusing applying of settings at a later time through mgcp_rtp_end_config(), which is no longer needed and can be dropped, simplifying steps during CRCX/MDCX/DLCX.
Change-Id: Ib7216a775f4ad126e62b9e99aff381fd45015819 --- M include/osmocom/mgcp/mgcp_protocol.h M include/osmocom/mgcp/mgcp_rtp_end.h M include/osmocom/mgcp/mgcp_trunk.h M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_rtp_end.c M src/libosmo-mgcp/mgcp_vty.c M tests/mgcp/mgcp_test.c 8 files changed, 15 insertions(+), 40 deletions(-)
Approvals: pespin: Looks good to me, approved Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve daniel: Looks good to me, but someone else must approve
diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h index ddb226b..77f17d5 100644 --- a/include/osmocom/mgcp/mgcp_protocol.h +++ b/include/osmocom/mgcp/mgcp_protocol.h @@ -26,8 +26,6 @@
struct mgcp_rtp_end; struct mgcp_endpoint; -void mgcp_rtp_end_config(struct mgcp_endpoint *endp, int expect_ssrc_change, - struct mgcp_rtp_end *rtp);
uint32_t mgcp_rtp_packet_duration(const struct mgcp_endpoint *endp, const struct mgcp_rtp_end *rtp); diff --git a/include/osmocom/mgcp/mgcp_rtp_end.h b/include/osmocom/mgcp/mgcp_rtp_end.h index 02554cb..df06569 100644 --- a/include/osmocom/mgcp/mgcp_rtp_end.h +++ b/include/osmocom/mgcp/mgcp_rtp_end.h @@ -30,7 +30,7 @@ int force_output_ptime;
/* RTP patching */ - int force_constant_ssrc; /* -1: always, 0: don't, 1: once */ + bool force_constant_ssrc; /* should we perform align_rtp_timestamp_offset() (1) or not (0) */ int force_aligned_timing; bool rfc5993_hr_convert; diff --git a/include/osmocom/mgcp/mgcp_trunk.h b/include/osmocom/mgcp/mgcp_trunk.h index 1679239..68d37dd 100644 --- a/include/osmocom/mgcp/mgcp_trunk.h +++ b/include/osmocom/mgcp/mgcp_trunk.h @@ -34,7 +34,7 @@ int keepalive_interval;
/* RTP patching */ - int force_constant_ssrc; /* 0: don't, 1: once */ + bool force_constant_ssrc; int force_aligned_timing; bool rfc5993_hr_convert;
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 476e3b1..894ca2a 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -542,16 +542,16 @@
if (!state->initialized) { state->initialized = 1; + state->packet_duration = mgcp_rtp_packet_duration(endp, rtp_end); state->in_stream.last_seq = seq - 1; - state->in_stream.ssrc = state->patch.orig_ssrc = ssrc; + state->in_stream.ssrc = ssrc; state->in_stream.last_tsdelta = 0; - state->packet_duration = - mgcp_rtp_packet_duration(endp, rtp_end); state->out_stream.last_seq = seq - 1; - state->out_stream.ssrc = state->patch.orig_ssrc = ssrc; state->out_stream.last_tsdelta = 0; state->out_stream.last_timestamp = timestamp; state->out_stream.ssrc = ssrc - 1; /* force output SSRC change */ + state->patch.orig_ssrc = ssrc; + state->patch.patch_ssrc = rtp_end->force_constant_ssrc; LOGPENDP(endp, DRTP, LOGL_INFO, "initializing stream, SSRC: %u timestamp: %u " "pkt-duration: %d, from %s:%d\n", @@ -578,7 +578,7 @@ osmo_sockaddr_port(&addr->u.sa));
state->in_stream.ssrc = ssrc; - if (rtp_end->force_constant_ssrc) { + if (state->patch.patch_ssrc) { int16_t delta_seq;
/* Always increment seqno by 1 */ @@ -594,10 +594,7 @@ adjust_rtp_timestamp_offset(endp, state, rtp_end, addr, delta_seq, timestamp, marker_bit);
- state->patch.patch_ssrc = true; ssrc = state->patch.orig_ssrc; - if (rtp_end->force_constant_ssrc != -1) - rtp_end->force_constant_ssrc -= 1;
LOGPENDP(endp, DRTP, LOGL_NOTICE, "SSRC patching enabled, SSRC: %u " diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 97eb43d..bbd38da 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -722,22 +722,6 @@ return 0; }
-void mgcp_rtp_end_config(struct mgcp_endpoint *endp, int expect_ssrc_change, - struct mgcp_rtp_end *rtp) -{ - struct mgcp_trunk *trunk = endp->trunk; - - bool patch_ssrc = expect_ssrc_change && trunk->force_constant_ssrc; - - rtp->force_constant_ssrc = patch_ssrc ? 1 : 0; - - LOGPENDP(endp, DLMGCP, LOGL_DEBUG, - "Configuring RTP endpoint: local port %d%s%s\n", - osmo_sockaddr_port(&rtp->addr.u.sa), - rtp->force_aligned_timing ? ", force constant timing" : "", - rtp->force_constant_ssrc ? ", force constant ssrc" : ""); -} - uint32_t mgcp_rtp_packet_duration(const struct mgcp_endpoint *endp, const struct mgcp_rtp_end *rtp) { @@ -1087,8 +1071,6 @@ rc = mgcp_conn_iuup_init(conn_rtp); }
- mgcp_rtp_end_config(endp, 0, &conn_rtp->end); - /* Find a local address for conn based on policy and initial SDP remote information, then find a free port for it */ if (mgcp_get_local_addr(conn_rtp->end.local_addr, conn_rtp) < 0) { @@ -1340,8 +1322,6 @@ goto error3; }
- mgcp_rtp_end_config(endp, 1, &conn_rtp->end); - /* modify */ LOGPCONN(conn, DLMGCP, LOGL_DEBUG, "MDCX: modified conn:%s\n", mgcp_conn_dump(conn)); diff --git a/src/libosmo-mgcp/mgcp_rtp_end.c b/src/libosmo-mgcp/mgcp_rtp_end.c index 819a922..34614fe 100644 --- a/src/libosmo-mgcp/mgcp_rtp_end.c +++ b/src/libosmo-mgcp/mgcp_rtp_end.c @@ -55,6 +55,7 @@ end->maximum_packet_time = -1;
end->force_aligned_timing = trunk->force_aligned_timing; + end->force_constant_ssrc = trunk->force_constant_ssrc; end->rfc5993_hr_convert = trunk->rfc5993_hr_convert;
if (cfg->force_ptime) { diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c index e90ceaa..1d9079c 100644 --- a/src/libosmo-mgcp/mgcp_vty.c +++ b/src/libosmo-mgcp/mgcp_vty.c @@ -857,7 +857,7 @@ { struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID); OSMO_ASSERT(trunk); - trunk->force_constant_ssrc = 1; + trunk->force_constant_ssrc = true; return CMD_SUCCESS; }
@@ -868,7 +868,7 @@ { struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID); OSMO_ASSERT(trunk); - trunk->force_constant_ssrc = 0; + trunk->force_constant_ssrc = false; return CMD_SUCCESS; }
@@ -923,7 +923,7 @@ { struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID); OSMO_ASSERT(trunk); - trunk->force_constant_ssrc = 0; + trunk->force_constant_ssrc = false; trunk->force_aligned_timing = 0; trunk->rfc5993_hr_convert = false; return CMD_SUCCESS; @@ -1197,7 +1197,7 @@ "rtp-patch ssrc", RTP_PATCH_STR "Force a fixed SSRC\n") { struct mgcp_trunk *trunk = vty->index; - trunk->force_constant_ssrc = 1; + trunk->force_constant_ssrc = true; return CMD_SUCCESS; }
@@ -1207,7 +1207,7 @@ "no rtp-patch ssrc", NO_STR RTP_PATCH_STR "Force a fixed SSRC\n") { struct mgcp_trunk *trunk = vty->index; - trunk->force_constant_ssrc = 0; + trunk->force_constant_ssrc = false; return CMD_SUCCESS; }
@@ -1257,7 +1257,7 @@ "no rtp-patch", NO_STR RTP_PATCH_STR) { struct mgcp_trunk *trunk = vty->index; - trunk->force_constant_ssrc = 0; + trunk->force_constant_ssrc = false; trunk->force_aligned_timing = 0; trunk->rfc5993_hr_convert = false; return CMD_SUCCESS; @@ -1361,7 +1361,7 @@ if (conn->type == MGCP_CONN_TYPE_RTP) { /* Handle it like a MDCX, switch on SSRC patching if enabled */ struct mgcp_conn_rtp *conn_rtp = mgcp_conn_get_conn_rtp(conn); - mgcp_rtp_end_config(endp, 1, &conn_rtp->end); + conn_rtp->state.patch.patch_ssrc = true; } else { /* FIXME: Introduce support for other connection (E1) * types when implementation is available */ diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 9aa7326..1c1a41e 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1479,7 +1479,6 @@ OSMO_ASSERT(info->len >= 0); msg->l3h = msgb_put(msg, info->len); memcpy((char*)msgb_l3(msg), info->data, info->len); - mgcp_rtp_end_config(&endp, 1, rtp);
mgcp_patch_and_count(&endp, &state, rtp, &addr, msg);
-
pespin