lchan release: always Deact SACCH

If an lchan is being released and had a SACCH active, there is no reason to
omit the Deact SACCH message ever. All of the callers that passed
do_deact_sacch = false did so for no good reason.

Drop the do_deact_sacch flag everywhere and, when the lchan type matches and
SAPI[0] is still active, simply always send a Deact SACCH message.

The do_deact_sacch flag was carried over from legacy code, by me, mainly
because I never really understood why it was there. I do hope I'm correct now,
asserting that having this flag makes no sense.

Change-Id: Id3301df059582da2377ef82feae554e94fa42035

8 files changed, 27 insertions, 28 deletions

diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index 0ec8fbc44..c16f0446e 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -896,7 +896,7 @@ static int rsl_rx_conn_fail(struct msgb *msg)
 	 * the connection will presumably be torn down and lead to an lchan release. During initial
 	 * Channel Request from the MS, an lchan has no conn yet, so in that case release now. */
 	if (!lchan->conn) {
-		lchan_release(lchan, false, false, true, *cause_p);
+		lchan_release(lchan, false, true, *cause_p);
 	} else
 		osmo_fsm_inst_dispatch(lchan->conn->fi, GSCON_EV_RSL_CONN_FAIL, (void*)cause_p);
 
diff --git a/src/osmo-bsc/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c
index 653681ed0..93362f85b 100644
--- a/src/osmo-bsc/assignment_fsm.c
+++ b/src/osmo-bsc/assignment_fsm.c
@@ -98,7 +98,7 @@ void assignment_reset(struct gsm_subscriber_connection *conn)
 	if (conn->assignment.new_lchan) {
 		struct gsm_lchan *lchan = conn->assignment.new_lchan;
 		conn->assignment.new_lchan = NULL;
-		lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+		lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 	}
 
 	if (conn->assignment.created_ci_for_msc) {
@@ -213,7 +213,7 @@ static void assignment_success(struct gsm_subscriber_connection *conn)
 	if (!conn->assignment.fi) {
 		/* The lchan was ready, and we failed to tell the MSC about it. By releasing this lchan,
 		 * the conn will notice that its primary lchan is gone and should clean itself up. */
-		lchan_release(conn->lchan, false, true, true, RSL_ERR_EQUIPMENT_FAIL);
+		lchan_release(conn->lchan, true, true, RSL_ERR_EQUIPMENT_FAIL);
 		return;
 	}
 
diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index 5856d7abd..ec06e9bce 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -152,7 +152,7 @@ static void forward_dtap(struct gsm_subscriber_connection *conn, struct msgb *ms
 
 /* Release an lchan in such a way that it doesn't fire events back to the conn. */
 static void gscon_release_lchan(struct gsm_subscriber_connection *conn, struct gsm_lchan *lchan,
-				bool do_sacch_deact, bool do_rr_release, bool err, enum gsm48_rr_cause cause_rr)
+				bool do_rr_release, bool err, enum gsm48_rr_cause cause_rr)
 {
 	if (!lchan || !conn)
 		return;
@@ -164,17 +164,17 @@ static void gscon_release_lchan(struct gsm_subscriber_connection *conn, struct g
 		conn->ho.new_lchan = NULL;
 	if (conn->assignment.new_lchan == lchan)
 		conn->assignment.new_lchan = NULL;
-	lchan_release(lchan, do_sacch_deact, do_rr_release, err, cause_rr);
+	lchan_release(lchan, do_rr_release, err, cause_rr);
 }
 
-void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool do_sacch_deact, bool do_rr_release)
+void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool do_rr_release)
 {
 	if (conn->ho.fi)
 		handover_end(conn, HO_RESULT_CONN_RELEASE);
 
 	assignment_reset(conn);
 
-	gscon_release_lchan(conn, conn->lchan, do_sacch_deact, do_rr_release, false, 0);
+	gscon_release_lchan(conn, conn->lchan, do_rr_release, false, 0);
 }
 
 static void handle_bssap_n_connect(struct osmo_fsm_inst *fi, struct osmo_scu_prim *scu_prim)
@@ -620,7 +620,7 @@ void gscon_change_primary_lchan(struct gsm_subscriber_connection *conn, struct g
 		osmo_fsm_inst_dispatch(conn->lchan->fi_rtp, LCHAN_RTP_EV_ESTABLISHED, 0);
 
 	if (old_lchan && (old_lchan != new_lchan))
-		gscon_release_lchan(conn, old_lchan, false, false, false, 0);
+		gscon_release_lchan(conn, old_lchan, false, false, 0);
 }
 
 void gscon_lchan_releasing(struct gsm_subscriber_connection *conn, struct gsm_lchan *lchan)
