June 2023 - gerrit-log - lists.osmocom.org

[S] Change in osmo-bts[master]: pcu_sock: move variable declaration of imsi[4] into related scope
by fixeria 07 Jun '23

07 Jun '23

Attention is currently required from: dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33215 ) Change subject: pcu_sock: move variable declaration of imsi[4] into related scope ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33215 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I62aed4f1c600ce2a80d2df928a60b6a2e0ae1889 Gerrit-Change-Number: 33215 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 10:36:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: paging: do not confirm PAGING COMMAND messages
by fixeria 07 Jun '23

07 Jun '23

Attention is currently required from: dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32734 ) Change subject: paging: do not confirm PAGING COMMAND messages ...................................................................... Patch Set 14: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32734 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8b8264d28b1b1deb08774cdba58dd4c6dafe115d Gerrit-Change-Number: 32734 Gerrit-PatchSet: 14 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: Wed, 07 Jun 2023 10:36:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: pcu_sock: move variable declaration of imsi[4] into related scope
by pespin 07 Jun '23

07 Jun '23

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33215 ) Change subject: pcu_sock: move variable declaration of imsi[4] into related scope ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33215 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I62aed4f1c600ce2a80d2df928a60b6a2e0ae1889 Gerrit-Change-Number: 33215 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 10:34:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: pcu_sock: move variable declaration of imsi[4] into related scope
by dexter 07 Jun '23

07 Jun '23

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/33215 ) Change subject: pcu_sock: move variable declaration of imsi[4] into related scope ...................................................................... pcu_sock: move variable declaration of imsi[4] into related scope The variable imsi[4] may be moved into the scope of case PCU_IF_SAPI_PCH Related: OS#5927 Change-Id: I62aed4f1c600ce2a80d2df928a60b6a2e0ae1889 --- M src/common/pcu_sock.c 1 file changed, 13 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/15/33215/1 diff --git a/src/common/pcu_sock.c b/src/common/pcu_sock.c index 811cea4..16048e2 100644 --- a/src/common/pcu_sock.c +++ b/src/common/pcu_sock.c @@ -667,7 +667,6 @@ struct gsm_bts_trx_ts *ts; struct msgb *msg; int rc = 0; - char imsi[4]; LOGP(DPCU, LOGL_DEBUG, "Data request received: sapi=%s arfcn=%d " "block=%d data=%s\n", sapi_string[data_req->sapi], @@ -679,6 +678,7 @@ { const struct gsm48_imm_ass *gsm48_imm_ass; bool confirm; + char imsi[4]; OSMO_STRLCPY_ARRAY(imsi, (char *)data_req->data); if (data_req->len-3 != GSM_MACBLOCK_LEN) { LOGP(DPCU, LOGL_ERROR, "MAC block with invalid length %d (expecting GSM_MACBLOCK_LEN = %d)\n", -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33215 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I62aed4f1c600ce2a80d2df928a60b6a2e0ae1889 Gerrit-Change-Number: 33215 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-bts[master]: paging: do not confirm PAGING COMMAND messages
by dexter 07 Jun '23

07 Jun '23

Attention is currently required from: fixeria. Hello Jenkins Builder, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32734 to look at the new patch set (#14). Change subject: paging: do not confirm PAGING COMMAND messages ...................................................................... paging: do not confirm PAGING COMMAND messages when osmo-bts receives a MAC block from osmo-pcu through the PCUIF it puts it in the review queue without further interpreting it. This also means that it will send confirmations to the PCU for IMMEDIATE ASSIGNMENT and PAGING COMMAND. This is not entirely correct because only IMMEDIATE ASSIGNMENT messages should be confirmed. osmo-pcu has no problem with this since it silently drops the confirmations for PAGING COMMAND messages. This peculiarity of the PCUIF implementation makes the confirmation logic hard to understand, so let's add some logic to osmo-bts that makes sure that only IMMEDIATE ASSIGNMENT messages are confirmed. Related: OS#5927 Change-Id: I8b8264d28b1b1deb08774cdba58dd4c6dafe115d --- M include/osmo-bts/paging.h M src/common/paging.c M src/common/pcu_sock.c 3 files changed, 36 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/34/32734/14 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32734 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8b8264d28b1b1deb08774cdba58dd4c6dafe115d Gerrit-Change-Number: 32734 Gerrit-PatchSet: 14 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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-bts[master]: paging: do not confirm PAGING COMMAND messages
by dexter 07 Jun '23

07 Jun '23

Attention is currently required from: fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32734 ) Change subject: paging: do not confirm PAGING COMMAND messages ...................................................................... Patch Set 14: (3 comments) File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/32734/comment/a0cd5984_759e75b1 PS12, Line 670: imsi > This array can be moved to the respective scope of use now. I have put that in a follow up patch https://gerrit.osmocom.org/c/osmo-bts/+/32734/comment/546c9222_36cddc9a PS12, Line 680: struct gsm48_imm_ass *gsm48_imm_ass > `const` please Done https://gerrit.osmocom.org/c/osmo-bts/+/32734/comment/d8e9db8e_d4d59073 PS12, Line 683: (struct gsm48_imm_ass *)data_req->data + 3 > This looks wrong to me. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32734 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8b8264d28b1b1deb08774cdba58dd4c6dafe115d Gerrit-Change-Number: 32734 Gerrit-PatchSet: 14 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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 10:26:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: paging: do not confirm PAGING COMMAND messages
by dexter 07 Jun '23

07 Jun '23

Attention is currently required from: fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32734 ) Change subject: paging: do not confirm PAGING COMMAND messages ...................................................................... Patch Set 13: (1 comment) File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/32734/comment/a6220441_3a4f45f4 PS12, Line 684: gsm48_imm_ass->msg_type > Another problem here is that you're accessing `data_req->data` (via `struct gsm48_imm_ass *`) before […] That makes sense of course. (This all will get better with PCUIF v.11) -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32734 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8b8264d28b1b1deb08774cdba58dd4c6dafe115d Gerrit-Change-Number: 32734 Gerrit-PatchSet: 13 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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 10:11:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: paging: do not confirm PAGING COMMAND messages
by dexter 07 Jun '23

07 Jun '23

Attention is currently required from: fixeria. Hello Jenkins Builder, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32734 to look at the new patch set (#13). Change subject: paging: do not confirm PAGING COMMAND messages ...................................................................... paging: do not confirm PAGING COMMAND messages when osmo-bts receives a MAC block from osmo-pcu through the PCUIF it puts it in the review queue without further interpreting it. This also means that it will send confirmations to the PCU for IMMEDIATE ASSIGNMENT and PAGING COMMAND. This is not entirely correct because only IMMEDIATE ASSIGNMENT messages should be confirmed. osmo-pcu has no problem with this since it silently drops the confirmations for PAGING COMMAND messages. This peculiarity of the PCUIF implementation makes the confirmation logic hard to understand, so let's add some logic to osmo-bts that makes sure that only IMMEDIATE ASSIGNMENT messages are confirmed. Related: OS#5927 Change-Id: I8b8264d28b1b1deb08774cdba58dd4c6dafe115d --- M include/osmo-bts/paging.h M src/common/paging.c M src/common/pcu_sock.c 3 files changed, 36 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/34/32734/13 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32734 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8b8264d28b1b1deb08774cdba58dd4c6dafe115d Gerrit-Change-Number: 32734 Gerrit-PatchSet: 13 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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmocore[master]: osmo_io: Fix sending msgb structures
by laforge 07 Jun '23

07 Jun '23

Attention is currently required from: arehbein, pespin, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33084 ) Change subject: osmo_io: Fix sending msgb structures ...................................................................... Patch Set 4: Code-Review+1 (1 comment) Patchset: PS2: > Ok then I'll prepare a patch removing osmo_iofd_write_{en,dis}able > > Those were only ever intended as public API for the user. Internally the txqueue will take care of dis-/enabling the writes. great. Then let's not introduce API where we don't have a clear use case for. IF it should ever be needed, we can introduce it then. > The next question then is whether read_{en,dis}able is actually needed in osmo_io. I'd treat it like above, i.e. not introduce it now if we have no user in our existing codebase, and introduce it _if_ ever needed later. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33084 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ie2a9c93f820fa372a1d527c805fd0fe2cff0eb49 Gerrit-Change-Number: 33084 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 10:05:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in pysim[master]: serial: return a return code in reset_card()
by laforge 07 Jun '23

07 Jun '23

laforge has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/33152 ) Change subject: serial: return a return code in reset_card() ...................................................................... serial: return a return code in reset_card() The method reset_card does not return a return code, while the coresponding pcsc implementation does return 1 on success. Change-Id: I658dd6857580652696b4a77e7d6cfe5778f09eff --- M pySim/transport/serial.py 1 file changed, 13 insertions(+), 0 deletions(-) Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified diff --git a/pySim/transport/serial.py b/pySim/transport/serial.py index 924df9e..3313a5e 100644 --- a/pySim/transport/serial.py +++ b/pySim/transport/serial.py @@ -104,6 +104,7 @@ raise NoCardError() elif rv < 0: raise ProtocolError() + return rv def _reset_card(self): self._atr = None -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33152 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I658dd6857580652696b4a77e7d6cfe5778f09eff Gerrit-Change-Number: 33152 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tobias <t-osmocom.org(a)tobias.org> Gerrit-MessageType: merged

1 0