laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-abis/+/28063 )
Change subject: input/ipaccess: Don't start zero-ms timer on every write
......................................................................
input/ipaccess: Don't start zero-ms timer on every write
Historically, before November 15, 2010 when commit
d49fc5ae24fc9d44d2b284392ab619cc7a69a876 was merged to [back then]
OpenBSC, before libosmo-abis became a separate library, we used to
have a 10us delay timer for subsequent writes to ip.access nanoBTS 900.
ts: Reduce the delay to 0 for OML and RSL
This is possible after not sending more than one OML command that
requires an extra ACK. For the RSL line we do not need any speed
limitation.
Ever since the above-mentioned commit, the BSC always sets that timeout
to zero, which makes libosmo-abis start a zero-microsecond libosmocore timer,
which in turn will make libosmocore call select/poll with zero timeout, which
makes the kernel return immediately.
Why not remove the timer completely? Because ipaccess-config.c still specifies
a non-zero signaling delay, and we cannot be sure that this is really not
needed.
So let's alter the code to only start the timer if it's non-zero
Change-Id: I9c379364e7e6afce35fc6316392b5b33748980f7
---
M src/input/ipaccess.c
1 file changed, 10 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/63/28063/1
diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c
index dbb8b2e..79504d2 100644
--- a/src/input/ipaccess.c
+++ b/src/input/ipaccess.c
@@ -491,12 +491,12 @@
int ret;
e1i_ts = ipaccess_line_ts(bfd, line);
- osmo_fd_write_disable(bfd);
/* get the next msg for this timeslot */
msg = e1inp_tx_ts(e1i_ts, &sign_link);
if (!msg) {
/* no message after tx delay timer */
+ osmo_fd_write_disable(bfd);
return 0;
}
@@ -524,11 +524,15 @@
goto err;
}
- /* set tx delay timer for next event */
- osmo_timer_setup(&e1i_ts->sign.tx_timer, timeout_ts1_write, e1i_ts);
-
- /* Reducing this might break the nanoBTS 900 init. */
- osmo_timer_schedule(&e1i_ts->sign.tx_timer, 0, e1i_ts->sign.delay);
+ /* this is some ancient code that apparently exists to slow down writes towards
+ * some even more ancient nanoBTS 900 units. See git commit
+ * d49fc5ae24fc9d44d2b284392ab619cc7a69a876 of openbsc.git (now osmo-bsc.git) */
+ if (e1i_ts->sign.delay) {
+ osmo_fd_write_disable(bfd);
+ /* set tx delay timer for next event */
+ osmo_timer_setup(&e1i_ts->sign.tx_timer, timeout_ts1_write, e1i_ts);
+ osmo_timer_schedule(&e1i_ts->sign.tx_timer, 0, e1i_ts->sign.delay);
+ }
out:
msgb_free(msg);
--
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/28063
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I9c379364e7e6afce35fc6316392b5b33748980f7
Gerrit-Change-Number: 28063
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newchange
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28061 )
Change subject: fix fallout from: 'stats: new trackers for lchan life duration'
......................................................................
fix fallout from: 'stats: new trackers for lchan life duration'
In lchan_fsm_cleanup(), ensure that the time_cc timer is actually inactive
before deallocating. Do so via lchan_reset(), to also make sure the
timer is stopped in all other situations where the lchan is deactivated.
This fixes an infinite-loop deadlock as described in OS#5554:
- run BSC_Tests.TC_chan_act_ack_est_ind_noreply
- restart the BTS process after the test is done
- osmo-bsc enters infinite loop in osmo_timer_del()
The reason is that lchan_fsm_cleanup() fails to stop a running active_cc
timer upon lchan deallocation. TC_chan_act_ack_est_ind_noreply
incidentally terminates OML while the timer is still active.
Related: OS#5554
Change-Id: I901bb86a78d7d021c8efe751fd9d93e5956ac0e0
---
M src/osmo-bsc/lchan_fsm.c
1 file changed, 8 insertions(+), 0 deletions(-)
Approvals:
neels: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
osmith: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c
index d693189..6854465 100644
--- a/src/osmo-bsc/lchan_fsm.c
+++ b/src/osmo-bsc/lchan_fsm.c
@@ -528,6 +528,14 @@
lchan->mgw_endpoint_ci_bts = NULL;
}
+ /* Ensure that the osmo_timer in lchan->active_cc is stopped. This is particularly important for lchan FSM
+ * deallocation, so that the timer is no longer active when the lchan FSM instance gets discarded
+ * (lchan_fsm_cleanup() calls this function), see OS#5554.
+ *
+ * Besides that, it is also good to make sure the timer is stopped when the lchan resets, to avoid any false
+ * counts being accumulated, however obscure an error situation may be. */
+ osmo_time_cc_cleanup(&lchan->active_cc);
+
/* NUL all volatile state */
*lchan = (struct gsm_lchan){
.ts = lchan->ts,
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28061
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I901bb86a78d7d021c8efe751fd9d93e5956ac0e0
Gerrit-Change-Number: 28061
Gerrit-PatchSet: 3
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: iedemam, laforge.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28061 )
Change subject: fix fallout from: 'stats: new trackers for lchan life duration'
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
Patchset:
PS2:
> Well, that's an embarrassing result after such a long review process on my patch. […]
it's usually the test suite that catches obscurely appearing bugs.
it can help to run the ttcn3 tests locally, but things regularly get caught by jenkins.
i've added you to the group of reviewers.
Please skim this, it defines how we work with +1 and +2 votes:
https://osmocom.org/projects/cellular-infrastructure/wiki/Gerrit#Voting-Rul…
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28061
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I901bb86a78d7d021c8efe751fd9d93e5956ac0e0
Gerrit-Change-Number: 28061
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: iedemam <michael(a)kapsulate.com>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Fri, 06 May 2022 15:46:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: iedemam <michael(a)kapsulate.com>
Gerrit-MessageType: comment
Attention is currently required from: laforge.
pespin has uploaded a new patch set (#2) to the change originally created by laforge. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28059 )
Change subject: paging: Implement upper bound of 60s for dynamic T3113
......................................................................
paging: Implement upper bound of 60s for dynamic T3113
Change-Id: Ib8228f8485527d34794048a9927e62b6ec8d802a
Closes: OS#5553
---
M src/osmo-bsc/paging.c
M tests/paging/paging_test.ok
2 files changed, 1,512 insertions(+), 1,503 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/59/28059/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28059
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ib8228f8485527d34794048a9927e62b6ec8d802a
Gerrit-Change-Number: 28059
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge.
pespin has uploaded a new patch set (#2) to the change originally created by laforge. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28060 )
Change subject: Add stat_item for per-bts [dynamic] T3113 timer
......................................................................
Add stat_item for per-bts [dynamic] T3113 timer
This allows external monitoring to see where the T3113 timer has been
adjusted to, in case it is set dynamically.
Change-Id: I533f2ca3c8e66c143154cbf03b827c9cbbacccdf
---
M include/osmocom/bsc/bts.h
M src/osmo-bsc/bts.c
M src/osmo-bsc/paging.c
3 files changed, 12 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/60/28060/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28060
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I533f2ca3c8e66c143154cbf03b827c9cbbacccdf
Gerrit-Change-Number: 28060
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels, laforge.
iedemam has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/28061 )
Change subject: fix fallout from: 'stats: new trackers for lchan life duration'
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Well, that's an embarrassing result after such a long review process on my patch. My testing never showed anything like this so apologies for missing it!
+1 from me (no permissions to do so)
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/28061
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I901bb86a78d7d021c8efe751fd9d93e5956ac0e0
Gerrit-Change-Number: 28061
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: iedemam <michael(a)kapsulate.com>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Fri, 06 May 2022 14:17:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment