Change in osmo-bsc[master]: Handle BTS/BBTRANSC Get Attributes (Ack) in NM FSMs

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Fri Dec 4 18:33:32 UTC 2020


laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/21502 )

Change subject: Handle BTS/BBTRANSC Get Attributes (Ack) in NM FSMs
......................................................................

Handle BTS/BBTRANSC Get Attributes (Ack) in NM FSMs

Before this patch, Get Attributes was sent quicklyafter the OML link
became up, even if the BTS/BB_TRANSC objects were still powered off,
which is wrong since attributes should only be available after the
objects transition out of the Power off state.

Furthermore, information about get attr response already received will
be required in future patches to delay NSVC setting.

Related: OS#4870
Change-Id: I8ec39c7e1f956ffce9aecd58a5590c43200ba086
---
M include/osmocom/bsc/gsm_data.h
M include/osmocom/bsc/nm_common_fsm.h
M src/osmo-bsc/bts_ipaccess_nanobts.c
M src/osmo-bsc/nm_bb_transc_fsm.c
M src/osmo-bsc/nm_bts_fsm.c
M src/osmo-bsc/nm_common_fsm.c
M src/osmo-bsc/osmo_bsc_main.c
7 files changed, 97 insertions(+), 24 deletions(-)

Approvals:
  Jenkins Builder: Verified
  fixeria: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved



diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index d8f8be2..6904266 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -399,6 +399,8 @@
 	struct osmo_fsm_inst *fi;
 	bool opstart_sent;
 	bool adm_unlock_sent;
+	bool get_attr_sent;
+	bool get_attr_rep_received;
 	bool set_attr_sent;
 	bool set_attr_ack_received;
 	bool force_rf_lock;
diff --git a/include/osmocom/bsc/nm_common_fsm.h b/include/osmocom/bsc/nm_common_fsm.h
index 7ad3df6..bceefbe 100644
--- a/include/osmocom/bsc/nm_common_fsm.h
+++ b/include/osmocom/bsc/nm_common_fsm.h
@@ -30,6 +30,7 @@
 enum nm_fsm_events {
 	NM_EV_SW_ACT_REP,
 	NM_EV_STATE_CHG_REP,
+	NM_EV_GET_ATTR_REP,
 	NM_EV_SET_ATTR_ACK,
 	NM_EV_OPSTART_ACK,
 	NM_EV_OPSTART_NACK,
diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c b/src/osmo-bsc/bts_ipaccess_nanobts.c
index 6d8f91f..c84d750 100644
--- a/src/osmo-bsc/bts_ipaccess_nanobts.c
+++ b/src/osmo-bsc/bts_ipaccess_nanobts.c
@@ -339,6 +339,27 @@
 	}
 }
 
