[PATCH 03/10] rlcmac: Reduce the depedency on the global gprs_rlcmac_bts variable

Holger Freyther hfreyther at sysmocom.de
Thu Aug 22 07:16:57 UTC 2013


From: Holger Hans Peter Freyther <holger at moiji-mobile.com>

For mocking/unit-testing/emulation (and a dual trx-systems) having
global state is quite bad. Cut back on the usage of the global
struct gprs_rlcmac_bts. It also makes the complexity of certain
routines more clear.
---
 src/gprs_bssgp_pcu.cpp   | 20 +++++++++++---------
 src/gprs_bssgp_pcu.h     |  8 ++++----
 src/gprs_rlcmac.cpp      | 27 +++++++++++++--------------
 src/gprs_rlcmac.h        | 19 ++++++++++++-------
 src/gprs_rlcmac_data.cpp | 15 ++++++++-------
 src/pcu_l1_if.cpp        |  2 +-
 6 files changed, 49 insertions(+), 42 deletions(-)

diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index a0ef206..acd30a4 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -25,6 +25,8 @@ struct osmo_pcu {
 	struct gprs_nsvc *nsvc;
 	struct bssgp_bvc_ctx *bctx;
 
+	struct gprs_rlcmac_bts *bts;
+
 	struct osmo_timer_list bvc_timer;
 
 	int nsvc_unblocked;
@@ -187,15 +189,14 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp)
 		} else {
 			/* the TBF exists, so we must write it in the queue
 			 * we prepend lifetime in front of PDU */
-			struct gprs_rlcmac_bts *bts = gprs_rlcmac_bts;
 			struct timeval *tv;
 			struct msgb *llc_msg = msgb_alloc(len + sizeof(*tv),
 				"llc_pdu_queue");
 			if (!llc_msg)
 				return -ENOMEM;
 			tv = (struct timeval *)msgb_put(llc_msg, sizeof(*tv));
-			if (bts->force_llc_lifetime)
-				delay_csec = bts->force_llc_lifetime;
+			if (the_pcu.bts->force_llc_lifetime)
+				delay_csec = the_pcu.bts->force_llc_lifetime;
 			/* keep timestap at 0 for infinite delay */
 			if (delay_csec != 0xffff) {
 				/* calculate timestamp of timeout */
@@ -247,14 +248,14 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp)
 		}
 
 		// Create new TBF (any TRX)
-		tfi = tfi_alloc(GPRS_RLCMAC_DL_TBF, &trx, use_trx);
+		tfi = tfi_alloc(the_pcu.bts, GPRS_RLCMAC_DL_TBF, &trx, use_trx);
 		if (tfi < 0) {
 			LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource\n");
 			/* FIXME: send reject */
 			return -EBUSY;
 		}
 		/* set number of downlink slots according to multislot class */
