Attention is currently required from: laforge.
Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/30633 )
Change subject: logging: add log level cache
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
> ping?
Pong.
There is nothing to resolve here as far as I am concerned, the comment above the single line that apparently warrants lengthy discussion clearly says: /* init with invalid high level */ - and that's it.
--
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: 9
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:44:01 +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: 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/b16a02ce_f3d699da
PS1, Line 1779: GSM48_IE_CIP_MODE_SET
> But then you'll break stuff for people building older osmo-bsc against recent libosmocore...
1- Stuff is already broken. It's not possible to encode and decode those IEs properly in a sane way.
2- That what libversion is for. We make them depend on newer libversion in master and next releases.
--
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:43:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: 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/dabd7bed_5de57e5e
PS1, Line 1779: GSM48_IE_CIP_MODE_SET
> Then we need to fix those projects and make them depend on libosmocore > 1.8. […]
But then you'll break stuff for people building older osmo-bsc against recent libosmocore...
--
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: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 22 Mar 2023 12:41:13 +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: 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:
(2 comments)
File include/osmocom/gsm/protocol/gsm_04_08.h:
https://gerrit.osmocom.org/c/libosmocore/+/32014/comment/92651c09_4a8210bf
PS1, Line 1779: GSM48_IE_CIP_MODE_SET
> In osmo-bsc.git I see this IEI used as follows: […]
Then we need to fix those projects and make them depend on libosmocore > 1.8.0, because with current state anyway encoding/decoding of those IEs is broken.
File src/gsm/tlv_parser.c:
https://gerrit.osmocom.org/c/libosmocore/+/32014/comment/abfd622d_5af11b5a
PS1, Line 244: & 0xf0
> Applying the mask in not needed anymore, given that you shift `uint8_t`?
Ack
--
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:39:53 +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: 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: Code-Review-1
(2 comments)
File include/osmocom/gsm/protocol/gsm_04_08.h:
https://gerrit.osmocom.org/c/libosmocore/+/32014/comment/89fce96b_70e7a455
PS1, Line 1779: GSM48_IE_CIP_MODE_SET
In osmo-bsc.git I see this IEI used as follows:
msgb_v_put(msg, GSM48_IE_CIP_MODE_SET | (cms & 0x0f));
so your patch is fixing libosmocore, but breaking osmo-bsc and potentially other projects.
File src/gsm/tlv_parser.c:
https://gerrit.osmocom.org/c/libosmocore/+/32014/comment/50066660_7cd50211
PS1, Line 244: & 0xf0
Applying the mask in not needed anymore, given that you shift `uint8_t`?
--
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: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 22 Mar 2023 12:36:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
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 src/gsm/tlv_parser.c:
https://gerrit.osmocom.org/c/libosmocore/+/32014/comment/5ddc5d3f_77bcb170
PS1, Line 157: msgb_v_put(msg, (tag << 4) | (val[0] & 0xf));
here's the shift in the encoding path, as expected.
--
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-Comment-Date: Wed, 22 Mar 2023 12:28:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/32014 )
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/tlv_parser.c
2 files changed, 25 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/14/32014/1
diff --git a/include/osmocom/gsm/protocol/gsm_04_08.h b/include/osmocom/gsm/protocol/gsm_04_08.h
index 66aa135..07ce09a 100644
--- a/include/osmocom/gsm/protocol/gsm_04_08.h
+++ b/include/osmocom/gsm/protocol/gsm_04_08.h
@@ -1734,6 +1734,10 @@
#define GSM48_IE_FRQSHORT_AFTER 0x02
#define GSM48_IE_MUL_RATE_CFG 0x03 /* 10.5.2.21aa */
#define GSM48_IE_FREQ_L_AFTER 0x05
+#define GSM48_IE_GROUP_CIP_SEQ 0x08
+#define GSM48_IE_CIP_MODE_SET 0x09
+#define GSM48_IE_GPRS_RESUMPT 0xc0
+#define GSM48_IE_SYNC_IND 0x0d
#define GSM48_IE_MSLOT_DESC 0x10
#define GSM48_IE_CHANMODE_2 0x11
#define GSM48_IE_FRQSHORT_BEFORE 0x12
@@ -1775,20 +1779,16 @@
#define GSM48_IE_START_TIME 0x7c
#define GSM48_IE_INDIVIDUAL_PRIORITIES 0x7c /* 44.018 Section 9.1.7 */
#define GSM48_IE_TIMING_ADVANCE 0x7d
-#define GSM48_IE_GROUP_CIP_SEQ 0x80
-#define GSM48_IE_CIP_MODE_SET 0x90
-#define GSM48_IE_GPRS_RESUMPT 0xc0
-#define GSM48_IE_SYNC_IND 0xd0
/* System Information 4 (types are equal IEs above) */
#define GSM48_IE_CBCH_CHAN_DESC 0x64
#define GSM48_IE_CBCH_MOB_AL 0x72
/* Additional MM elements */
+#define GSM48_IE_PRIORITY_LEV 0x08
#define GSM48_IE_LOCATION_AREA 0x13
#define GSM48_IE_AUTN 0x20
#define GSM48_IE_AUTH_RES_EXT 0x21
#define GSM48_IE_AUTS 0x22
-#define GSM48_IE_PRIORITY_LEV 0x80
#define GSM48_IE_FOLLOW_ON_PROC 0xa1
#define GSM48_IE_CTS_PERMISSION 0xa2
diff --git a/src/gsm/tlv_parser.c b/src/gsm/tlv_parser.c
index de76688..f60da8c 100644
--- a/src/gsm/tlv_parser.c
+++ b/src/gsm/tlv_parser.c
@@ -241,7 +241,7 @@
*o_tag = tag;
/* single octet TV IE */
- if (def->def[tag & 0xf0].type == TLV_TYPE_SINGLE_TV) {
+ if (def->def[(tag & 0xf0) >> 4].type == TLV_TYPE_SINGLE_TV) {
*o_tag = tag & 0xf0;
*o_val = buf;
*o_len = 1;
--
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-MessageType: newchange