Harald Welte has submitted this change and it was merged. Change subject: CC: intentionally release T308 on BSSMAP Clear Request from BSC ......................................................................
CC: intentionally release T308 on BSSMAP Clear Request from BSC So far we hit a running T308 during CC release when caused by a BSSMAP Clear Request, and we loudly log that as error. However, now I understand that T308 is a direct cause of the dispatch of a REL IND towards MNCC, which is used to indicate teardown to MNCC. So during _gsm48_cc_trans_free(), we first clear all timers, then invoke mncc_release_ind() which starts another timer (useful for graceful CC Release, but in this code path the intention is immediate release). Simply immediately cancel the timer again and release the conn. A separate question is whether a BSSMAP Clear Request should be less aggressive in releasing the connections; i.e. instead of calling trans_free() all around, to rather ask each transaction to "please stop soon", somehow. Related: OS#3062 Change-Id: I231fdb574a086a206321148474cbdc7ca9cf39f0 --- M src/libmsc/gsm_04_08.c M src/libmsc/transaction.c M tests/msc_vlr/msc_vlr_test_call.err 3 files changed, 4 insertions(+), 10 deletions(-) Approvals: Vadim Yanitskiy: Looks good to me, but someone else must approve Harald Welte: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c index e5ddb44..f29c0b6 100644 --- a/src/libmsc/gsm_04_08.c +++ b/src/libmsc/gsm_04_08.c @@ -1422,6 +1422,9 @@ mncc_release_ind(trans->net, trans, trans->callref, GSM48_CAUSE_LOC_PRN_S_LU, GSM48_CC_CAUSE_RESOURCE_UNAVAIL); + /* This is a final freeing of the transaction. The MNCC release may have triggered the + * T308 release timer, but we don't have the luxury of graceful CC Release here. */ + gsm48_stop_cc_timer(trans); } if (trans->cc.state != GSM_CSTATE_NULL) new_cc_state(trans, GSM_CSTATE_NULL); diff --git a/src/libmsc/transaction.c b/src/libmsc/transaction.c index 66b929a..147099e 100644 --- a/src/libmsc/transaction.c +++ b/src/libmsc/transaction.c @@ -122,15 +122,6 @@ switch (trans->protocol) { case GSM48_PDISC_CC: _gsm48_cc_trans_free(trans); - if (osmo_timer_pending(&trans->cc.timer)) { - LOGP(DCC, LOGL_ERROR, - "%s Timer 0x%x is still running while discarding transaction" - " -- this is a bug: we were still expecting a response but" - " are freeing the transaction anyway\n", - vlr_subscr_name(trans->conn->vsub), trans->cc.Tcurrent); - osmo_timer_del(&trans->cc.timer); - trans->cc.Tcurrent = 0; - } conn_usage_token = MSC_CONN_USE_TRANS_CC; break; case GSM48_PDISC_SMS: diff --git a/tests/msc_vlr/msc_vlr_test_call.err b/tests/msc_vlr/msc_vlr_test_call.err index 6142464..1b8002b 100644 --- a/tests/msc_vlr/msc_vlr_test_call.err +++ b/tests/msc_vlr/msc_vlr_test_call.err @@ -1085,8 +1085,8 @@ DMNCC transmit message MNCC_REL_CNF DCC Sending 'MNCC_REL_CNF' to MNCC. MSC --> MNCC: callref 0x423: MNCC_REL_CNF +DCC stopping pending timer T308 DCC (ti 00 sub MSISDN:42342) new state RELEASE_REQ -> NULL -DCC MSISDN:42342 Timer 0x308 is still running while discarding transaction -- this is a bug: we were still expecting a response but are freeing the transaction anyway DREF VLR subscr MSISDN:42342 usage decreases to: 2 DREF MSISDN:42342: MSC conn use - trans_cc == 1 (0x4) - Iu Release --RAN_UTRAN_IU--> MS -- To view, visit https://gerrit.osmocom.org/7704 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I231fdb574a086a206321148474cbdc7ca9cf39f0 Gerrit-PatchSet: 3 Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Vadim Yanitskiy <axilira...@gmail.com>