[MERGED] osmo-msc[master]: CC: intentionally release T308 on BSSMAP Clear Request from BSC

2018-04-11 Thread Harald Welte 
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(>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(>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 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy

osmo-msc[master]: CC: intentionally release T308 on BSSMAP Clear Request from BSC

2018-04-11 Thread Harald Welte 

Patch Set 2:

If the bsc tells us the (radio) connection is gone , there would be no point in 
giving transactions more time to clean up.

-- 
To view, visit https://gerrit.osmocom.org/7704
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I231fdb574a086a206321148474cbdc7ca9cf39f0
Gerrit-PatchSet: 2
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No

osmo-msc[master]: CC: intentionally release T308 on BSSMAP Clear Request from BSC

2018-04-11 Thread Harald Welte 

Patch Set 2: Code-Review+2

-- 
To view, visit https://gerrit.osmocom.org/7704
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I231fdb574a086a206321148474cbdc7ca9cf39f0
Gerrit-PatchSet: 2
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No

osmo-msc[master]: CC: intentionally release T308 on BSSMAP Clear Request from BSC

2018-04-11 Thread Vadim Yanitskiy 

Patch Set 2: Code-Review+1

-- 
To view, visit https://gerrit.osmocom.org/7704
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I231fdb574a086a206321148474cbdc7ca9cf39f0
Gerrit-PatchSet: 2
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy 
Gerrit-HasComments: No

[PATCH] osmo-msc[master]: CC: intentionally release T308 on BSSMAP Clear Request from BSC

2018-04-09 Thread Neels Hofmeyr 
Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/7704

to look at the new patch set (#2).

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(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/04/7704/2

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(>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(>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: newpatchset
Gerrit-Change-Id: I231fdb574a086a206321148474cbdc7ca9cf39f0
Gerrit-PatchSet: 2
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder

[PATCH] osmo-msc[master]: CC: intentionally release T308 on BSSMAP Clear Request from BSC

2018-04-09 Thread Neels Hofmeyr 

Review at  https://gerrit.osmocom.org/7704

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(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/04/7704/1

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(>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(>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: newchange
Gerrit-Change-Id: I231fdb574a086a206321148474cbdc7ca9cf39f0
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr