May 2023 - gerrit-log - lists.osmocom.org

[S] Change in libosmocore[master]: gsm630: make osmo_hr_check_sid compatible with RFC 5993

by fixeria

Attention is currently required from: falconia, dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32740 ) Change subject: gsm630: make osmo_hr_check_sid compatible with RFC 5993 ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I am also somewhat uncertain if we really need this flexibility. > Wouldn't it be much cleaner to check the RTP encoding format ... FYI: osmo-bts-trx already does this in some places, for instance: https://cgit.osmocom.org/osmo-bts/tree/src/osmo-bts-trx/sched_lchan_tchh.c?… -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32740 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I25728299b757fbc87dd1b3f5adaec9b8b240c5d1 Gerrit-Change-Number: 32740 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: falconia <falcon(a)freecalypso.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 May 2023 14:24:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: falconia <falcon(a)freecalypso.org> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in libosmocore[master]: gsm630: make osmo_hr_check_sid compatible with RFC 5993

by falconia

Attention is currently required from: fixeria, dexter. falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32740 ) Change subject: gsm630: make osmo_hr_check_sid compatible with RFC 5993 ...................................................................... Patch Set 1: (2 comments) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/32740/comment/4718014e_019ba0b1 PS1, Line 7: gsm630: make osmo_hr_check_sid compatible with RFC 5993 I assume you meant gsm620 here. Patchset: PS1: I am not certain at all that the right solution to OS#5688 is to propagate the knowledge of two different RTP formats deep into the innards of our code. Wouldn't it be much cleaner to check the RTP encoding format at the very beginning of RTP input processing in OsmoBTS (or in OsmoMGW feeding E1 Abis), in the same place where calls to osmo_{fr,efr}_sid_preen() have been inserted recently, strip the extraneous RFC 5993 ToC octet if one is present, and then work only with the TS 101 318 format internally throughout our code base? And in the other direction, if the desire is to emit RFC 5993 format to the outside world (configurable per vty, ideally), likewise work in TS 101 318 format internally, but prepend the extra ToC octet just before RTP output. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32740 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I25728299b757fbc87dd1b3f5adaec9b8b240c5d1 Gerrit-Change-Number: 32740 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 May 2023 14:19:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in osmo-bts[master]: l1sap: cosmetic: rename payload_len to rtp_pl_len

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32752 ) Change subject: l1sap: cosmetic: rename payload_len to rtp_pl_len ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32752 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8a0e0357aab2a78e25811f66b1b870e8c6ebffe9 Gerrit-Change-Number: 32752 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, 17 May 2023 14:12:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload

by pespin

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32630 ) Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/32630/comment/43370faf_dc1cddbf PS3, Line 1949: static struct msgb *rtppayload_convert_hr(const uint8_t *rtp_pl, uint8_t rtp_pl_len, struct gsm_lchan *lchan) This looks much clearer now, thanks! Maybe add a comment somewhere that packets with rtp_pl_len different than GSM_HR_BYTES_RTP_TS101318 or GSM_HR_BYTES_RTP_RFC5993 have already been filtered out in previous code paths here. Maybe even use a switch (rtp_pl_len) and OSMO_ASSERT(0) in default branch. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32630 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I9419b40c1171876879d41aba4f51c93e8ef5673c Gerrit-Change-Number: 32630 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: Wed, 17 May 2023 14:12:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in osmo-bts[master]: l1sap: Drop invalid HR GSM payload

by pespin

