Attention is currently required from: lynxis lazus, pespin. fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38398?usp=email ) Change subject: SGSN: BSSGP_ConnHdlr: f_gmm_attach(): allow the SGSN to request the IMEI ...................................................................... Patch Set 6: Code-Review-1 (4 comments) File sgsn/BSSGP_ConnHdlr.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38398/comment/396ec4f1_40c5… : PS6, Line 240: as_mm_identity Maybe deriving two smaller altsteps (`as_mm_identity_imsi` and `as_mm_identity_imei`) from this one would be more flexible? The existing `as_mm_identity` would just combine both like this: ``` altstep as_mm_identity(integer ran_index := 0) runs on BSSGP_ConnHdlr { [] as_mm_identity_imsi(ran_index); [] as_mm_identity_imei(ran_index); } ``` This would allow the API user to expect (or not expect) specific identity, e.g. you could use `as_mm_identity_imei` below to expect the IMEI request specifically. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38398/comment/083f37a4_f7a1… : PS6, Line 265: as_receive_l3 This again looks 99% identical to the existing `f_receive_l3()`, so again code duplication. An altstep is indeed more flexible, but having more than one API doing the same thing is not good. I suggest removing `f_receive_l3()` and changing the code to use your new altstep. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38398/comment/d22ac246_96e4… : PS6, Line 578: allow_id_imei_req We may still receive an identity request here (regardless of the expectations), and this would block the altstep until `Tguard` fires... With the approach of two smaller altsteps I suggested above, you could do something like this: ``` [allow_id_imei_req] as_mm_identity_imei(ran_index) { repeat; } [] as_mm_identity(ran_index) { setverdict(fail, "Rx unexpected MM IDENTITY Req"); } [] as_receive_l3(tr_GMM_ATTACH_ACCEPT('001'B, ?, ?), l3_mt, ran_index) { ... } } ``` https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38398/comment/c663b540_e8af… : PS6, Line 579: f_process_attach_accept (cosmetic, but) please move the function call to its own line. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38398?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Id49c9e6ef7517a6a831315ac1f9915c50b88beb6 Gerrit-Change-Number: 38398 Gerrit-PatchSet: 6 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Sun, 24 Nov 2024 18:45:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes