Attention is currently required from: laforge.
wbokslag has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-tetra/+/34169 )
Change subject: Added crypto support
......................................................................
Patch Set 2:
(1 comment)
File src/crypto/tetra_crypto.c:
https://gerrit.osmocom.org/c/osmo-tetra/+/34169/comment/7eea84e2_89994c3f
PS2, Line 249: // key->addr, key->index, tcs->hn, tetra_tdma_time_dump(tdma_time), tmpdu_offset, ct_len);
> why is this (and similar lines below) commented-out in this patch? I'd suspect it would work just as […]
You are right, I have been making some general modifications to have the code conform more to what I believe to be the osmocom standard, I could/should probably have refrained from making such changes in this patch. I have removed it in an amended patch.
Log levels seem like a good idea for the future. The particular print in this example is rarely of interest. These, and some other prints elsewhere in the code, may be useful when debugging/understanding network traffic, but not to the majority of users.
--
To view, visit https://gerrit.osmocom.org/c/osmo-tetra/+/34169
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-tetra
Gerrit-Branch: master
Gerrit-Change-Id: I0c0227cf5b747bd5032602390175b898173f6ae6
Gerrit-Change-Number: 34169
Gerrit-PatchSet: 2
Gerrit-Owner: wbokslag <w.bokslag(a)midnightblue.nl>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 29 Aug 2023 10:22:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
Attention is currently required from: dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/34059 )
Change subject: pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
File include/osmo-bts/bts.h:
https://gerrit.osmocom.org/c/osmo-bts/+/34059/comment/235ff057_4fc6370b
PS3, Line 448: bool confirm;
changing the field order would probably benefit from better alignment :)
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34059
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I29858fa20ad8bd0aefe81a5c40ad77a2559a8c10
Gerrit-Change-Number: 34059
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 29 Aug 2023 10:04:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/34059
to look at the new patch set (#3).
Change subject: pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
......................................................................
pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
In PCUIF v.11 we use PCU_IF_SAPI_AGCH_2 exclusively. We use this SAPI
to transfer IMMEDIATE ASSIGNMENT messages for uplink and downlink. In
both cases we send a confirmation back to the PCU. For details see
coresponding patch in osmo-pcu.git (see Depends)
CAUTION: This patch breaks compatibility to current master osmo-pcu (See
also "Depends")
Related: OS#5927
Depends: osmo-pcu.git I9effdcec1da91a6e2e7a7c41f95d3300ad1bb292
Depends: osmo-ttcn3-hacks.git Iec00d8144dfb2cd8bcee9093c96a3cc98aea6458
Change-Id: I29858fa20ad8bd0aefe81a5c40ad77a2559a8c10
---
M include/osmo-bts/bts.h
M include/osmo-bts/pcu_if.h
M include/osmo-bts/pcuif_proto.h
M src/common/bts.c
M src/common/paging.c
M src/common/pcu_sock.c
6 files changed, 67 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/59/34059/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34059
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I29858fa20ad8bd0aefe81a5c40ad77a2559a8c10
Gerrit-Change-Number: 34059
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: osmith, dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34192 )
Change subject: pcuif_proto: check confirm flag in struct gsm_pcu_if_pch
......................................................................
Patch Set 3:
(1 comment)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34192/comment/b99acf52_ce71fc66
PS1, Line 552: if (pch->confirmed_imm_ass)
> I don't think that it is required to mention this, we do changes to PCUIF all the time. […]
I disagree with this. I'm now more or less understanding the different changes you are doing to the protocol, but even for myself it's starting to be confusing the amount of changes. Imagine for others, or ourselves in a few months time. It will be difficult to understand which changes are part of which versions of the protocol. So yeah, in general I'd love seeing a bit more thoughts and clear explanations on what these changes are part of (eg v10->v11 changes).
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34192
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I3d2842626b7e8325860ea3160c7d900d39e953a0
Gerrit-Change-Number: 34192
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 29 Aug 2023 09:59:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: osmith, dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34192 )
Change subject: pcuif_proto: check confirm flag in struct gsm_pcu_if_pch
......................................................................
Patch Set 3:
(2 comments)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34192/comment/d1b91702_425d377d
PS2, Line 559: if (!pch->confirm)
I'm not really following.
> The problem is that we can not really chose here if we want a confirmation or not.
Who is "we" here? in which piece of software?
> The Ericsson RBS BTS will send the IMM ASS send message whether we want it or not. And this will generate a confirmation.
What do you mean here with "will send a IMM ASS whether we want it or not"?
> As far as I understand, you would use the pch->confirm flag to decide if the message has to go through pcu_rx_rr_imm_ass_pch() or pcu_rx_rr_paging_pch()
why do you want to do that? pch->confim is to know whether the PCUIF primitive has to be confirmed. You look the message type to know whether it's an ImmAss or a Paging.
https://gerrit.osmocom.org/c/osmo-bsc/+/34192/comment/068d3d07_a2fcde4f
PS2, Line 566: if (pch->confirm)
> In theory osmo-bts would be capable to confirm paging MAC blocks. […]
Just send the confirmation when you transmit the related block over RSL. We can later on add some extra parameters in osmo-pcu to incresae the delay, get estimation delays from the other PCUIF side, etc.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34192
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I3d2842626b7e8325860ea3160c7d900d39e953a0
Gerrit-Change-Number: 34192
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 29 Aug 2023 09:57:18 +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 posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/34059 )
Change subject: pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH
......................................................................
Patch Set 2:
This change is ready for review.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34059
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I29858fa20ad8bd0aefe81a5c40ad77a2559a8c10
Gerrit-Change-Number: 34059
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 29 Aug 2023 09:56:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: osmith, pespin.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34192 )
Change subject: pcuif_proto: check confirm flag in struct gsm_pcu_if_pch
......................................................................
Patch Set 3:
(1 comment)
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34192/comment/13e623d3_978cef5d
PS1, Line 552: if (pch->confirmed_imm_ass)
> @osmith this should not be a problem since there's a PCUIF version bump 10->11. […]
I don't think that it is required to mention this, we do changes to PCUIF all the time. We take care that we do not break compatibility (Depends) and expect that users always use the current master and do not mix different states from different days. If someone uses nightly, nothing should be broken.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34192
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I3d2842626b7e8325860ea3160c7d900d39e953a0
Gerrit-Change-Number: 34192
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 29 Aug 2023 09:54:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34230 )
Change subject: DIAMETER: Origin-State-Id in DWR and DWA messages is optional
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34230
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I02e6f97a308f1752711aafed39c1d7534a382c70
Gerrit-Change-Number: 34230
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 29 Aug 2023 09:51:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment