osmo-bts-trx: init OML only once by sending AVSTATE_OK with OPSTATE_ENABLED

When receiving an OPSTART for the BTS object, also set the availability state
to OK.

Before, the availability would remain at NM_AVSTATE_DEPENDENCY, which caused an
unfortunate chain reaction resulting in osmo-bts-trx going through the
initialization sequence twice:

  BTS    BSC
   |<-----|   SITE_MANAGER OPSTART
 n |----->|   BTS state change: OPSTATE_DISABLED, AVSTATE_DEPENDENCY
 o |      |     This signals to nm_statechg_event() in bts_ipaccess_nanobts.c
 r |      |     to (a) Set BTS Attributes and (b) send BTS OPSTART
 m |<-----|   Set BTS Attributes (a)
 a |      |     When osmo-bts-trx receives a Set BTS Attributes, it sends
 l |----->|   CHANNEL state change: OPSTATE_DISABLED  x8
   |      |     This signals the BSC to Set CHANNEL Attributes and OPSTART
 i |<-----|   Set CHANNEL Attributes  x8
 n |<-----|   CHANNEL OPSTART  x8
 i |----->|   CHANNEL state change: OPSTATE_ENABLED, AVSTATE_OK  x8
 t |      |
   |<-----|   BTS OPSTART (b)
   |      |     osmo-bts-trx immediately replies with:
   |----->|   BTS state change: OPSTATE_ENABLED, AVSTATE_DEPENDENCY
   |      |     Unfortunately, availability is left at DEPENDENCY,
   |      |     and the NM_OC_BTS case in nm_statechg_event() only
   |      |     checks for availability, not for the opstate.
   |      |     Hence nm_statechg_event() again feels inclined to
   |      |     to (a) Set BTS Attributes and (b) send BTS OPSTART,
   |      |
 --+------+----- This is where the second round starts
   |      |
 s |<-----|   Set BTS Attributes (a)
 e |      |     When osmo-bts-trx receives a Set BTS Attributes, it sends
 c |----->|   CHANNEL state change: OPSTATE_DISABLED  x8
 o |      |     All channels are disabled again, and then re-launched:
 n |<-----|   Set CHANNEL Attributes  x8
 d |<-----|   CHANNEL OPSTART  x8
   |----->|   CHANNEL state change: OPSTATE_ENABLED, AVSTATE_OK  x8
   |      |
 i |<-----|   BTS OPSTART (b)
 n |      |     osmo-bts-trx again sets the OPSTATE_ENABLED, but since
 i |      |     this time it was already enabled, no further state change
 t |      |     is sent back to the BSC.

This nightmare pivots on two hinges:

1. osmo-bts-trx fails to set BTS availability to AVSTATE_OK.
2. nm_statechg_event() fails to heed the OPSTATE_ENABLED of the BTS state
   change.

Note, the configured channels from the first round were not actually taken
down, only the OML OPSTATE_DISABLED were sent.

In this commit, fix the osmo-bts-trx side: send AVSTATE_OK for the BTS object
upon sending OPSTATE_ENABLED, so that only the part marked "normal init" above
is run.

This change applies the same fix to other OML objects, which should make sense
in the same manner, within the current hackish OML implementation:
* NM_OC_BTS
* NM_OC_SITE_MANAGER
* NM_OC_BASEB_TRANSC
* NM_OC_GPRS_NSE
* NM_OC_GPRS_CELL
* NM_OC_GPRS_NSVC

This means that the NM_OC_CHANNEL case just above is identical, and thus
collapse NM_OC_CHANNEL onto the other cases. Drop the comments from
NM_OC_CHANNEL since they merely rephrase the commands themselves.

See OS#1770 for BTS and NITB logs.

Fixes: OS#1770

Change-Id: I08aa861f6100568c79750f4fbc9a32e1557b9304

1 files changed, 1 insertions, 11 deletions

diff --git a/src/osmo-bts-trx/l1_if.c b/src/osmo-bts-trx/l1_if.c
index 7f4b9f7e..49bcdbb8 100644
--- a/src/osmo-bts-trx/l1_if.c
+++ b/src/osmo-bts-trx/l1_if.c
@@ -694,23 +694,13 @@ int bts_model_opstart(struct gsm_bts *bts, struct gsm_abis_mo *mo,
 		rc = trx_init(obj);
 		break;
 	case NM_OC_CHANNEL:
-		/* configure timeslot */
-		rc = 0; //ts_connect(obj);
-
-		/* Set to Operational State: Enabled */
-		oml_mo_state_chg(mo, NM_OPSTATE_ENABLED, NM_AVSTATE_OK);
-
-		/* Send OPSTART ack */
-		rc = oml_mo_opstart_ack(mo);
-
-		break;
 	case NM_OC_BTS:
 	case NM_OC_SITE_MANAGER:
 	case NM_OC_BASEB_TRANSC:
 	case NM_OC_GPRS_NSE:
 	case NM_OC_GPRS_CELL:
 	case NM_OC_GPRS_NSVC:
-		oml_mo_state_chg(mo, NM_OPSTATE_ENABLED, -1);
+		oml_mo_state_chg(mo, NM_OPSTATE_ENABLED, NM_AVSTATE_OK);
 		rc = oml_mo_opstart_ack(mo);
 		break;
 	default: