pespin has submitted this change. ( 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(-)
Approvals: fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved msuraev: Looks good to me, but someone else must approve Jenkins Builder: Verified
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);