-		tbf = tbf_alloc(tbf, GPRS_RLCMAC_DL_TBF, tfi, trx, ms_class,
+		tbf = tbf_alloc(the_pcu.bts, tbf, GPRS_RLCMAC_DL_TBF, tfi, trx, ms_class,
 			ss);
 		if (!tbf) {
 			LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH ressource\n");
@@ -560,8 +561,6 @@ int gprs_bssgp_tx_fc_bvc(void)
 
 static void bvc_timeout(void *_priv)
 {
-	struct gprs_rlcmac_bts *bts = gprs_rlcmac_bts;
-
 	if (!the_pcu.bvc_sig_reset) {
 		LOGP(DBSSGP, LOGL_INFO, "Sending reset on BVCI 0\n");
 		bssgp_tx_bvc_reset(the_pcu.bctx, 0, BSSGP_CAUSE_OML_INTERV);
@@ -588,11 +587,12 @@ static void bvc_timeout(void *_priv)
 	LOGP(DBSSGP, LOGL_DEBUG, "Sending flow control info on BVCI %d\n",
 		the_pcu.bctx->bvci);
 	gprs_bssgp_tx_fc_bvc();
-	osmo_timer_schedule(&the_pcu.bvc_timer, bts->fc_interval, 0);
+	osmo_timer_schedule(&the_pcu.bvc_timer, the_pcu.bts->fc_interval, 0);
 }
 
 /* create BSSGP/NS layer instances */
-int gprs_bssgp_create_and_connect(uint16_t local_port, uint32_t sgsn_ip,
+int gprs_bssgp_create_and_connect(struct gprs_rlcmac_bts *bts,
+	uint16_t local_port, uint32_t sgsn_ip,
 	uint16_t sgsn_port, uint16_t nsei, uint16_t nsvci, uint16_t bvci,
 	uint16_t mcc, uint16_t mnc, uint16_t lac, uint16_t rac,
 	uint16_t cell_id)
@@ -607,6 +607,8 @@ int gprs_bssgp_create_and_connect(uint16_t local_port, uint32_t sgsn_ip,
 	if (the_pcu.bctx)
 		return 0; /* if already created, must return 0: no error */
 
+	the_pcu.bts = bts;
+
 	bssgp_nsi = gprs_ns_instantiate(&sgsn_ns_cb, tall_pcu_ctx);
 	if (!bssgp_nsi) {
 		LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
diff --git a/src/gprs_bssgp_pcu.h b/src/gprs_bssgp_pcu.h
index 9f0adc9..15c8d9f 100644
--- a/src/gprs_bssgp_pcu.h
+++ b/src/gprs_bssgp_pcu.h
@@ -41,10 +41,10 @@ struct bssgp_bvc_ctx *btsctx_alloc(uint16_t bvci, uint16_t nsei);
 #define NS_HDR_LEN 4
 #define IE_LLC_PDU 14
 
-int gprs_bssgp_create_and_connect(uint16_t local_port, uint32_t sgsn_ip, uint16_t
-		sgsn_port, uint16_t nsei, uint16_t nsvci, uint16_t bvci,
-		uint16_t mcc, uint16_t mnc, uint16_t lac, uint16_t rac,
-		uint16_t cell_id);
+int gprs_bssgp_create_and_connect(struct gprs_rlcmac_bts *bts, uint16_t local_port,
+		uint32_t sgsn_ip, uint16_t sgsn_port, uint16_t nsei,
+		uint16_t nsvci, uint16_t bvci, uint16_t mcc, uint16_t mnc,
+		uint16_t lac, uint16_t rac, uint16_t cell_id);
 
 void gprs_bssgp_exit_on_destroy(void);
 void gprs_bssgp_destroy_or_exit(void);
diff --git a/src/gprs_rlcmac.cpp b/src/gprs_rlcmac.cpp
index f3a24f7..8874983 100644
--- a/src/gprs_rlcmac.cpp
+++ b/src/gprs_rlcmac.cpp
@@ -176,9 +176,9 @@ void debug_diagram(int diag, const char *format, ...)
 /* FIXME: spread ressources over multiple TRX. Also add option to use same
  * TRX in case of existing TBF for TLLI in the other direction. */
 /* search for free TFI and return TFI, TRX */
-int tfi_alloc(enum gprs_rlcmac_tbf_direction dir, uint8_t *_trx, int8_t use_trx)
+int tfi_alloc(struct gprs_rlcmac_bts *bts, enum gprs_rlcmac_tbf_direction dir,
+		uint8_t *_trx, int8_t use_trx)
 {
-	struct gprs_rlcmac_bts *bts = gprs_rlcmac_bts;
 	struct gprs_rlcmac_pdch *pdch;
 	struct gprs_rlcmac_tbf **tbfp;
 	uint8_t trx_from, trx_to, trx, ts, tfi;
@@ -252,11 +252,10 @@ static inline int8_t find_free_usf(struct gprs_rlcmac_pdch *pdch, uint8_t ts)
 }
 
 /* lookup TBF Entity (by TFI) */
-struct gprs_rlcmac_tbf *tbf_by_tfi(uint8_t tfi, uint8_t trx,
-	enum gprs_rlcmac_tbf_direction dir)
+struct gprs_rlcmac_tbf *tbf_by_tfi(struct gprs_rlcmac_bts *bts,
+	uint8_t tfi, uint8_t trx, enum gprs_rlcmac_tbf_direction dir)
 {
 	struct gprs_rlcmac_tbf *tbf;
-	struct gprs_rlcmac_bts *bts = gprs_rlcmac_bts;
 
 	if (tfi >= 32 || trx >= 8)
 		return NULL;
@@ -318,11 +317,11 @@ struct gprs_rlcmac_tbf *tbf_by_poll_fn(uint32_t fn, uint8_t trx, uint8_t ts)
 	return NULL;
 }
 
-struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_tbf *old_tbf,
-	enum gprs_rlcmac_tbf_direction dir, uint8_t tfi, uint8_t trx,
+struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts,
+	struct gprs_rlcmac_tbf *old_tbf, enum gprs_rlcmac_tbf_direction dir,
+	uint8_t tfi, uint8_t trx,
 	uint8_t ms_class, uint8_t single_slot)
 {
-	struct gprs_rlcmac_bts *bts = gprs_rlcmac_bts;
 	struct gprs_rlcmac_tbf *tbf;
 	int rc;
 
@@ -367,7 +366,7 @@ next_diagram:
 	tbf->ws = 64;
 	tbf->sns = 128;
 	/* select algorithm */
-	rc = bts->alloc_algorithm(old_tbf, tbf, bts->alloc_algorithm_curst,
+	rc = bts->alloc_algorithm(bts, old_tbf, tbf, bts->alloc_algorithm_curst,
 		single_slot);
 	/* if no ressource */
 	if (rc < 0) {
@@ -406,10 +405,10 @@ next_diagram:
  *
  * Assign single slot for uplink and downlink
  */
-int alloc_algorithm_a(struct gprs_rlcmac_tbf *old_tbf,
+int alloc_algorithm_a(struct gprs_rlcmac_bts *bts,
+	struct gprs_rlcmac_tbf *old_tbf,
 	struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single)
 {
-	struct gprs_rlcmac_bts *bts = gprs_rlcmac_bts;
 	struct gprs_rlcmac_pdch *pdch;
 	uint8_t ts;
 	int8_t usf; /* must be signed */
@@ -462,10 +461,10 @@ int alloc_algorithm_a(struct gprs_rlcmac_tbf *old_tbf,
  * Assign one uplink slot. (With free USF)
  *
  */
-int alloc_algorithm_b(struct gprs_rlcmac_tbf *old_tbf,
+int alloc_algorithm_b(struct gprs_rlcmac_bts *bts,
+	struct gprs_rlcmac_tbf *old_tbf,
 	struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single)
 {
-	struct gprs_rlcmac_bts *bts = gprs_rlcmac_bts;
 	struct gprs_rlcmac_pdch *pdch;
 	struct gprs_ms_multislot_class *ms_class;
 	uint8_t Rx, Tx, Sum;	/* Maximum Number of Slots: RX, Tx, Sum Rx+Tx */
@@ -964,7 +963,7 @@ int tbf_update(struct gprs_rlcmac_tbf *tbf)
 	ul_tbf = tbf_by_tlli(tbf->tlli, GPRS_RLCMAC_UL_TBF);
 
 	tbf_unlink_pdch(tbf);
-	rc = bts->alloc_algorithm(ul_tbf, tbf, bts->alloc_algorithm_curst, 0);
+	rc = bts->alloc_algorithm(bts, ul_tbf, tbf, bts->alloc_algorithm_curst, 0);
 	/* if no ressource */
 	if (rc < 0) {
 		LOGP(DRLCMAC, LOGL_ERROR, "No ressource after update???\n");
diff --git a/src/gprs_rlcmac.h b/src/gprs_rlcmac.h
index c5af602..ac437f5 100644
--- a/src/gprs_rlcmac.h
+++ b/src/gprs_rlcmac.h
@@ -84,7 +84,8 @@ struct gprs_rlcmac_bts {
 	uint8_t n3103;
 	uint8_t n3105;
 	struct gprs_rlcmac_trx trx[8];
-	int (*alloc_algorithm)(struct gprs_rlcmac_tbf *old_tbf,
+	int (*alloc_algorithm)(struct gprs_rlcmac_bts *bts,
+		struct gprs_rlcmac_tbf *old_tbf,
 		struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single);
 	uint32_t alloc_algorithm_curst; /* options to customize algorithm */
 	uint8_t force_two_phase;
@@ -308,14 +309,16 @@ int sba_alloc(uint8_t *_trx, uint8_t *_ts, uint32_t *_fn, uint8_t ta);
 
 struct gprs_rlcmac_sba *sba_find(uint8_t trx, uint8_t ts, uint32_t fn);
 
-int tfi_alloc(enum gprs_rlcmac_tbf_direction dir, uint8_t *_trx,
-	int8_t use_trx);
+int tfi_alloc(struct gprs_rlcmac_bts *bts, enum gprs_rlcmac_tbf_direction dir,
+	uint8_t *_trx, int8_t use_trx);
 
-struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_tbf *old_tbf,
+struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts,
+	struct gprs_rlcmac_tbf *old_tbf,
 	enum gprs_rlcmac_tbf_direction dir, uint8_t tfi, uint8_t trx,
 	uint8_t ms_class, uint8_t single_slot);
 
-struct gprs_rlcmac_tbf *tbf_by_tfi(uint8_t tfi, uint8_t trx,
+struct gprs_rlcmac_tbf *tbf_by_tfi(struct gprs_rlcmac_bts *bts,
+	uint8_t tfi, uint8_t trx,
         enum gprs_rlcmac_tbf_direction dir);
 
 struct gprs_rlcmac_tbf *tbf_by_tlli(uint32_t tlli,
@@ -431,10 +434,12 @@ int flush_timing_advance(void);
 
 extern "C" {
 #endif
-int alloc_algorithm_a(struct gprs_rlcmac_tbf *old_tbf,
+int alloc_algorithm_a(struct gprs_rlcmac_bts *bts,
+	struct gprs_rlcmac_tbf *old_tbf,
 	struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single);
 
-int alloc_algorithm_b(struct gprs_rlcmac_tbf *old_tbf,
+int alloc_algorithm_b(struct gprs_rlcmac_bts *bts,
+	struct gprs_rlcmac_tbf *old_tbf,
 	struct gprs_rlcmac_tbf *tbf, uint32_t cust, uint8_t single);
 #ifdef __cplusplus
 }
diff --git a/src/gprs_rlcmac_data.cpp b/src/gprs_rlcmac_data.cpp
index 99fac1b..4d5acb9 100644
--- a/src/gprs_rlcmac_data.cpp
+++ b/src/gprs_rlcmac_data.cpp
@@ -239,14 +239,14 @@ static struct gprs_rlcmac_tbf *alloc_ul_tbf(int8_t use_trx, uint8_t ms_class,
 	uint8_t tfi;
 
 	/* create new TBF, use sme TRX as DL TBF */
-	tfi = tfi_alloc(GPRS_RLCMAC_UL_TBF, &trx, use_trx);
+	tfi = tfi_alloc(bts, GPRS_RLCMAC_UL_TBF, &trx, use_trx);
 	if (tfi < 0) {
 		LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH ressource\n");
 		/* FIXME: send reject */
 		return NULL;
 	}
 	/* use multislot class of downlink TBF */
-	tbf = tbf_alloc(dl_tbf, GPRS_RLCMAC_UL_TBF, tfi, trx, ms_class, 0);
+	tbf = tbf_alloc(bts, dl_tbf, GPRS_RLCMAC_UL_TBF, tfi, trx, ms_class, 0);
 	if (!tbf) {
 		LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH ressource\n");
 		/* FIXME: send reject */
@@ -273,6 +273,7 @@ int gprs_rlcmac_rcv_control_block(bitvec *rlc_block, uint8_t trx, uint8_t ts,
 	uint32_t tlli = 0;
 	struct gprs_rlcmac_tbf *tbf;
 	struct gprs_rlcmac_sba *sba;
+	struct gprs_rlcmac_bts *bts = gprs_rlcmac_bts;
 	int rc;
 
 	RlcMacUplink_t * ul_control_block = (RlcMacUplink_t *)talloc_zero(tall_pcu_ctx, RlcMacUplink_t);
@@ -478,14 +479,14 @@ int gprs_rlcmac_rcv_control_block(bitvec *rlc_block, uint8_t trx, uint8_t ts,
 		} else {
 			if (ul_control_block->u.Packet_Resource_Request.ID.u.Global_TFI.UnionType) {
 				tfi = ul_control_block->u.Packet_Resource_Request.ID.u.Global_TFI.u.DOWNLINK_TFI;
-				tbf = tbf_by_tfi(tfi, trx, GPRS_RLCMAC_DL_TBF);
+				tbf = tbf_by_tfi(bts, tfi, trx, GPRS_RLCMAC_DL_TBF);
 				if (!tbf) {
 					LOGP(DRLCMAC, LOGL_NOTICE, "PACKET RESSOURCE REQ unknown downlink TBF=%d\n", tlli);
 					break;
 				}
 			} else {
 				tfi = ul_control_block->u.Packet_Resource_Request.ID.u.Global_TFI.u.UPLINK_TFI;
-				tbf = tbf_by_tfi(tfi, trx, GPRS_RLCMAC_UL_TBF);
+				tbf = tbf_by_tfi(bts, tfi, trx, GPRS_RLCMAC_UL_TBF);
 				if (!tbf) {
 					LOGP(DRLCMAC, LOGL_NOTICE, "PACKET RESSOURCE REQ unknown uplink TBF=%d\n", tlli);
 					break;
@@ -876,7 +877,7 @@ int gprs_rlcmac_rcv_data_block_acknowledged(uint8_t trx, uint8_t ts,
 	}
 
 	/* find TBF inst from given TFI */
-	tbf = tbf_by_tfi(rh->tfi, trx, GPRS_RLCMAC_UL_TBF);
+	tbf = tbf_by_tfi(bts, rh->tfi, trx, GPRS_RLCMAC_UL_TBF);
 	if (!tbf) {
 		LOGP(DRLCMACUL, LOGL_NOTICE, "UL DATA unknown TBF=%d\n",
 			rh->tfi);
@@ -1173,14 +1174,14 @@ int gprs_rlcmac_rcv_rach(uint8_t ra, uint32_t Fn, int16_t qta)
 			"(AGCH)\n");
 	} else {
 		// Create new TBF
-		tfi = tfi_alloc(GPRS_RLCMAC_UL_TBF, &trx, -1);
+		tfi = tfi_alloc(bts, GPRS_RLCMAC_UL_TBF, &trx, -1);
 		if (tfi < 0) {
 			LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH ressource\n");
 			/* FIXME: send reject */
 			return -EBUSY;
 		}
 		/* set class to 0, since we don't know the multislot class yet */
-		tbf = tbf_alloc(NULL, GPRS_RLCMAC_UL_TBF, tfi, trx, 0, 1);
+		tbf = tbf_alloc(bts, NULL, GPRS_RLCMAC_UL_TBF, tfi, trx, 0, 1);
 		if (!tbf) {
 			LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH ressource\n");
 			/* FIXME: send reject */
diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp
index b7b0269..e53b683 100644
--- a/src/pcu_l1_if.cpp
+++ b/src/pcu_l1_if.cpp
@@ -418,7 +418,7 @@ bssgp_failed:
 	ia.s_addr = htonl(info_ind->remote_ip[0]);
 	LOGP(DL1IF, LOGL_DEBUG, " remote_ip=%s\n", inet_ntoa(ia));
 
-	rc = gprs_bssgp_create_and_connect(info_ind->local_port[0],
+	rc = gprs_bssgp_create_and_connect(bts, info_ind->local_port[0],
 		info_ind->remote_ip[0], info_ind->remote_port[0],
 		info_ind->nsei, info_ind->nsvci[0], info_ind->bvci,
 		info_ind->mcc, info_ind->mnc, info_ind->lac, info_ind->rac,
-- 
1.8.3.2





More information about the osmocom-net-gprs mailing list