[MERGED] osmo-bts[master]: osmo-bts-trx: init OML only once by sending AVSTATE_OK with ...

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/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Thu Jul 14 12:45:14 UTC 2016


Neels Hofmeyr has submitted this change and it was merged.

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


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
---
M src/osmo-bts-trx/l1_if.c
1 file changed, 1 insertion(+), 11 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Holger Freyther: Looks good to me, approved



diff --git a/src/osmo-bts-trx/l1_if.c b/src/osmo-bts-trx/l1_if.c
index 7f4b9f7..49bcdbb 100644
--- a/src/osmo-bts-trx/l1_if.c
+++ b/src/osmo-bts-trx/l1_if.c
@@ -694,23 +694,13 @@
 		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:

-- 
To view, visit https://gerrit.osmocom.org/498
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I08aa861f6100568c79750f4fbc9a32e1557b9304
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Tom Tsou <tom at tsou.cc>



More information about the gerrit-log mailing list