pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/32603 )

Change subject: bbtransc/rcarrier: Fix statechg done twice upon NM_EV_RX_OPSTART ......................................................................

bbtransc/rcarrier: Fix statechg done twice upon NM_EV_RX_OPSTART

When the NM_EV_RX_OPSTART event is received, it will call bts model specific function bts_model_opstart(), which is responisble for answering back with NM_EV_OPSTART_ACK or NM_EV_OPSTART_NACK. Since that answer could be done sequentially in same callback code path, we could end up twice at the end of the st_op_disabled_offline() function checking for statechg (due to reentring that function). As a result, one can see the following message appear during OML bring up: nm_bb_transc_fsm.c:185 NM_BBTRANSC_OP(bts0-trx0){ENABLED}: transition to state ENABLED not permitted!

Fix the issue by avoiding ending up at the end of the function in code paths which should not be triggering any change. The case of bbtransc is a bit different than that of rcarrier for NM_EV_RX_SETATTR, since the former really doesn't receive any such message from the BSC yet, so if we checked for that one before continuing, it would never go on.

Change-Id: I5184a33dd8da9244e8aacf3ab8bb8930f732a136 --- M src/common/nm_bb_transc_fsm.c M src/common/nm_radio_carrier_fsm.c 2 files changed, 30 insertions(+), 4 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/03/32603/1

diff --git a/src/common/nm_bb_transc_fsm.c b/src/common/nm_bb_transc_fsm.c index d42130a..57ccdfa 100644 --- a/src/common/nm_bb_transc_fsm.c +++ b/src/common/nm_bb_transc_fsm.c @@ -136,7 +136,7 @@ &bb_transc->mo, bb_transc); bb_transc->mo.setattr_success = rc == 0; oml_fom_ack_nack_copy_msg(setattr_data->msg, rc); - break; + return; case NM_EV_RX_OPSTART: #if 0 /* Disabled because osmo-bsc doesn't send SetAttr on BB_TRANSC object */ @@ -146,7 +146,7 @@ } #endif bts_model_opstart(trx->bts, &bb_transc->mo, bb_transc); - break; + return; case NM_EV_OPSTART_ACK: bb_transc->mo.opstart_success = true; oml_mo_opstart_ack(&bb_transc->mo); diff --git a/src/common/nm_radio_carrier_fsm.c b/src/common/nm_radio_carrier_fsm.c index d7590bc..851f71c 100644 --- a/src/common/nm_radio_carrier_fsm.c +++ b/src/common/nm_radio_carrier_fsm.c @@ -120,14 +120,14 @@ &trx->mo, trx); trx->mo.setattr_success = rc == 0; oml_fom_ack_nack_copy_msg(setattr_data->msg, rc); - break; + break; /* check statechg below */ case NM_EV_RX_OPSTART: if (!trx->mo.setattr_success) { oml_mo_opstart_nack(&trx->mo, NM_NACK_CANT_PERFORM); return; } bts_model_opstart(trx->bts, &trx->mo, trx); - break; + return; case NM_EV_OPSTART_ACK: trx->mo.opstart_success = true; oml_mo_opstart_ack(&trx->mo);