TBF: move window parameters to UL/DL level

The UL and DL TBF use different classes implementing window
management. Hence it's better to use it explicitly instead of using the
common window management superclass inside common TBF superclass. While
at it, also remove the direct access to window class - use accessor
functions instead.

Related: OS#1759
Change-Id: I0b55aa8947db65f7206adcf53ea32b74a831d9e6

6 files changed, 42 insertions, 32 deletions

diff --git a/src/bts.cpp b/src/bts.cpp
index c6059670..14c05f2f 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -1203,7 +1203,7 @@ void gprs_rlcmac_pdch::rcv_control_dl_ack_nack(Packet_Downlink_Ack_Nack_t *ack_n
 
 	num_blocks = Decoding::decode_gprs_acknack_bits(
 		&ack_nack->Ack_Nack_Description, &bits,
-		&bsn_begin, &bsn_end, &tbf->m_window);
+		&bsn_begin, &bsn_end, tbf->window());
 
 	LOGP(DRLCMAC, LOGL_DEBUG,
 		"Got GPRS DL ACK bitmap: SSN: %d, BSN %d to %d - 1 (%d blocks), "
@@ -1279,8 +1279,8 @@ void gprs_rlcmac_pdch::rcv_control_egprs_dl_ack_nack(EGPRS_PD_AckNack_t *ack_nac
 		(void *)&ack_nack->EGPRS_AckNack.Desc,
 		(int)offsetof(EGPRS_AckNack_t, Desc),
 		(int)offsetof(EGPRS_AckNack_w_len_t, Desc),
-		tbf->m_window.v_a(),
-		tbf->m_window.v_s(),
+		tbf->window()->v_a(),
+		tbf->window()->v_s(),
 		osmo_hexdump((const uint8_t *)&ack_nack->EGPRS_AckNack.Desc.URBB,
 			sizeof(ack_nack->EGPRS_AckNack.Desc.URBB)));
 
@@ -1290,7 +1290,7 @@ void gprs_rlcmac_pdch::rcv_control_egprs_dl_ack_nack(EGPRS_PD_AckNack_t *ack_nac
 
 	num_blocks = Decoding::decode_egprs_acknack_bits(
 		&ack_nack->EGPRS_AckNack.Desc, &bits,
-		&bsn_begin, &bsn_end, &tbf->m_window);
+		&bsn_begin, &bsn_end, tbf->window());
 
 	LOGP(DRLCMAC, LOGL_DEBUG,
 		"Got EGPRS DL ACK bitmap: SSN: %d, BSN %d to %d - 1 (%d blocks), "
diff --git a/src/encoding.cpp b/src/encoding.cpp
index 87c6f086..b2da1e84 100644
--- a/src/encoding.cpp
+++ b/src/encoding.cpp
@@ -695,7 +695,7 @@ static void write_packet_uplink_ack_gprs(
 {
 
 	bitvec_write_field(dest, &wp, tbf->current_cs().to_num() - 1, 2); // CHANNEL_CODING_COMMAND
-	write_packet_ack_nack_desc_gprs(bts, dest, wp, &tbf->m_window, is_final);
+	write_packet_ack_nack_desc_gprs(bts, dest, wp, tbf->window(), is_final);
 
 	bitvec_write_field(dest, &wp, 1, 1); // 1: have CONTENTION_RESOLUTION_TLLI
 	bitvec_write_field(dest, &wp, tbf->tlli(), 32); // CONTENTION_RESOLUTION_TLLI
@@ -892,7 +892,7 @@ static void write_packet_uplink_ack_egprs(
 
 	/* -2 for last bit 0 mandatory and REL5 not supported */
 	unsigned bits_ack_nack = dest->data_len * 8 - wp - 2;
-	write_packet_ack_nack_desc_egprs(bts, dest, wp, &tbf->m_window, is_final, bits_ack_nack);
+	write_packet_ack_nack_desc_egprs(bts, dest, wp, tbf->window(), is_final, bits_ack_nack);
 
 	bitvec_write_field(dest, &wp, 0, 1); // fixed 0
 	bitvec_write_field(dest, &wp, 0, 1); // 0: don't have REL 5
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 6847e181..48cfb6f3 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -1491,6 +1491,11 @@ bool gprs_rlcmac_tbf::is_control_ts(uint8_t ts) const
 	return ts == control_ts;
 }
 
+gprs_rlc_ul_window *gprs_rlcmac_ul_tbf::window()
+{
+	return &m_window;
+}
+
 struct gprs_rlcmac_ul_tbf *handle_tbf_reject(struct gprs_rlcmac_bts *bts,
 			GprsMs *ms, uint32_t tlli, uint8_t trx_no, uint8_t ts)
 {
diff --git a/src/tbf.h b/src/tbf.h
index 6d7edbcf..21762d3b 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -193,8 +193,6 @@ struct gprs_rlcmac_tbf {
 	GprsMs *ms() const;
 	void set_ms(GprsMs *ms);
 
-	gprs_rlc_window *window();
-
 	uint8_t tsc() const;
 
 	int rlcmac_diag();
@@ -456,7 +454,7 @@ inline void gprs_rlcmac_tbf::disable_egprs()
 
 struct gprs_rlcmac_dl_tbf : public gprs_rlcmac_tbf {
 	gprs_rlcmac_dl_tbf(BTS *bts);
-
+	gprs_rlc_dl_window *window();
 	void cleanup();
 	void enable_egprs();
 	/* dispatch Unitdata.DL messages */
@@ -496,7 +494,6 @@ struct gprs_rlcmac_dl_tbf : public gprs_rlcmac_tbf {
 	 * All states that need reset must be in this struct, so this is why
 	 * variables are in both (dl and ul) structs and not outside union.
 	 */
-	gprs_rlc_dl_window m_window;
 	int32_t m_tx_counter; /* count all transmitted blocks */
 	uint8_t m_wait_confirm; /* wait for CCCH IMM.ASS cnf */
 	bool m_dl_ack_requested;
@@ -547,11 +544,18 @@ protected:
 	enum egprs_rlcmac_dl_spb get_egprs_dl_spb(int bsn);
 
 	struct osmo_timer_list m_llc_timer;
+
+	/* Please note that all variables below will be reset when changing
+	 * from WAIT RELEASE back to FLOW state (re-use of TBF).
+	 * All states that need reset must be in this struct, so this is why
+	 * variables are in both (dl and ul) structs and not outside union.
+	 */
+	gprs_rlc_dl_window m_window;
 };
 
 struct gprs_rlcmac_ul_tbf : public gprs_rlcmac_tbf {
 	gprs_rlcmac_ul_tbf(BTS *bts);
-
+	gprs_rlc_ul_window *window();
 	struct msgb *create_ul_ack(uint32_t fn, uint8_t ts);
 	bool ctrl_ack_to_toggle();
 	bool handle_ctrl_ack();
@@ -590,7 +594,6 @@ struct gprs_rlcmac_ul_tbf : public gprs_rlcmac_tbf {
 	 * All states that need reset must be in this struct, so this is why
 	 * variables are in both (dl and ul) structs and not outside union.
 	 */
-	gprs_rlc_ul_window m_window;
 	int32_t m_rx_counter; /* count all received blocks */
 	uint8_t m_n3103;	/* N3103 counter */
 	uint8_t m_usf[8];	/* list USFs per PDCH (timeslot) */
@@ -602,6 +605,13 @@ struct gprs_rlcmac_ul_tbf : public gprs_rlcmac_tbf {
 
 protected:
 	void maybe_schedule_uplink_acknack(const gprs_rlc_data_info *rlc);
+
+	/* Please note that all variables below will be reset when changing
+	 * from WAIT RELEASE back to FLOW state (re-use of TBF).
+	 * All states that need reset must be in this struct, so this is why
+	 * variables are in both (dl and ul) structs and not outside union.
+	 */
+	gprs_rlc_ul_window m_window;
 };
 
 #ifdef __cplusplus
@@ -657,14 +667,4 @@ inline gprs_rlcmac_dl_tbf *as_dl_tbf(gprs_rlcmac_tbf *tbf)
 		return NULL;
 }
 
-inline gprs_rlc_window *gprs_rlcmac_tbf::window()
-{
-	switch (direction)
-	{
-	case GPRS_RLCMAC_UL_TBF: return &as_ul_tbf(this)->m_window;
-	case GPRS_RLCMAC_DL_TBF: return &as_dl_tbf(this)->m_window;
-	}
-	return NULL;
-}
-
 #endif
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index e3b0a9de..45c9a8f4 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -969,6 +969,11 @@ int gprs_rlcmac_dl_tbf::analyse_errors(char *show_rbb, uint8_t ssn,
 	return lost * 100 / (lost + received);
 }
 
+gprs_rlc_dl_window *gprs_rlcmac_dl_tbf::window()
+{
+	return &m_window;
+}
+
 int gprs_rlcmac_dl_tbf::update_window(unsigned first_bsn,
 	const struct bitvec *rbb)
 {
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index fbbe366b..ac15b2cc 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -297,9 +297,9 @@ static void test_tbf_final_ack(enum test_tbf_final_ack_mode test_mode)
 
 /* Receive an ACK */
 #define RCV_ACK(fin, tbf, rbb) do {					\
-		tbf->rcvd_dl_ack(fin, tbf->m_window.v_s(), rbb);	\
+		tbf->rcvd_dl_ack(fin, tbf->window()->v_s(), rbb);	\
 		if (!fin)						\
-			OSMO_ASSERT(tbf->m_window.window_empty());	\
+			OSMO_ASSERT(tbf->window()->window_empty());	\
 	} while(0)
 
 static void test_tbf_delayed_release()
@@ -1725,7 +1725,7 @@ static void test_tbf_egprs_two_phase_puan(void)
 		"Got MS: TLLI = 0x%08x, TA = %d\n", ms->tlli(), ms->ta());
 	send_dl_data(&the_bts, tlli, imsi, test_data, sizeof(test_data));
 
-	ul_tbf->m_window.reset_state();
+	ul_tbf->window()->reset_state();
 	/* Function to generate URBB with length */
 	ul_tbf = establish_ul_tbf_two_phase_puan_URBB_with_length(&the_bts, ts_no, tlli, &fn,
 		qta, ms_class, egprs_ms_class, ul_tbf);
@@ -1737,7 +1737,7 @@ static void test_tbf_egprs_two_phase_puan(void)
 
 	send_dl_data(&the_bts, tlli, imsi, test_data, sizeof(test_data));
 
-	ul_tbf->m_window.reset_state();
+	ul_tbf->window()->reset_state();
 	/* Function to generate CRBB */
 	bts->ws_base = 128;
 	bts->ws_pdch = 64;
@@ -2529,7 +2529,7 @@ static void test_tbf_epdan_out_of_rx_window(void)
 
 	dl_tbf = create_dl_tbf(&the_bts, ms_class, egprs_ms_class, &trx_no);
 	dl_tbf->update_ms(tlli, GPRS_RLCMAC_DL_TBF);
-	prlcdlwindow = &dl_tbf->m_window;
+	prlcdlwindow = dl_tbf->window();
 	prlcmvb = &prlcdlwindow->m_v_b;
 	prlcdlwindow->m_v_s = 1288;
 	prlcdlwindow->m_v_a = 1176;
@@ -2561,7 +2561,7 @@ static void test_tbf_epdan_out_of_rx_window(void)
 
 	Decoding::decode_egprs_acknack_bits(
 		&ack_nack->EGPRS_AckNack.Desc, &bits,
-		&bsn_begin, &bsn_end, &dl_tbf->m_window);
+		&bsn_begin, &bsn_end, dl_tbf->window());
 
 	dl_tbf->rcvd_dl_ack(
 		ack_nack->EGPRS_AckNack.Desc.FINAL_ACK_INDICATION,
@@ -2755,17 +2755,17 @@ static void tbf_cleanup(gprs_rlcmac_dl_tbf *dl_tbf)
 }
 
 #define NACK(tbf, x) do {					\
-		tbf->m_window.m_v_b.mark_nacked(x);		\
-		OSMO_ASSERT(tbf->m_window.m_v_b.is_nacked(x));	\
+		tbf->window()->m_v_b.mark_nacked(x);		\
+		OSMO_ASSERT(tbf->window()->m_v_b.is_nacked(x));	\
 	} while(0)
 
 #define CHECK_UNACKED(tbf, cs, bsn) do {				             \
-		OSMO_ASSERT(tbf->m_window.m_v_b.is_unacked(bsn));	             \
+		OSMO_ASSERT(tbf->window()->m_v_b.is_unacked(bsn));	             \
 		OSMO_ASSERT(tbf->m_rlc.block(bsn)->cs_current_trans.to_num() == cs); \
 	} while(0)
 
 #define CHECK_NACKED(tbf, cs, bsn) do {					             \
-		OSMO_ASSERT(tbf->m_window.m_v_b.is_nacked(bsn));	             \
+		OSMO_ASSERT(tbf->window()->m_v_b.is_nacked(bsn));	             \
 		OSMO_ASSERT(tbf->m_rlc.block(bsn)->cs_current_trans.to_num() == cs); \
 	} while(0)