Change in osmo-pcu[master]: Revert "fix: handle NULL return of as_dl_tbf() and as_ul_tbf()"

pespin gerrit-no-reply at lists.osmocom.org
Mon Aug 23 16:27:59 UTC 2021


pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/25222 )

Change subject: Revert "fix: handle NULL return of as_dl_tbf() and as_ul_tbf()"
......................................................................

Revert "fix: handle NULL return of as_dl_tbf() and as_ul_tbf()"

This reverts commit d8e8ea9c8f16e0a1d09c2ea4395e15eac7358ed2.

Change-Id: I8000e78515b25b9be5c28a249bde330dac915dcb
---
M src/bts.cpp
M src/gprs_ms.c
M src/gprs_rlcmac_sched.cpp
M src/gprs_rlcmac_ts_alloc.cpp
M src/pdch.cpp
M src/tbf.cpp
6 files changed, 37 insertions(+), 57 deletions(-)

Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, approved



diff --git a/src/bts.cpp b/src/bts.cpp
index 0453e4e..1fbb00e 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -1165,21 +1165,22 @@
 {
 	struct gprs_rlcmac_pdch *pdch = &bts->trx[trx_no].pdch[ts];
 	struct pdch_ulc_node *poll = pdch_ulc_get_node(pdch->ulc, fn);
-	struct gprs_rlcmac_ul_tbf *ul_tbf = as_ul_tbf(poll->tbf_poll.poll_tbf);
+	struct gprs_rlcmac_ul_tbf *tbf;
 	if (!poll || poll->type !=PDCH_ULC_NODE_TBF_POLL ||
 	    poll->tbf_poll.poll_tbf->direction != GPRS_RLCMAC_UL_TBF)
 		LOGP(DL1IF, LOGL_DEBUG, "[%s] update TA = %u ignored due to "
 		     "unknown UL TBF on TRX = %d, TS = %d, FN = %d\n",
 		     p, ta, trx_no, ts, fn);
-	else if (ul_tbf) {
+	else {
+		tbf = as_ul_tbf(poll->tbf_poll.poll_tbf);
 		/* we need to distinguish TA information provided by L1
 		 * from PH-DATA-IND and PHY-RA-IND so that we can properly
 		 * update TA for given TBF
 		 */
 		if (is_rach)
-			set_tbf_ta(ul_tbf, (uint8_t)ta);
+			set_tbf_ta(tbf, (uint8_t)ta);
 		else
-			update_tbf_ta(ul_tbf, ta);
+			update_tbf_ta(tbf, ta);
 
 	}
 }
diff --git a/src/gprs_ms.c b/src/gprs_ms.c
index b9d130a..0d6be4d 100644
--- a/src/gprs_ms.c
+++ b/src/gprs_ms.c
@@ -335,13 +335,9 @@
 
 void ms_attach_tbf(struct GprsMs *ms, struct gprs_rlcmac_tbf *tbf)
 {
-	struct gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(tbf);
-	struct gprs_rlcmac_ul_tbf *ul_tbf = as_ul_tbf(tbf);
-	/* cannot be both DL and UL */
-	OSMO_ASSERT(!(dl_tbf && ul_tbf));
-	if (dl_tbf)
+	if (tbf_direction(tbf) == GPRS_RLCMAC_DL_TBF)
 		ms_attach_dl_tbf(ms, as_dl_tbf(tbf));
-	if (ul_tbf)
+	else
 		ms_attach_ul_tbf(ms, as_ul_tbf(tbf));
 }
 
diff --git a/src/gprs_rlcmac_sched.cpp b/src/gprs_rlcmac_sched.cpp
index b99ef9e..2adf1f3 100644
--- a/src/gprs_rlcmac_sched.cpp
+++ b/src/gprs_rlcmac_sched.cpp
@@ -50,8 +50,7 @@
 
 	llist_for_each_entry(pos, &pdch->trx->ul_tbfs, list) {
 		ul_tbf = as_ul_tbf((struct gprs_rlcmac_tbf *)pos->entry);
-		if (!ul_tbf)
-			continue;
+		OSMO_ASSERT(ul_tbf);
 		/* this trx, this ts */
 		if (!ul_tbf->is_control_ts(pdch->ts_no))
 			continue;
@@ -70,8 +69,7 @@
 	}
 	llist_for_each_entry(pos, &pdch->trx->dl_tbfs, list) {
 		dl_tbf = as_dl_tbf((struct gprs_rlcmac_tbf *)pos->entry);
-		if (!dl_tbf)
-			continue;
+		OSMO_ASSERT(dl_tbf);
 		/* this trx, this ts */
 		if (!dl_tbf->is_control_ts(pdch->ts_no))
 			continue;
@@ -461,7 +459,6 @@
 			"single block allocation at FN=%d\n", fn, block_nr, sba->fn);
 	/* else, check uplink resource for polling */
 	} else if ((poll_tbf = pdch_ulc_get_tbf_poll(pdch->ulc, poll_fn))) {
-		struct gprs_rlcmac_ul_tbf *ul_tbf;
 		LOGPDCH(pdch, DRLCMACSCHED, LOGL_DEBUG, "Received RTS for PDCH: FN=%d "
 			"block_nr=%d scheduling free USF for polling at FN=%d of %s\n",
 			fn, block_nr, poll_fn, tbf_name(poll_tbf));
@@ -469,9 +466,8 @@
 		 * let's set its USF in the DL msg. This is not really needed,
 		 * but it helps understand better the flow when looking at
 		 * pcaps. */
-		ul_tbf = as_ul_tbf(poll_tbf);
-		if (ul_tbf && poll_tbf->direction == GPRS_RLCMAC_UL_TBF && ul_tbf->m_usf[ts] != USF_INVALID)
-			usf_tbf = ul_tbf;
+		if (poll_tbf->direction == GPRS_RLCMAC_UL_TBF && as_ul_tbf(poll_tbf)->m_usf[ts] != USF_INVALID)
+			usf_tbf = as_ul_tbf(poll_tbf);
 	/* else, search for uplink tbf */
 	} else if ((usf_tbf = sched_select_uplink(pdch, require_gprs_only))) {
 		LOGPDCH(pdch, DRLCMACSCHED, LOGL_DEBUG, "Received RTS for PDCH: FN=%d "
diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 88dc024..5ccbb9f 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -357,8 +357,6 @@
 	struct GprsMs *ms = tbf_->ms();
 	const gprs_rlcmac_tbf *tbf = tbf_;
 	gprs_rlcmac_trx *trx = ms_current_trx(ms);
-	struct gprs_rlcmac_dl_tbf *dl_tbf;
-	struct gprs_rlcmac_ul_tbf *ul_tbf;
 
 	LOGPAL(tbf, "A", single, use_trx, LOGL_DEBUG, "Alloc start\n");
 
@@ -408,15 +406,12 @@
 
 	/* The allocation will be successful, so the system state and tbf_/ms_
 	 * may be modified from now on. */
-	dl_tbf = as_dl_tbf(tbf_);
-	ul_tbf = as_ul_tbf(tbf_);
-	/* cannot be both DL and UL */
-	OSMO_ASSERT(!(dl_tbf && ul_tbf));
-	if (ul_tbf) {
+	if (tbf->direction == GPRS_RLCMAC_UL_TBF) {
+		struct gprs_rlcmac_ul_tbf *ul_tbf = as_ul_tbf(tbf_);
 		LOGPSL(tbf, LOGL_DEBUG, "Assign uplink TS=%d TFI=%d USF=%d\n", ts, tfi, usf);
 		assign_uplink_tbf_usf(pdch, ul_tbf, tfi, usf);
-	}
-	if (dl_tbf) {
+	} else {
+		struct gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(tbf_);
 		LOGPSL(tbf, LOGL_DEBUG, "Assign downlink TS=%d TFI=%d\n", ts, tfi);
 		assign_dlink_tbf(pdch, dl_tbf, tfi);
 	}
@@ -883,8 +878,6 @@
 	struct GprsMs *ms = tbf_->ms();
 	const gprs_rlcmac_tbf *tbf = tbf_;
 	gprs_rlcmac_trx *trx;
-	struct gprs_rlcmac_dl_tbf *dl_tbf;
-	struct gprs_rlcmac_ul_tbf *ul_tbf;
 
 	LOGPAL(tbf, "B", single, use_trx, LOGL_DEBUG, "Alloc start\n");
 
@@ -967,14 +960,10 @@
 	tbf_->first_common_ts = first_common_ts;
 	tbf_->first_ts = first_ts;
 
-	dl_tbf = as_dl_tbf(tbf_);
-	ul_tbf = as_ul_tbf(tbf_);
-	/* cannot be both DL and UL */
-	OSMO_ASSERT(!(dl_tbf && ul_tbf));
-	if (dl_tbf)
-		assign_dl_tbf_slots(dl_tbf, trx, dl_slots, tfi);
-	if (ul_tbf)
-		assign_ul_tbf_slots(ul_tbf, trx, ul_slots, tfi, usf);
+	if (tbf->direction == GPRS_RLCMAC_DL_TBF)
+		assign_dl_tbf_slots(as_dl_tbf(tbf_), trx, dl_slots, tfi);
+	else
+		assign_ul_tbf_slots(as_ul_tbf(tbf_), trx, ul_slots, tfi, usf);
 
 	bts_do_rate_ctr_inc(bts, CTR_TBF_ALLOC_ALGO_B);
 
diff --git a/src/pdch.cpp b/src/pdch.cpp
index f955444..2ec40ce 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -452,7 +452,7 @@
 		return;
 	}
 	tbf = as_dl_tbf(poll->tbf_poll.poll_tbf);
-	if (!tbf || tbf->tfi() != tfi) {
+	if (tbf->tfi() != tfi) {
 		LOGPTBFDL(tbf, LOGL_NOTICE,
 			  "PACKET DOWNLINK ACK with wrong TFI=%d, ignoring!\n", tfi);
 		return;
@@ -522,7 +522,7 @@
 		return;
 	}
 	tbf = as_dl_tbf(poll->tbf_poll.poll_tbf);
-	if (!tbf || tbf->tfi() != tfi) {
+	if (tbf->tfi() != tfi) {
 		LOGPDCH(this, DRLCMAC, LOGL_NOTICE, "EGPRS PACKET DOWNLINK ACK with "
 			"wrong TFI=%d, ignoring!\n", tfi);
 		return;
@@ -1063,8 +1063,8 @@
 			m_tbfs[tbf->direction][tbf->tfi()]->name());
 
 	m_num_tbfs[tbf->direction] += 1;
-	ul_tbf = as_ul_tbf(tbf);
-	if (ul_tbf) {
+	if (tbf->direction == GPRS_RLCMAC_UL_TBF) {
+		ul_tbf = as_ul_tbf(tbf);
 		m_assigned_usf |= 1 << ul_tbf->m_usf[ts_no];
 	}
 	m_assigned_tfi[tbf->direction] |= 1UL << tbf->tfi();
@@ -1083,8 +1083,8 @@
 	OSMO_ASSERT(m_num_tbfs[tbf->direction] > 0);
 
 	m_num_tbfs[tbf->direction] -= 1;
-	ul_tbf = as_ul_tbf(tbf);
-	if (ul_tbf) {
+	if (tbf->direction == GPRS_RLCMAC_UL_TBF) {
+		ul_tbf = as_ul_tbf(tbf);
 		m_assigned_usf &= ~(1 << ul_tbf->m_usf[ts_no]);
 	}
 	m_assigned_tfi[tbf->direction] &= ~(1UL << tbf->tfi());
diff --git a/src/tbf.cpp b/src/tbf.cpp
index e9e584f..fcad879 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -270,18 +270,15 @@
 void tbf_free(struct gprs_rlcmac_tbf *tbf)
 {
 	/* update counters */
-	gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(tbf);
-	gprs_rlcmac_ul_tbf *ul_tbf = as_ul_tbf(tbf);
-	/* cannot be both DL and UL */
-	OSMO_ASSERT(!(dl_tbf && ul_tbf));
-	if (ul_tbf) {
+	if (tbf->direction == GPRS_RLCMAC_UL_TBF) {
+		gprs_rlcmac_ul_tbf *ul_tbf = as_ul_tbf(tbf);
 		bts_do_rate_ctr_inc(tbf->bts, CTR_TBF_UL_FREED);
 		if (tbf->state_is(TBF_ST_FLOW))
 			bts_do_rate_ctr_inc(tbf->bts, CTR_TBF_UL_ABORTED);
 		rate_ctr_group_free(ul_tbf->m_ul_egprs_ctrs);
 		rate_ctr_group_free(ul_tbf->m_ul_gprs_ctrs);
-	}
-	if (dl_tbf) {
+	} else {
+		gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(tbf);
 		if (tbf->is_egprs_enabled()) {
 			rate_ctr_group_free(dl_tbf->m_dl_egprs_ctrs);
 		} else {
@@ -294,7 +291,9 @@
 
 	/* Give final measurement report */
 	gprs_rlcmac_rssi_rep(tbf);
-	if (dl_tbf) {
+	if (tbf->direction == GPRS_RLCMAC_DL_TBF) {
+		gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(tbf);
+
 		dl_tbf->abort();
 		dl_tbf->cleanup();
 	}
@@ -624,10 +623,7 @@
 void gprs_rlcmac_tbf::poll_timeout(struct gprs_rlcmac_pdch *pdch, uint32_t poll_fn, enum pdch_ulc_tbf_poll_reason reason)
 {
 	uint16_t pgroup;
-	gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(this);
 	gprs_rlcmac_ul_tbf *ul_tbf = as_ul_tbf(this);
-	/* cannot be both DL and UL */
-	OSMO_ASSERT(!(dl_tbf && ul_tbf));
 
 	LOGPTBF(this, LOGL_NOTICE, "poll timeout for FN=%d, TS=%d (curr FN %d)\n",
 		poll_fn, pdch->ts_no, bts_current_frame_number(bts));
@@ -694,7 +690,9 @@
 		/* Timeout waiting for CTRL ACK acking Pkt Cell Change Continue */
 		osmo_fsm_inst_dispatch(m_ms->nacc->fi, NACC_EV_TIMEOUT_CELL_CHG_CONTINUE, NULL);
 		return;
-	} else if (dl_tbf) {
+	} else if (direction == GPRS_RLCMAC_DL_TBF) {
+		gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(this);
+
 		if (!(dl_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK))) {
 			LOGPTBF(this, LOGL_NOTICE,
 				"Timeout for polling PACKET DOWNLINK ACK: %s\n",
@@ -786,7 +784,6 @@
 void gprs_rlcmac_tbf::handle_timeout()
 {
 	int current_fn = bts_current_frame_number(bts);
-	gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(this);
 
 	LOGPTBF(this, LOGL_DEBUG, "timer 0 expired. cur_fn=%d\n", current_fn);
 
@@ -801,7 +798,8 @@
 	}
 
 	/* Finish waiting after IMM.ASS confirm timer for CCCH assignment (see timer X2002) */
-	if (dl_tbf && (state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) {
+	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) {
+		gprs_rlcmac_dl_tbf *dl_tbf = as_dl_tbf(this);
 		dl_tbf->m_wait_confirm = 0;
 		if (dl_tbf->state_is(TBF_ST_ASSIGN)) {
 			tbf_assign_control_ts(dl_tbf);

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

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I8000e78515b25b9be5c28a249bde330dac915dcb
Gerrit-Change-Number: 25222
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210823/23d20fac/attachment.htm>


More information about the gerrit-log mailing list