Change in osmo-bsc[master]: cosmetic: lchan: introduce sub-struct lchan->release.*

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Thu Nov 8 03:58:22 UTC 2018


Neels Hofmeyr has uploaded this change for review. ( https://gerrit.osmocom.org/11668


Change subject: cosmetic: lchan: introduce sub-struct lchan->release.*
......................................................................

cosmetic: lchan: introduce sub-struct lchan->release.*

Put all lchan release related flags and settings in a sub-struct named
'release' to better indicate what those fields are for. There is no functional
change.

Change-Id: Icfddc6010e5d7c309f1a7ed3526b5b635ffeaf11
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/bsc_subscr_conn_fsm.c
M src/osmo-bsc/lchan_fsm.c
M src/osmo-bsc/lchan_rtp_fsm.c
4 files changed, 53 insertions(+), 50 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/68/11668/1

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index b257b67..fb1db80 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -492,6 +492,8 @@
 	uint8_t nr;
 	char *name;
 
+	char *last_error;
+
 	struct osmo_fsm_inst *fi;
 	struct osmo_fsm_inst *fi_rtp;
 	struct mgwep_ci *mgw_endpoint_ci_bts;
@@ -510,21 +512,21 @@
 		struct gsm_lchan *re_use_mgw_endpoint_from_lchan;
 	} activate;
 
-	/* If an event to release the lchan comes in while still waiting for responses, just mark this
-	 * flag, so that the lchan will gracefully release at the next sensible junction. */
-	bool release_requested;
-	bool do_rr_release;
+	struct {
+		/* If an event to release the lchan comes in while still waiting for responses, just mark this
+		 * flag, so that the lchan will gracefully release at the next sensible junction. */
+		bool release_requested;
+		bool do_rr_release;
 
-	char *last_error;
+		/* There is an RSL error cause of value 0, so we need a separate flag. */
+		bool release_in_error;
+		/* RSL error code, RSL_ERR_* */
+		uint8_t rsl_error_cause;
 
-	/* There is an RSL error cause of value 0, so we need a separate flag. */
-	bool release_in_error;
-	/* RSL error code, RSL_ERR_* */
-	uint8_t rsl_error_cause;
-
-	/* If a release event is being handled, ignore other ricocheting release events until that
-	 * release handling has concluded. */
-	bool in_release_handler;
+		/* If a release event is being handled, ignore other ricocheting release events until that
+		 * release handling has concluded. */
+		bool in_release_handler;
+	} release;
 
 	/* The logical channel type */
 	enum gsm_chan_t type;
diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index 86f8354..c0febaa 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -715,8 +715,8 @@
 		break;
 	case GSCON_EV_RSL_CONN_FAIL:
 		if (conn->lchan) {
-			conn->lchan->release_in_error = true;
-			conn->lchan->rsl_error_cause = data ? *(uint8_t*)data : RSL_ERR_IE_ERROR;
+			conn->lchan->release.release_in_error = true;
+			conn->lchan->release.rsl_error_cause = data ? *(uint8_t*)data : RSL_ERR_IE_ERROR;
 		}
 		gscon_bssmap_clear(conn, GSM0808_CAUSE_RADIO_INTERFACE_FAILURE);
 		break;
diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c
index 9964fcf..012239c 100644
--- a/src/osmo-bsc/lchan_fsm.c
+++ b/src/osmo-bsc/lchan_fsm.c
@@ -485,7 +485,7 @@
 		OSMO_ASSERT(!lchan->conn);
 		OSMO_ASSERT(!lchan->mgw_endpoint_ci_bts);
 		lchan_set_last_error(lchan, NULL);
-		lchan->release_requested = false;
+		lchan->release.release_requested = false;
 
 		lchan->conn = info->for_conn;
 		lchan->activate.activ_for = info->activ_for;
@@ -557,7 +557,7 @@
 	struct gsm_lchan *lchan = lchan_fi_lchan(fi);
 	struct mgwep_ci *use_mgwep_ci = lchan_use_mgw_endpoint_ci_bts(lchan);
 
