neels has submitted this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/37272?usp=email )
Change subject: do not invoke two Assignments (fixup for re-assignment)
......................................................................
do not invoke two Assignments (fixup for re-assignment)
Make sure we wait for Assignment responses before dispatching an
Assignment Request towards RAN.
In particular, when the remote call leg sends its codecs, we may trigger
Re-Assignment to match that codec. Make sure this is done only once:
when receiving another SDP message, wait for the first Assignment.
Implement as an osmo_timer, since there still is no osmo_fsm
implementation for Assignment nor for CC trans FSM. Also it doesn't
belong in the msc_a FSM (it should remain in state COMMUNICATING).
Without this patch, new ttcn3 test TC_lu_and_mo_call_reass_for_mt_codec
sporadically fails, if MNCC with SDP falls in-between Assignment Request
and Assignment Complete.
Related: osmo-msc d767c73a1f93253a54d6a8650a4cf2143353bba0 ==
I8760feaa8598047369ef8c3ab2673013bac8ac8a
Related: osmo-ttcn3-hacks I402ed0523a2a87b83f29c5577b2c828102005d53
Change-Id: I0f1e1a551aed545b555b9f236fc5967b1e4cc940
---
M include/osmocom/msc/msc_a.h
M src/libmsc/gsm_04_08_cc.c
M src/libmsc/msc_a.c
M src/libmsc/ran_infra.c
4 files changed, 75 insertions(+), 1 deletion(-)
Approvals:
fixeria: Looks good to me, but someone else must approve
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
neels: Looks good to me, approved
diff --git a/include/osmocom/msc/msc_a.h b/include/osmocom/msc/msc_a.h
index a5b624c..d180e38 100644
--- a/include/osmocom/msc/msc_a.h
+++ b/include/osmocom/msc/msc_a.h
@@ -137,6 +137,8 @@
/* There may be up to 7 incoming calls for this subscriber.
This is the currently serviced voice call,
* as in, the other person the subscriber is currently talking
to. */
struct gsm_trans *active_trans;
+
+ struct osmo_timer_list assignment_request_pending;
} cc;
struct msc_ho_state ho;
diff --git a/src/libmsc/gsm_04_08_cc.c b/src/libmsc/gsm_04_08_cc.c
index 63b1699..fc4f730 100644
--- a/src/libmsc/gsm_04_08_cc.c
+++ b/src/libmsc/gsm_04_08_cc.c
@@ -772,7 +772,8 @@
const struct gsm_mncc_bearer_cap *bcap)
{
struct codec_filter *codecs = &trans->cc.codecs;
- struct call_leg *cl = trans->msc_a ? trans->msc_a->cc.call_leg : NULL;
+ struct msc_a *msc_a = trans->msc_a;
+ struct call_leg *cl = msc_a ? msc_a->cc.call_leg : NULL;
struct rtp_stream *rtp_cn = cl ? cl->rtp[RTP_TO_CN] : NULL;
if (sdp[0]) {
@@ -818,6 +819,16 @@
return;
}
+ if (msc_a && osmo_timer_pending(&msc_a->cc.assignment_request_pending))
{
+ /* Still waiting for an Assignment Response.
+ * For example, when the remote call leg sends some MNCC with
SDP with a mismatching codec,
+ * we start Re-Assignment to match that codec: we send an
Assignment Request and wait for a response.
+ * When we receive another MNCC with SDP from the remote call
leg before this Re-Assignment is
+ * completed, we must not trigger *another* Assignment Request,
but instead wait for the Re-Assignment
+ * to come back with a response first. */
+ return;
+ }
+
/* We've already completed Assignment of a voice channel (some time
ago), and now the remote side has changed
* to a mismatching codec (list). Try to re-assign this side to a
matching codec. */
LOG_TRANS(trans, LOGL_INFO, "Remote call leg mismatches assigned codec:
%s\n",
diff --git a/src/libmsc/msc_a.c b/src/libmsc/msc_a.c
index bff5e67..2e88a32 100644
--- a/src/libmsc/msc_a.c
+++ b/src/libmsc/msc_a.c
@@ -673,6 +673,8 @@
rtp_to_ran->codecs_known ?
&rtp_to_ran->codecs : NULL, NULL);
}
+static void assignment_request_timeout_cb(void *data);
+
/* The MGW has given us a local IP address for the RAN side. Ready to start
the Assignment of a voice channel. */
void msc_a_tx_assignment_cmd(struct msc_a *msc_a)
{
@@ -680,6 +682,19 @@
struct gsm_trans *cc_trans = msc_a->cc.active_trans;
struct gsm0808_channel_type channel_type;
+ /* Do not dispatch another Assignment Command before an earlier
assignment is completed. This is a sanity
+ * safeguard, ideally callers should not even invoke this function when
an Assignment is already ongoing.
+ * (There is no osmo_fsm for Assignment / the CC trans code; when we
refactor that one day, this timer should be
+ * an FSM state.) */
+ if (osmo_timer_pending(&msc_a->cc.assignment_request_pending)) {
+ LOG_MSC_A(msc_a, LOGL_ERROR,
+ "Not transmitting Assignment, still waiting for the
response to an earlier Assignment\n");
+ return;
+ }
+ osmo_timer_setup(&msc_a->cc.assignment_request_pending,
assignment_request_timeout_cb, msc_a);
+ osmo_timer_schedule(&msc_a->cc.assignment_request_pending,
+ osmo_tdef_get(msc_a->c.ran->tdefs, -37,
OSMO_TDEF_S, 10), 0);
+
if (!cc_trans) {
LOG_MSC_A(msc_a, LOGL_ERROR, "No CC transaction active\n");
call_leg_release(msc_a->cc.call_leg);
@@ -943,6 +958,9 @@
vlr_subscr_enable_expire_lu(vsub);
}
+ /* We no longer care about assignment responses. */
+ osmo_timer_del(&msc_a->cc.assignment_request_pending);
+
/* If we're closing in a middle of a trans, we need to clean up */
trans_conn_closed(msc_a);
@@ -1051,6 +1069,7 @@
vsub->msc_conn_ref = NULL;
osmo_timer_del(&msc_a->lu_delay_timer);
+ osmo_timer_del(&msc_a->cc.assignment_request_pending);
}
const struct value_string msc_a_fsm_event_names[] = {
@@ -1479,6 +1498,9 @@
const struct gsm0808_speech_codec *codec_if_known =
ac->assignment_complete.codec_present ?
&ac->assignment_complete.codec : NULL;
+ /* Pending assignment has worked out. We're no longer waiting for a
response now. */
+ osmo_timer_del(&msc_a->cc.assignment_request_pending);
+
/* For a voice group call, handling is performed by VGCS FSM */
gcc_trans = trans_find_by_type(msc_a, TRANS_GCC);
if (gcc_trans) {
@@ -1582,10 +1604,15 @@
}
}
+/* Invoked when Assignment has failed, either by a failure response, or by
timeout. When failing on timeout,
+ * pass af == NULL. */
static void msc_a_up_call_assignment_failure(struct msc_a *msc_a, const struct
ran_msg *af)
{
struct gsm_trans *trans;
+ /* Pending assignment has failed. We're no longer waiting for a
response now. */
+ osmo_timer_del(&msc_a->cc.assignment_request_pending);
+
/* For a normal voice call, there will be an rtp_stream FSM. */
if (msc_a->cc.call_leg && msc_a->cc.call_leg->rtp[RTP_TO_RAN]) {
LOG_MSC_A(msc_a, LOGL_ERROR, "Assignment Failure, releasing
call\n");
@@ -1617,6 +1644,12 @@
msc_a_release_cn(msc_a);
}
+static void assignment_request_timeout_cb(void *data)
+{
+ struct msc_a *msc_a = data;
+ msc_a_up_call_assignment_failure(msc_a, NULL);
+}
+
static void msc_a_up_classmark_update(struct msc_a *msc_a, const struct
osmo_gsm48_classmark *classmark,
struct osmo_gsm48_classmark *dst)
{
diff --git a/src/libmsc/ran_infra.c b/src/libmsc/ran_infra.c
index 79de7d2..0656cca 100644
--- a/src/libmsc/ran_infra.c
+++ b/src/libmsc/ran_infra.c
@@ -47,6 +47,8 @@
{ .T = -36, .default_val = 0, .unit = OSMO_TDEF_MS, \
.desc = "Delay connection release after LU. Useful to optimize an
SMSC to dispatch " \
"pending messages within the initial connection." }, \
+ { .T = -37, .default_val = 10, \
+ .desc = "Voice channel Assignment sanity timeout, when no response is
received (should never happen)." }, \
struct osmo_tdef msc_tdefs_geran[] = {
RAN_TDEFS
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/37272?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I0f1e1a551aed545b555b9f236fc5967b1e4cc940
Gerrit-Change-Number: 37272
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: pespin <pes...@sysmocom.de>
Gerrit-MessageType: merged