+static void nm_rx_get_attr_rep(struct msgb *oml_msg)
+{
+	struct abis_om_fom_hdr *foh = msgb_l3(oml_msg);
+	struct e1inp_sign_link *sign_link = oml_msg->dst;
+	struct gsm_bts *bts = sign_link->trx->bts;
+	struct gsm_bts_trx *trx;
+
+	switch (foh->obj_class) {
+	case NM_OC_BTS:
+		osmo_fsm_inst_dispatch(bts->mo.fi, NM_EV_GET_ATTR_REP, NULL);
+		break;
+	case NM_OC_BASEB_TRANSC:
+		if (!(trx = gsm_bts_trx_num(bts, foh->obj_inst.trx_nr)))
+			return;
+		osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, NM_EV_GET_ATTR_REP, NULL);
+		break;
+	default:
+		LOGPFOH(DNM, LOGL_ERROR, foh, "Get Attributes Response received on incorrect object class %d!\n", foh->obj_class);
+	}
+}
+
 static void nm_rx_set_bts_attr_ack(struct msgb *oml_msg)
 {
 	struct abis_om_fom_hdr *foh = msgb_l3(oml_msg);
@@ -432,6 +453,9 @@
 	case S_NM_OPSTART_NACK:
 		nm_rx_opstart_nack(signal_data);
 		return 0;
+	case S_NM_GET_ATTR_REP:
+		nm_rx_get_attr_rep(signal_data);
+		return 0;
 	case S_NM_SET_BTS_ATTR_ACK:
 		nm_rx_set_bts_attr_ack(signal_data);
 		return 0;
diff --git a/src/osmo-bsc/nm_bb_transc_fsm.c b/src/osmo-bsc/nm_bb_transc_fsm.c
index e7132e8..31e5f34 100644
--- a/src/osmo-bsc/nm_bb_transc_fsm.c
+++ b/src/osmo-bsc/nm_bb_transc_fsm.c
@@ -48,6 +48,8 @@
 {
 	struct gsm_bts_bb_trx *bb_transc = (struct gsm_bts_bb_trx *)fi->priv;
 
+	bb_transc->mo.get_attr_sent = false;
+	bb_transc->mo.get_attr_rep_received = false;
 	bb_transc->mo.adm_unlock_sent = false;
 	bb_transc->mo.opstart_sent = false;
 }
@@ -87,7 +89,20 @@
 static void configure_loop(struct gsm_bts_bb_trx *bb_transc, struct gsm_nm_state *state, bool allow_opstart)
 {
 	struct gsm_bts_trx *trx = gsm_bts_bb_trx_get_trx(bb_transc);
-	if (state->administrative != NM_STATE_UNLOCKED && !bb_transc->mo.adm_unlock_sent) {
+
+	/* Request TRX-level attributes */
+	if (!bb_transc->mo.get_attr_sent && !bb_transc->mo.get_attr_rep_received) {
+		bb_transc->mo.get_attr_sent = true;
+		/* N. B: we rely on attribute order when parsing response in abis_nm_rx_get_attr_resp() */
+		const uint8_t trx_attr[] = { NM_ATT_MANUF_STATE, NM_ATT_SW_CONFIG, };
+		/* we should not request more attributes than we're ready to handle */
+		OSMO_ASSERT(sizeof(trx_attr) < MAX_BTS_ATTR);
+		abis_nm_get_attr(trx->bts, NM_OC_BASEB_TRANSC, 0, trx->nr, 0xff,
+				 trx_attr, sizeof(trx_attr));
+	}
+
+	if (bb_transc->mo.get_attr_rep_received &&
+	    state->administrative != NM_STATE_UNLOCKED && !bb_transc->mo.adm_unlock_sent) {
 		bb_transc->mo.adm_unlock_sent = true;
 		/* Note: nanoBTS sometimes fails NACKing the BaseBand
 		   Transceiver Unlock command while in Dependency, specially
@@ -105,7 +120,7 @@
 		/* TRX software is active, tell it to initiate RSL Link */
 		abis_nm_ipaccess_rsl_connect(trx, trx->bts->ip_access.rsl_ip,
 					     3003, trx->rsl_tei);
-	    }
+	}
 }
 
 static void st_op_disabled_dependency_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)
@@ -127,6 +142,11 @@
 	struct gsm_nm_state *new_state;
 
 	switch (event) {
+	case NM_EV_GET_ATTR_REP:
+		bb_transc->mo.get_attr_rep_received = true;
+		bb_transc->mo.get_attr_sent = false;
+		configure_loop(bb_transc, &bb_transc->mo.nm_state, false);
+		return;
 	case NM_EV_STATE_CHG_REP:
 		nsd = (struct nm_statechg_signal_data *)data;
 		new_state = nsd->new_state;
@@ -172,6 +192,11 @@
 	struct gsm_nm_state *new_state;
 
 	switch (event) {
+	case NM_EV_GET_ATTR_REP:
+		bb_transc->mo.get_attr_rep_received = true;
+		bb_transc->mo.get_attr_sent = false;
+		configure_loop(bb_transc, &bb_transc->mo.nm_state, true);
+		return;
 	case NM_EV_STATE_CHG_REP:
 		nsd = (struct nm_statechg_signal_data *)data;
 		new_state = nsd->new_state;
@@ -214,6 +239,8 @@
 
 	/* Reset state, we don't need it in this state and it will need to be
 	  reused as soon as we move back to Disabled */
+	bb_transc->mo.get_attr_sent = false;
+	bb_transc->mo.get_attr_rep_received = false;
 	bb_transc->mo.opstart_sent = false;
 	bb_transc->mo.adm_unlock_sent = false;
 }
@@ -282,7 +309,8 @@
 	},
 	[NM_BB_TRANSC_ST_OP_DISABLED_DEPENDENCY] = {
 		.in_event_mask =
-			X(NM_EV_STATE_CHG_REP),
+			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_GET_ATTR_REP),
 		.out_state_mask =
 			X(NM_BB_TRANSC_ST_OP_DISABLED_NOTINSTALLED) |
 			X(NM_BB_TRANSC_ST_OP_DISABLED_OFFLINE) |