-	if (lchan->release_requested) {
+	if (lchan->release.release_requested) {
 		lchan_fail("Release requested while activating");
 		return;
 	}
@@ -593,7 +593,7 @@
 
 	case LCHAN_EV_RTP_RELEASED:
 	case LCHAN_EV_RTP_ERROR:
-		if (lchan->in_release_handler) {
+		if (lchan->release.in_release_handler) {
 			/* Already in release, the RTP is not the initial cause of failure.
 			 * Just ignore. */
 			return;
@@ -616,7 +616,7 @@
 	uint8_t ho_ref = 0;
 	struct gsm_lchan *lchan = lchan_fi_lchan(fi);
 
-	if (lchan->release_requested) {
+	if (lchan->release.release_requested) {
 		lchan_fail_to(LCHAN_ST_UNUSED, "Release requested while activating");
 		return;
 	}
@@ -653,12 +653,12 @@
 		break;
 
 	case LCHAN_EV_RSL_CHAN_ACTIV_NACK:
-		lchan->in_release_handler = true;
+		lchan->release.in_release_handler = true;
 		if (data) {
 			uint32_t next_state;
-			lchan->rsl_error_cause = *(uint8_t*)data;
-			lchan->release_in_error = true;
-			if (lchan->rsl_error_cause != RSL_ERR_RCH_ALR_ACTV_ALLOC)
+			lchan->release.rsl_error_cause = *(uint8_t*)data;
+			lchan->release.release_in_error = true;
+			if (lchan->release.rsl_error_cause != RSL_ERR_RCH_ALR_ACTV_ALLOC)
 				next_state = LCHAN_ST_BORKEN;
 			else
 				/* Taking this over from legacy code: send an RF Chan Release even though
@@ -666,18 +666,18 @@
 				next_state = LCHAN_ST_WAIT_RF_RELEASE_ACK;
 
 			lchan_fail_to(next_state, "Chan Activ NACK: %s (0x%x)",
-				      rsl_err_name(lchan->rsl_error_cause), lchan->rsl_error_cause);
+				      rsl_err_name(lchan->release.rsl_error_cause), lchan->release.rsl_error_cause);
 		} else {
-			lchan->rsl_error_cause = RSL_ERR_IE_NONEXIST;
-			lchan->release_in_error = true;
+			lchan->release.rsl_error_cause = RSL_ERR_IE_NONEXIST;
+			lchan->release.release_in_error = true;
 			lchan_fail_to(LCHAN_ST_BORKEN, "Chan Activ NACK without cause IE");
 		}
-		lchan->in_release_handler = false;
+		lchan->release.in_release_handler = false;
 		break;
 
 	case LCHAN_EV_RTP_RELEASED:
 	case LCHAN_EV_RTP_ERROR:
-		if (lchan->in_release_handler) {
+		if (lchan->release.in_release_handler) {
 			/* Already in release, the RTP is not the initial cause of failure.
 			 * Just ignore. */
 			return;
@@ -698,7 +698,7 @@
 {
 	int rc;
 	struct gsm_lchan *lchan = lchan_fi_lchan(fi);
-	if (lchan->release_requested) {
+	if (lchan->release.release_requested) {
 		lchan_fail_to(LCHAN_ST_WAIT_RF_RELEASE_ACK, "Release requested while activating");
 		return;
 	}
@@ -789,7 +789,7 @@
 
 	case LCHAN_EV_RTP_RELEASED:
 	case LCHAN_EV_RTP_ERROR:
-		if (lchan->in_release_handler) {
+		if (lchan->release.in_release_handler) {
 			/* Already in release, the RTP is not the initial cause of failure.
 			 * Just ignore. */
 			return;
@@ -809,7 +809,7 @@
 {
 	struct gsm_lchan *lchan = lchan_fi_lchan(fi);
 
-	if (lchan->release_requested) {
+	if (lchan->release.release_requested) {
 		lchan_fail("Release requested while activating");
 		return;
 	}
@@ -896,7 +896,7 @@
 
 	case LCHAN_EV_RTP_RELEASED:
 	case LCHAN_EV_RTP_ERROR:
-		if (lchan->in_release_handler) {
+		if (lchan->release.in_release_handler) {
 			/* Already in release, the RTP is not the initial cause of failure.
 			 * Just ignore. */
 			return;
@@ -929,7 +929,7 @@
 
 static void lchan_do_release(struct gsm_lchan *lchan)
 {
-	if (lchan->do_rr_release && lchan->sapis[0] != LCHAN_SAPI_UNUSED)
+	if (lchan->release.do_rr_release && lchan->sapis[0] != LCHAN_SAPI_UNUSED)
 		gsm48_send_rr_release(lchan);
 
 	if (lchan->fi_rtp)
@@ -1025,7 +1025,7 @@
 	switch (event) {
 
 	case LCHAN_EV_RSL_RF_CHAN_REL_ACK:
-		if (lchan->rsl_error_cause)
+		if (lchan->release.release_in_error)
 			lchan_fsm_state_chg(LCHAN_ST_WAIT_AFTER_ERROR);
 		else
 			lchan_fsm_state_chg(LCHAN_ST_UNUSED);
@@ -1053,7 +1053,7 @@
 
 	case LCHAN_EV_RSL_CHAN_ACTIV_ACK:
 		/* A late Chan Activ ACK? Release. */
-		lchan->release_in_error = true;
+		lchan->release.release_in_error = true;
 		lchan_fsm_state_chg(LCHAN_ST_WAIT_RF_RELEASE_ACK);
 		return;
 
@@ -1064,7 +1064,7 @@
 
 	case LCHAN_EV_RSL_RF_CHAN_REL_ACK:
 		/* A late Release ACK? */
-		lchan->release_in_error = true;
+		lchan->release.release_in_error = true;
 		lchan_fsm_state_chg(LCHAN_ST_WAIT_AFTER_ERROR);
 		/* TODO: we used to do this only for sysmobts:
 			int do_free = is_sysmobts_v2(ts->trx->bts);
@@ -1288,7 +1288,7 @@
 		return 0;
 
 	default:
-		lchan->release_in_error = true;
+		lchan->release.release_in_error = true;
 		lchan_fail("Timeout");
 		return 0;
 	}
@@ -1300,25 +1300,26 @@
 	if (!lchan || !lchan->fi)
 		return;
 
-	if (lchan->in_release_handler)
+	if (lchan->release.in_release_handler)
 		return;
-	lchan->in_release_handler = true;
+	lchan->release.in_release_handler = true;
 
 	struct osmo_fsm_inst *fi = lchan->fi;
-	lchan->release_in_error = err;
-	lchan->rsl_error_cause = cause_rr;
-	lchan->do_rr_release = do_rr_release;
+
+	lchan->release.release_in_error = err;
+	lchan->release.rsl_error_cause = cause_rr;
+	lchan->release.do_rr_release = do_rr_release;
 
 	/* States waiting for events will notice the desire to release when done waiting, so it is enough
 	 * to mark for release. */
-	lchan->release_requested = true;
+	lchan->release.release_requested = true;
 
 	/* If we took the RTP over from another lchan, put it back. */
-	if (lchan->fi_rtp && lchan->release_in_error)
+	if (lchan->fi_rtp && lchan->release.release_in_error)
 		osmo_fsm_inst_dispatch(lchan->fi_rtp, LCHAN_RTP_EV_ROLLBACK, 0);
 
 	/* But when in error, don't wait for the next state to pick up release_requested. */
-	if (lchan->release_in_error) {
+	if (lchan->release.release_in_error) {
 		switch (lchan->fi->state) {
 		default:
 			/* Normally we signal release in lchan_fsm_wait_rll_rtp_released_onenter(). When
@@ -1343,7 +1344,7 @@
 		lchan_fsm_state_chg(LCHAN_ST_WAIT_RLL_RTP_RELEASED);
 
 exit_release_handler:
-	lchan->in_release_handler = false;
+	lchan->release.in_release_handler = false;
 }
 
 void lchan_fsm_cleanup(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause cause)
diff --git a/src/osmo-bsc/lchan_rtp_fsm.c b/src/osmo-bsc/lchan_rtp_fsm.c
index bf73429..dd42fc4 100644
--- a/src/osmo-bsc/lchan_rtp_fsm.c
+++ b/src/osmo-bsc/lchan_rtp_fsm.c
@@ -256,7 +256,7 @@
 	int val;
 	struct gsm_lchan *lchan = lchan_rtp_fi_lchan(fi);
 
-	if (lchan->release_requested) {
+	if (lchan->release.release_requested) {
 		lchan_rtp_fail("Release requested while activating");
 		return;
 	}
@@ -324,7 +324,7 @@
 	struct gsm_lchan *lchan = lchan_rtp_fi_lchan(fi);
 	const struct mgcp_conn_peer *mgw_rtp;
 
-	if (lchan->release_requested) {
+	if (lchan->release.release_requested) {
 		lchan_rtp_fail("Release requested while activating");
 		return;
 	}
@@ -439,7 +439,7 @@
 	struct gsm_lchan *lchan = lchan_rtp_fi_lchan(fi);
 	struct gsm_lchan *old_lchan = lchan->activate.re_use_mgw_endpoint_from_lchan;
 
-	if (lchan->release_requested) {
+	if (lchan->release.release_requested) {
 		lchan_rtp_fail("Release requested while activating");
 		return;
 	}
@@ -724,7 +724,7 @@
 int lchan_rtp_fsm_timer_cb(struct osmo_fsm_inst *fi)
 {
 	struct gsm_lchan *lchan = lchan_rtp_fi_lchan(fi);
-	lchan->release_in_error = true;
+	lchan->release.release_in_error = true;
 	lchan_rtp_fail("Timeout");
 	return 0;
 }

-- 
To view, visit https://gerrit.osmocom.org/11668
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icfddc6010e5d7c309f1a7ed3526b5b635ffeaf11
Gerrit-Change-Number: 11668
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181108/b10f2efa/attachment-0001.html>


More information about the gerrit-log mailing list