Attention is currently required from: dexter.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/24146 )
Change subject: cards: move methods read_aids and select_adf_by_aid to class UsimCard
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> @dexter: is this patch still needed? if yes, please update. If not, please abandon.
ping?
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/24146
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I71667b7b38c930b4e0409fe2ba2786e1db9cf9a3
Gerrit-Change-Number: 24146
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 22 Mar 2023 13:20:55 +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: laforge, pespin.
Hello Jenkins Builder, laforge, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/32014
to look at the new patch set (#2).
Change subject: Fix parsing of TLV_TYPE_SINGLE_TV
......................................................................
Fix parsing of TLV_TYPE_SINGLE_TV
The decoding path of TLV_TYPE_SINGLE_TV is wrong, since it is not
shifting right the tag before using it. On the other hand, the encoding
path (tlv_encode_one) is doing that, so it is clear there's a bug.
It seems that in order to workaround the bug some IEs in gsm_04_08.h (TS
24.008 and TS 44.018) were defined incorrectly (eg 0x80) while the spec
clearly assigns eg. "8" to it, and makes sure no full byte IEI collides.
Some other IEIs like GSM48_IE_GMM_CIPH_CKSN which are also of the same
type were already correctly defined as 0x08.
Change-Id: I799e35dc8d4d153fa63bf50563a5482cdf4de2d7
---
M include/osmocom/gsm/protocol/gsm_04_08.h
M src/gsm/gsm48.c
M src/gsm/tlv_parser.c
3 files changed, 37 insertions(+), 11 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/14/32014/2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32014
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I799e35dc8d4d153fa63bf50563a5482cdf4de2d7
Gerrit-Change-Number: 32014
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32014 )
Change subject: Fix parsing of TLV_TYPE_SINGLE_TV
......................................................................
Patch Set 1:
(1 comment)
File include/osmocom/gsm/protocol/gsm_04_08.h:
https://gerrit.osmocom.org/c/libosmocore/+/32014/comment/8d0c93de_c9d67ffb
PS1, Line 1779: GSM48_IE_CIP_MODE_SET
> 1- Stuff is already broken. It's not possible to encode and decode those IEs properly in a sane way.
Let's clarify: the libosmocore's TLV codec is handling `TLV_TYPE_SINGLE_TV` incorrectly, yes. But not all projects are using it, osmo-bsc.git does the encoding of some PDUs manually (by using the msgb API), and it's doing it correctly. So not everything is broken.
> 2- That what libversion is for. We make them depend on newer libversion in master and next releases.
Not sure if I am following you. AFAIU, you're talking about fixing osmo-bsc.git and making it depend on more recent fixed libosmocore. But I am talking about compiling old osmo-bsc (e.g. while bisecting) against recent (to be fixed) libosmocore. If you change define values, old code doing manual encoding of these IEIs will start producing wrong encoding results. To me it's even worse than breaking the API and breaking compilation for old code, because it will cause problems at run-time.
IMO, we should rather keep the old GSM48_IE_ for backwards compatibility with projects using them like osmo-bsc does, define new ones with the correct values and use these new ones in `gsm48_rr_att_tlvdef[]`.
> I know it's a pain changing those and breaking the ABI, but otherwise we are just mtairning a broken system and keep adding workaround everywhere.
Well, yes, it's a pain. But I don't remember we ever changed our policy about keeping backwards compatibility and started breaking the API here and there? It's similar to deprecating functions: just add a comment marking old defines as deprecated, add new ones with the correct values, and use these new ones everywhere.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32014
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I799e35dc8d4d153fa63bf50563a5482cdf4de2d7
Gerrit-Change-Number: 32014
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 22 Mar 2023 13:00:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge.
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30633 )
Change subject: logging: add log level cache
......................................................................
Patch Set 10:
(1 comment)
File src/core/logging.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5332):
https://gerrit.osmocom.org/c/libosmocore/+/30633/comment/f7787d70_d4083f28
PS10, Line 120: for (int i = 0; i < osmo_log_info->num_cat; i++) {
braces {} are not necessary for single statement blocks
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30633
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
Gerrit-Change-Number: 30633
Gerrit-PatchSet: 10
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 22 Mar 2023 12:55:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/30633
to look at the new patch set (#10).
Change subject: logging: add log level cache
......................................................................
logging: add log level cache
This ensures multithreaded logging attempts, in particular ones that do
nothing, do not hold the lock just for checking the level, which
interferes with other logging attempts.
Closes: OS#5818
Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
---
M include/osmocom/core/logging.h
M src/core/logging.c
M src/vty/logging_vty.c
M tests/ctrl/ctrl_test.c
4 files changed, 139 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/30633/10
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30633
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
Gerrit-Change-Number: 30633
Gerrit-PatchSet: 10
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: fixeria, pespin.
arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31534 )
Change subject: common: Make socket queue max. length configurable
......................................................................
Patch Set 6:
(1 comment)
File tests/osmo-bts.vty:
https://gerrit.osmocom.org/c/osmo-bts/+/31534/comment/cbf19bc5_a7801b06
PS6, Line 228: help
> The libosmovty's own command are intentionally masked with '...', please keep it this way. […]
I tried the ellipsis before, but for whatever reason, it didn't and doesn't work after my changes. I have diffed the outputs quoted from the testlog and the only difference between 'expected' and 'got' was the three dots. What is happening/what do I have to do to fix this?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31534
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Id6ba6e4eadce9ce82ef2407f4e28346e7fe4abfa
Gerrit-Change-Number: 31534
Gerrit-PatchSet: 6
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 22 Mar 2023 12:47:20 +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.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32014 )
Change subject: Fix parsing of TLV_TYPE_SINGLE_TV
......................................................................
Patch Set 1:
(1 comment)
File include/osmocom/gsm/protocol/gsm_04_08.h:
https://gerrit.osmocom.org/c/libosmocore/+/32014/comment/8bbfa772_a6d92003
PS1, Line 1779: GSM48_IE_CIP_MODE_SET
> 1- Stuff is already broken. It's not possible to encode and decode those IEs properly in a sane way. […]
I know it's a pain changing those and breaking the ABI, but otherwise we are just mtairning a broken system and keep adding workaround everywhere.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32014
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I799e35dc8d4d153fa63bf50563a5482cdf4de2d7
Gerrit-Change-Number: 32014
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 22 Mar 2023 12:44:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment