Change in osmo-bsc[master]: fix gscon clear 2/n: proper state transition to ST_CLEARING

[neels]

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/26613 )


Change subject: fix gscon clear 2/n: proper state transition to ST_CLEARING
......................................................................

fix gscon clear 2/n: proper state transition to ST_CLEARING

The way the ST_CLEARING is entered before this patch has various
symptoms of how I / we used osmo_fsm when we were still FSM amateurs in
Osmocom. Patch that up:

- Do state transitioning into gscon_bssmap_clear(), move the Clear
  Request messaging to ST_CLEARING's onenter function.

- In gscon_bssmap_clear(), ask for a state transition to ST_CLEARING
  first. Go ahead only if it is allowed.

- Fix the timeout behavior: by using conn_fsm_state_chg(), use the
  actual proper X4 timer value from VTY configuration instead of
  hardcoded magic numbers in various places.

Related: OS#5337
Change-Id: I234b2a754d0c98031056981823cdbc187e977741
---
M src/osmo-bsc/bsc_subscr_conn_fsm.c
1 file changed, 20 insertions(+), 15 deletions(-)



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

diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index ff742a2..e33fe43 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -141,10 +141,24 @@
 static void gscon_bssmap_clear(struct gsm_subscriber_connection *conn,
 			       enum gsm0808_cause cause)
 {
+	/* already clearing? */
+	switch (conn->fi->state) {
+	case ST_CLEARING:
+		return;
+	default:
+		break;
+	}
 
+	conn->clear_cause = cause;
+	conn_fsm_state_chg(ST_CLEARING);
+}
+
+static void gscon_fsm_clearing_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)
+{
 	struct msgb *resp;
 	int rc;
-	conn->clear_cause = cause;
+	struct gsm_subscriber_connection *conn = fi->priv;
+	enum gsm0808_cause cause = conn->clear_cause;
 
 	if (conn->rx_clear_command) {
 		LOGPFSML(conn->fi, LOGL_DEBUG, "Not sending BSSMAP CLEAR REQUEST, already got CLEAR COMMAND from MSC\n");
@@ -152,14 +166,14 @@
 	}
 
 	if (!conn->sccp.msc) {
-		LOGPFSML(conn->fi, LOGL_ERROR, "Unable to deliver BSSMAP Clear Request message, no MSC for this conn\n");
+		LOGPFSML(fi, LOGL_ERROR, "Unable to deliver BSSMAP Clear Request message, no MSC for this conn\n");
 		return;
 	}
 
-	LOGPFSML(conn->fi, LOGL_DEBUG, "Tx BSSMAP CLEAR REQUEST(%s) to MSC\n", gsm0808_cause_name(cause));
+	LOGPFSML(fi, LOGL_DEBUG, "Tx BSSMAP CLEAR REQUEST(%s) to MSC\n", gsm0808_cause_name(cause));
 	resp = gsm0808_create_clear_rqst(cause);
 	if (!resp) {
-		LOGPFSML(conn->fi, LOGL_ERROR, "Unable to compose BSSMAP Clear Request message\n");
+		LOGPFSML(fi, LOGL_ERROR, "Unable to compose BSSMAP Clear Request message\n");
 		return;
 	}
 
@@ -355,8 +369,6 @@
 			       osmo_fsm_inst_state_name(conn->fi));
 		}
 		gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE);
-		if (conn->fi->state != ST_CLEARING)
-			osmo_fsm_inst_state_chg(fi, ST_CLEARING, 60, -4);
 		return;
 	default:
 		OSMO_ASSERT(false);
@@ -376,7 +388,6 @@
 			   confirmed connection, then instead simply drop the connection */
 			LOGPFSML(fi, LOGL_INFO,
 				 "Connection confirmed but lchan was dropped previously, clearing conn\n");
-			osmo_fsm_inst_state_chg(conn->fi, ST_CLEARING, 60, -4);
 			gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE);
 			break;
 		}
@@ -705,6 +716,7 @@
 	},
 	[ST_CLEARING] = {
 		.name = "CLEARING",
+		.onenter = gscon_fsm_clearing_onenter,
 		/* dead end state */
 	 },
 };
@@ -763,11 +775,6 @@
 			   yet so we cannot release it. First wait for the CC, and release in gscon_fsm_wait_cc(). */
 			break;
 		default:
-			/* Ensure that the FSM is in ST_CLEARING. */
-			osmo_fsm_inst_state_chg(conn->fi, ST_CLEARING, 60, -4);
-			/* fall thru, omit an error log if already in ST_CLEARING */
-		case ST_CLEARING:
-			/* Request a Clear Command from the MSC. */
 			gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE);
 			break;
 		}
@@ -806,8 +813,7 @@
 				 osmo_fsm_inst_name(conn->fi), detach_label);
 	}
 
-	if ((conn->fi && conn->fi->state != ST_CLEARING)
-	    && !conn->lchan
+	if (!conn->lchan
 	    && !conn->ho.new_lchan
 	    && !conn->assignment.new_lchan
 	    && !conn->lcs.loc_req)
@@ -1205,7 +1211,6 @@
 failed_to_send:
 	LOGPFSML(conn->fi, LOGL_ERROR, "Tx BSSMAP CLEAR REQUEST to MSC\n");
 	gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE);
-	osmo_fsm_inst_state_chg(conn->fi, ST_ACTIVE, 0, 0);
 }
 
 void gscon_submit_rsl_dtap(struct gsm_subscriber_connection *conn,

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I234b2a754d0c98031056981823cdbc187e977741
Gerrit-Change-Number: 26613
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: ipse <Alexander.Chemeris at gmail.com>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211217/accb0cbb/attachment.htm>