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
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
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
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
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
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
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