Attention is currently required from: jolly, neels, laforge, fixeria, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31417 ) Change subject: l1sap: Drop invalid HR GSM payload ...................................................................... Patch Set 11: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Gerrit-Change-Number: 31417 Gerrit-PatchSet: 11 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 May 2023 14:09:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in osmo-bts[master]: l1sap: cosmetic: rename payload_len to rtp_pl_len

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/32752 ) Change subject: l1sap: cosmetic: rename payload_len to rtp_pl_len ...................................................................... l1sap: cosmetic: rename payload_len to rtp_pl_len The function signature of rtppayload_is_octet_aligned has a parameter rtp_pl for the payload and a parameter payload_len for the length of the payload, while other functions use rtp_pl and rtp_pl_len. Change-Id: I8a0e0357aab2a78e25811f66b1b870e8c6ebffe9 --- M src/common/l1sap.c 1 file changed, 15 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/52/32752/1 diff --git a/src/common/l1sap.c b/src/common/l1sap.c index 3939294..cff82cf 100644 --- a/src/common/l1sap.c +++ b/src/common/l1sap.c @@ -1216,7 +1216,7 @@ return 1; } -static bool rtppayload_is_octet_aligned(const uint8_t *rtp_pl, uint8_t payload_len) +static bool rtppayload_is_octet_aligned(const uint8_t *rtp_pl, uint8_t rtp_pl_len) { /* * Logic: If 1st bit padding is not zero, packet is either: @@ -1237,7 +1237,7 @@ #define AMR_PADDING1(rtp_pl) (rtp_pl[0] & 0x0f) #define AMR_PADDING2(rtp_pl) (rtp_pl[1] & 0x03) - if (payload_len < 2 || AMR_PADDING1(rtp_pl) || AMR_PADDING2(rtp_pl)) + if (rtp_pl_len < 2 || AMR_PADDING1(rtp_pl) || AMR_PADDING2(rtp_pl)) return false; return true; -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32752 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8a0e0357aab2a78e25811f66b1b870e8c6ebffe9 Gerrit-Change-Number: 32752 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload

by dexter

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/32630 to look at the new patch set (#3). Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... l1sap: Accept RFC5993 and TS 101.318 HR GSM payload Unfotunately there are two different RTP formats for HR GSM specified and it is unclear which should be used with GSM networks. Also each BTS model may support only either one or the other format. Let's use the BTS feature flags to determine which format the BTS model supports and then let's use this information to convert incoming RTP packets into the supported format. Doing so we will make sure that always both formats are accepted. Related: OS#5688 Change-Id: I9419b40c1171876879d41aba4f51c93e8ef5673c --- M include/osmo-bts/bts.h M src/common/bts.c M src/common/l1sap.c M src/osmo-bts-lc15/main.c M src/osmo-bts-oc2g/main.c M src/osmo-bts-sysmo/main.c M src/osmo-bts-trx/main.c M src/osmo-bts-virtual/main.c 8 files changed, 87 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/30/32630/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32630 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I9419b40c1171876879d41aba4f51c93e8ef5673c Gerrit-Change-Number: 32630 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 1 month

1
0
0 0

[S] Change in osmo-bts[master]: l1sap: Drop invalid HR GSM payload

by dexter

Attention is currently required from: jolly, neels, laforge, pespin, fixeria. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31417 ) Change subject: l1sap: Drop invalid HR GSM payload ...................................................................... Patch Set 11: (2 comments) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/a0e12409_def8fc1b PS8, Line 1969: break; > This turns out even more ugly due to the l1sap_msgb_alloc and the fallthrough. […] Done File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/31417/comment/c1ec497f_d6409f4d PS10, Line 1284: /* BTS flags specify that ETSI TS 101.318 is not understood by lower layers */ > In the next patch you are converting between formats, so these 2 block checks below returning false […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Gerrit-Change-Number: 31417 Gerrit-PatchSet: 11 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 May 2023 14:06:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

[S] Change in osmo-bts[master]: l1sap: Drop invalid HR GSM payload

by dexter

Attention is currently required from: jolly, neels, laforge, pespin, fixeria. Hello Jenkins Builder, jolly, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 to look at the new patch set (#11). Change subject: l1sap: Drop invalid HR GSM payload ...................................................................... l1sap: Drop invalid HR GSM payload HR GSM payload is currently not validated. Let's add an rtppayload_validate_hr function that checks the incoming RTP payload by its length. Two lengts are valid, depending on which of the two HR GSM RTP formats (RFC 5993 or TS 101 318) are used. Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Related: OS#5688 --- M src/common/l1sap.c 1 file changed, 26 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/17/31417/11 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31417 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I453562da412fde5b928bd2b588129c58ec8e2a7e Gerrit-Change-Number: 31417 Gerrit-PatchSet: 11 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 1 month

1
0
0 0

[M] Change in osmo-bts[master]: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload

by dexter

Attention is currently required from: pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32630 ) Change subject: l1sap: Accept RFC5993 and TS 101.318 HR GSM payload ...................................................................... Patch Set 3: (2 comments) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/32630/comment/bece4b45_1b5af44f PS2, Line 1963: if (OSMO_UNLIKELY(rfc5993 && !ts101318 && rtp_pl_len == GSM_HR_BYTES_RTP_TS101318)) { > I'd really appreciate a more "step by step logic procedure" here, as in how one would thing about it […] Done https://gerrit.osmocom.org/c/osmo-bts/+/32630/comment/9d91398d_c43009d8 PS2, Line 1970: } else if (OSMO_UNLIKELY(ts101318 && !rfc5993 && rtp_pl_len == GSM_HR_BYTES_RTP_RFC5993)) { > else not needed due to break immediatelly above. Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32630 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I9419b40c1171876879d41aba4f51c93e8ef5673c Gerrit-Change-Number: 32630 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 17 May 2023 14:06:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 1 month

1
0
0 0

2025

2024

2023

2022

gerrit-log May 2023