[PATCH] osmo-bts[master]: common/rsl: move decision whether to chan act ack/nack to co...

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
Mon Aug 22 22:00:17 UTC 2016


Review at  https://gerrit.osmocom.org/747

common/rsl: move decision whether to chan act ack/nack to common function

Prepare for a dyn TS patch that needs to call rsl_tx_chan_act_ack() directly
without the rel_act_kind decision.

Add function rsl_tx_chan_act_acknack() to wrap rsl_tx_chan_act_ack() and
rsl_tx_chan_act_nack(). Move the decision whether to drop the ack/nack, based
on lchan->rel_act_kind, to the new function, losing some code dup.

Change all callers to use the new function; drop the two older ones from rsl.h
and make them static.

Note: for nack, the exception for dyn TS in PDCH mode was missing
(rsl_tx_chan_act_nack() had only the rel_act_kind != LCHAN_REL_ACT_RSL
condition, but should also have had the dyn TS exception as in
rsl_tx_chan_act_ack()). I already know that this exception will again be
removed in an upcoming commit, but for patch readability it logically makes
sense to add it here. To easily include the nack case, drop the check for which
pchan the dyn TS is operating as, because a rel_act_kind == LCHAN_REL_ACT_PCU
implies that it is either already in or trying to become PDCH mode.

Change-Id: I57ba60c670730c6d7877a6a9b96ece0a7679a0bb
---
M include/osmo-bts/rsl.h
M src/common/l1sap.c
M src/common/rsl.c
3 files changed, 32 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/47/747/1

diff --git a/include/osmo-bts/rsl.h b/include/osmo-bts/rsl.h
index 72ed2fc..093e9cb 100644
--- a/include/osmo-bts/rsl.h
+++ b/include/osmo-bts/rsl.h
@@ -19,8 +19,7 @@
 		    uint8_t ra, uint8_t acc_delay);
 int rsl_tx_est_ind(struct gsm_lchan *lchan, uint8_t link_id, uint8_t *data, int len);
 
-int rsl_tx_chan_act_ack(struct gsm_lchan *lchan);
-int rsl_tx_chan_act_nack(struct gsm_lchan *lchan, uint8_t cause);
+int rsl_tx_chan_act_acknack(struct gsm_lchan *lchan, uint8_t cause);
 int rsl_tx_conn_fail(struct gsm_lchan *lchan, uint8_t cause);
 int rsl_tx_rf_rel_ack(struct gsm_lchan *lchan);
 int rsl_tx_hando_det(struct gsm_lchan *lchan, uint8_t *ho_delay);
diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index 6498103..3802e25 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -466,10 +466,7 @@
 
 	lchan = get_lchan_by_chan_nr(trx, info_act_cnf->chan_nr);
 
-	if (info_act_cnf->cause)
-		rsl_tx_chan_act_nack(lchan, info_act_cnf->cause);
-	else
-		rsl_tx_chan_act_ack(lchan);
+	rsl_tx_chan_act_acknack(lchan, info_act_cnf->cause);
 
 	/* During PDCH ACT, this is where we know that the PCU is done
 	 * activating a PDCH, and PDCH switchover is complete.  See
diff --git a/src/common/rsl.c b/src/common/rsl.c
index 490ae28..37c142c 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -535,26 +535,12 @@
 }
 
 /* 8.4.2 sending CHANnel ACTIVation ACKnowledge */
-int rsl_tx_chan_act_ack(struct gsm_lchan *lchan)
+static int rsl_tx_chan_act_ack(struct gsm_lchan *lchan)
 {
 	struct gsm_time *gtime = get_time(lchan->ts->trx->bts);
 	struct msgb *msg;
 	uint8_t chan_nr = gsm_lchan2chan_nr(lchan);
 	uint8_t ie[2];
-
-	/*
-	 * Normally, PDCH activation via PCU does not ack back to the BSC.
-	 * But for GSM_PCHAN_TCH_F_TCH_H_PDCH, send a non-standard act ack for
-	 * LCHAN_REL_ACT_PCU, since the act req came from RSL initially.
-	 */
-	if (lchan->rel_act_kind != LCHAN_REL_ACT_RSL
-	    && !(lchan->ts->pchan == GSM_PCHAN_TCH_F_TCH_H_PDCH
-		 && lchan->ts->dyn.pchan_is == GSM_PCHAN_PDCH
-		 && lchan->rel_act_kind == LCHAN_REL_ACT_PCU)) {
-		LOGP(DRSL, LOGL_NOTICE, "%s not sending CHAN ACT ACK\n",
-			gsm_lchan_name(lchan));
-		return 0;
-	}
 
 	LOGP(DRSL, LOGL_NOTICE, "%s Tx CHAN ACT ACK\n", gsm_lchan_name(lchan));
 
@@ -596,16 +582,10 @@
 }
 
 /* 8.4.3 sending CHANnel ACTIVation Negative ACK */
