[PATCH] osmo-pcu[master]: Simplify TS alloc: adjust allocator signatures

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

Max gerrit-no-reply at lists.osmocom.org
Thu Sep 28 15:51:10 UTC 2017


Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/3807

to look at the new patch set (#10).

Simplify TS alloc: adjust allocator signatures

* drop unused parameters (from both functions and structs)
* document used parameters and return values
* tighten types used for parameters
* use consistent formatting

Tests are adjusted accordingly but test results are left untouched to
avoid regressions.

Change-Id: I39d81ab64ff790b9c4c2d0312a574485cd83e755
Related: OS#228
---
M src/bts.h
M src/gprs_rlcmac.h
M src/gprs_rlcmac_ts_alloc.cpp
M src/tbf.cpp
M src/tbf.h
M src/tbf_dl.cpp
M tests/alloc/AllocTest.cpp
7 files changed, 59 insertions(+), 60 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/07/3807/10

diff --git a/src/bts.h b/src/bts.h
index 1d13a64..0ce5123 100644
--- a/src/bts.h
+++ b/src/bts.h
@@ -205,11 +205,9 @@
 	struct gsmtap_inst *gsmtap;
 	uint32_t gsmtap_categ_mask;
 	struct gprs_rlcmac_trx trx[8];
-	int (*alloc_algorithm)(struct gprs_rlcmac_bts *bts,
-		struct GprsMs *ms,
-		struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single,
-		int use_tbf);
-	uint32_t alloc_algorithm_curst; /* options to customize algorithm */
+	int (*alloc_algorithm)(struct gprs_rlcmac_bts *bts, struct GprsMs *ms, struct gprs_rlcmac_tbf *tbf,
+			       bool single, int8_t use_tbf);
+
 	uint8_t force_two_phase;
 	uint8_t alpha, gamma;
 	uint8_t egprs_enabled;
diff --git a/src/gprs_rlcmac.h b/src/gprs_rlcmac.h
index be1e686..c16a954 100644
--- a/src/gprs_rlcmac.h
+++ b/src/gprs_rlcmac.h
@@ -21,6 +21,8 @@
 #ifndef GPRS_RLCMAC_H
 #define GPRS_RLCMAC_H
 
+#include <stdbool.h>
+
 #ifdef __cplusplus
 #include <gsm_rlcmac.h>
 #include <gsm_timer.h>
@@ -98,20 +100,14 @@
 
 extern "C" {
 #endif
-int alloc_algorithm_a(struct gprs_rlcmac_bts *bts,
-	struct GprsMs *ms,
-	struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single,
-	int use_trx);
+int alloc_algorithm_a(struct gprs_rlcmac_bts *bts, struct GprsMs *ms, struct gprs_rlcmac_tbf *tbf, bool single,
+		      int8_t use_trx);
 
-int alloc_algorithm_b(struct gprs_rlcmac_bts *bts,
-	struct GprsMs *ms,
-	struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single,
-	int use_trx);
+int alloc_algorithm_b(struct gprs_rlcmac_bts *bts, struct GprsMs *ms, struct gprs_rlcmac_tbf *tbf, bool single,
+		      int8_t use_trx);
 
-int alloc_algorithm_dynamic(struct gprs_rlcmac_bts *bts,
-	struct GprsMs *ms,
-	struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single,
-	int use_trx);
+int alloc_algorithm_dynamic(struct gprs_rlcmac_bts *bts, struct GprsMs *ms, struct gprs_rlcmac_tbf *tbf, bool single,
+			    int8_t use_trx);
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/gprs_rlcmac_ts_alloc.cpp b/src/gprs_rlcmac_ts_alloc.cpp
index 3b596f4..9ceceb2 100644
--- a/src/gprs_rlcmac_ts_alloc.cpp
+++ b/src/gprs_rlcmac_ts_alloc.cpp
@@ -396,14 +396,19 @@
 	return tfi;
 }
 
-/* Slot Allocation: Algorithm A
+/*! Slot Allocation: Algorithm A
  *
  * Assign single slot for uplink and downlink
+ *
+ *  \param[in,out] bts Pointer to BTS struct
+ *  \param[in,out] ms_ Pointer to MS object
+ *  \param[in,out] tbf_ Pointer to TBF struct
+ *  \param[in] single flag indicating if we should force single-slot allocation
+ *  \param[in] use_trx which TRX to use or -1 if it should be selected during allocation
+ *  \returns negative error code or 0 on success
  */
-int alloc_algorithm_a(struct gprs_rlcmac_bts *bts,
-	GprsMs *ms_,
-	struct gprs_rlcmac_tbf *tbf_, uint32_t cust, uint8_t single,
-	int use_trx)
+int alloc_algorithm_a(struct gprs_rlcmac_bts *bts, GprsMs *ms_, struct gprs_rlcmac_tbf *tbf_, bool single,
+		      int8_t use_trx)
 {
 	struct gprs_rlcmac_pdch *pdch;
 	int ts = -1;
@@ -796,15 +801,20 @@
 	return 0;
 }
 
-/* Slot Allocation: Algorithm B
+/*! Slot Allocation: Algorithm B
  *
  * Assign as many downlink slots as possible.
  * Assign one uplink slot. (With free USF)
  *
+ *  \param[in,out] bts Pointer to BTS struct
+ *  \param[in,out] ms_ Pointer to MS object
+ *  \param[in,out] tbf_ Pointer to TBF struct
+ *  \param[in] single flag indicating if we should force single-slot allocation
+ *  \param[in] use_trx which TRX to use or -1 if it should be selected during allocation
+ *  \returns negative error code or 0 on success
  */
-int alloc_algorithm_b(struct gprs_rlcmac_bts *bts,
-	GprsMs *ms_, struct gprs_rlcmac_tbf *tbf_,
-	uint32_t cust, uint8_t single, int use_trx)
+int alloc_algorithm_b(struct gprs_rlcmac_bts *bts, GprsMs *ms_, struct gprs_rlcmac_tbf *tbf_, bool single,
+		      int8_t use_trx)
 {
 	uint8_t dl_slots;
 	uint8_t ul_slots;
@@ -1025,7 +1035,7 @@
 	return 0;
 }
 
-/* Slot Allocation: Algorithm dynamic
+/*! Slot Allocation: Algorithm dynamic
  *
  * This meta algorithm automatically selects on of the other algorithms based
  * on the current system state.
@@ -1033,10 +1043,15 @@
  * The goal is to support as many MS and TBF as possible. On low usage, the
  * goal is to provide the highest possible bandwidth per MS.
  *
+ *  \param[in,out] bts Pointer to BTS struct
+ *  \param[in,out] ms_ Pointer to MS object
+ *  \param[in,out] tbf_ Pointer to TBF struct
+ *  \param[in] single flag indicating if we should force single-slot allocation
+ *  \param[in] use_trx which TRX to use or -1 if it should be selected during allocation
+ *  \returns negative error code or 0 on success
  */
-int alloc_algorithm_dynamic(struct gprs_rlcmac_bts *bts,
-	GprsMs *ms_, struct gprs_rlcmac_tbf *tbf_,
-	uint32_t cust, uint8_t single, int use_trx)
+int alloc_algorithm_dynamic(struct gprs_rlcmac_bts *bts, GprsMs *ms_, struct gprs_rlcmac_tbf *tbf_, bool single,
+			    int8_t use_trx)
 {
 	int rc;
 
@@ -1048,7 +1063,7 @@
 	}
 
 	if (!bts->multislot_disabled) {
-		rc = alloc_algorithm_b(bts, ms_, tbf_, cust, single, use_trx);
+		rc = alloc_algorithm_b(bts, ms_, tbf_, single, use_trx);
 		if (rc >= 0)
 			return rc;
 
@@ -1057,8 +1072,7 @@
 		bts->multislot_disabled = 1;
 	}
 
-	rc = alloc_algorithm_a(bts, ms_, tbf_, cust, single, use_trx);
-	return rc;
+	return alloc_algorithm_a(bts, ms_, tbf_, single, use_trx);
 }
 
 int gprs_alloc_max_dl_slots_per_ms(struct gprs_rlcmac_bts *bts, uint8_t ms_class)
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 8e54157..d470c19 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -489,8 +489,7 @@
 		return -EINVAL;
 
 	tbf_unlink_pdch(this);
