This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
neels gerrit-no-reply at lists.osmocom.orgneels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/15186 )
Change subject: gprs_gmm.c: Call mmctx_set_(p)mm_state only on related ran_type
......................................................................
Patch Set 2:
(7 comments)
I'm advocating for OSMO_ASSERTS(); they would probably give us more SGSN crashes, but then we stand a chance of finding bugs related to a subscriber switching RAN types. The aim could also be to not crash and instead log errors and detach the subscriber, not so sure.
https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c
File src/gprs/gprs_gmm.c:
https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@140
PS2, Line 140: {
Vadim's idea seems very good, and I would add:
Place an OSMO_ASSERT for matching RAN type to catch all bugs that mix and mess it up.
(Am not aware of the callers but would assume no mismatching RAN type state should ever be passed to this.)
https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@153
PS2, Line 153: case PMM_CONNECTED:
(while at it we could lose this 'case', or even make the entire switch into an if())
https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@204
PS2, Line 204: mmctx_set_pmm_state(mm, PMM_IDLE);
how could a RANAP_IU_EVENT come in on a ran_type != MM_CTX_T_UTRAN_Iu? I guess that should be an OSMO_ASSERT()?
https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@1096
PS2, Line 1096: case GSM48_MT_GMM_SERVICE_REQ:
again this looks like it is clearly related to Iu and ran_type should always be UTRAN_Iu?
https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@2084
PS2, Line 2084: mmctx->gmm_state = GMM_REGISTERED_NORMAL;
I know this was in the code before, but it seems to overwrite the previous state. Both RAN types set the state after this, so this is redundant and may confuse logging?
(It says "Changing state from X to Y", where the X is here always GMM_REGISTERED_NORMAL, although the state would have been something else before overwriting?)
https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@2097
PS2, Line 2097: case MM_CTX_T_GERAN_Iu:
(instead of all of these dead switch cases, maybe we should rather "#if 0" away the unused enum member)
https://gerrit.osmocom.org/#/c/15186/2/src/gprs/gprs_gmm.c@2116
PS2, Line 2116: mmctx->gmm_state = GMM_REGISTERED_NORMAL;
(again weird overwriting of the current state, like above)
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15186
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I65ad9e180177bc9fc7c4a037cd85cfe33b161f73
Gerrit-Change-Number: 15186
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <axilirator at gmail.com>
Gerrit-CC: neels <nhofmeyr at sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:39:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190814/683a95ad/attachment.htm>