Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/12526 )
Change subject: Optionally store IMEI in subscriber table ...................................................................... Patch Set 4: Code-Review-1 (3 comments) https://gerrit.osmocom.org/#/c/12526/4/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/12526/4/src/hlr.c@442 PS4, Line 442: LOGP(DMAIN, LOGL_INFO, "%s: has IMEI: %s (consider setting 'store-imei 1')\n", gsup->imsi, imei); if the subscriber doesn't exist, we should respond with an error. I'm not saying to implement an IMEI black/whitelist, but at least to error if the IMSI isn't known at all. i.e. always first db_subscr_get_by_imsi(). I'm not sure if we need to optimize for db performance, but if we do, then this would be an obvious improvement: To have less DB hits instead implement db_update_imei_by_imsi(), so that we check IMSI presence and update the IMEI in one DB hit. A returned value of zero modified rows would then mean "IMSI not known". https://gerrit.osmocom.org/#/c/12526/4/src/hlr_vty.c File src/hlr_vty.c: https://gerrit.osmocom.org/#/c/12526/4/src/hlr_vty.c@75 PS4, Line 75: vty_out(vty, " store-imei 1%s", VTY_NEWLINE); looks like too much indent. Add a vty test for this to one or other .vty transcript test, please (check output of 'show running-config') https://gerrit.osmocom.org/#/c/12526/4/src/hlr_vty.c@313 PS4, Line 313: " to set 'check-imei-rqd 1' in osmo-msc.cfg.\n" wording: maybe rather "Note that an MSC does not necessarily send Check IMEI requests (for OsmoMSC, you may want to set 'check-imei-rqd 1')." consistency: AFAICT osmo-hlr so far uses only the 'foo' / 'no foo' syntax, so better also make this 'store-imei' 'no store-imei' ( If you decide to keep (0|1), then the first doc should be general for switching it off or on: store-imei Configure whether to store a received IMEI in the database. 0 Do not store... 1 Store received IMEIs. Note that an MSC does not necessarily send Check IMEI requests (for OsmoMSC, you may want to set 'check-imei-rqd 1'). ) -- To view, visit https://gerrit.osmocom.org/12526 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I09274ecbed64224f7ae305e09ede773931da2a57 Gerrit-Change-Number: 12526 Gerrit-PatchSet: 4 Gerrit-Owner: osmith <osm...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: osmith <osm...@sysmocom.de> Gerrit-Comment-Date: Mon, 14 Jan 2019 17:00:33 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