-	rc = bts_data->alloc_algorithm(bts_data, ms(), this,
-		bts_data->alloc_algorithm_curst, 0, -1);
+	rc = bts_data->alloc_algorithm(bts_data, ms(), this, false, -1);
 	/* if no resource */
 	if (rc < 0) {
 		LOGP(DRLCMAC, LOGL_ERROR, "No resource after update???\n");
@@ -752,9 +751,8 @@
 		LOGP(DRLCMAC, LOGL_ERROR, "- Poll Timeout, but no event!\n");
 }
 
-static int setup_tbf(struct gprs_rlcmac_tbf *tbf,
-	GprsMs *ms, int8_t use_trx,
-	uint8_t ms_class, uint8_t egprs_ms_class, uint8_t single_slot)
+static int setup_tbf(struct gprs_rlcmac_tbf *tbf, GprsMs *ms, int8_t use_trx, uint8_t ms_class, uint8_t egprs_ms_class,
+		     bool single_slot)
 {
 	int rc;
 	struct gprs_rlcmac_bts *bts;
@@ -769,8 +767,7 @@
 	tbf->m_created_ts = time(NULL);
 	tbf->set_ms_class(ms_class);
 	/* select algorithm */
-	rc = bts->alloc_algorithm(bts, ms, tbf, bts->alloc_algorithm_curst,
-		single_slot, use_trx);
+	rc = bts->alloc_algorithm(bts, ms, tbf, single_slot, use_trx);
 	/* if no resource */
 	if (rc < 0) {
 		return -1;
@@ -830,9 +827,8 @@
 	}
 }
 