-int rsl_tx_chan_act_nack(struct gsm_lchan *lchan, uint8_t cause)
+static int rsl_tx_chan_act_nack(struct gsm_lchan *lchan, uint8_t cause)
 {
 	struct msgb *msg;
 	uint8_t chan_nr = gsm_lchan2chan_nr(lchan);
-
-	if (lchan->rel_act_kind != LCHAN_REL_ACT_RSL) {
-		LOGP(DRSL, LOGL_DEBUG, "%s not sending CHAN ACT NACK.\n",
-			gsm_lchan_name(lchan));
-		return 0;
-	}
 
 	LOGP(DRSL, LOGL_NOTICE,
 		"%s Sending Channel Activated NACK: cause = 0x%02x\n",
@@ -621,6 +601,27 @@
 	msg->trx = lchan->ts->trx;
 
 	return abis_bts_rsl_sendmsg(msg);
+}
+
+/* Send an RSL Channel Activation Ack if cause is zero, a Nack otherwise. */
+int rsl_tx_chan_act_acknack(struct gsm_lchan *lchan, uint8_t cause)
+{
+	/*
+	 * Normally, PDCH activation via PCU does not ack back to the BSC.
+	 * But for GSM_PCHAN_TCH_F_TCH_H_PDCH, send a non-standard act ack for
+	 * LCHAN_REL_ACT_PCU, since the act req came from RSL initially.
+	 */
+	if (lchan->rel_act_kind != LCHAN_REL_ACT_RSL
+	    && !(lchan->ts->pchan == GSM_PCHAN_TCH_F_TCH_H_PDCH
+		 && lchan->rel_act_kind == LCHAN_REL_ACT_PCU)) {
+		LOGP(DRSL, LOGL_NOTICE, "%s not sending CHAN ACT %s\n",
+			gsm_lchan_name(lchan), rc ? "NACK" : "ACK");
+		return 0;
+	}
+
+	if (rc)
+		return rsl_tx_chan_act_nack(lchan);
+	return rsl_tx_chan_act_ack(lchan);
 }
 
 /* 8.4.4 sending CONNection FAILure */
@@ -782,7 +783,7 @@
 		LOGP(DRSL, LOGL_ERROR,
 		     "%s: error: lchan is not available, but in state: %s.\n",
 		     gsm_lchan_name(lchan), gsm_lchans_name(lchan->state));
-		return rsl_tx_chan_act_nack(lchan, RSL_ERR_EQUIPMENT_FAIL);
+		return rsl_tx_chan_act_acknack(lchan, RSL_ERR_EQUIPMENT_FAIL);
 	}
 
 	if (ts->pchan == GSM_PCHAN_TCH_F_TCH_H_PDCH) {
@@ -797,8 +798,8 @@
 			 */
 			rc = dyn_ts_l1_reconnect(ts, msg);
 			if (rc)
-				return rsl_tx_chan_act_nack(lchan,
-							    RSL_ERR_NORMAL_UNSPEC);
+				return rsl_tx_chan_act_acknack(lchan,
+						       RSL_ERR_NORMAL_UNSPEC);
 			/* indicate that the msgb should not be freed. */
 			return 1;
 		}
@@ -814,7 +815,7 @@
 	/* 9.3.3 Activation Type */
 	if (!TLVP_PRESENT(&tp, RSL_IE_ACT_TYPE)) {
 		LOGP(DRSL, LOGL_NOTICE, "missing Activation Type\n");
-		return rsl_tx_chan_act_nack(lchan, RSL_ERR_MAND_IE_ERROR);
+		return rsl_tx_chan_act_acknack(lchan, RSL_ERR_MAND_IE_ERROR);
 	}
 	type = *TLVP_VAL(&tp, RSL_IE_ACT_TYPE);
 
@@ -822,7 +823,8 @@
 	if (type != RSL_ACT_OSMO_PDCH) {
 		if (!TLVP_PRESENT(&tp, RSL_IE_CHAN_MODE)) {
 			LOGP(DRSL, LOGL_NOTICE, "missing Channel Mode\n");
-			return rsl_tx_chan_act_nack(lchan, RSL_ERR_MAND_IE_ERROR);
+			return rsl_tx_chan_act_acknack(lchan,
+						       RSL_ERR_MAND_IE_ERROR);
 		}
 		cm = (struct rsl_ie_chan_mode *) TLVP_VAL(&tp, RSL_IE_CHAN_MODE);
 		lchan_tchmode_from_cmode(lchan, cm);
@@ -954,7 +956,7 @@
 	/* actually activate the channel in the BTS */
 	rc = l1sap_chan_act(lchan->ts->trx, dch->chan_nr, &tp);
 	if (rc < 0)
-		return rsl_tx_chan_act_nack(lchan, -rc);
+		return rsl_tx_chan_act_acknack(lchan, -rc);
 
 	return 0;
 }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I57ba60c670730c6d7877a6a9b96ece0a7679a0bb
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list