@@ -293,7 +321,8 @@
 	},
 	[NM_BB_TRANSC_ST_OP_DISABLED_OFFLINE] = {
 		.in_event_mask =
-			X(NM_EV_STATE_CHG_REP),
+			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_GET_ATTR_REP),
 		.out_state_mask =
 			X(NM_BB_TRANSC_ST_OP_DISABLED_NOTINSTALLED) |
 			X(NM_BB_TRANSC_ST_OP_DISABLED_DEPENDENCY) |
diff --git a/src/osmo-bsc/nm_bts_fsm.c b/src/osmo-bsc/nm_bts_fsm.c
index 6c577bd..731b578 100644
--- a/src/osmo-bsc/nm_bts_fsm.c
+++ b/src/osmo-bsc/nm_bts_fsm.c
@@ -48,6 +48,8 @@
 {
 	struct gsm_bts *bts = (struct gsm_bts *)fi->priv;
 
+	bts->mo.get_attr_sent = false;
+	bts->mo.get_attr_rep_received = false;
 	bts->mo.set_attr_sent = false;
 	bts->mo.set_attr_ack_received = false;
 	bts->mo.adm_unlock_sent = false;
@@ -89,14 +91,27 @@
 static void configure_loop(struct gsm_bts *bts, struct gsm_nm_state *state, bool allow_opstart) {
 	struct msgb *msgb;
 
-	if (!bts->mo.set_attr_sent && !bts->mo.set_attr_ack_received) {
+	/* Request generic BTS-level attributes */
+	if (!bts->mo.get_attr_sent && !bts->mo.get_attr_rep_received) {
+		bts->mo.get_attr_sent = true;
+		/* N. B: we rely on attribute order when parsing response in abis_nm_rx_get_attr_resp() */
+		const uint8_t bts_attr[] = { NM_ATT_MANUF_ID, NM_ATT_SW_CONFIG, };
+		/* we should not request more attributes than we're ready to handle */
+		OSMO_ASSERT(sizeof(bts_attr) < MAX_BTS_ATTR);
+		abis_nm_get_attr(bts, NM_OC_BTS, 0, 0xff, 0xff,
+				 bts_attr, sizeof(bts_attr));
+	}
+
+	if (bts->mo.get_attr_rep_received &&
+	    !bts->mo.set_attr_sent && !bts->mo.set_attr_ack_received) {
 		bts->mo.set_attr_sent = true;
 		msgb = nanobts_attr_bts_get(bts);
 		abis_nm_set_bts_attr(bts, msgb->data, msgb->len);
 		msgb_free(msgb);
 	}
 
-	if (state->administrative != NM_STATE_UNLOCKED && !bts->mo.adm_unlock_sent) {
+	if (bts->mo.set_attr_ack_received &&
+	    state->administrative != NM_STATE_UNLOCKED && !bts->mo.adm_unlock_sent) {
 		bts->mo.adm_unlock_sent = true;
 		abis_nm_chg_adm_state(bts, NM_OC_BTS,
 				      bts->bts_nr, 0xff, 0xff,
@@ -105,11 +120,11 @@
 
 	if (allow_opstart && state->administrative == NM_STATE_UNLOCKED &&
 	    bts->mo.set_attr_ack_received) {
-		    if (!bts->mo.opstart_sent) {
-			    bts->mo.opstart_sent = true;
-			    abis_nm_opstart(bts, NM_OC_BTS, bts->bts_nr, 0xff, 0xff);
-		    }
-	    }
+		if (!bts->mo.opstart_sent) {
+		    bts->mo.opstart_sent = true;
+		    abis_nm_opstart(bts, NM_OC_BTS, bts->bts_nr, 0xff, 0xff);
+		}
+	}
 }
 
 static void st_op_disabled_dependency_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)
@@ -133,6 +148,11 @@
 	struct gsm_nm_state *new_state;
 
 	switch (event) {
+	case NM_EV_GET_ATTR_REP:
+		bts->mo.get_attr_rep_received = true;
+		bts->mo.get_attr_sent = false;
+		configure_loop(bts, &bts->mo.nm_state, false);
+		return;
 	case NM_EV_SET_ATTR_ACK:
 		bts->mo.set_attr_ack_received = true;
 		bts->mo.set_attr_sent = false;
@@ -182,6 +202,11 @@
 	struct gsm_nm_state *new_state;
 
 	switch (event) {
+	case NM_EV_GET_ATTR_REP:
+		bts->mo.get_attr_rep_received = true;
+		bts->mo.get_attr_sent = false;
+		configure_loop(bts, &bts->mo.nm_state, true);
+		return;
 	case NM_EV_SET_ATTR_ACK:
 		bts->mo.set_attr_ack_received = true;
 		bts->mo.set_attr_sent = false;
@@ -231,6 +256,8 @@
 	  reused as soon as we move back to Disabled */
 	bts->mo.opstart_sent = false;
 	bts->mo.adm_unlock_sent = false;
+	bts->mo.get_attr_sent = false;
+	bts->mo.get_attr_rep_received = false;
 	bts->mo.set_attr_sent = false;
 	bts->mo.set_attr_ack_received = false;
 }
@@ -300,6 +327,7 @@
 	[NM_BTS_ST_OP_DISABLED_DEPENDENCY] = {
 		.in_event_mask =
 			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_GET_ATTR_REP) |
 			X(NM_EV_SET_ATTR_ACK),
 		.out_state_mask =
 			X(NM_BTS_ST_OP_DISABLED_NOTINSTALLED) |
@@ -312,6 +340,7 @@
 	[NM_BTS_ST_OP_DISABLED_OFFLINE] = {
 		.in_event_mask =
 			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_GET_ATTR_REP) |
 			X(NM_EV_SET_ATTR_ACK),
 		.out_state_mask =
 			X(NM_BTS_ST_OP_DISABLED_NOTINSTALLED) |
diff --git a/src/osmo-bsc/nm_common_fsm.c b/src/osmo-bsc/nm_common_fsm.c
index 42c01e0..2a529db 100644
--- a/src/osmo-bsc/nm_common_fsm.c
+++ b/src/osmo-bsc/nm_common_fsm.c
@@ -25,6 +25,7 @@
 const struct value_string nm_fsm_event_names[] = {
 	{ NM_EV_SW_ACT_REP, "SW_ACT_REP" },
 	{ NM_EV_STATE_CHG_REP, "STATE_CHG_REP" },
+	{ NM_EV_GET_ATTR_REP, "GET_ATTR_REP" },
 	{ NM_EV_SET_ATTR_ACK, "SET_ATTR_ACK" },
 	{ NM_EV_OPSTART_ACK, "OPSTART_ACK" },
 	{ NM_EV_OPSTART_NACK, "OPSTART_NACK" },
diff --git a/src/osmo-bsc/osmo_bsc_main.c b/src/osmo-bsc/osmo_bsc_main.c
index 3be8593..ca00ba0 100644
--- a/src/osmo-bsc/osmo_bsc_main.c
+++ b/src/osmo-bsc/osmo_bsc_main.c
@@ -371,13 +371,6 @@
 {
 	struct input_signal_data *isd = signal_data;
 	struct gsm_bts_trx *trx = isd->trx;
-	/* N. B: we rely on attribute order when parsing response in abis_nm_rx_get_attr_resp() */
-	const uint8_t bts_attr[] = { NM_ATT_MANUF_ID, NM_ATT_SW_CONFIG, };
-	const uint8_t trx_attr[] = { NM_ATT_MANUF_STATE, NM_ATT_SW_CONFIG, };
-
-	/* we should not request more attributes than we're ready to handle */
-	OSMO_ASSERT(sizeof(bts_attr) < MAX_BTS_ATTR);
-	OSMO_ASSERT(sizeof(trx_attr) < MAX_BTS_ATTR);
 
 	if (subsys != SS_L_INPUT)
 		return -EINVAL;
@@ -395,14 +388,8 @@
 			  set bts->si_common.cell_alloc */
 			generate_cell_chan_list(ca, trx->bts);
 
-			/* Request generic BTS-level attributes */
-			abis_nm_get_attr(trx->bts, NM_OC_BTS, 0xFF, 0xFF, 0xFF, bts_attr, sizeof(bts_attr));
-
 			llist_for_each_entry(cur_trx, &trx->bts->trx_list, list) {
 				int i;
-				/* Request TRX-level attributes */
-				abis_nm_get_attr(cur_trx->bts, NM_OC_BASEB_TRANSC, 0, cur_trx->nr, 0xFF,
-						 trx_attr, sizeof(trx_attr));
 				for (i = 0; i < ARRAY_SIZE(cur_trx->ts); i++)
 					generate_ma_for_ts(&cur_trx->ts[i]);
 			}

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I8ec39c7e1f956ffce9aecd58a5590c43200ba086
Gerrit-Change-Number: 21502
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201204/4d4e9f18/attachment.htm>


More information about the gerrit-log mailing list