pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/15483 )
Change subject: Iu: implement a user inactivity timer ...................................................................... Patch Set 2: Code-Review-1 (5 comments) https://gerrit.osmocom.org/#/c/15483/2//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/15483/2//COMMIT_MSG@9 PS2, Line 9: The user inactivity timer is similiar to the Gb READY timer and redruce reduces https://gerrit.osmocom.org/#/c/15483/2/src/sgsn/gprs_mm_state_iu_fsm.c File src/sgsn/gprs_mm_state_iu_fsm.c: https://gerrit.osmocom.org/#/c/15483/2/src/sgsn/gprs_mm_state_iu_fsm.c@15 PS2, Line 15: [ST_PMM_CONNECTED] = { .T=-3314 }, So this timer is not defined in the case of Iu but we add it ourselves? please clarify that somewhere and leave a comment in code too if that's the case. If timer comes from specs then it should be positive (and write a comment with the spec reference). https://gerrit.osmocom.org/#/c/15483/2/src/sgsn/gprs_mm_state_iu_fsm.c@50 PS2, Line 50: static const struct RANAP_Cause user_inactive_cause = { why static? it's fine having it in the stack right? https://gerrit.osmocom.org/#/c/15483/2/src/sgsn/gprs_mm_state_iu_fsm.c@96 PS2, Line 96: /* timer for pmm state. state=CONNECTED: -T3314 (User inactivity timer) */ Ah here it is. Still, it's not clear if it comes from us or it's defined in specs. Better add the comment also in the timer struct definition at the start of the file. https://gerrit.osmocom.org/#/c/15483/2/src/sgsn/sgsn_vty.c File src/sgsn/sgsn_vty.c: https://gerrit.osmocom.org/#/c/15483/2/src/sgsn/sgsn_vty.c@107 PS2, Line 107: /* non spec timers */ Ah here it is! non-spec. Please add same thing in the FSM timer declaration. -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/15483 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I66c2ac0350cb074aefd9a22c5121acf723f239d3 Gerrit-Change-Number: 15483 Gerrit-PatchSet: 2 Gerrit-Owner: lynxis lazus <lyn...@fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <lafo...@gnumonks.org> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-Comment-Date: Thu, 12 Sep 2019 11:12:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
