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_40c522ec?usp=email
 :
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_f7a10631?usp=email
 :
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_96e4391b?usp=email
 :
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_e8af26fa?usp=email
 :
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 <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-CC: pespin <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Attention: lynxis lazus <[email protected]>
Gerrit-Comment-Date: Sun, 24 Nov 2024 18:45:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Reply via email to