-struct gprs_rlcmac_ul_tbf *tbf_alloc_ul_tbf(struct gprs_rlcmac_bts *bts,
-	GprsMs *ms, int8_t use_trx,
-	uint8_t ms_class, uint8_t egprs_ms_class, uint8_t single_slot)
+struct gprs_rlcmac_ul_tbf *tbf_alloc_ul_tbf(struct gprs_rlcmac_bts *bts, GprsMs *ms, int8_t use_trx, uint8_t ms_class,
+					    uint8_t egprs_ms_class, bool single_slot)
 {
 	struct gprs_rlcmac_ul_tbf *tbf;
 	int rc;
@@ -921,9 +917,8 @@
 	return 0;
 }
 
-struct gprs_rlcmac_dl_tbf *tbf_alloc_dl_tbf(struct gprs_rlcmac_bts *bts,
-	GprsMs *ms, int8_t use_trx,
-	uint8_t ms_class, uint8_t egprs_ms_class, uint8_t single_slot)
+struct gprs_rlcmac_dl_tbf *tbf_alloc_dl_tbf(struct gprs_rlcmac_bts *bts, GprsMs *ms, int8_t use_trx, uint8_t ms_class,
+					    uint8_t egprs_ms_class, bool single_slot)
 {
 	struct gprs_rlcmac_dl_tbf *tbf;
 	int rc;
diff --git a/src/tbf.h b/src/tbf.h
index 95e1e89..8f92149 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -314,13 +314,11 @@
 	int8_t use_trx, uint8_t ms_class, uint8_t egprs_ms_class,
 	uint32_t tlli, uint8_t ta, GprsMs *ms);
 
-struct gprs_rlcmac_ul_tbf *tbf_alloc_ul_tbf(struct gprs_rlcmac_bts *bts,
-	GprsMs *ms, int8_t use_trx,
-	uint8_t ms_class, uint8_t egprs_ms_class, uint8_t single_slot);
+struct gprs_rlcmac_ul_tbf *tbf_alloc_ul_tbf(struct gprs_rlcmac_bts *bts, GprsMs *ms, int8_t use_trx, uint8_t ms_class,
+					    uint8_t egprs_ms_class, bool single_slot);
 
-struct gprs_rlcmac_dl_tbf *tbf_alloc_dl_tbf(struct gprs_rlcmac_bts *bts,
-	GprsMs *ms, int8_t use_trx,
-	uint8_t ms_class, uint8_t egprs_ms_class, uint8_t single_slot);
+struct gprs_rlcmac_dl_tbf *tbf_alloc_dl_tbf(struct gprs_rlcmac_bts *bts, GprsMs *ms, int8_t use_trx, uint8_t ms_class,
+					    uint8_t egprs_ms_class, bool single_slot);
 
 void tbf_free(struct gprs_rlcmac_tbf *tbf);
 
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 3d27883..cbde283 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -118,7 +118,7 @@
 				const uint8_t egprs_ms_class,
 				struct gprs_rlcmac_dl_tbf **tbf)
 {
-	uint8_t ss;
+	bool ss;
 	int8_t use_trx;
 	uint16_t ta = GSM48_TA_INVALID;
 	struct gprs_rlcmac_ul_tbf *ul_tbf = NULL, *old_ul_tbf;
@@ -136,11 +136,11 @@
 	if (ul_tbf && ul_tbf->m_contention_resolution_done
 	 && !ul_tbf->m_final_ack_sent) {
 		use_trx = ul_tbf->trx->trx_no;
-		ss = 0;
+		ss = false;
 		old_ul_tbf = ul_tbf;
 	} else {
 		use_trx = -1;
-		ss = 1; /* PCH assignment only allows one timeslot */
+		ss = true; /* PCH assignment only allows one timeslot */
 		old_ul_tbf = NULL;
 	}
 
diff --git a/tests/alloc/AllocTest.cpp b/tests/alloc/AllocTest.cpp
index 9bff38a..14aa44a 100644
--- a/tests/alloc/AllocTest.cpp
+++ b/tests/alloc/AllocTest.cpp
@@ -39,7 +39,7 @@
 static gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts,
 		GprsMs *ms, gprs_rlcmac_tbf_direction dir,
 		uint8_t use_trx,
-		uint8_t ms_class, uint8_t egprs_ms_class, uint8_t single_slot)
+		uint8_t ms_class, uint8_t egprs_ms_class, bool single_slot)
 {
 	if (dir == GPRS_RLCMAC_UL_TBF)
 		return tbf_alloc_ul_tbf(bts, ms, use_trx,
@@ -452,10 +452,8 @@
 	test_all_alloc_b();
 }
 
-typedef int (*algo_t)(struct gprs_rlcmac_bts *bts,
-		struct GprsMs *ms,
-		struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single,
-		int use_trx);
+typedef int (*algo_t)(struct gprs_rlcmac_bts *bts, struct GprsMs *ms, struct gprs_rlcmac_tbf *tbf, bool single,
+		      int8_t use_trx);
 
 static char get_dir_char(uint8_t mask, uint8_t tx, uint8_t rx, uint8_t busy)
 {

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I39d81ab64ff790b9c4c2d0312a574485cd83e755
Gerrit-PatchSet: 10
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list