Attention is currently required from: fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/27520 )
Change subject: osmo-bts-trx: rx_tchh_fn(): mark valid SID frames as such
......................................................................
Patch Set 1:
(1 comment)
File src/osmo-bts-trx/sched_lchan_tchh.c:
https://gerrit.osmocom.org/c/osmo-bts/+/27520/comment/2f65a9a2_c3839941
PS1, Line 154: tch_data[0] = (0x02 << 4);
why aren't you using enum osmo_amr_type here? Furthermore, 2 seems to be AMR_5_90, not sure why that one specifically?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/27520
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I09cec984bb60c754908126acf0300a2cc602485c
Gerrit-Change-Number: 27520
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 16 Mar 2022 11:59:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin, dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/27430 )
Change subject: osmo-bts-trx: rx_tchh_fn(): fix HR SID detection (wrong offset)
......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS4:
> to summarize: This change is correct (check only the HR codec payloads and not the TOC). […]
Updated the commit message, thanks for your references and explanation! Regarding your proposal to match SID frames in gsm0503_tch_hr_decode(), I am not sure if it's safe to change behavior of this library function. If you think it's worth it, please let me know. For now I submitted: https://gerrit.osmocom.org/c/osmo-bts/+/27520.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/27430
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ie5fa776dcb2b2203a97aed56ecbf2450af7d87c1
Gerrit-Change-Number: 27430
Gerrit-PatchSet: 6
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 16 Mar 2022 10:12:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/27520 )
Change subject: osmo-bts-trx: rx_tchh_fn(): mark valid SID frames as such
......................................................................
osmo-bts-trx: rx_tchh_fn(): mark valid SID frames as such
Set the FT (Frame Type) in the ToC (Type-of-Content) section as
defined in section 5.2 of RFC 5993. Before this change SID frames
had FT = 000 (Good Speech frame), because gsm0503_tch_hr_decode()
does not distinguish between speech and SID frames internally.
Change-Id: I09cec984bb60c754908126acf0300a2cc602485c
Related: SYS#5853
---
M src/osmo-bts-trx/sched_lchan_tchh.c
1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/20/27520/1
diff --git a/src/osmo-bts-trx/sched_lchan_tchh.c b/src/osmo-bts-trx/sched_lchan_tchh.c
index d2ee4f0..5d2c12c 100644
--- a/src/osmo-bts-trx/sched_lchan_tchh.c
+++ b/src/osmo-bts-trx/sched_lchan_tchh.c
@@ -149,7 +149,10 @@
fn_is_odd, &n_errors, &n_bits_total);
if (rc == (GSM_HR_BYTES + 1)) { /* only for valid *speech* frames */
/* gsm0503_tch_hr_decode() prepends a ToC octet (see RFC5993), skip it */
- lchan_set_marker(osmo_hr_check_sid(&tch_data[1], GSM_HR_BYTES), lchan); /* DTXu */
+ bool is_sid = osmo_hr_check_sid(&tch_data[1], GSM_HR_BYTES);
+ if (is_sid) /* Mark SID frames as such: F = 0, FT = 010 */
+ tch_data[0] = (0x02 << 4);
+ lchan_set_marker(is_sid, lchan); /* DTXu */
}
break;
case GSM48_CMODE_SPEECH_AMR: /* AMR */
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/27520
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I09cec984bb60c754908126acf0300a2cc602485c
Gerrit-Change-Number: 27520
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: fixeria, pespin, dexter.
Hello Jenkins Builder, laforge, pespin, dexter,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/27430
to look at the new patch set (#6).
Change subject: osmo-bts-trx: rx_tchh_fn(): fix HR SID detection (wrong offset)
......................................................................
osmo-bts-trx: rx_tchh_fn(): fix HR SID detection (wrong offset)
According to RFC5993, which is referenced by 3GPP TS 48.103, the
complete RTP HR payload consists of a Table-of-Contents (ToC) byte
followed by the 14 bytes of the HR codec frame. See section 5.2.
gsm0503_tch_hr_decode() outputs frames in the RTP format with
length of 15 bytes (120 bits): 1 (ToC) + 14 (HR frame):
+-------------+-------------------------
| ToC section | HR codec frame ...
+-------------+-------------------------
osmo_hr_check_sid() expects a HR codec frame of 14 bytes (112 bits)
as the input, so we should skip the ToC section when calling it.
Change-Id: Ie5fa776dcb2b2203a97aed56ecbf2450af7d87c1
Related: SYS#5853
---
M src/osmo-bts-trx/sched_lchan_tchh.c
1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/30/27430/6
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/27430
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ie5fa776dcb2b2203a97aed56ecbf2450af7d87c1
Gerrit-Change-Number: 27430
Gerrit-PatchSet: 6
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: fixeria, pespin, dexter.
Hello Jenkins Builder, laforge, pespin, dexter,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bts/+/27430
to look at the new patch set (#5).
Change subject: osmo-bts-trx: rx_tchh_fn(): fix HR SID detection (wrong offset)
......................................................................
osmo-bts-trx: rx_tchh_fn(): fix HR SID detection (wrong offset)
According to ETSI TS 101 318, section 5.2, the GSM half rate codec
has frame length of 14 bytes (112 bits). According to RFC5993,
which is referenced by 3GPP TS 48.103, the complete RTP payload
consists of a payload Table-of-Contents (ToC) byte followed by
the 14 bytes of the HR codec frame. See section 5.2.
gsm0503_tch_hr_decode() outputs frames in the RTP format with
length of 15 bytes (120 bits): 1 (ToC) + 14 (HR frame):
+-------------+-------------------------
| ToC section | HR codec frame ...
+-------------+-------------------------
osmo_hr_check_sid() expects a HR codec frame of 14 bytes (112 bits)
as the input, so we should skip the ToC section when calling it.
Change-Id: Ie5fa776dcb2b2203a97aed56ecbf2450af7d87c1
Related: SYS#5853
---
M src/osmo-bts-trx/sched_lchan_tchh.c
1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/30/27430/5
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/27430
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ie5fa776dcb2b2203a97aed56ecbf2450af7d87c1
Gerrit-Change-Number: 27430
Gerrit-PatchSet: 5
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels, laforge, fixeria, pespin.
iedemam has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27081 )
Change subject: stats: new trackers for lchan life duration
......................................................................
Patch Set 16:
(10 comments)
Patchset:
PS16:
Hi all,
Thanks again for the feedback. I think we're getting much closer. This version still needs additional testing on my side to confirm that I can still accurately see both per-lchan instance duration and total duration per lchan type.
I will keep testing this and await further feedback.
Regards,
-Michael
File src/osmo-bsc/lchan_fsm.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/2c18280f_a249ed27
PS14, Line 465: struct rate_ctr *active_ms_rate_ctr;
> You could simply set it to NULL here and keep the code unchanged, but not critical.
Done
File src/osmo-bsc/lchan_fsm.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/961f4f23_da339e28
PS15, Line 208: if (lchan->active_ms.cfg.rate_ctr) {
> I understand now: the rate_ctr is set only for SDCCH and TCH type.
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/ea462e35_9934bd0a
PS15, Line 209: osmo_time_cc_cleanup(&lchan->active_ms);
> osmo_time_cc_cleanup() is not necessary, the forget_sum configuration does this implicitly
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/9a20d4f0_4e8b368d
PS15, Line 465: uint
> I like to use "uint" in private projects, but there i typedef it manually as unsigned int. […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/eb1c8fac_e76441ee
PS15, Line 478: counter_id = -1;
> could, yes, but totally fine here IMHO. […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/1679d6e6_4c389747
PS15, Line 479: switch (lchan->type) {
> the lchan->type changes during operation of osmo-bsc, it is not known at the time of lchan_fsm_alloc […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/6dcb2dda_6b64141c
PS15, Line 493: .gran_usec = 1*1000,
> Sorry for my earlier comment, I've come to realize what exactly I recommended by mentioning 1 ms gra […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/954d0daf_01205823
PS15, Line 498: .T_forget_sum = -18,
> timers X16, X17, X18 are defined to configure a different counter (the all_allocated counters). […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/9bd5ac4e_af62f137
PS15, Line 562: /* Stop the timekeeper, which triggers a report */
> the comment suggests that the report is triggered only now. […]
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27081
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I1b0670c47cb5e0b7776eda89d1e71545ba0e3347
Gerrit-Change-Number: 27081
Gerrit-PatchSet: 16
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 15 Mar 2022 16:50:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: iedemam, laforge, fixeria, pespin.
Hello Jenkins Builder, neels, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/27081
to look at the new patch set (#16).
Change subject: stats: new trackers for lchan life duration
......................................................................
stats: new trackers for lchan life duration
This patch adds two stats which track cummulative lchan lifetime by
type TCH and SDCCH. These new counters will accomplish two things:
1) Provide a glanceable way to see if lchan durations look healthy. When
examining a site, short-lived (<5s) and long-lived (>30s) TCH lchans
are difficult to tell apart. If we only see short-lived TCH lchans,
there is most likely an RF or signaling problem to investigate. This
new counter will expose channel ages in the VTY output
2) Provide a more accurate count for Erlangs per site. Currently, we
are basing Erlangs on active TCH channel counts per stats period. This
method skews high very quickly. Each active TCH in that period
translates into the full 10s of activity. This counter should improve
accuracy by two orders of magnitude.
Change-Id: I1b0670c47cb5e0b7776eda89d1e71545ba0e3347
---
M include/osmocom/bsc/bts.h
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/bts.c
M src/osmo-bsc/bts_trx_vty.c
M src/osmo-bsc/bts_vty.c
M src/osmo-bsc/gsm_data.c
M src/osmo-bsc/lchan_fsm.c
7 files changed, 90 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/81/27081/16
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27081
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I1b0670c47cb5e0b7776eda89d1e71545ba0e3347
Gerrit-Change-Number: 27081
Gerrit-PatchSet: 16
Gerrit-Owner: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: iedemam <michael(a)kapsulate.com>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27509 )
Change subject: struct gsm_encr: store alg_id in human-readable format
......................................................................
Patch Set 3:
(6 comments)
This change is ready for review.
Commit Message:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/ec43a57c_06cadbdd
PS2, Line 30: ciph_mod_set = (lchan->encr.alg_id << 1) | 1;
> well, in Cipher Mode Setting, it has to be (a5_n - 1) << 1
Indeed, thanks! Fixed in the next patchset revision.
File include/osmocom/bsc/gsm_data.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/fa25f947_37a5464b
PS2, Line 613: uint8_t alg_id;
> alg_id is named after the Algorithm Identifier IE. […]
Agreed, done.
File src/osmo-bsc/abis_rsl.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/c95648f8_54016850
PS2, Line 163: switch (*out++) {
> nitpick (ok, it's correct, but IMHO the code becomes far less head scratchy when we don't ++ inline […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/dade52f4_2485fa17
PS2, Line 663: if (lchan->encr.alg_id > 0) {
> (here is a place where i have to think twice, I thought it is incorrect at first; if the name were a […]
Done
File src/osmo-bsc/gsm_04_08_rr.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/f9bf4d9c_85b2a568
PS2, Line 596: _id
> "- 1" is missing now! 44.018 10.5.2. […]
Done
File src/osmo-bsc/gsm_08_08.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27509/comment/38a76e39_eebd4e6a
PS2, Line 88: chosen_encr
> also rename this to chosen_a5_n?
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27509
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ieb50c9a352cfa5481aebac2379e0a461663543ec
Gerrit-Change-Number: 27509
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 15 Mar 2022 15:45:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment