[MERGED] osmo-pcu[master]: TBF: cleanup state flag handling

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

Harald Welte gerrit-no-reply at lists.osmocom.org
Fri Jan 12 14:17:52 UTC 2018


Harald Welte has submitted this change and it was merged.

Change subject: TBF: cleanup state flag handling
......................................................................


TBF: cleanup state flag handling

 * introduce generic function to check whether particular flag was set
   for'a TBF and clear it if necessary. Use this instead of
   clear_poll_timeout_flag()
 * add function to explicitly set assignment and appropriate state flags

Overall this makes the code easier to read and debug.

Related: OS#1759
Change-Id: Ic4560280c72f91700f2e19c6c7f6658dc29625c2
---
M src/bts.cpp
M src/tbf.cpp
M src/tbf.h
M src/tbf_dl.cpp
M src/tbf_ul.cpp
5 files changed, 36 insertions(+), 33 deletions(-)

Approvals:
  Stefan Sperling: Looks good to me, but someone else must approve
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/bts.cpp b/src/bts.cpp
index 4bc792a..37a9c80 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -1031,23 +1031,19 @@
 				tbf->direction == new_tbf->direction)
 			tbf_free(tbf);
 
-		if ((new_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) {
+		if (new_tbf->check_n_clear(GPRS_RLCMAC_FLAG_CCCH)) {
 			/* We now know that the PACCH really existed */
 			LOGPTBF(new_tbf, LOGL_INFO,
 				"The TBF has been confirmed on the PACCH, "
 				"changed type from CCCH to PACCH\n");
-			new_tbf->state_flags &= ~(1 << GPRS_RLCMAC_FLAG_CCCH);
 			new_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
 		}
 		new_tbf->set_state(GPRS_RLCMAC_FLOW);
 		/* stop pending assignment timer */
 		new_tbf->t_stop(T0, "control acked (DL-TBF)");
-		if ((new_tbf->state_flags &
-			(1 << GPRS_RLCMAC_FLAG_TO_DL_ASS))) {
-			new_tbf->state_flags &=
-				~(1 << GPRS_RLCMAC_FLAG_TO_DL_ASS);
+		if (new_tbf->check_n_clear(GPRS_RLCMAC_FLAG_TO_DL_ASS))
 			LOGPTBF(new_tbf, LOGL_NOTICE, "Recovered downlink assignment\n");
-		}
+
 		tbf_assign_control_ts(new_tbf);
 		return;
 	}
@@ -1068,12 +1064,9 @@
 			tbf_free(tbf);
 
 		new_tbf->set_state(GPRS_RLCMAC_FLOW);
-		if ((new_tbf->state_flags &
-			(1 << GPRS_RLCMAC_FLAG_TO_UL_ASS))) {
-			new_tbf->state_flags &=
-				~(1 << GPRS_RLCMAC_FLAG_TO_UL_ASS);
+		if (new_tbf->check_n_clear(GPRS_RLCMAC_FLAG_TO_UL_ASS))
 			LOGPTBF(new_tbf, LOGL_NOTICE, "Recovered uplink assignment for UL\n");
-		}
+
 		tbf_assign_control_ts(new_tbf);
 		/* there might be LLC packets waiting in the queue, but the DL
 		 * TBF might have been released while the UL TBF has been
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 9e3a8ef..6847e18 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -402,8 +402,7 @@
 		return NULL;
 	}
 	tbf->m_contention_resolution_done = 1;
-	tbf->set_state(GPRS_RLCMAC_ASSIGN);
-	tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
+	tbf->set_assigned_on(GPRS_RLCMAC_FLAG_PACCH, false);
 	T_START(tbf, T3169, bts->t3169, 0, "allocation (UL-TBF)", true);
 	tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
 	OSMO_ASSERT(tbf->ms());
@@ -1511,8 +1510,7 @@
 
 	llist_add(&ul_tbf->list(), &bts->bts->ul_tbfs());
 	ul_tbf->bts->tbf_ul_created();
-	ul_tbf->set_state(GPRS_RLCMAC_ASSIGN);
-	ul_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
+	ul_tbf->set_assigned_on(GPRS_RLCMAC_FLAG_PACCH, false);
 
 	ul_tbf->set_ms(ms);
 	ul_tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
diff --git a/src/tbf.h b/src/tbf.h
index 943ec92..6d7edbc 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -180,6 +180,8 @@
 	bool state_is(enum gprs_rlcmac_tbf_state rhs) const;
 	bool state_is_not(enum gprs_rlcmac_tbf_state rhs) const;
 	void set_state(enum gprs_rlcmac_tbf_state new_state);
+	bool check_n_clear(uint8_t state_flag);
+	void set_assigned_on(uint8_t state_flag, bool check_ccch);
 	const char *state_name() const;
 
 	const char *name() const;
@@ -368,12 +370,33 @@
 	return tbf_state_name[state];
 }
 
+/* Set assignment state and corrsponding flags */
+inline void gprs_rlcmac_tbf::set_assigned_on(uint8_t state_flag, bool check_ccch)
+{
+	set_state(GPRS_RLCMAC_ASSIGN);
+	if (check_ccch) {
+		if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH)))
+			state_flags |= (1 << state_flag);
+	} else
+		state_flags |= (1 << state_flag);
+}
+
 inline void gprs_rlcmac_tbf::set_state(enum gprs_rlcmac_tbf_state new_state)
 {
 	LOGP(DRLCMAC, LOGL_DEBUG, "%s changes state from %s to %s\n",
 		tbf_name(this),
 		tbf_state_name[state], tbf_state_name[new_state]);
 	state = new_state;
+}
+
+inline bool gprs_rlcmac_tbf::check_n_clear(uint8_t state_flag)
+{
+	if ((state_flags & (1 << state_flag))) {
+		state_flags &= ~(1 << state_flag);
+		return true;
+	}
+
+	return false;
 }
 
 inline LListHead<gprs_rlcmac_tbf>& gprs_rlcmac_tbf::list()
@@ -451,7 +474,7 @@
 	int rcvd_dl_ack(bool final_ack, unsigned first_bsn, struct bitvec *rbb);
 	struct msgb *create_dl_acked_block(uint32_t fn, uint8_t ts);
 	void trigger_ass(struct gprs_rlcmac_tbf *old_tbf);
-	void clear_poll_timeout_flag();
+
 	bool handle_ack_nack();
 	void request_dl_ack();
 	bool need_control_ts() const;
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index b871bc3..e3b0a9d 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -493,9 +493,7 @@
 		old_tbf->was_releasing = old_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE);
 
 		/* change state */
-		set_state(GPRS_RLCMAC_ASSIGN);
-		if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH)))
-			state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
+		set_assigned_on(GPRS_RLCMAC_FLAG_PACCH, true);
 
 		/* start timer */
 		T_START(this, T0, T_ASS_PACCH_SEC, 0, "assignment (PACCH)", true);
@@ -505,8 +503,7 @@
 		was_releasing = state_is(GPRS_RLCMAC_WAIT_RELEASE);
 
 		/* change state */
-		set_state(GPRS_RLCMAC_ASSIGN);
-		state_flags |= (1 << GPRS_RLCMAC_FLAG_CCCH);
+		set_assigned_on(GPRS_RLCMAC_FLAG_CCCH, false);
 
 		/* send immediate assignment */
 		bts->snd_dl_ass(this, 0, imsi());
@@ -634,18 +631,12 @@
 	return bsn;
 }
 
-void gprs_rlcmac_dl_tbf::clear_poll_timeout_flag()
-{
-	state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ACK);
-}
-
 bool gprs_rlcmac_dl_tbf::handle_ack_nack()
 {
 	bool ack_recovered = false;
 
 	state_flags |= (1 << GPRS_RLCMAC_FLAG_DL_ACK);
-	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK))) {
-		clear_poll_timeout_flag();
+	if (check_n_clear(GPRS_RLCMAC_FLAG_TO_DL_ACK)) {
 		ack_recovered = true;
 	}
 
@@ -856,7 +847,7 @@
 			if (is_final)
 				T_START(this, T3191, bts_data()->t3191, 0, "final block (DL-TBF)", true);
 
-			clear_poll_timeout_flag();
+			state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ACK); /* clear poll timeout flag */
 
 			/* Clear request flag */
 			m_dl_ack_requested = false;
diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp
index 45de7cd..6442b8f 100644
--- a/src/tbf_ul.cpp
+++ b/src/tbf_ul.cpp
@@ -96,10 +96,8 @@
 
 bool gprs_rlcmac_ul_tbf::ctrl_ack_to_toggle()
 {
-	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK))) {
-		state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_UL_ACK);
+	if (check_n_clear(GPRS_RLCMAC_FLAG_TO_UL_ACK))
 		return true; /* GPRS_RLCMAC_FLAG_TO_UL_ACK was set, now cleared */
-	}
 
 	state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK);
 	return false; /* GPRS_RLCMAC_FLAG_TO_UL_ACK was unset, now set */

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic4560280c72f91700f2e19c6c7f6658dc29625c2
Gerrit-PatchSet: 6
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>



More information about the gerrit-log mailing list