Change in osmo-bts[master]: bts_trx: Drop non-executed path in trx_link_estab()

[pespin]

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/25542 )

Change subject: bts_trx: Drop non-executed path in trx_link_estab()
......................................................................

bts_trx: Drop non-executed path in trx_link_estab()

This function is only called during sign_link_up() e1inp callback, hence
only the link!=NULL condition (UP) is ever executed. Let's drop the DOWN
path and make it a function only used to trigger events when link
becomes up, similar to what bts_link_estab() does with OML.

Here it becomes clear the NM_EV_RSL_DOWN was never sent. It's not much
of an issue though since it would only make transition RCARRIER/BBTRANSC
Enabled->DisabledOffline. However, since due to libosmo-abis limitation
we receive a sign_link_down() for the entire line when 1 of its links
goes down, we don't care much since we go for shutdown of the entire BTS
anyway. Ideally, libosmo-abis would support simply telling us 1 of the
links in the line went down and if it was not OML and not RSL TRX==C0,
then we could keep on running and simply disable the related TRX.

Change-Id: Iac553c68339c0da32fd313676995747eb4344087
---
M src/common/abis.c
M src/common/bts_trx.c
2 files changed, 13 insertions(+), 14 deletions(-)

Approvals:
  fixeria: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/common/abis.c b/src/common/abis.c
index 1722f3d..1b489c1 100644
--- a/src/common/abis.c
+++ b/src/common/abis.c
@@ -214,6 +214,14 @@
 			e1inp_sign_link_destroy(trx->rsl_link);
 			trx->rsl_link = NULL;
 		}
+		/* Note: Here we could send NM_EV_RSL_DOWN to each
+		 * trx->(bb_transc.)mo.fi, but we are starting shutdown of the
+		 * entire BTS anyway through bts_model_abis_close(), so simply
+		 * let bts_shutdown FSM take care of slowly powering down all
+		 * the TRX. It would make sense to send NM_EV_RSL_DOWN only if a
+		 * RSL link TRX!=C0 was going down, in order to selectively stop
+		 * that TRX only. But libosmo-abis expects us to drop the entire
+		 * line when something goes wrong... */
 	}
 	bts_model_abis_close(bts);
 	osmo_fsm_inst_state_chg(fi, ABIS_LINK_ST_WAIT_RECONNECT, OML_RETRY_TIMER, 0);
diff --git a/src/common/bts_trx.c b/src/common/bts_trx.c
index a5d7ed3..ed742d5 100644
--- a/src/common/bts_trx.c
+++ b/src/common/bts_trx.c
@@ -206,25 +206,16 @@
 /* RSL link is established, send status report */
 int trx_link_estab(struct gsm_bts_trx *trx)
 {
-	struct e1inp_sign_link *link = trx->rsl_link;
 	int rc;
 
-	LOGPTRX(trx, DSUM, LOGL_INFO, "RSL link %s\n",
-		link ? "up" : "down");
+	LOGPTRX(trx, DSUM, LOGL_INFO, "RSL link up\n");
 
-	osmo_fsm_inst_dispatch(trx->mo.fi, link ? NM_EV_RSL_UP : NM_EV_RSL_DOWN, NULL);
-	osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, link ? NM_EV_RSL_UP : NM_EV_RSL_DOWN, NULL);
+	osmo_fsm_inst_dispatch(trx->mo.fi, NM_EV_RSL_UP, NULL);
+	osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, NM_EV_RSL_UP, NULL);
 
-	if (link)
-		rc = rsl_tx_rf_res(trx);
-	else
-		rc = bts_model_trx_deact_rf(trx);
-	if (rc < 0) {
+	if ((rc = rsl_tx_rf_res(trx)) < 0)
 		oml_tx_failure_event_rep(&trx->bb_transc.mo, NM_SEVER_MAJOR, OSMO_EVT_MAJ_RSL_FAIL,
-					 link ?
-					 "Failed to establish RSL link (%d)" :
-					 "Failed to deactivate RF (%d)", rc);
-	}
+					 "Failed to establish RSL link (%d)", rc);
 
 	return 0;
 }

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/25542
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Iac553c68339c0da32fd313676995747eb4344087
Gerrit-Change-Number: 25542
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210923/b0c9c931/attachment.htm>