fixeria has uploaded this change for review.
abis_nm: delay configure_loop() until NM_MT_SW_ACTIVATED_REP
Even though the Abis/OML message flow looks the way it should look
on the wire, it does not actually reflect the sequence/flow of events
and actions in the NM FSMs. For example (extracted from a PCAP):
GPRS Cell(00,00,ff) State Changed Event Report
GPRS Cell(00,00,ff) Software Activate Request
GPRS Cell(00,00,ff) Software Activate Request ACK
GPRS Cell(00,00,ff) Activate Software
GPRS Cell(00,00,ff) Activate Software ACK
[a] GPRS Cell(00,00,ff) State Changed Event Report
[b] GPRS Cell(00,00,ff) Software Activated Report
[c] GPRS Cell(00,00,ff) Get Attributes
GPRS Cell(00,00,ff) Get Attributes Response
[d] GPRS Cell(00,00,ff) IPA Set Attributes
GPRS Cell(00,00,ff) IPA Set Attributes ACK
GPRS Cell(00,00,ff) Change Administrative State
GPRS Cell(00,00,ff) Change Administrative State ACK
GPRS Cell(00,00,ff) State Changed Event Report
GPRS Cell(00,00,ff) Opstart
GPRS Cell(00,00,ff) Opstart ACK
A follow-up patch [1] changes the logic generating message [d],
so that the IPA Object Version of the GPRS Cell MO is taken into
account when adding the attributes.
The problem is that both messages [c] and [d] are generated and
queued for transmission on the receipt of message [a], but *before*
message [b] has been processed. So the IPA Object Version is not
known and assumed to be 0 at that point in time.
This patch delays configure_loop() until message [b] is received.
So far only for nanoBTS and only for those MOs, for which Figure 2
in 3GPP TS 52.021 explicitly mentions that the SW downloading and
activation procedures may be required, plus for the ip.access
specific MOs which all seem to support the SW activation.
osmo-bts does send SW Activated Report only for a subset of MOs,
which does not include Baseband Transceiver, Radio Carrier, and
Radio Channel. 3GPP TS 52.021 is not clear on whether this
message shall be sent by all MOs either, so we consider it
optional and delay configure_loop() only for nanoBTS.
As a bonus, this patch is expected to fix the following warning:
NM_BTS_SM_OP(bts_sm){ENABLED}: Event SW_ACT_REP not permitted
Change-Id: I3953a5e41eb27165f9ff203cac7447ee9d311abf
Related: [1] Ie0fb3eaf76e1f70e5a19bb088e1674b7e553d32a
---
M include/osmocom/bsc/bts.h
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/nm_bb_transc_fsm.c
M src/osmo-bsc/nm_bts_fsm.c
M src/osmo-bsc/nm_bts_sm_fsm.c
M src/osmo-bsc/nm_rcarrier_fsm.c
6 files changed, 109 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/71/34471/1
diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h
index 3f7958c..6974dca 100644
--- a/include/osmocom/bsc/bts.h
+++ b/include/osmocom/bsc/bts.h
@@ -692,6 +692,17 @@
return false;
}
+static inline bool is_nanobts(const struct gsm_bts *bts)
+{
+ switch (bts->type) {
+ case GSM_BTS_TYPE_NANOBTS:
+ return true;
+ default:
+ break;
+ }
+ return false;
+}
+
static inline bool is_osmobts(const struct gsm_bts *bts)
{
switch (bts->type) {
diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 2876d24..275e4f1 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -528,6 +528,7 @@
struct gsm_nm_state nm_state;
struct gsm_bts *bts;
struct osmo_fsm_inst *fi;
+ bool sw_act_rep_received;
bool opstart_sent;
bool adm_unlock_sent;
bool get_attr_sent;
diff --git a/src/osmo-bsc/nm_bb_transc_fsm.c b/src/osmo-bsc/nm_bb_transc_fsm.c
index ce2f3e9..82da6e1 100644
--- a/src/osmo-bsc/nm_bb_transc_fsm.c
+++ b/src/osmo-bsc/nm_bb_transc_fsm.c
@@ -61,6 +61,7 @@
{
struct gsm_bts_bb_trx *bb_transc = (struct gsm_bts_bb_trx *)fi->priv;
+ bb_transc->mo.sw_act_rep_received = false;
bb_transc->mo.get_attr_sent = false;
bb_transc->mo.get_attr_rep_received = false;
bb_transc->mo.adm_unlock_sent = false;
@@ -71,11 +72,14 @@
static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
{
+ struct gsm_bts_bb_trx *bb_transc = (struct gsm_bts_bb_trx *)fi->priv;
struct nm_statechg_signal_data *nsd;
const struct gsm_nm_state *new_state;
switch (event) {
case NM_EV_SW_ACT_REP:
+ bb_transc->mo.sw_act_rep_received = true;
+ break;
case NM_EV_SETUP_RAMP_READY:
break;
case NM_EV_STATE_CHG_REP:
@@ -109,6 +113,10 @@
if (bts_setup_ramp_wait(trx->bts))
return;
+ /* nanoBTS only: delay until SW Activated Report is received */
+ if (is_nanobts(trx->bts) && !bb_transc->mo.sw_act_rep_received)
+ return;
+
/* Request TRX-level attributes */
if (!bb_transc->mo.get_attr_sent && !bb_transc->mo.get_attr_rep_received) {
uint8_t attr_buf[3]; /* enlarge if needed */
@@ -176,6 +184,7 @@
switch (event) {
case NM_EV_SW_ACT_REP:
+ bb_transc->mo.sw_act_rep_received = true;
configure_loop(bb_transc, &bb_transc->mo.nm_state, false);
break;
case NM_EV_GET_ATTR_REP:
@@ -238,6 +247,7 @@
switch (event) {
case NM_EV_SW_ACT_REP:
+ bb_transc->mo.sw_act_rep_received = true;
configure_loop(bb_transc, &bb_transc->mo.nm_state, true);
break;
case NM_EV_GET_ATTR_REP:
diff --git a/src/osmo-bsc/nm_bts_fsm.c b/src/osmo-bsc/nm_bts_fsm.c
index 937b63d..6350171 100644
--- a/src/osmo-bsc/nm_bts_fsm.c
+++ b/src/osmo-bsc/nm_bts_fsm.c
@@ -48,6 +48,7 @@
{
struct gsm_bts *bts = (struct gsm_bts *)fi->priv;
+ bts->mo.sw_act_rep_received = false;
bts->mo.get_attr_sent = false;
bts->mo.get_attr_rep_received = false;
bts->mo.set_attr_sent = false;
@@ -58,11 +59,14 @@
static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
{
+ struct gsm_bts *bts = (struct gsm_bts *)fi->priv;
struct nm_statechg_signal_data *nsd;
const struct gsm_nm_state *new_state;
switch (event) {
case NM_EV_SW_ACT_REP:
+ bts->mo.sw_act_rep_received = true;
+ break;
case NM_EV_SETUP_RAMP_READY:
break;
case NM_EV_STATE_CHG_REP:
@@ -96,6 +100,10 @@
if (bts_setup_ramp_wait(bts))
return;
+ /* nanoBTS only: delay until SW Activated Report is received */
+ if (is_nanobts(bts) && !bts->mo.sw_act_rep_received)
+ return;
+
/* Request generic BTS-level attributes */
if (!bts->mo.get_attr_sent && !bts->mo.get_attr_rep_received) {
uint8_t attr_buf[3]; /* enlarge if needed */
@@ -180,6 +188,7 @@
switch (event) {
case NM_EV_SW_ACT_REP:
+ bts->mo.sw_act_rep_received = true;
configure_loop(bts, &bts->mo.nm_state, false);
break;
case NM_EV_GET_ATTR_REP:
@@ -236,6 +245,7 @@
switch (event) {
case NM_EV_SW_ACT_REP:
+ bts->mo.sw_act_rep_received = true;
configure_loop(bts, &bts->mo.nm_state, true);
break;
case NM_EV_GET_ATTR_REP:
diff --git a/src/osmo-bsc/nm_bts_sm_fsm.c b/src/osmo-bsc/nm_bts_sm_fsm.c
index 70b0656..6279005 100644
--- a/src/osmo-bsc/nm_bts_sm_fsm.c
+++ b/src/osmo-bsc/nm_bts_sm_fsm.c
@@ -49,6 +49,7 @@
struct gsm_bts *bts = gsm_bts_sm_get_bts(site_mgr);
site_mgr->peer_has_no_avstate_offline = (bts->type == GSM_BTS_TYPE_NANOBTS);
+ site_mgr->mo.sw_act_rep_received = false;
site_mgr->mo.opstart_sent = false;
}
@@ -61,6 +62,8 @@
switch (event) {
case NM_EV_SW_ACT_REP:
+ site_mgr->mo.sw_act_rep_received = true;
+ break;
case NM_EV_SETUP_RAMP_READY:
break;
case NM_EV_STATE_CHG_REP:
@@ -107,6 +110,10 @@
if (bts_setup_ramp_wait(bts))
return;
+ /* nanoBTS only: delay until SW Activated Report is received */
+ if (is_nanobts(bts) && !site_mgr->mo.sw_act_rep_received)
+ return;
+
if (allow_opstart && !site_mgr->mo.opstart_sent) {
site_mgr->mo.opstart_sent = true;
abis_nm_opstart(bts, NM_OC_SITE_MANAGER, 0xff, 0xff, 0xff);
@@ -116,11 +123,14 @@
static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event, void *data)
{
+ struct gsm_bts_sm *site_mgr = (struct gsm_bts_sm *)fi->priv;
struct nm_statechg_signal_data *nsd;
const struct gsm_nm_state *new_state;
switch (event) {
case NM_EV_SW_ACT_REP:
+ site_mgr->mo.sw_act_rep_received = true;
+ break;
case NM_EV_SETUP_RAMP_READY:
break;
case NM_EV_STATE_CHG_REP:
@@ -162,6 +172,7 @@
switch (event) {
case NM_EV_SW_ACT_REP:
+ site_mgr->mo.sw_act_rep_received = true;
configure_loop(site_mgr, &site_mgr->mo.nm_state, true);
break;
case NM_EV_STATE_CHG_REP:
diff --git a/src/osmo-bsc/nm_rcarrier_fsm.c b/src/osmo-bsc/nm_rcarrier_fsm.c
index 15e7c9f..798dfdc 100644
--- a/src/osmo-bsc/nm_rcarrier_fsm.c
+++ b/src/osmo-bsc/nm_rcarrier_fsm.c
@@ -58,6 +58,7 @@
{
struct gsm_bts_trx *trx = (struct gsm_bts_trx *)fi->priv;
+ trx->mo.sw_act_rep_received = false;
trx->mo.set_attr_sent = false;
trx->mo.set_attr_ack_received = false;
trx->mo.adm_unlock_sent = false;
@@ -66,11 +67,14 @@
static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
{
+ struct gsm_bts_trx *trx = (struct gsm_bts_trx *)fi->priv;
struct nm_statechg_signal_data *nsd;
const struct gsm_nm_state *new_state;
switch (event) {
case NM_EV_SW_ACT_REP:
+ trx->mo.sw_act_rep_received = true;
+ break;
case NM_EV_SETUP_RAMP_READY:
break;
case NM_EV_STATE_CHG_REP:
@@ -104,6 +108,10 @@
if (bts_setup_ramp_wait(trx->bts))
return;
+ /* nanoBTS only: delay until SW Activated Report is received */
+ if (is_nanobts(trx->bts) && !trx->mo.sw_act_rep_received)
+ return;
+
if (!trx->mo.set_attr_sent && !trx->mo.set_attr_ack_received) {
trx->mo.set_attr_sent = true;
msgb = nanobts_gen_set_radio_attr(trx->bts, trx);
@@ -152,6 +160,7 @@
switch (event) {
case NM_EV_SW_ACT_REP:
+ trx->mo.sw_act_rep_received = true;
configure_loop(trx, &trx->mo.nm_state, false);
break;
case NM_EV_SET_ATTR_ACK:
@@ -205,6 +214,7 @@
switch (event) {
case NM_EV_SW_ACT_REP:
+ trx->mo.sw_act_rep_received = true;
configure_loop(trx, &trx->mo.nm_state, true);
break;
case NM_EV_SET_ATTR_ACK:
To view, visit change 34471. To unsubscribe, or for help writing mail filters, visit settings.