Attention is currently required from: laforge.
wbokslag has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-tetra/+/33943 )
Change subject: Added first steps towards tetra crypto support
......................................................................
Patch Set 4:
(9 comments)
Patchset:
PS4:
Thanks for the remarks, I simplified some stuff, and am waiting for your opinion on merging tetra_crypto_state into tetra_mac_state while keeping tetra_crypto_database a global.
File src/crypto/tetra_crypto.h:
https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/a653f2ce_7e63b875
PS3, Line 47: mcc
> it's been a long time since I read TETRA specs. […]
ETSI EN 300 392-1 V1.4.1 Clause 7.2.5 states:
The MNC is coded in binary using 14 bits. The range of MNC shall be limited to four decimal digits i.e. the maximum value is 9 999 decimal or 270F in hexadecimal format.
This does not in any way seem to be a character based representation so I think we're good here.
https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/01b4abdd_13b0be75
PS3, Line 54: int is_gssi_range;
: int start;
: int end;
> are there situations where those integers really can be negative?
Since I'm not really using address ranges I've decided to remove them from this patch. We can re-introduce those when needed.
https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/82e93cb4_6ba25440
PS3, Line 61: mnc
> is there a reeason to use int for mnc/mcc in 'struct network_info' but uint32_t here?
You're right, I changed them to uint32_t in the network info struct
https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/7c6b69cd_98946b5a
PS3, Line 71: int num_keys;
> I guess all of these could be unsigned as they simply count the number of list elements? Using the […]
Done
https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/07b52ca7_952ac2f5
PS3, Line 93: const char *tetra_get_key_type_name(int key_type);
> didn't you define enums above for those types? Wondering why the argument type is int instead of the […]
Done
File src/crypto/tetra_crypto.c:
https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/9b060a51_c0b8d002
PS3, Line 14: struct tet
> I'd generally suggest to a void global variables for state that might exist multiple times (think of […]
I introduce two new global variables. How would you feel about merging the tetra_crypto_state into the tetra_mac_state? Since the MAC handles encryption, it would not be out of place.
The second question then remains the tetra_crypto_database which serves as a database that stores user-supplied network specifications and key material. I think this neither fits in the tetra_mac_state, nor does it seem appropriate to pass it around through all the functions that are involved.
To me, it seems okay to make the tetra_crypto_database a global since it is reasonable to assume one would want to use it across multiple tetra_mac_states. Agree?
https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/92efdc1d_85f4d289
PS3, Line 82: calloc
> we generally use talloc in the existing osmo-tetra code base, and it would make sense to not mix dif […]
Done
https://gerrit.osmocom.org/c/osmo-tetra/+/33943/comment/b5ee42d5_5893be8a
PS3, Line 96: k->mcc, k->mnc, k->key_type);
> we have something like OSMO_STRBUF_APPEND() in osmocom/core/utils. […]
I'll leave it as-is for now
--
To view, visit https://gerrit.osmocom.org/c/osmo-tetra/+/33943
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-tetra
Gerrit-Branch: master
Gerrit-Change-Id: I63bc712630ae5dbaa049c129d456f7aef5bda863
Gerrit-Change-Number: 33943
Gerrit-PatchSet: 4
Gerrit-Owner: wbokslag <w.bokslag(a)midnightblue.nl>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Thu, 27 Jul 2023 16:16:57 +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: wbokslag.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-tetra/+/33943
to look at the new patch set (#4).
Change subject: Added first steps towards tetra crypto support
......................................................................
Added first steps towards tetra crypto support
A crypto folder has been added containing only a single c and h file at the moment. These files contain structs and high level functionality pertaining to TETRA crypto support which can be added in future patches.
Change-Id: I63bc712630ae5dbaa049c129d456f7aef5bda863
---
M src/Makefile
A src/crypto/tetra_crypto.c
A src/crypto/tetra_crypto.h
3 files changed, 355 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-tetra refs/changes/43/33943/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-tetra/+/33943
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-tetra
Gerrit-Branch: master
Gerrit-Change-Id: I63bc712630ae5dbaa049c129d456f7aef5bda863
Gerrit-Change-Number: 33943
Gerrit-PatchSet: 4
Gerrit-Owner: wbokslag <w.bokslag(a)midnightblue.nl>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: wbokslag <w.bokslag(a)midnightblue.nl>
Gerrit-MessageType: newpatchset
Attention is currently required from: Hoernchen, pespin.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-trx/+/33944
to look at the new patch set (#6).
Change subject: ms: restructure the va code to add rach support
......................................................................
ms: restructure the va code to add rach support
This commit adds support for rach bursts to the viterbi equalizer, which
is currently only being used by the ms side, so the equalizer can be used
by the osmo-trx network side, too. The difference is that rach bursts are
shorter than any other burst type (due to unknown TA) and start with
diffrent tail bits.
Change-Id: I4a5cedc8c9a3289c75ce7b914eac286e601ebed0
---
M Transceiver52M/grgsm_vitac/constants.h
M Transceiver52M/grgsm_vitac/grgsm_vitac.cpp
M Transceiver52M/grgsm_vitac/grgsm_vitac.h
M Transceiver52M/ms/ms_rx_lower.cpp
M Transceiver52M/ms/ms_upper.cpp
5 files changed, 131 insertions(+), 96 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/44/33944/6
--
To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/33944
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Change-Id: I4a5cedc8c9a3289c75ce7b914eac286e601ebed0
Gerrit-Change-Number: 33944
Gerrit-PatchSet: 6
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Hoernchen, pespin.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-trx/+/33944
to look at the new patch set (#5).
Change subject: ms: restructure the va code to add rach support
......................................................................
ms: restructure the va code to add rach support
This commit adds support for rach bursts to the viterbi equalizer, which is currently only being used by the ms side, so the equalizer can be used by the osmo-trx network side, too. The difference is that rach bursts are shorter than any other burst type (due to unknown TA) and start with diffrent tail bits.
Change-Id: I4a5cedc8c9a3289c75ce7b914eac286e601ebed0
---
M Transceiver52M/grgsm_vitac/constants.h
M Transceiver52M/grgsm_vitac/grgsm_vitac.cpp
M Transceiver52M/grgsm_vitac/grgsm_vitac.h
M Transceiver52M/ms/ms_rx_lower.cpp
M Transceiver52M/ms/ms_upper.cpp
5 files changed, 127 insertions(+), 96 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/44/33944/5
--
To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/33944
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Change-Id: I4a5cedc8c9a3289c75ce7b914eac286e601ebed0
Gerrit-Change-Number: 33944
Gerrit-PatchSet: 5
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, fixeria.
Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/33926 )
Change subject: ms: drop the tx burst padding
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> not really the nicest way of a commit log message, but well. […]
well it has been useless for quite some time but there was no reason to change it because everything was working and the proper time to touch it is while working on burst related code, but I don't feel like writing a long story for a one character change (and a two line fix thanks to werror for unused vars which was already annoying enough).
--
To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/33926
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Change-Id: Ied5c3ab5dde975e11b0ef6d9cbc86be19173c4e8
Gerrit-Change-Number: 33926
Gerrit-PatchSet: 3
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 27 Jul 2023 14:50:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment