Attention is currently required from: laforge, fixeria.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/33690 )
Change subject: pySim/cards: Split legacy classes away from core SIM + UICC
......................................................................
Patch Set 5:
(1 comment)
File pySim/cards.py:
https://gerrit.osmocom.org/c/pysim/+/33690/comment/9ae38531_887226bc
PS4, Line 319: SimCard
> indeed. […]
I personally would put a comment into UiccCardBase that describes the situation. I would try to avoid the code dup for now since it also looks strange. Should we later add more classic SIM specific code to SimCardBase that technically wouldn't fit into UiccCardBase, then I would think about changing it. (but I think that is unlikely.)
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/33690
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Id36140675def5fc44eedce81fc7b09e0adc527e1
Gerrit-Change-Number: 33690
Gerrit-PatchSet: 5
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 17 Jul 2023 15:52:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: jolly, fixeria, pespin.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33448 )
Change subject: ASCI: Add Notification/FACCH support
......................................................................
Patch Set 5: Code-Review+1
(3 comments)
File include/osmo-bts/notification.h:
https://gerrit.osmocom.org/c/osmo-bts/+/33448/comment/83d8200b_fe60fe77
PS5, Line 60: int facch_gen_msg(struct gsm_bts *bts, uint8_t *out_buf, const uint8_t *group_call_ref, const uint8_t *chan_desc,
As far as I understand this is an ASCI related function. Maybe the name should make this clear as well. Like bts_asci_facch_gen_msg() ?
File src/common/rsl.c:
https://gerrit.osmocom.org/c/osmo-bts/+/33448/comment/f7f3f360_a88f8501
PS5, Line 814: broadcast_facch
> AFAIU, this function is broadcasting FACCH on all channels, even if they're not related to VGCS/VBS. […]
Maybe it would be good to add a comment to clarify this?
https://gerrit.osmocom.org/c/osmo-bts/+/33448/comment/0c3c46bb_0d390762
PS5, Line 814: static int broadcast_facch(struct gsm_bts *bts, const uint8_t *group_call_ref, const uint8_t *chan_desc,
maybe rename this asci_broadcast_facch, so that it is more clear that this function is asci related? In rsl.c a lot stuff comes together and since ASCI is a bit special I think it would be good to make everything ASCI related distinguishable from the regular GSM stuff.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33448
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I3ed14fa54a907891e492a7ada8e745a2c56cd46d
Gerrit-Change-Number: 33448
Gerrit-PatchSet: 5
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 17 Jul 2023 15:35:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33768 )
Change subject: bts: send IMMEDIATE ASSIGNMENT via AGCH when no IMSI is available
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> This means we would confirm any AGCH message, regardless if the IMMEDIATE ASSIGNMENT is for DL or UL […]
ACK. This will allow us for better time(out) estimation at PCU. For instance avoid scheduling blocks until at least the ImmAss has been transmitted towards the MS.
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33768
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: Iae1dd5d6be9ee8813a9c0b092926e8eda63f1e8e
Gerrit-Change-Number: 33768
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 17 Jul 2023 15:30:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33766 )
Change subject: paging: also accept zero length IMSI strings 3
......................................................................
paging: also accept zero length IMSI strings 3
When an IMMEDIATE ASSIGNMENT MAC block (from PCUIF) is added to the
paging queue, then also an IMSI is required. The paging queue uses the
last three digits of the IMSI to calculate the paging group. In case no
IMSI is given, the MAC block is rejected. This was handeled differently
before. Even an IMSI of length 0 would still be interpreted as "000" and
not rejected. See also:
I9f3799916e8102bf1ce0f21891f2d24f43091f01
Let's restore the behaviour we had before and accept short IMSI
strings again.
Change-Id: Iab1c3f1c39dd59bb53aa74b2c9e9e135e3985e0b
Related: OS#6099
---
M src/common/paging.c
1 file changed, 44 insertions(+), 10 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, but someone else must approve
fixeria: Looks good to me, approved
diff --git a/src/common/paging.c b/src/common/paging.c
index b1aad7c..edc2c1f 100644
--- a/src/common/paging.c
+++ b/src/common/paging.c
@@ -270,6 +270,27 @@
return 0;
}
+/* Convert the last three digits of a given IMSI string to their decimal representation. In case the given IMSI string
+ * is shorter than three or zero digits, it will be assumed as "000" */
+static uint16_t convert_imsi_to_decimal(const char *imsi)
+{
+ uint16_t _imsi;
+ size_t imsi_len = strlen(imsi);
+
+ /* Tha paging group is calculated from the last three digits of the IMSI */
+ if (imsi_len < 3) {
+ LOGP(DPAG, LOGL_ERROR, "short IMSI (%zu digits), will assume \"000\" to calculate paging group\n", imsi_len);
+ _imsi = 0;
+ } else {
+ imsi = imsi + imsi_len - 3;
+ _imsi = 100 * ((*(imsi++)) - '0');
+ _imsi += 10 * ((*(imsi++)) - '0');
+ _imsi += (*(imsi++)) - '0';
+ }
+
+ return _imsi;
+}
+
/* Add a ready formatted MAC block message to the paging queue, this can be an IMMEDIATE ASSIGNMENT, or a
* PAGING COMMAND (from the PCU) */
int paging_add_macblock(struct paging_state *ps, uint32_t tlli, const char *imsi, bool confirm, const uint8_t *macblock)
@@ -278,7 +299,6 @@
struct paging_record *pr;
uint16_t paging_group;
uint16_t _imsi;
- size_t imsi_len = strlen(imsi);
check_congestion(ps);
@@ -289,15 +309,7 @@
return -ENOSPC;
}
- /* Tha paging group is calculated from the last three digits of the IMSI */
- if (imsi_len < 3) {
- LOGP(DPAG, LOGL_ERROR, "IMSI with invalid length %zu (expecting at least the last 3 digits)\n", imsi_len);
- return -EINVAL;
- }
- imsi = imsi + imsi_len - 3;
- _imsi = 100 * ((*(imsi++)) - '0');
- _imsi += 10 * ((*(imsi++)) - '0');
- _imsi += (*(imsi++)) - '0';
+ _imsi = convert_imsi_to_decimal(imsi);
paging_group = gsm0502_calc_paging_group(&ps->chan_desc, _imsi);
group_q = &ps->paging_queue[paging_group];
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33766
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Iab1c3f1c39dd59bb53aa74b2c9e9e135e3985e0b
Gerrit-Change-Number: 33766
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: pespin.
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33768 )
Change subject: bts: send IMMEDIATE ASSIGNMENT via AGCH when no IMSI is available
......................................................................
Patch Set 2:
(1 comment)
File src/bts.cpp:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-9697):
https://gerrit.osmocom.org/c/osmo-pcu/+/33768/comment/68468488_b5eaa8d0
PS2, Line 1136: * be necessary for tranmission on PCH. Since the IMSI is usually only unknown during the GMM
'tranmission' may be misspelled - perhaps 'transmission'?
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33768
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: Iae1dd5d6be9ee8813a9c0b092926e8eda63f1e8e
Gerrit-Change-Number: 33768
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 17 Jul 2023 15:11:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33768 )
Change subject: bts: send IMMEDIATE ASSIGNMENT via AGCH when no IMSI is available
......................................................................
Patch Set 2:
(5 comments)
Patchset:
PS1:
> This cannot be merged until we confirm the ImmAss on AGCH back to PCU. […]
This means we would confirm any AGCH message, regardless if the IMMEDIATE ASSIGNMENT is for DL or UL TBF?
(I would send the confirmation when the AGCH message is pulled from the queue in bts.c:bts_ccch_copy_msg)
File src/bts.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/33768/comment/0fa73c00_7307b10e
PS1, Line 1126: if (strlen(tbf->imsi()) < 3) {
> ms_imsi_is_valid()
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/33768/comment/03b38989_22e2fe4c
PS1, Line 1127: /* In case the IMSI is not yet known, the IMMEDIATE ASSIGNMENT is sent on the AGCH. The reason
> Maybe explain that the usual case around here is during GMM attach, where the SGSN still didn't lear […]
Done
File src/pcu_l1_if.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/33768/comment/2c597f33_bb2d75e5
PS1, Line 278: /* OS#6097: if strlen(imsi) == 0: We assume the MS is in non-DRX
> imsi_len = strlen(imsi); […]
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/33768/comment/b1d1505a_d6567e41
PS1, Line 310: /* OS#6097: if strlen(pch_dt.imsi) == 0: We assume the MS is in non-DRX
> Same as above.
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33768
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: Iae1dd5d6be9ee8813a9c0b092926e8eda63f1e8e
Gerrit-Change-Number: 33768
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 17 Jul 2023 15:10:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
Hello Jenkins Builder, pespin, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-pcu/+/33768
to look at the new patch set (#2).
Change subject: bts: send IMMEDIATE ASSIGNMENT via AGCH when no IMSI is available
......................................................................
bts: send IMMEDIATE ASSIGNMENT via AGCH when no IMSI is available
There may be situations where a downlink TBF has to be assigned but the
IMSI is not yet known to the PCU. In this case we should not just send
the IMMEDIATE ASSIGNMENT via the PCH anyway. Instead, the AGCH should be
used.
Related: OS#6097
Change-Id: Iae1dd5d6be9ee8813a9c0b092926e8eda63f1e8e
---
M src/bts.cpp
M src/pcu_l1_if.cpp
2 files changed, 42 insertions(+), 18 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/68/33768/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33768
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: Iae1dd5d6be9ee8813a9c0b092926e8eda63f1e8e
Gerrit-Change-Number: 33768
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset