Merge branch 'sysmocom/gprs-window-handling' into sysmocom/master

Daniel and me worked on the window handling. We now test that what
we encode is compatible to what we decode. The char array now includes
SSN-1 to the right of the array (reflecting a time axis that grows
towards the right..)

8 files changed, 170 insertions, 34 deletions

diff --git a/src/decoding.cpp b/src/decoding.cpp
index 0f70872a..fce8124f 100644
--- a/src/decoding.cpp
+++ b/src/decoding.cpp
@@ -85,14 +85,16 @@ uint8_t Decoding::get_ms_class_by_capability(MS_Radio_Access_capability_t *cap)
 
 /**
  * show_rbb needs to be an array with 65 elements
+ * The index of the array is the bit position in the rbb
+ * (show_rbb[63] relates to BSN ssn-1)
  */
 void Decoding::extract_rbb(const uint8_t *rbb, char *show_rbb)
 {
-	for (int i = 63; i >= 0; i--) {
+	for (int i = 0; i < 64; i++) {
 		uint8_t bit;
 
-		bit = (rbb[i >> 3]  >>  (7 - (i&7)))   & 1;
-		show_rbb[i] = bit ? '1' : 'o';
+		bit = !!(rbb[i/8] & (1<<(7-i%8)));
+		show_rbb[i] = bit ? 'R' : 'I';
 	}
 
 	show_rbb[64] = '\0';
diff --git a/src/encoding.cpp b/src/encoding.cpp
index 085e7c34..94799868 100644
--- a/src/encoding.cpp
+++ b/src/encoding.cpp
@@ -352,6 +352,27 @@ int Encoding::write_paging_request(bitvec * dest, uint8_t *ptmsi, uint16_t ptmsi
 	return plen;
 }
 
+/**
+ * The index of the array show_rbb is the bit position inside the rbb
+ * (show_rbb[63] relates to BSN ssn-1)
+ */
+void Encoding::encode_rbb(const char *show_rbb, uint8_t *rbb)
+{
+	uint8_t rbb_byte = 0;
+
+	// RECEIVE_BLOCK_BITMAP
+	for (int i = 0; i < 64; i++) {
+		/* Set bit at the appropriate position (see 3GPP TS 04.60 9.1.8.1) */
+		if (show_rbb[i] == 'R')
+			rbb_byte |= 1<< (7-(i%8));
+
+		if((i%8) == 7) {
+			rbb[i/8] = rbb_byte;
+			rbb_byte = 0;
+		}
+	}
+}
+
 /* generate uplink ack */
 void Encoding::write_packet_uplink_ack(struct gprs_rlcmac_bts *bts,
 	RlcMacDownlink_t * block, struct gprs_rlcmac_tbf *tbf,
@@ -359,12 +380,9 @@ void Encoding::write_packet_uplink_ack(struct gprs_rlcmac_bts *bts,
 {
 	// Packet Uplink Ack/Nack  TS 44.060 11.2.28
 
-	char show_v_n[65];
+	char rbb[65];
 
-	uint8_t rbb = 0;
-	uint16_t i, bbn;
-	uint16_t mod_sns = (tbf->sns() - 1);
-	char bit;
+	tbf->dir.ul.window.update_rbb(&tbf->dir.ul.v_n, rbb);
 
 	LOGP(DRLCMACUL, LOGL_DEBUG, "Encoding Ack/Nack for %s "
 		"(final=%d)\n", tbf_name(tbf), final);
@@ -381,25 +399,14 @@ void Encoding::write_packet_uplink_ack(struct gprs_rlcmac_bts *bts,
 	block->u.Packet_Uplink_Ack_Nack.UnionType    = 0x0;      // PU_AckNack_GPRS = on
 	block->u.Packet_Uplink_Ack_Nack.u.PU_AckNack_GPRS_Struct.CHANNEL_CODING_COMMAND                        = bts->initial_cs_ul - 1;             // CS1
 	block->u.Packet_Uplink_Ack_Nack.u.PU_AckNack_GPRS_Struct.Ack_Nack_Description.FINAL_ACK_INDICATION     = final;           // FINAL ACK INDICATION
-	block->u.Packet_Uplink_Ack_Nack.u.PU_AckNack_GPRS_Struct.Ack_Nack_Description.STARTING_SEQUENCE_NUMBER = tbf->dir.ul.window.v_r(); // STARTING_SEQUENCE_NUMBER
-	// RECEIVE_BLOCK_BITMAP
-	for (i = 0, bbn = (tbf->dir.ul.window.v_r() - 64) & mod_sns; i < 64;
-	     i++, bbn = (bbn + 1) & mod_sns) {
-		bit = tbf->dir.ul.v_n.state(bbn);
-		show_v_n[i] = bit;
-		if (bit == 'R')
-			rbb = (rbb << 1)|1;
-		else
-			rbb = (rbb << 1);
-		if((i%8) == 7)
-		{
-			block->u.Packet_Uplink_Ack_Nack.u.PU_AckNack_GPRS_Struct.Ack_Nack_Description.RECEIVED_BLOCK_BITMAP[i/8] = rbb;
-			rbb = 0;
-		}
-	}
-	show_v_n[64] = '\0';
+	block->u.Packet_Uplink_Ack_Nack.u.PU_AckNack_GPRS_Struct.Ack_Nack_Description.STARTING_SEQUENCE_NUMBER = tbf->dir.ul.window.ssn(); // STARTING_SEQUENCE_NUMBER
+
+	encode_rbb(rbb, block->u.Packet_Uplink_Ack_Nack.u.PU_AckNack_GPRS_Struct.Ack_Nack_Description.RECEIVED_BLOCK_BITMAP);
+
+	/* rbb is not NULL terminated */
+	rbb[64] = 0;
 	LOGP(DRLCMACUL, LOGL_DEBUG, "- V(N): \"%s\" R=Received "
-		"N=Not-Received\n", show_v_n);
+		"I=Invalid\n", rbb);
 
 	block->u.Packet_Uplink_Ack_Nack.u.PU_AckNack_GPRS_Struct.UnionType              = 0x0; // Fixed Allocation Dummy = on
 	block->u.Packet_Uplink_Ack_Nack.u.PU_AckNack_GPRS_Struct.u.FixedAllocationDummy = 0x0; // Fixed Allocation Dummy
diff --git a/src/encoding.h b/src/encoding.h
index d0f8f0ab..e62c2c8f 100644
--- a/src/encoding.h
+++ b/src/encoding.h
@@ -54,6 +54,8 @@ public:
 			uint8_t old_downlink, struct gprs_rlcmac_tbf *tbf, uint8_t poll,
 			uint8_t alpha, uint8_t gamma, int8_t ta_idx, uint8_t ta_ts);
 
+	static void encode_rbb(const char *show_rbb, uint8_t *rbb);
+
 	static void write_packet_uplink_ack(struct gprs_rlcmac_bts *bts, RlcMacDownlink_t * block, struct gprs_rlcmac_tbf *tbf,
 		        uint8_t final);
 
diff --git a/src/rlc.cpp b/src/rlc.cpp
index 915da87f..f31670ea 100644
--- a/src/rlc.cpp
+++ b/src/rlc.cpp
@@ -84,19 +84,23 @@ int gprs_rlc_v_b::count_unacked(const gprs_rlc_dl_window &w)
 	return unacked;
 }
 
+static uint16_t bitnum_to_bsn(int bitnum, uint16_t ssn, uint16_t mod_sns)
+{
+	return (ssn - 1 - bitnum) & mod_sns;
+}
+
 void gprs_rlc_v_b::update(BTS *bts, char *show_rbb, uint8_t ssn,
 			const gprs_rlc_dl_window &w,
 			uint16_t *lost, uint16_t *received)
 {
-	uint16_t bsn;
-	int i;
-
 	/* SSN - 1 is in range V(A)..V(S)-1 */
-	for (i = 63, bsn = (ssn - 1) & w.mod_sns();
-	     i >= 0 && bsn != ((w.v_a() - 1) & w.mod_sns());
-	     i--, bsn = (bsn - 1) & w.mod_sns()) {
+	for (int bitpos = 0; bitpos < w.ws(); bitpos++) {
+		uint16_t bsn = bitnum_to_bsn(bitpos, ssn, w.mod_sns());
 
-		if (show_rbb[i] == '1') {
+		if (bsn == ((w.v_a() - 1) & w.mod_sns()))
+			break;
+
+		if (show_rbb[w.ws() - 1 - bitpos] == 'R') {
 			LOGP(DRLCMACDL, LOGL_DEBUG, "- got ack for BSN=%d\n", bsn);
 			if (!is_acked(bsn))
 				*received += 1;
@@ -146,6 +150,18 @@ void gprs_rlc_v_n::reset()
 	memset(m_v_n, 0x0, sizeof(m_v_n));
 }
 
+/* Update the receive block bitmap */
+void gprs_rlc_ul_window::update_rbb(const gprs_rlc_v_n *v_n, char *rbb)
+{
+	int i;
+	for (i=0; i < ws(); i++) {
+		if (v_n->is_received(ssn()-1-i))
+			rbb[ws()-1-i] = 'R';
+		else
+			rbb[ws()-1-i] = 'I';
+	}
+}
+
 /* Raise V(R) to highest received sequence number not received. */
 void gprs_rlc_ul_window::raise_v_r(const uint16_t bsn, gprs_rlc_v_n *v_n)
 {
diff --git a/src/rlc.h b/src/rlc.h
index 0ecce407..d68afab8 100644
--- a/src/rlc.h
+++ b/src/rlc.h
@@ -89,8 +89,11 @@ struct gprs_rlc_ul_window {
 	const uint16_t v_r() const;
 	const uint16_t v_q() const;
 
+	const uint16_t ssn() const;
+
 	bool is_in_window(uint8_t bsn) const;
 
+	void update_rbb(const gprs_rlc_v_n *v_n, char *rbb);
 	void raise_v_r(int moves);
 	void raise_v_r(const uint16_t bsn, gprs_rlc_v_n *v_n);
 	uint16_t raise_v_q(gprs_rlc_v_n *v_n);
@@ -339,6 +342,11 @@ inline const uint16_t gprs_rlc_ul_window::v_q() const
 	return m_v_q;
 }
 
+inline const uint16_t gprs_rlc_ul_window::ssn() const
+{
+	return m_v_r;
+}
+
 inline void gprs_rlc_ul_window::raise_v_r(int moves)
 {
 	m_v_r = (m_v_r + moves) & mod_sns();
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 3f4da560..8ffafce8 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -1348,7 +1348,7 @@ int gprs_rlcmac_tbf::update_window(const uint8_t ssn, const uint8_t *rbb)
 	Decoding::extract_rbb(rbb, show_rbb);
 	/* show received array in debug (bit 64..1) */
 	LOGP(DRLCMACDL, LOGL_DEBUG, "- ack:  (BSN=%d)\"%s\""
-		"(BSN=%d)  1=ACK o=NACK\n", (ssn - 64) & mod_sns,
+		"(BSN=%d)  R=ACK I=NACK\n", (ssn - 64) & mod_sns,
 		show_rbb, (ssn - 1) & mod_sns);
 
 	/* apply received array to receive state (SSN-64..SSN-1) */
diff --git a/tests/types/TypesTest.cpp b/tests/types/TypesTest.cpp
index 3859dfb9..ea9fdbeb 100644
--- a/tests/types/TypesTest.cpp
+++ b/tests/types/TypesTest.cpp
@@ -22,6 +22,8 @@
 #include "bts.h"
 #include "tbf.h"
 #include "gprs_debug.h"
+#include "encoding.h"
+#include "decoding.h"
 
 extern "C" {
 #include <osmocom/core/application.h>
@@ -30,6 +32,14 @@ extern "C" {
 #include <osmocom/core/utils.h>
 }
 
+#define OSMO_ASSERT_STR_EQ(a, b) \
+	do { \
+		if (strcmp(a, b)) { \
+			printf("String mismatch:\nGot:\t%s\nWant:\t%s\n", a, b); \
+			OSMO_ASSERT(false); \
+		} \
+	} while (0)
+
 void *tall_pcu_ctx;
 int16_t spoof_mnc = 0, spoof_mcc = 0;
 
@@ -185,6 +195,10 @@ static void test_rlc_dl_ul_basic()
 		gprs_rlc_ul_window ul_win = { 0, };
 		gprs_rlc_v_n v_n;
 		int count;
+		const char *rbb;
+		char win_rbb[65];
+		uint8_t bin_rbb[8];
+		win_rbb[64] = '\0';
 
 		v_n.reset();
 
@@ -194,6 +208,15 @@ static void test_rlc_dl_ul_basic()
 
 		OSMO_ASSERT(!v_n.is_received(0));
 
+		rbb = "IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII";
+		OSMO_ASSERT(ul_win.ssn() == 0);
+		ul_win.update_rbb(&v_n, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+		Encoding::encode_rbb(win_rbb, bin_rbb);
+		printf("rbb: %s\n", osmo_hexdump(bin_rbb, sizeof(bin_rbb)));
+		Decoding::extract_rbb(bin_rbb, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+
 		/* simulate to have received 0, 1 and 5 */
 		OSMO_ASSERT(ul_win.is_in_window(0));
 		v_n.mark_received(0);
@@ -204,6 +227,15 @@ static void test_rlc_dl_ul_basic()
 		OSMO_ASSERT(ul_win.v_r() == 1);
 		OSMO_ASSERT(count == 1);
 
+		rbb = "IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIR";
+		OSMO_ASSERT(ul_win.ssn() == 1);
+		ul_win.update_rbb(&v_n, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+		Encoding::encode_rbb(win_rbb, bin_rbb);
+		printf("rbb: %s\n", osmo_hexdump(bin_rbb, sizeof(bin_rbb)));
+		Decoding::extract_rbb(bin_rbb, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+
 		OSMO_ASSERT(ul_win.is_in_window(1));
 		v_n.mark_received(1);
 		ul_win.raise_v_r(1, &v_n);
@@ -213,6 +245,15 @@ static void test_rlc_dl_ul_basic()
 		OSMO_ASSERT(ul_win.v_r() == 2);
 		OSMO_ASSERT(count == 1);
 
+		rbb = "IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIRR";
+		OSMO_ASSERT(ul_win.ssn() == 2);
+		ul_win.update_rbb(&v_n, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+		Encoding::encode_rbb(win_rbb, bin_rbb);
+		printf("rbb: %s\n", osmo_hexdump(bin_rbb, sizeof(bin_rbb)));
+		Decoding::extract_rbb(bin_rbb, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+
 		OSMO_ASSERT(ul_win.is_in_window(5));
 		v_n.mark_received(5);
 		ul_win.raise_v_r(5, &v_n);
@@ -222,6 +263,15 @@ static void test_rlc_dl_ul_basic()
 		OSMO_ASSERT(ul_win.v_r() == 6);
 		OSMO_ASSERT(count == 0);
 
+		rbb = "IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIRRIIIR";
+		OSMO_ASSERT(ul_win.ssn() == 6);
+		ul_win.update_rbb(&v_n, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+		Encoding::encode_rbb(win_rbb, bin_rbb);
+		printf("rbb: %s\n", osmo_hexdump(bin_rbb, sizeof(bin_rbb)));
+		Decoding::extract_rbb(bin_rbb, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+
 		OSMO_ASSERT(ul_win.is_in_window(65));
 		OSMO_ASSERT(ul_win.is_in_window(2));
 		OSMO_ASSERT(v_n.is_received(5));
@@ -233,6 +283,15 @@ static void test_rlc_dl_ul_basic()
 		OSMO_ASSERT(ul_win.v_q() == 2);
 		OSMO_ASSERT(ul_win.v_r() == 66);
 
+		rbb = "IIIRIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIR";
+		OSMO_ASSERT(ul_win.ssn() == 66);
+		ul_win.update_rbb(&v_n, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+		Encoding::encode_rbb(win_rbb, bin_rbb);
+		printf("rbb: %s\n", osmo_hexdump(bin_rbb, sizeof(bin_rbb)));
+		Decoding::extract_rbb(bin_rbb, win_rbb);
+		OSMO_ASSERT_STR_EQ(win_rbb, rbb);
+
 		OSMO_ASSERT(ul_win.is_in_window(2));
 		OSMO_ASSERT(!ul_win.is_in_window(66));
 		v_n.mark_received(2);
@@ -274,6 +333,42 @@ static void test_rlc_dl_ul_basic()
 		count = ul_win.raise_v_q(&v_n);
 		OSMO_ASSERT(count == 0);
 	}
+
+	{
+		int count;
+		uint8_t rbb[8];
+		uint16_t lost = 0, recv = 0;
+		char show_rbb[65];
+		BTS dummy_bts;
+		gprs_rlc_dl_window dl_win = { 0, };
+		gprs_rlc_v_b v_b;
+
+		v_b.reset();
+
+		OSMO_ASSERT(dl_win.window_empty());
+		OSMO_ASSERT(!dl_win.window_stalled());
+		OSMO_ASSERT(dl_win.distance() == 0);
+
+		dl_win.increment_send();
+		OSMO_ASSERT(!dl_win.window_empty());
+		OSMO_ASSERT(!dl_win.window_stalled());
+		OSMO_ASSERT(dl_win.distance() == 1);
+
+		for (int i = 0; i < 35; ++i) {
+			dl_win.increment_send();
+			OSMO_ASSERT(!dl_win.window_empty());
+			OSMO_ASSERT(dl_win.distance() == i + 2);
+		}
+
+		uint8_t rbb_cmp[8] = { 0x00, 0x00, 0x00, 0x07, 0xff, 0xff, 0xff, 0xff };
+		Decoding::extract_rbb(rbb_cmp, show_rbb);
+		printf("show_rbb: %s\n", show_rbb);
+
+		v_b.update(&dummy_bts, show_rbb, 35, dl_win, &lost, &recv);
+		OSMO_ASSERT(lost == 0);
+		OSMO_ASSERT(recv == 35);
+
+	}
 }
 
 int main(int argc, char **argv)
diff --git a/tests/types/TypesTest.ok b/tests/types/TypesTest.ok
index 9d0cbfbb..6ca2717c 100644
--- a/tests/types/TypesTest.ok
+++ b/tests/types/TypesTest.ok
@@ -1 +1,7 @@
 Making some basic type testing.
+rbb: 00 00 00 00 00 00 00 00 
+rbb: 00 00 00 00 00 00 00 01 
+rbb: 00 00 00 00 00 00 00 03 
+rbb: 00 00 00 00 00 00 00 31 
+rbb: 10 00 00 00 00 00 00 01 
+show_rbb: IIIIIIIIIIIIIIIIIIIIIIIIIIIIIRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR