Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/27722 )
Change subject: osmo-bts-trx: rx_tchh_fn(): use a lookup table for FACCH/H
......................................................................
Patch Set 1:
(1 comment)
File src/osmo-bts-trx/sched_lchan_tchh.c:
https://gerrit.osmocom.org/c/osmo-bts/+/27722/comment/d3b98858_1e14fdb5
PS1, Line 85: int fn_is_odd = (((bi->fn + 26 - 10) % 26) >> 2) & 1;
> Then having a static inline function fn_is_facch which does "!sched_tchh_ul_facch_map[bi->fn % 26]" […]
I am not a big fan of this kind of functions, sorry. This statement:
sched_tchh_ul_facch_map[bi->fn % 26]
is self-explaining, IMO. The negation is required because gsm0503_tch_hr_decode() involves double-negation: it would try to decode the buffer as FACCH/H if its argument 'odd' is false.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/27722
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I7c4e3befdd33bef4d89a04a4c7cb1a2d4078c156
Gerrit-Change-Number: 27722
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 11 Apr 2022 13:18:33 +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/osmo-bts/+/27722 )
Change subject: osmo-bts-trx: rx_tchh_fn(): use a lookup table for FACCH/H
......................................................................
Patch Set 1:
(1 comment)
File src/osmo-bts-trx/sched_lchan_tchh.c:
https://gerrit.osmocom.org/c/osmo-bts/+/27722/comment/51ac4a16_9f609060
PS1, Line 85: int fn_is_odd = (((bi->fn + 26 - 10) % 26) >> 2) & 1;
> Yes, this naming looks weird to me too. 'fn_is_facch' would have been a lot cleaner. […]
Then having a static inline function fn_is_facch which does "!sched_tchh_ul_facch_map[bi->fn % 26]" would be helpful to understand better the code imho. I'd welcome it at least.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/27722
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I7c4e3befdd33bef4d89a04a4c7cb1a2d4078c156
Gerrit-Change-Number: 27722
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
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-Comment-Date: Mon, 11 Apr 2022 12:55:01 +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/osmo-bts/+/27722 )
Change subject: osmo-bts-trx: rx_tchh_fn(): use a lookup table for FACCH/H
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
File src/osmo-bts-trx/sched_lchan_tchh.c:
https://gerrit.osmocom.org/c/osmo-bts/+/27722/comment/c2ac8cf9_c77c14ba
PS1, Line 85: int fn_is_odd = (((bi->fn + 26 - 10) % 26) >> 2) & 1;
> iiuc the "fn_is_odd" before was actually wrong, at least in the naming, since it's no only checking […]
Yes, this naming looks weird to me too. 'fn_is_facch' would have been a lot cleaner. But in reality this flag is true when no FACCH is possible at the given TDMA FN, so then 'fn_is_not_facch'. So yeah, with a lookup table it's a lot cleaner.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/27722
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I7c4e3befdd33bef4d89a04a4c7cb1a2d4078c156
Gerrit-Change-Number: 27722
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 11 Apr 2022 12:47:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: iedemam, neels.
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 (#21).
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, 91 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/81/27081/21
--
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: 21
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: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: osmith.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27711 )
Change subject: Make subscr_conn_get_next_id RAN specific
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27711
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc-nat
Gerrit-Branch: master
Gerrit-Change-Id: I77b0ef33f94c401d24f38eb8d79e1007234e0ab4
Gerrit-Change-Number: 27711
Gerrit-PatchSet: 3
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 11 Apr 2022 12:25:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: neels.
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 20:
(3 comments)
File src/osmo-bsc/bts_trx_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/3fad8e64_c0f35692
PS19, Line 576: VTY_NEWLINE);
> this will print nonsense values for lchan types other than SDCCH and TCH, unless we remove those con […]
Done
File src/osmo-bsc/lchan_fsm.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/b5d3897c_8eafe655
PS19, Line 225: if (lchan->active_cc.cfg.rate_ctr) {
> let's remove this condition: always (re)start the activity time_cc, also for lchan types other than […]
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/27081/comment/7b7bef8f_d8649b0d
PS19, Line 560: if (lchan->active_cc.cfg.rate_ctr) {
> remove this condition, always set the flag. Does not matter wheter a rate_ctr is set.
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: 20
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-Comment-Date: Mon, 11 Apr 2022 12:19:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment