[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
jolly has submitted this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email ) ( 7 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: Refactoring encoding of mobile identity at mobile application .. Refactoring encoding of mobile identity at mobile application Deprecated functions gsm48_generate_mid_from_*() are replaced by osmo_mobile_identity_encode_msgb(). Clean up code. Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 --- M src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h M src/host/layer23/src/mobile/gsm48_mm.c M src/host/layer23/src/mobile/gsm48_rr.c 3 files changed, 80 insertions(+), 61 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h b/src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h index 3ece82e..8ec1b7a 100644 --- a/src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h +++ b/src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h @@ -240,6 +240,8 @@ int state; }; +int gsm48_encode_mi_lv(struct osmocom_ms *ms, struct msgb *msg, uint8_t mi_type, bool emergency_imsi); +int gsm48_encode_mi_tlv(struct osmocom_ms *ms, struct msgb *msg, uint8_t mi_type, bool emergency_imsi); uint8_t gsm48_current_pwr_lev(struct gsm_settings *set, uint16_t arfcn); int gsm48_mm_init(struct osmocom_ms *ms); int gsm48_mm_exit(struct osmocom_ms *ms); diff --git a/src/host/layer23/src/mobile/gsm48_mm.c b/src/host/layer23/src/mobile/gsm48_mm.c index ed8cdf8..488416d 100644 --- a/src/host/layer23/src/mobile/gsm48_mm.c +++ b/src/host/layer23/src/mobile/gsm48_mm.c @@ -272,46 +272,68 @@ return length; } -/* encode 'mobile identity' */ -int gsm48_encode_mi(uint8_t *buf, struct msgb *msg, struct osmocom_ms *ms, - uint8_t mi_type) +/* Encode and append 'mobile identity' of given type to message, based on current settings. */ +int gsm48_encode_mi_lv(struct osmocom_ms *ms, struct msgb *msg, uint8_t mi_type, bool emergency_imsi) { struct gsm_subscriber *subscr = >subscr; struct gsm_settings *set = >settings; - uint8_t *ie; + struct osmo_mobile_identity mi = { }; + int rc; + uint8_t *l; - switch(mi_type) { + /* Copy MI values according to their types. */ + switch (mi_type) { case GSM_MI_TYPE_TMSI: - gsm48_generate_mid_from_tmsi(buf, subscr->tmsi); + mi.tmsi = subscr->tmsi; break; case GSM_MI_TYPE_IMSI: - gsm48_generate_mid_from_imsi(buf, subscr->imsi); + if (emergency_imsi) + OSMO_STRLCPY_ARRAY(mi.imsi, set->emergency_imsi); + else + OSMO_STRLCPY_ARRAY(mi.imsi, subscr->imsi); break; case GSM_MI_TYPE_IMEI: - gsm48_generate_mid_from_imsi(buf, set->imei); + OSMO_STRLCPY_ARRAY(mi.imei, set->imei); break; case GSM_MI_TYPE_IMEISV: - gsm48_generate_mid_from_imsi(buf, set->imeisv); + OSMO_STRLCPY_ARRAY(mi.imeisv, set->imeisv); + break; + } + + /* Generate MI or 'NONE', if not available. */ + switch (mi_type) { + case GSM_MI_TYPE_TMSI: + case GSM_MI_TYPE_IMSI: + case GSM_MI_TYPE_IMEI: + case GSM_MI_TYPE_IMEISV: + mi.type = mi_type; + l = msgb_put(msg, 1); + rc = osmo_mobile_identity_encode_msgb(msg, , true); + if (rc < 0) { + LOGP(DMM, LOGL_ERROR, "Failed to encode mobile identity type %d. Please fix!\n", mi_type); + *l = 1; + msgb_put_u8(msg, 0xf0 | GSM_MI_TYPE_NONE); + break; + } + *l = rc; break; case GSM_MI_TYPE_NONE: default: - buf[0] = GSM48_IE_MOBILE_ID; - buf[1] = 1; - buf[2] = 0xf0; - break; - } - /* alter MI type */ - buf[2] = (buf[2] & ~GSM_MI_TYPE_MASK) | mi_type; - - if (msg) { - /* MI as LV */ - ie = msgb_put(msg, 1 + buf[1]); - memcpy(ie, buf + 1, 1 + buf[1]); + msgb_put_u8(msg, 1); + msgb_put_u8(msg, 0xf0 | mi_type); } return 0; } +int gsm48_encode_mi_tlv(struct osmocom_ms *ms, struct msgb *msg, uint8_t mi_type, bool emergency_imsi) +{ + /* Append IE type. */ + msgb_put_u8(msg, GSM48_IE_MOBILE_ID); + + return gsm48_encode_mi_lv(ms, msg, mi_type, emergency_imsi); +} + /* encode 'classmark 1' */ int
[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
Attention is currently required from: jolly. Hello Jenkins Builder, fixeria, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email to look at the new patch set (#9). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder The change is no longer submittable: Verified is unsatisfied now. Change subject: Refactoring encoding of mobile identity at mobile application .. Refactoring encoding of mobile identity at mobile application Deprecated functions gsm48_generate_mid_from_*() are replaced by osmo_mobile_identity_encode_msgb(). Clean up code. Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 --- M src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h M src/host/layer23/src/mobile/gsm48_mm.c M src/host/layer23/src/mobile/gsm48_rr.c 3 files changed, 80 insertions(+), 61 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/84/34484/9 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 Gerrit-Change-Number: 34484 Gerrit-PatchSet: 9 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: jolly Gerrit-MessageType: newpatchset
[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
Attention is currently required from: jolly. Hello Jenkins Builder, fixeria, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder The change is no longer submittable: Verified is unsatisfied now. Change subject: Refactoring encoding of mobile identity at mobile application .. Refactoring encoding of mobile identity at mobile application Deprecated functions gsm48_generate_mid_from_*() are replaced by osmo_mobile_identity_encode_msgb(). Clean up code. Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 --- M src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h M src/host/layer23/src/mobile/gsm48_mm.c M src/host/layer23/src/mobile/gsm48_rr.c 3 files changed, 80 insertions(+), 61 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/84/34484/8 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 Gerrit-Change-Number: 34484 Gerrit-PatchSet: 8 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: jolly Gerrit-MessageType: newpatchset
[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
Attention is currently required from: jolly. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email ) Change subject: Refactoring encoding of mobile identity at mobile application .. Patch Set 7: Code-Review+1 (2 comments) Patchset: PS7: BTW, this is already the 2nd time we're migrating from old deprecated to a new (not yet deprecated) API. Let's see how long this new API will last :P File src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h: https://gerrit.osmocom.org/c/osmocom-bb/+/34484/comment/d6b33307_18808dba PS7, Line 243: gsm48_encode_mi_lv I am afraid we may end up having a naming conflict with libosmocore some day, since I am seeing plenty of `gsm48_encode_*` symbols in there. Not blocking, just wanted to point this out. -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 Gerrit-Change-Number: 34484 Gerrit-PatchSet: 7 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: jolly Gerrit-Comment-Date: Tue, 26 Sep 2023 14:14:10 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
Attention is currently required from: jolly. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email ) Change subject: Refactoring encoding of mobile identity at mobile application .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 Gerrit-Change-Number: 34484 Gerrit-PatchSet: 7 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: jolly Gerrit-Comment-Date: Tue, 26 Sep 2023 13:56:22 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
Attention is currently required from: jolly. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email ) Change subject: Refactoring encoding of mobile identity at mobile application .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 Gerrit-Change-Number: 34484 Gerrit-PatchSet: 7 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-Attention: jolly Gerrit-Comment-Date: Tue, 26 Sep 2023 11:41:17 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
Attention is currently required from: pespin. jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email ) Change subject: Refactoring encoding of mobile identity at mobile application .. Patch Set 6: (3 comments) Commit Message: https://gerrit.osmocom.org/c/osmocom-bb/+/34484/comment/764ccc95_b155d748 PS5, Line 9: Deprecated functions gsm48_generate_mid_from_*() are replaced by > yay! \o/ Done Patchset: PS5: > Just to make sure, did you test a bit this? This kind of patches can easily > add regressions. […] Yes, it is tested. It is used for every attach/detach/location update using IMSI and TMSI. Also I tested it with voice group call, where it is used to identify the talker. File src/host/layer23/src/mobile/gsm48_mm.c: https://gerrit.osmocom.org/c/osmocom-bb/+/34484/comment/a18d5ba1_023dba70 PS5, Line 276: int gsm48_encode_mi(struct osmocom_ms *ms, struct msgb *msg, bool tlv, uint8_t mi_type, bool emergency_imsi) > Did you think about having 2 functions gsm48_encode_mi_tlv() and > gsm48_encode_mi_lv()? […] Done -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 Gerrit-Change-Number: 34484 Gerrit-PatchSet: 6 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 26 Sep 2023 10:25:48 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
Attention is currently required from: jolly. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: Refactoring encoding of mobile identity at mobile application .. Refactoring encoding of mobile identity at mobile application Deprecated functions gsm48_generate_mid_from_*() are replaced by osmo_mobile_identity_encode_msgb(). Clean up code. Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 --- M src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h M src/host/layer23/src/mobile/gsm48_mm.c M src/host/layer23/src/mobile/gsm48_rr.c 3 files changed, 80 insertions(+), 61 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/84/34484/6 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 Gerrit-Change-Number: 34484 Gerrit-PatchSet: 6 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-MessageType: newpatchset
[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
Attention is currently required from: jolly. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email ) Change subject: Refactoring encoding of mobile identity at mobile application .. Patch Set 5: (3 comments) Commit Message: https://gerrit.osmocom.org/c/osmocom-bb/+/34484/comment/d318655e_786fbc18 PS5, Line 9: Deprecated functions gsm48_generate_mid_from_*() are replaced by yay! \o/ Patchset: PS5: Just to make sure, did you test a bit this? This kind of patches can easily add regressions. laforge already submitted a similar once a few weeks/months ago and I think it is still in gerrit because I spotted some possible regressions. File src/host/layer23/src/mobile/gsm48_mm.c: https://gerrit.osmocom.org/c/osmocom-bb/+/34484/comment/1f598f90_00b0d62c PS5, Line 276: int gsm48_encode_mi(struct osmocom_ms *ms, struct msgb *msg, bool tlv, uint8_t mi_type, bool emergency_imsi) Did you think about having 2 functions gsm48_encode_mi_tlv() and gsm48_encode_mi_lv()? Then have: gsm48_encode_mi_tlv(..) { msgb_put_u8(msg, GSM48_IE_MOBILE_ID); return gsm48_encode_mi_lv(...) } -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 Gerrit-Change-Number: 34484 Gerrit-PatchSet: 5 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Comment-Date: Thu, 21 Sep 2023 12:48:56 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in osmocom-bb[master]: Refactoring encoding of mobile identity at mobile application
jolly has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34484?usp=email ) Change subject: Refactoring encoding of mobile identity at mobile application .. Refactoring encoding of mobile identity at mobile application Deprecated functions gsm48_generate_mid_from_*() are replaced by osmo_mobile_identity_encode_msgb(). Clean up code. Change-Id: I9ff429bd50d718530fdad64a276053a35c8928f2 --- M src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h M src/host/layer23/src/mobile/gsm48_mm.c M src/host/layer23/src/mobile/gsm48_rr.c 3 files changed, 75 insertions(+), 61 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/84/34484/1 diff --git a/src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h b/src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h index 3ece82e..4d8a353 100644 --- a/src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h +++ b/src/host/layer23/include/osmocom/bb/mobile/gsm48_mm.h @@ -240,6 +240,7 @@ int state; }; +int gsm48_encode_mi(struct osmocom_ms *ms, struct msgb *msg, bool tlv, uint8_t mi_type, bool emergency_imsi); uint8_t gsm48_current_pwr_lev(struct gsm_settings *set, uint16_t arfcn); int gsm48_mm_init(struct osmocom_ms *ms); int gsm48_mm_exit(struct osmocom_ms *ms); diff --git a/src/host/layer23/src/mobile/gsm48_mm.c b/src/host/layer23/src/mobile/gsm48_mm.c index 70a35f8..14a2721 100644 --- a/src/host/layer23/src/mobile/gsm48_mm.c +++ b/src/host/layer23/src/mobile/gsm48_mm.c @@ -272,41 +272,59 @@ return length; } -/* encode 'mobile identity' */ -int gsm48_encode_mi(uint8_t *buf, struct msgb *msg, struct osmocom_ms *ms, - uint8_t mi_type) +/* Encode and append 'mobile identity' of given type to message, based on current settings. */ +int gsm48_encode_mi(struct osmocom_ms *ms, struct msgb *msg, bool tlv, uint8_t mi_type, bool emergency_imsi) { struct gsm_subscriber *subscr = >subscr; struct gsm_settings *set = >settings; - uint8_t *ie; + struct osmo_mobile_identity mi = { }; + int rc; + uint8_t *l; - switch(mi_type) { + /* Copy MI values according to their types. */ + switch (mi_type) { case GSM_MI_TYPE_TMSI: - gsm48_generate_mid_from_tmsi(buf, subscr->tmsi); + mi.tmsi = subscr->tmsi; break; case GSM_MI_TYPE_IMSI: - gsm48_generate_mid_from_imsi(buf, subscr->imsi); + if (emergency_imsi) + OSMO_STRLCPY_ARRAY(mi.imsi, set->emergency_imsi); + else + OSMO_STRLCPY_ARRAY(mi.imsi, subscr->imsi); break; case GSM_MI_TYPE_IMEI: - gsm48_generate_mid_from_imsi(buf, set->imei); + OSMO_STRLCPY_ARRAY(mi.imei, set->imei); break; case GSM_MI_TYPE_IMEISV: - gsm48_generate_mid_from_imsi(buf, set->imeisv); + OSMO_STRLCPY_ARRAY(mi.imeisv, set->imeisv); + break; + } + + /* Append IE type. */ + if (tlv) + msgb_put_u8(msg, GSM48_IE_MOBILE_ID); + + /* Generate MI or 'NONE', if not available. */ + switch (mi_type) { + case GSM_MI_TYPE_TMSI: + case GSM_MI_TYPE_IMSI: + case GSM_MI_TYPE_IMEI: + case GSM_MI_TYPE_IMEISV: + mi.type = mi_type; + l = msgb_put(msg, 1); + rc = osmo_mobile_identity_encode_msgb(msg, , true); + if (rc < 0) { + LOGP(DMM, LOGL_ERROR, "Failed to encode mobile identity type %d. Please fix!\n", mi_type); + *l = 1; + msgb_put_u8(msg, 0xf0 | GSM_MI_TYPE_NONE); + break; + } + *l = rc; break; case GSM_MI_TYPE_NONE: default: - buf[0] = GSM48_IE_MOBILE_ID; - buf[1] = 1; - buf[2] = 0xf0; - break; - } - /* alter MI type */ - buf[2] = (buf[2] & ~GSM_MI_TYPE_MASK) | mi_type; - - if (msg) { - /* MI as LV */ - ie = msgb_put(msg, 1 + buf[1]); - memcpy(ie, buf + 1, 1 + buf[1]); + msgb_put_u8(msg, 1); + msgb_put_u8(msg, 0xf0 | mi_type); } return 0; @@ -1816,7 +1834,6 @@ { struct msgb *nmsg; struct gsm48_hdr *ngh; - uint8_t buf[11]; LOGP(DMM, LOGL_INFO, "IDENTITY RESPONSE\n"); @@ -1828,8 +1845,8 @@ ngh->proto_discr = GSM48_PDISC_MM; ngh->msg_type = GSM48_MT_MM_ID_RESP; - /* MI */ - gsm48_encode_mi(buf, nmsg, ms, mi_type); + /* MI (LV) */ + gsm48_encode_mi(ms, nmsg, false, mi_type, false); /* push RR header and send down */ return gsm48_mm_to_rr(ms, nmsg, GSM48_RR_DATA_REQ, 0, 0); @@ -1847,7