@@ -716,7 +716,7 @@ static void gscon_fsm_allstate(struct osmo_fsm_inst *fi, uint32_t event, void *d
 		if (conn->fi->state != ST_CLEARING)
 			osmo_fsm_inst_state_chg(fi, ST_CLEARING, 60, 999);
 		LOGPFSML(fi, LOGL_DEBUG, "Releasing all lchans (if any) after BSSMAP Clear Command\n");
-		gscon_release_lchans(conn, true, true);
+		gscon_release_lchans(conn, true);
 		/* FIXME: Release all terestrial resources in ST_CLEARING */
 		/* According to 3GPP 48.008 3.1.9.1. "The BSS need not wait for the radio channel
 		 * release to be completed or for the guard timer to expire before returning the
@@ -792,7 +792,7 @@ static void gscon_pre_term(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause ca
 	}
 
 	LOGPFSML(fi, LOGL_DEBUG, "Releasing all lchans (if any) because this conn is terminating\n");
-	gscon_release_lchans(conn, false, true);
+	gscon_release_lchans(conn, true);
 
 	/* drop pending messages */
 	gscon_dtap_queue_flush(conn, 0);
@@ -806,7 +806,7 @@ static int gscon_timer_cb(struct osmo_fsm_inst *fi)
 
 	switch (fi->T) {
 	case 993210:
-		gscon_release_lchan(conn, conn->lchan, false, true, true, RSL_ERR_INTERWORKING);
+		gscon_release_lchan(conn, conn->lchan, true, true, RSL_ERR_INTERWORKING);
 
 		/* MSC has not responded/confirmed connection with CC, this
 		 * could indicate a bad SCCP connection. We now inform the the
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index 9f42d8a74..8eb069293 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -4588,7 +4588,7 @@ DEFUN(lchan_act, lchan_act_cmd,
 		}
 		vty_out(vty, "%% Asking for release of %s in state %s%s", gsm_lchan_name(lchan),
 			osmo_fsm_inst_state_name(lchan->fi), VTY_NEWLINE);
-		lchan_release(lchan, false, !!(lchan->conn), false, 0);
+		lchan_release(lchan, !!(lchan->conn), false, 0);
 	}
 
 	return CMD_SUCCESS;
diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c
index c3dd3075d..4659c1ac9 100644
--- a/src/osmo-bsc/gsm_04_08_rr.c
+++ b/src/osmo-bsc/gsm_04_08_rr.c
@@ -942,7 +942,7 @@ int gsm0408_rcvmsg(struct msgb *msg, uint8_t link_id)
 		/* allocate a new connection */
 		lchan->conn = bsc_subscr_con_allocate(msg->lchan->ts->trx->bts->network);
 		if (!lchan->conn) {
-			lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+			lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 			return -1;
 		}
 		lchan->conn->lchan = lchan;
diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c
index 48e87fcb8..062c878d3 100644
--- a/src/osmo-bsc/gsm_08_08.c
+++ b/src/osmo-bsc/gsm_08_08.c
@@ -525,7 +525,7 @@ static int move_to_msc(struct gsm_subscriber_connection *_conn,
 		/* FIXME: I have not the slightest idea what move_to_msc() intends to do; during lchan
 		 * FSM introduction, I changed this and hope it is the appropriate action. I actually
 		 * assume this is unused legacy code for osmo-bsc_nat?? */
-		gscon_release_lchans(_conn, false, false);
+		gscon_release_lchans(_conn, false);
 		return 1;
 	}
 
diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c
index 8076eebfe..f2836cf37 100644
--- a/src/osmo-bsc/handover_fsm.c
+++ b/src/osmo-bsc/handover_fsm.c
@@ -243,7 +243,7 @@ static void handover_reset(struct gsm_subscriber_connection *conn)
 	struct mgwep_ci *ci;
 	if (conn->ho.new_lchan)
 		/* New lchan was activated but never passed to a conn */
-		lchan_release(conn->ho.new_lchan, true, false, true, RSL_ERR_EQUIPMENT_FAIL);
+		lchan_release(conn->ho.new_lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 
 	ci = conn->ho.created_ci_for_msc;
 	if (ci) {
@@ -725,7 +725,7 @@ void handover_end(struct gsm_subscriber_connection *conn, enum handover_result r
 				/* 3GPP TS 48.008 3.1.5.3.3 "Abnormal Conditions": if neither MS reports
 				 * HO Failure nor the MSC sends a Clear Command, we should release the
 				 * dedicated radio resources and send a Clear Request to the MSC. */
-				lchan_release(conn->lchan, false, true, true, GSM48_RR_CAUSE_ABNORMAL_TIMER);
+				lchan_release(conn->lchan, true, true, GSM48_RR_CAUSE_ABNORMAL_TIMER);
 				/* Once the channel release is through, the BSSMAP Clear will follow. */
 				break;
 			}
@@ -756,7 +756,7 @@ void handover_end(struct gsm_subscriber_connection *conn, enum handover_result r
 	/* Detach the new_lchan last, so we can still see it in above logging */
 	if (ho->new_lchan) {
 		/* Release new lchan, it didn't work out */
-		lchan_release(ho->new_lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+		lchan_release(ho->new_lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 		ho->new_lchan = NULL;
 	}
 
diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c
index f43264410..5428d973e 100644
--- a/src/osmo-bsc/lchan_fsm.c
+++ b/src/osmo-bsc/lchan_fsm.c
@@ -156,14 +156,14 @@ static void lchan_on_fully_established(struct gsm_lchan *lchan)
 			LOG_LCHAN(lchan, LOGL_ERROR,
 				  "lchan activation for assignment succeeded, but lchan has no conn:"
 				  " cannot trigger appropriate actions. Release.\n");
-			lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+			lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 			break;
 		}
 		if (!lchan->conn->assignment.fi) {
 			LOG_LCHAN(lchan, LOGL_ERROR,
 				  "lchan activation for assignment succeeded, but lchan has no"
 				  " assignment ongoing: cannot trigger appropriate actions. Release.\n");
-			lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+			lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 			break;
 		}
 		osmo_fsm_inst_dispatch(lchan->conn->assignment.fi, ASSIGNMENT_EV_LCHAN_ESTABLISHED,
@@ -177,14 +177,14 @@ static void lchan_on_fully_established(struct gsm_lchan *lchan)
 		if (!lchan->conn) {
 			LOG_LCHAN(lchan, LOGL_ERROR,
 				  "lchan activation for handover succeeded, but lchan has no conn\n");
-			lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+			lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 			break;
 		}
 		if (!lchan->conn->ho.fi) {
 			LOG_LCHAN(lchan, LOGL_ERROR,
 				  "lchan activation for handover succeeded, but lchan has no"
 				  " handover ongoing\n");
-			lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+			lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 			break;
 		}
 		osmo_fsm_inst_dispatch(lchan->conn->ho.fi, HO_EV_LCHAN_ESTABLISHED, lchan);
@@ -720,14 +720,14 @@ static void lchan_fsm_post_activ_ack(struct osmo_fsm_inst *fi)
 			LOG_LCHAN(lchan, LOGL_ERROR,
 				  "lchan activation for assignment succeeded, but lchan has no conn:"
 				  " cannot trigger appropriate actions. Release.\n");
-			lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+			lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 			break;
 		}
 		if (!lchan->conn->assignment.fi) {
 			LOG_LCHAN(lchan, LOGL_ERROR,
 				  "lchan activation for assignment succeeded, but lchan has no"
 				  " assignment ongoing: cannot trigger appropriate actions. Release.\n");
-			lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+			lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 			break;
 		}
 		/* After the Chan Activ Ack, the MS expects to receive an RR Assignment Command.
@@ -740,14 +740,14 @@ static void lchan_fsm_post_activ_ack(struct osmo_fsm_inst *fi)
 			LOG_LCHAN(lchan, LOGL_ERROR,
 				  "lchan activation for handover succeeded, but lchan has no conn:"
 				  " cannot trigger appropriate actions. Release.\n");
-			lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+			lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 			break;
 		}
 		if (!lchan->conn->ho.fi) {
 			LOG_LCHAN(lchan, LOGL_ERROR,
 				  "lchan activation for handover succeeded, but lchan has no"
 				  " handover ongoing: cannot trigger appropriate actions. Release.\n");
-			lchan_release(lchan, false, false, true, RSL_ERR_EQUIPMENT_FAIL);
+			lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
 			break;
 		}
 		/* After the Chan Activ Ack of the new lchan, send the MS an RR Handover Command on the
@@ -935,7 +935,7 @@ static void lchan_do_release(struct gsm_lchan *lchan)
 	if (lchan->fi_rtp)
 		osmo_fsm_inst_dispatch(lchan->fi_rtp, LCHAN_RTP_EV_RELEASE, 0);
 
-	if (lchan->deact_sacch && should_sacch_deact(lchan))
+	if (should_sacch_deact(lchan))
 		rsl_deact_sacch(lchan);
 }
 
@@ -1296,7 +1296,7 @@ int lchan_fsm_timer_cb(struct osmo_fsm_inst *fi)
 	}
 }
 
-void lchan_release(struct gsm_lchan *lchan, bool sacch_deact, bool do_rr_release,
+void lchan_release(struct gsm_lchan *lchan, bool do_rr_release,
 		   bool err, enum gsm48_rr_cause cause_rr)
 {
 	if (!lchan || !lchan->fi)
@@ -1309,7 +1309,6 @@ void lchan_release(struct gsm_lchan *lchan, bool sacch_deact, bool do_rr_release
 	struct osmo_fsm_inst *fi = lchan->fi;
 	lchan->release_in_error = err;
 	lchan->rsl_error_cause = cause_rr;
-	lchan->deact_sacch = sacch_deact;
 	lchan->do_rr_release = do_rr_release;
 
 	/* States waiting for events will notice the desire to release when done waiting, so it is enough