Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Patch Set 12: Code-Review+1 Let's move it forward. -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 12 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-CC: Stefan Sperling Gerrit-Comment-Date: Thu, 21 Feb 2019 10:19:08 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Vadim Yanitskiy has submitted this change and it was merged. (
https://gerrit.osmocom.org/11992 )
Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..
libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
It may happen that either the MS or an EUSE would become
unresponsive during a call independent SS session, e.g.
due to a bug, or a dropped message. In such cases, the
corresponding transaction would remain unfreed forever.
This change introduces a guard timer, that prevents keeping
'stalled' NCSS sessions forever. As soon as it expires, both
sides (i.e. MS and EUSE) are getting notified, and the
transaction is being released.
By default, the timer expires after 30 seconds. As soon as
either the MS, or an EUSE initiates any activity,
the watchdog timer is rescheduled.
The timeout value can be configured from the VTY:
msc
...
! Use 0 to disable this timer
ncss guard-timeout 30
Please note that changing the timeout value at run-time
doesn't affect the existing NCSS sessions, excepting the
case when the timer is disabled at run-time.
This change makes TC_lu_and_ss_session_timeout pass.
Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Related Change-Id: (TTCN) I3e1791773d56617172ae27a46889a1ae4d400e2f
Related: OS#3655
---
M include/osmocom/msc/gsm_data.h
M include/osmocom/msc/transaction.h
M src/libmsc/gsm_09_11.c
M src/libmsc/msc_vty.c
M src/libmsc/osmo_msc.c
M tests/test_nodes.vty
6 files changed, 90 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
Vadim Yanitskiy: Looks good to me, but someone else must approve
diff --git a/include/osmocom/msc/gsm_data.h b/include/osmocom/msc/gsm_data.h
index dab082d..7d3a1e7 100644
--- a/include/osmocom/msc/gsm_data.h
+++ b/include/osmocom/msc/gsm_data.h
@@ -192,6 +192,8 @@
/* Global MNCC guard timer value */
int mncc_guard_timeout;
+ /* Global guard timer value for NCSS sessions */
+ int ncss_guard_timeout;
struct {
struct mgcp_client_conf conf;
diff --git a/include/osmocom/msc/transaction.h
b/include/osmocom/msc/transaction.h
index 39b09ae..830328b 100644
--- a/include/osmocom/msc/transaction.h
+++ b/include/osmocom/msc/transaction.h
@@ -87,6 +87,8 @@
* a subscriber after successful Paging Response
*/
struct msgb *msg;
+ /* Inactivity timer, triggers transaction release */
+ struct osmo_timer_list timer_guard;
} ss;
};
diff --git a/src/libmsc/gsm_09_11.c b/src/libmsc/gsm_09_11.c
index d2ad0b7..c133656 100644
--- a/src/libmsc/gsm_09_11.c
+++ b/src/libmsc/gsm_09_11.c
@@ -51,6 +51,39 @@
/* FIXME: choose a proper range */
static uint32_t new_callref = 0x2001;
+static void ncss_session_timeout_handler(void *_trans)
+{
+ struct gsm_trans *trans = (struct gsm_trans *) _trans;
+ struct osmo_gsup_message gsup_msg = { 0 };
+
+ /* The timeout might be disabled from the VTY */
+ if (trans->net->ncss_guard_timeout == 0)
+ return;
+
+ LOGP(DMM, LOGL_NOTICE, "SS/USSD session timeout, releasing "
+ "transaction (trans=%p, callref=%x)\n", trans, trans->callref);
+
+ /* Indicate connection release to subscriber (if active) */
+ if (trans->conn != NULL) {
+ /* This pair of cause location and value is used by commercial
networks */
+ msc_send_ussd_release_complete_cause(trans->conn,
trans->transaction_id,
+ GSM48_CAUSE_LOC_PUN_S_LU, GSM48_CC_CAUSE_NORMAL_UNSPEC);
+ }
+
+ /* Terminate GSUP session with EUSE */
+ gsup_msg.message_type = OSMO_GSUP_MSGT_PROC_SS_ERROR;
+ OSMO_STRLCPY_ARRAY(gsup_msg.imsi, trans->vsub->imsi);
+
+ gsup_msg.session_state = OSMO_GSUP_SESSION_STATE_END;
+ gsup_msg.session_id = trans->callref;
+ gsup_msg.cause = GMM_CAUSE_NET_FAIL;
+
+ osmo_gsup_client_enc_send(trans->net->vlr->gsup_client, _msg);
+
+ /* Finally, release this transaction */
+ trans_free(trans);
+}
+
/* Entry point for call independent MO SS messages */
int gsm0911_rcv_nc_ss(struct ran_conn *conn, struct msgb *msg)
{
@@ -108,6 +141,10 @@
return -ENOMEM;
}
+ /* Init inactivity timer */
+ osmo_timer_setup(>ss.timer_guard,
+ ncss_session_timeout_handler, trans);
+
/* Count active NC SS/USSD sessions */
osmo_counter_inc(conn->network->active_nc_ss);
@@ -116,6 +153,12 @@
cm_service_request_concludes(conn, msg);
}
+ /* (Re)schedule the inactivity timer */
+ if (conn->network->ncss_guard_timeout > 0) {
+ osmo_timer_schedule(>ss.timer_guard,
+
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Patch Set 12: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 12 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-CC: Stefan Sperling Gerrit-Comment-Date: Thu, 21 Feb 2019 07:14:45 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Vadim Yanitskiy has posted comments on this change. (
https://gerrit.osmocom.org/11992 )
Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..
Patch Set 10:
(1 comment)
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c
File src/libmsc/gsm_09_11.c:
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c@156
PS10, Line 156: }
> else if (osmo_timer_pending(>ss.watchdog)) { […]
Done in ncss_session_timeout_handler().
--
To view, visit https://gerrit.osmocom.org/11992
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Gerrit-Change-Number: 11992
Gerrit-PatchSet: 10
Gerrit-Owner: Vadim Yanitskiy
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: Vadim Yanitskiy
Gerrit-Reviewer: daniel
Gerrit-CC: Stefan Sperling
Gerrit-Comment-Date: Tue, 19 Feb 2019 20:29:18 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Hello daniel, Max, Neels Hofmeyr, Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11992 to look at the new patch set (#12). Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. libmsc/gsm_09_11.c: implement guard timer for NCSS sessions It may happen that either the MS or an EUSE would become unresponsive during a call independent SS session, e.g. due to a bug, or a dropped message. In such cases, the corresponding transaction would remain unfreed forever. This change introduces a guard timer, that prevents keeping 'stalled' NCSS sessions forever. As soon as it expires, both sides (i.e. MS and EUSE) are getting notified, and the transaction is being released. By default, the timer expires after 30 seconds. As soon as either the MS, or an EUSE initiates any activity, the watchdog timer is rescheduled. The timeout value can be configured from the VTY: msc ... ! Use 0 to disable this timer ncss guard-timeout 30 Please note that changing the timeout value at run-time doesn't affect the existing NCSS sessions, excepting the case when the timer is disabled at run-time. This change makes TC_lu_and_ss_session_timeout pass. Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Related Change-Id: (TTCN) I3e1791773d56617172ae27a46889a1ae4d400e2f Related: OS#3655 --- M include/osmocom/msc/gsm_data.h M include/osmocom/msc/transaction.h M src/libmsc/gsm_09_11.c M src/libmsc/msc_vty.c M src/libmsc/osmo_msc.c M tests/test_nodes.vty 6 files changed, 90 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/92/11992/12 -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 12 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-CC: Stefan Sperling
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Harald Welte has posted comments on this change. (
https://gerrit.osmocom.org/11992 )
Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..
Patch Set 10:
(1 comment)
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c
File src/libmsc/gsm_09_11.c:
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c@153
PS10, Line 153: if (conn->network->ncss_guard_timeout > 0) {
> Hmm, I am not sure if we really need to care about changing the configuration
> at run-time...
it is a valid scenario to change the timeout at runtime in the VTY. However, I
don't think it's required to affect all ongoing NCSS sessions. It's sufficient
if all newly-established NCSS sessions are affected by the new timer.
--
To view, visit https://gerrit.osmocom.org/11992
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Gerrit-Change-Number: 11992
Gerrit-PatchSet: 10
Gerrit-Owner: Vadim Yanitskiy
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: Vadim Yanitskiy
Gerrit-Reviewer: daniel
Gerrit-CC: Stefan Sperling
Gerrit-Comment-Date: Wed, 13 Feb 2019 12:56:11 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Hello daniel, Max, Neels Hofmeyr, Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11992 to look at the new patch set (#11). Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. libmsc/gsm_09_11.c: implement guard timer for NCSS sessions It may happen that either the MS or an EUSE would become unresponsive during a call independent SS session, e.g. due to a bug, or a dropped message. In such cases, the corresponding transaction would remain unfreed forever. This change introduces a guard timer, that prevents keeping 'stalled' NCSS sessions forever. As soon as it expires, both sides (i.e. MS and EUSE) are getting notified, and the transaction is being released. By default, the timer expires after 30 seconds. As soon as either the MS, or an EUSE initiates any activity, the watchdog timer is rescheduled. The timeout value can be configured from the VTY: msc ... ! Use 0 to disable this timer ncss guard-timeout 30 This change makes TC_lu_and_ss_session_timeout pass. Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Related Change-Id: (TTCN) I3e1791773d56617172ae27a46889a1ae4d400e2f Related: OS#3655 --- M include/osmocom/msc/gsm_data.h M include/osmocom/msc/transaction.h M src/libmsc/gsm_09_11.c M src/libmsc/msc_vty.c M src/libmsc/osmo_msc.c M tests/test_nodes.vty 6 files changed, 86 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/92/11992/11 -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 11 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Reviewer: daniel Gerrit-CC: Stefan Sperling
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Vadim Yanitskiy has posted comments on this change. (
https://gerrit.osmocom.org/11992 )
Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..
Patch Set 10:
(4 comments)
https://gerrit.osmocom.org/#/c/11992/10//COMMIT_MSG
Commit Message:
https://gerrit.osmocom.org/#/c/11992/10//COMMIT_MSG@9
PS10, Line 9: ESME
> EUSE?
Ahh, sure. Thanks!
https://gerrit.osmocom.org/#/c/11992/10/include/osmocom/msc/transaction.h
File include/osmocom/msc/transaction.h:
https://gerrit.osmocom.org/#/c/11992/10/include/osmocom/msc/transaction.h@89
PS10, Line 89: watchdog;
> in the CC sub-struct above we call it "timer_guard". […]
Done.
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c
File src/libmsc/gsm_09_11.c:
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c@153
PS10, Line 153: if (conn->network->ncss_guard_timeout > 0) {
> This seems like possible race condition if you disable the ncss_guard_timeout
> from VTY while USSD se […]
Hmm, I am not sure if we really need to care about changing the configuration
at run-time...
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c@156
PS10, Line 156: }
> else if (osmo_timer_pending(>ss.watchdog)) { […]
This solution wouldn't help if the timer is disabled from the VTY, but there is
no any activity on a given session. A proper solution would be to reschedule
all NCSS timers from the VTY command handler...
--
To view, visit https://gerrit.osmocom.org/11992
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Gerrit-Change-Number: 11992
Gerrit-PatchSet: 10
Gerrit-Owner: Vadim Yanitskiy
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: Vadim Yanitskiy
Gerrit-Reviewer: daniel
Gerrit-CC: Stefan Sperling
Gerrit-Comment-Date: Sun, 10 Feb 2019 13:07:18 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
daniel has posted comments on this change. ( https://gerrit.osmocom.org/11992 )
Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..
Patch Set 10: Code-Review-1
(3 comments)
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c
File src/libmsc/gsm_09_11.c:
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c@153
PS10, Line 153: if (conn->network->ncss_guard_timeout > 0) {
This seems like possible race condition if you disable the ncss_guard_timeout
from VTY while USSD sessions are active. Existing timers will not be
rescheduled.
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c@156
PS10, Line 156: }
else if (osmo_timer_pending(>ss.watchdog)) {
osmo_timer_del(>ss.watchdog);
}
https://gerrit.osmocom.org/#/c/11992/10/src/libmsc/gsm_09_11.c@279
PS10, Line 279: }
My comment above may apply here and elsewere as well
--
To view, visit https://gerrit.osmocom.org/11992
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Gerrit-Change-Number: 11992
Gerrit-PatchSet: 10
Gerrit-Owner: Vadim Yanitskiy
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: Vadim Yanitskiy
Gerrit-Reviewer: daniel
Gerrit-CC: Stefan Sperling
Gerrit-Comment-Date: Thu, 07 Feb 2019 10:33:59 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Patch Set 10: (2 comments) looks fine to me, except for two typo (ESME/EUSE) in the commit message, and inconsistent naming watchdog vs. guard timer. https://gerrit.osmocom.org/#/c/11992/10//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/11992/10//COMMIT_MSG@9 PS10, Line 9: ESME EUSE? https://gerrit.osmocom.org/#/c/11992/10/include/osmocom/msc/transaction.h File include/osmocom/msc/transaction.h: https://gerrit.osmocom.org/#/c/11992/10/include/osmocom/msc/transaction.h@89 PS10, Line 89: watchdog; in the CC sub-struct above we call it "timer_guard". I would appreciate if we could align here and refer to it as a guard timer, not as a watchdog. They both do the same thing, right? -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 10 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Stefan Sperling Gerrit-Comment-Date: Thu, 07 Feb 2019 10:21:45 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Hello Max, Neels Hofmeyr, Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11992 to look at the new patch set (#10). Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. libmsc/gsm_09_11.c: implement guard timer for NCSS sessions It may happen that either the MS or an ESME would become unresponsive during a call independent SS session, e.g. due to a bug, or a dropped message. In such cases, the corresponding transaction would remain unfreed forever. This change introduces a guard timer, that prevents keeping 'stalled' NCSS sessions forever. As soon as it expires, both sides (i.e. MS and ESME) are getting notified, and the transaction is being released. By default, the timer expires after 30 seconds. As soon as either the MS, or an ESME initiates any activity, the watchdog timer is rescheduled. The timeout value can be configured from the VTY: msc ... ! Use 0 to disable this timer ncss guard-timeout 30 This change makes TC_lu_and_ss_session_timeout pass. Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Related Change-Id: (TTCN) I3e1791773d56617172ae27a46889a1ae4d400e2f Related: OS#3655 --- M include/osmocom/msc/gsm_data.h M include/osmocom/msc/transaction.h M src/libmsc/gsm_09_11.c M src/libmsc/msc_vty.c M src/libmsc/osmo_msc.c M tests/test_nodes.vty 6 files changed, 86 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/92/11992/10 -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 10 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Stefan Sperling
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Hello Max, Neels Hofmeyr, Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11992 to look at the new patch set (#9). Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. libmsc/gsm_09_11.c: implement guard timer for NCSS sessions It may happen that either the MS or an ESME would become unresponsive during a call independent SS session, e.g. due to a bug, or a dropped message. In such cases, the corresponding transaction would remain unfreed forever. This change introduces a guard timer, that prevents keeping 'stalled' NCSS sessions forever. As soon as it expires, both sides (i.e. MS and ESME) are getting notified, and the transaction is being released. By default, the timer expires after 30 seconds. As soon as either the MS, or an ESME initiates any activity, the watchdog timer is rescheduled. The timeout value can be configured from the VTY: msc ... ! Use 0 to disable this timer ncss guard-timeout 30 This change makes TC_lu_and_ss_session_timeout pass. Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Depends Change-Id: (libosmocore) Ie3ac85fcef90a5e532334ba3482804d5305c88d7 Related Change-Id: (TTCN) I3e1791773d56617172ae27a46889a1ae4d400e2f Related: OS#3655 --- M include/osmocom/msc/gsm_data.h M include/osmocom/msc/transaction.h M src/libmsc/gsm_09_11.c M src/libmsc/msc_vty.c M src/libmsc/osmo_msc.c M tests/test_nodes.vty 6 files changed, 86 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/92/11992/9 -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 9 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Stefan Sperling
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Patch Set 8: Hi Max, > Same comments as before. > Patch set 8: Code-Review -1 same opinion as before: not critical. > If that's the only reason than it's better to use uint8_t > (which can be casted to int without any issues) to keep it > consistent with vty code. Your suggestion to use 'uint8_t' wouldn't make it better, as there is no need to use fixed-size type in this particular case. We can extend the VTY later on to allow longer timeout values, so using generic 'unsigned int' would make more sense. Anyway, in general we neither use 'unsigned int' nor fixed-types in the 'for' statements where the counter 'i' is always > 0 - we just use 'int' and leave it up to the compiler. Nobody does complain about this. This patch is consistent with the current code, where we use 'int' for MNCC timeout, as well as with the libosmocore's API, where osmo_timer_schedule() does accept 'int'. The change does what is written in the commit message, and it also has the test case, so I don't see any reason to block it due to such minor issues. Again, if you have enough time and strong desire, feel free to submit a patch that changes both variables to 'unsigned int'. -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 8 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Stefan Sperling Gerrit-Comment-Date: Wed, 06 Feb 2019 10:30:47 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Max has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Patch Set 8: Code-Review-1 Same comments as before. -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 8 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Stefan Sperling Gerrit-Comment-Date: Wed, 06 Feb 2019 10:08:41 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Hello Max, Neels Hofmeyr, Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11992 to look at the new patch set (#8). Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. libmsc/gsm_09_11.c: implement guard timer for NCSS sessions It may happen that either the MS or an ESME would become unresponsive during a call independent SS session, e.g. due to a bug, or a dropped message. In such cases, the corresponding transaction would remain unfreed forever. This change introduces a guard timer, that prevents keeping 'stalled' NCSS sessions forever. As soon as it expires, both sides (i.e. MS and ESME) are getting notified, and the transaction is being released. By default, the timer expires after 30 seconds. As soon as either the MS, or an ESME initiates any activity, the watchdog timer is rescheduled. The timeout value can be configured from the VTY: msc ... ! Use 0 to disable this timer ncss guard-timeout 30 This change makes TC_lu_and_ss_session_timeout pass. Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Depends Change-Id: (libosmocore) Ie3ac85fcef90a5e532334ba3482804d5305c88d7 Related Change-Id: (TTCN) I3e1791773d56617172ae27a46889a1ae4d400e2f Related: OS#3655 --- M include/osmocom/msc/gsm_data.h M include/osmocom/msc/transaction.h M src/libmsc/gsm_09_11.c M src/libmsc/msc_vty.c M src/libmsc/osmo_msc.c M tests/test_nodes.vty 6 files changed, 88 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/92/11992/8 -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 8 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Stefan Sperling
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Stefan Sperling has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Patch Set 7: (1 comment) https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c File src/libmsc/gsm_09_11.c: https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c@62 PS7, Line 62: /* Release connection (if any) with subscriber */ There is a comment elsewhere in this file, above another call to trans_free(trans) which says: /* TODO: release connection with subscriber */ It looks like a general cleanup function might be useful, which could be invoked from this timeout handler and from other places? Perhaps not in this patch, but in a future patch? -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 7 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Stefan Sperling Gerrit-Comment-Date: Mon, 17 Dec 2018 10:58:53 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Max has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Patch Set 7: (1 comment) https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h File include/osmocom/msc/gsm_data.h: https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h@188 PS7, Line 188: int ncss_guard_timeout; > Same as mncc_guard_timeout. Integer is expected by libosmocore: […] If that's the only reason than it's better to use uint8_t (which can be casted to int without any issues) to keep it consistent with vty code. -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 7 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Fri, 14 Dec 2018 14:29:43 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Vadim Yanitskiy has posted comments on this change. (
https://gerrit.osmocom.org/11992 )
Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..
Patch Set 7:
(3 comments)
https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h
File include/osmocom/msc/gsm_data.h:
https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h@188
PS7, Line 188: int ncss_guard_timeout;
> Why do you use int in here?
Same as mncc_guard_timeout. Integer is expected by libosmocore:
void osmo_timer_schedule(struct osmo_timer_list *timer, int seconds, int
microseconds);
We can change it in a separate change. Not critical.
https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c
File src/libmsc/gsm_09_11.c:
https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c@477
PS7, Line 477: if (net->ncss_guard_timeout > 0) {
> Seems like we always check ncss_guard_timeout > 0
Because it's optional. What's wrong with that?
https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/msc_vty.c
File src/libmsc/msc_vty.c:
https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/msc_vty.c@382
PS7, Line 382: "guard timer value (sec.), or 0 to disable\n")
> So ncss_guard_timeout is always positive and fit into uint8_t.
Please see my comment above.
--
To view, visit https://gerrit.osmocom.org/11992
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Gerrit-Change-Number: 11992
Gerrit-PatchSet: 7
Gerrit-Owner: Vadim Yanitskiy
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: Vadim Yanitskiy
Gerrit-Comment-Date: Fri, 14 Dec 2018 13:34:46 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Max has posted comments on this change. ( https://gerrit.osmocom.org/11992 )
Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
..
Patch Set 7: Code-Review-1
(4 comments)
https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h
File include/osmocom/msc/gsm_data.h:
https://gerrit.osmocom.org/#/c/11992/7/include/osmocom/msc/gsm_data.h@188
PS7, Line 188: int ncss_guard_timeout;
Why do you use int in here?
https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c
File src/libmsc/gsm_09_11.c:
https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c@141
PS7, Line 141: /* Init self-destruction timer */
That sounds ominous - I like it :)
https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/gsm_09_11.c@477
PS7, Line 477: if (net->ncss_guard_timeout > 0) {
Seems like we always check ncss_guard_timeout > 0
https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/msc_vty.c
File src/libmsc/msc_vty.c:
https://gerrit.osmocom.org/#/c/11992/7/src/libmsc/msc_vty.c@382
PS7, Line 382: "guard timer value (sec.), or 0 to disable\n")
So ncss_guard_timeout is always positive and fit into uint8_t.
--
To view, visit https://gerrit.osmocom.org/11992
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb
Gerrit-Change-Number: 11992
Gerrit-PatchSet: 7
Gerrit-Owner: Vadim Yanitskiy
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Max
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: Vadim Yanitskiy
Gerrit-Comment-Date: Fri, 14 Dec 2018 13:14:49 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Patch Set 7: > Is this some named timer from the spec or it's just smth which > makes sense in practice? Second. No such timer in the specs. -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 7 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Max Gerrit-Comment-Date: Fri, 14 Dec 2018 13:05:29 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Max has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Patch Set 7: Is this some named timer from the spec or it's just smth which makes sense in practice? -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 7 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Max Gerrit-Comment-Date: Fri, 14 Dec 2018 12:59:55 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Hello Neels Hofmeyr, Harald Welte, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11992 to look at the new patch set (#7). Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. libmsc/gsm_09_11.c: implement guard timer for NCSS sessions It may happen that either the MS or an ESME would become unresponsive during a call independent SS session, e.g. due to a bug, or a dropped message. In such cases, the corresponding transaction would remain unfreed forever. This change introduces a guard timer, that prevents keeping 'stalled' NCSS sessions forever. As soon as it expires, both sides (i.e. MS and ESME) are getting notified, and the transaction is being released. By default, the timer expires after 30 seconds. As soon as either the MS, or an ESME initiates any activity, the timer is being rescheduled. The timeout value can be configured from the VTY: msc ... ! Use 0 to disable this timer ncss guard-timeout 30 Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Depends Change-Id: (OsmoHLR) I0589ff27933e9bca2bcf93b8259004935778db8f Related Change-Id: (TTCN) I3e1791773d56617172ae27a46889a1ae4d400e2f Related: OS#3655 --- M include/osmocom/msc/gsm_data.h M include/osmocom/msc/transaction.h M src/libmsc/gsm_09_11.c M src/libmsc/msc_vty.c M src/libmsc/osmo_msc.c M tests/test_nodes.vty 6 files changed, 86 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/92/11992/7 -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 7 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy
Change in osmo-msc[master]: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/11992 ) Change subject: libmsc/gsm_09_11.c: implement guard timer for NCSS sessions .. Set Ready For Review -- To view, visit https://gerrit.osmocom.org/11992 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf4d87c45e90324764073e8230e0fb9cb96dd9cb Gerrit-Change-Number: 11992 Gerrit-PatchSet: 6 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Sun, 02 Dec 2018 19:42:21 + Gerrit-HasComments: No Gerrit-HasLabels: No
