openbsc[master]: fix handling of state changes in acc ramping
Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/7825 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I235d2c5fa962f2f338e77d0c11502921b37f4c36 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pau Espin Pedrol Gerrit-HasComments: No
[MERGED] openbsc[master]: fix handling of state changes in acc ramping
Harald Welte has submitted this change and it was merged.
Change subject: fix handling of state changes in acc ramping
..
fix handling of state changes in acc ramping
Take both the operative and administrative states into account
when deciding whether to start ACC ramping, and examine old/new
state values to avoid triggering ramping for a no-op state change.
This requires a fix to gsm_trx_lock_rf(): This function overwrote
the old administrative state of a trx before enqueuing a state
change request towards the BTS.
The BTS will confirm this request with an ACK, at which time a
signal is generated which the ACC ramp code listens to. We must
not overwrite the old state value until the signal has been handled,
otherwise the signal handler cannot tell what the old state was.
Tested with a virtphy setup, nanobts, and osmo-bts.
This is a port of osmo-bsc commit cda994edb20d24032d6ab4e916d0e9411671cfc0
Change-Id: I235d2c5fa962f2f338e77d0c11502921b37f4c36
Related: OS#2591
---
M openbsc/src/libbsc/abis_nm.c
M openbsc/src/libbsc/acc_ramp.c
M openbsc/src/libbsc/bsc_init.c
3 files changed, 113 insertions(+), 37 deletions(-)
Approvals:
Pau Espin Pedrol: Looks good to me, but someone else must approve
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 8eadfa8..cbb255a 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -2797,13 +2797,17 @@
{
uint8_t new_state = locked ? NM_STATE_LOCKED : NM_STATE_UNLOCKED;
- LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Changing adm. state %s -> %s
[%s]\n", trx->bts->nr, trx->nr,
+
+ if (!trx->bts || !trx->bts->oml_link) {
+ /* Set initial state which will be sent when BTS connects. */
+ trx->mo.nm_state.administrative = new_state;
+ return;
+ }
+
+ LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Requesting administrative state
change %s -> %s [%s]\n",
+trx->bts->nr, trx->nr,
get_value_string(abis_nm_adm_state_names,
trx->mo.nm_state.administrative),
get_value_string(abis_nm_adm_state_names, new_state), reason);
-
- trx->mo.nm_state.administrative = new_state;
- if (!trx->bts || !trx->bts->oml_link)
- return;
abis_nm_chg_adm_state(trx->bts, NM_OC_RADIO_CARRIER,
trx->bts->bts_nr, trx->nr, 0xff,
diff --git a/openbsc/src/libbsc/acc_ramp.c b/openbsc/src/libbsc/acc_ramp.c
index 6e64109..c90d087 100644
--- a/openbsc/src/libbsc/acc_ramp.c
+++ b/openbsc/src/libbsc/acc_ramp.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
/*
* Check if an ACC has been permanently barred for a BTS,
@@ -144,6 +145,7 @@
struct nm_statechg_signal_data *nsd = signal_data;
struct acc_ramp *acc_ramp = handler_data;
struct gsm_bts_trx *trx = NULL;
+ bool trigger_ramping = false, abort_ramping = false;
/* Handled signals map to an Administrative State Change ACK, or a
State Changed Event Report. */
if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER)
@@ -154,37 +156,109 @@
trx = nsd->obj;
+ LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: administrative state
%s -> %s\n",
+ acc_ramp->bts->nr, trx->nr,
+ get_value_string(abis_nm_adm_state_names,
nsd->old_state->administrative),
+ get_value_string(abis_nm_adm_state_names,
nsd->new_state->administrative));
+ LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: operational state %s
-> %s\n",
+ acc_ramp->bts->nr, trx->nr,
+ abis_nm_opstate_name(nsd->old_state->operational),
+ abis_nm_opstate_name(nsd->new_state->operational));
+
/* We only care about state changes of the first TRX. */
if (trx->nr != 0)
return 0;
/* RSL must already be up. We cannot send RACH system information to
the BTS otherwise. */
- if (trx->rsl_link == NULL)
+ if (trx->rsl_link == NULL) {
+ LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: ignoring
state change because RSL link is down\n",
+acc_ramp->bts->nr, trx->nr);
return 0;
-
- /* Trigger or abort ACC ramping based on the new 'RF lock' state of
this TRX. */
- switch (nsd->new_state->administrative) {
- case NM_STATE_UNLOCKED:
- /*
-* Do not re-trigger ACC ramping if ramping is already in
progress.
-* A BTS might send several "unlock" change events: One in the
Administrative
-* State Change ACK, and/or another in a State Changed Event
Report.
-* For instance, the nanobts is known to send both.
-*/
- if (!osmo_timer_pending(&acc_ramp->step_timer))
- acc_ramp_tri
openbsc[master]: fix handling of state changes in acc ramping
Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/7825 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I235d2c5fa962f2f338e77d0c11502921b37f4c36 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pau Espin Pedrol Gerrit-HasComments: No
[PATCH] openbsc[master]: fix handling of state changes in acc ramping
Review at https://gerrit.osmocom.org/7825
fix handling of state changes in acc ramping
Take both the operative and administrative states into account
when deciding whether to start ACC ramping, and examine old/new
state values to avoid triggering ramping for a no-op state change.
This requires a fix to gsm_trx_lock_rf(): This function overwrote
the old administrative state of a trx before enqueuing a state
change request towards the BTS.
The BTS will confirm this request with an ACK, at which time a
signal is generated which the ACC ramp code listens to. We must
not overwrite the old state value until the signal has been handled,
otherwise the signal handler cannot tell what the old state was.
Tested with a virtphy setup, nanobts, and osmo-bts.
This is a port of osmo-bsc commit cda994edb20d24032d6ab4e916d0e9411671cfc0
Change-Id: I235d2c5fa962f2f338e77d0c11502921b37f4c36
Related: OS#2591
---
M openbsc/src/libbsc/abis_nm.c
M openbsc/src/libbsc/acc_ramp.c
M openbsc/src/libbsc/bsc_init.c
3 files changed, 113 insertions(+), 37 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/25/7825/1
diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 8eadfa8..cbb255a 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -2797,13 +2797,17 @@
{
uint8_t new_state = locked ? NM_STATE_LOCKED : NM_STATE_UNLOCKED;
- LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Changing adm. state %s -> %s
[%s]\n", trx->bts->nr, trx->nr,
+
+ if (!trx->bts || !trx->bts->oml_link) {
+ /* Set initial state which will be sent when BTS connects. */
+ trx->mo.nm_state.administrative = new_state;
+ return;
+ }
+
+ LOGP(DNM, LOGL_NOTICE, "(bts=%d,trx=%d) Requesting administrative state
change %s -> %s [%s]\n",
+trx->bts->nr, trx->nr,
get_value_string(abis_nm_adm_state_names,
trx->mo.nm_state.administrative),
get_value_string(abis_nm_adm_state_names, new_state), reason);
-
- trx->mo.nm_state.administrative = new_state;
- if (!trx->bts || !trx->bts->oml_link)
- return;
abis_nm_chg_adm_state(trx->bts, NM_OC_RADIO_CARRIER,
trx->bts->bts_nr, trx->nr, 0xff,
diff --git a/openbsc/src/libbsc/acc_ramp.c b/openbsc/src/libbsc/acc_ramp.c
index 6e64109..c90d087 100644
--- a/openbsc/src/libbsc/acc_ramp.c
+++ b/openbsc/src/libbsc/acc_ramp.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
/*
* Check if an ACC has been permanently barred for a BTS,
@@ -144,6 +145,7 @@
struct nm_statechg_signal_data *nsd = signal_data;
struct acc_ramp *acc_ramp = handler_data;
struct gsm_bts_trx *trx = NULL;
+ bool trigger_ramping = false, abort_ramping = false;
/* Handled signals map to an Administrative State Change ACK, or a
State Changed Event Report. */
if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER)
@@ -154,37 +156,109 @@
trx = nsd->obj;
+ LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: administrative state
%s -> %s\n",
+ acc_ramp->bts->nr, trx->nr,
+ get_value_string(abis_nm_adm_state_names,
nsd->old_state->administrative),
+ get_value_string(abis_nm_adm_state_names,
nsd->new_state->administrative));
+ LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: operational state %s
-> %s\n",
+ acc_ramp->bts->nr, trx->nr,
+ abis_nm_opstate_name(nsd->old_state->operational),
+ abis_nm_opstate_name(nsd->new_state->operational));
+
/* We only care about state changes of the first TRX. */
if (trx->nr != 0)
return 0;
/* RSL must already be up. We cannot send RACH system information to
the BTS otherwise. */
- if (trx->rsl_link == NULL)
+ if (trx->rsl_link == NULL) {
+ LOGP(DRSL, LOGL_DEBUG, "(bts=%d,trx=%d) ACC RAMP: ignoring
state change because RSL link is down\n",
+acc_ramp->bts->nr, trx->nr);
return 0;
-
- /* Trigger or abort ACC ramping based on the new 'RF lock' state of
this TRX. */
- switch (nsd->new_state->administrative) {
- case NM_STATE_UNLOCKED:
- /*
-* Do not re-trigger ACC ramping if ramping is already in
progress.
-* A BTS might send several "unlock" change events: One in the
Administrative
-* State Change ACK, and/or another in a State Changed Event
Report.
-* For instance, the nanobts is known to send both.
-*/
- if (!osmo_timer_pending(&acc_ramp->step_timer))
- acc_ramp_trigger(acc_ramp);
- break;
- case NM_STATE_LOCKED:
- case NM_STATE_SHUTDOWN:
- acc_ramp_abort(acc_ramp);
- break;
- case NM_STATE_NULL:
- break;
- d
