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_40c52... : 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_f7a10... : 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_96e43... : 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_e8af2... : PS6, Line 579: f_process_attach_accept (cosmetic, but) please move the function call to its own line.