Attention is currently required from: laforge, pespin, fixeria, keith, dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28509 )
Change subject: Change CC_CAUSE returned on unanswered MT Call ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File src/libmsc/gsm_04_08_cc.c: https://gerrit.osmocom.org/c/osmo-msc/+/28509/comment/4cd300a7_318ddd36 PS1, Line 281: (trans->cc.state == GSM_CSTATE_CALL_RECEIVED) ? > In adding this check, I should point out that I do not know what other CC > state is possible when we […] related: the cc state transitioning was written a very long time ago, before we had osmo_fsm. Specially for inter-MSC HO i implemented a new "proper" FSM for CC via MNCC in 2018, in mncc_fsm.[hc], because it was not possible to use the older cc implementation for what inter-msc requires. It is well possible and loosely intended to replace the old cc implementation with this new mncc_fsm (i wrote that intention into the comment on top of mncc_call.c). IMO it would clarify things and have "proper" state transition checking and so forth. Given the relatively low attention to osmo-msc in the past years the cc refactoring hasn't happened, and other things are more pressing, it doesn't seem to be happening any time soon. So until then we're left with figuring out the old cc implementation that osmo-msc still uses in the usual voice call scenarios. So: It seems to me that you have already figured out at least one case where sending USER_NOTRESPOND makes more sense, and i trust you with that judgement. I think this patch is fine, but if you would want to limit this cause to mgw X2 expiry: in call_leg.c, in call_leg_fsm_timer_cb(), you could detect the T that expired: if (fi->T == -2) ... the usual pattern we use is to store state about whether a cause value has already been determined, for example add trans.cc.release_cause, and here use that if it is nonzero: trans->cc.release_cause ? : GSM48_CAUSE_RESOURCE_UNAVAIL It seems that X2 is also used for a timeout during call release (though actually that seems like dead code), so probably if (fi->T == -2 && fi->state == CALL_LEG_ST_ESTABLISHING) trans->cc.release_cause = GSM48_CC_CAUSE_USER_NOTRESPOND; The missing link is that call_leg doesn't know its cc trans yet, so in call_leg we would need to add a backpointer to the trans, like rtp_stream already has: for_trans. All of this just if you think it is worthwhile... -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28509 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I4a9cfc388ec9ecb743d154a114a6db638eac4701 Gerrit-Change-Number: 28509 Gerrit-PatchSet: 1 Gerrit-Owner: keith <ke...@rhizomatica.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pma...@sysmocom.de> Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-CC: laforge <lafo...@osmocom.org> Gerrit-Attention: laforge <lafo...@osmocom.org> Gerrit-Attention: pespin <pes...@sysmocom.de> Gerrit-Attention: fixeria <vyanits...@sysmocom.de> Gerrit-Attention: keith <ke...@rhizomatica.org> Gerrit-Attention: dexter <pma...@sysmocom.de> Gerrit-Comment-Date: Fri, 22 Jul 2022 17:03:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: keith <ke...@rhizomatica.org> Gerrit-MessageType: comment
