Change in osmo-ttcn3-hacks[master]: PCU: refactor and simplify f_rx_rlcmac_dl_block_exp_data()

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

fixeria gerrit-no-reply at lists.osmocom.org
Sun May 10 20:50:01 UTC 2020


fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18186 )


Change subject: PCU: refactor and simplify f_rx_rlcmac_dl_block_exp_data()
......................................................................

PCU: refactor and simplify f_rx_rlcmac_dl_block_exp_data()

This function was written in a way that it tries to do as
many different (but related) things as possible:

  a) send an RTS.req to the IUT, expect a DATA.ind in return,
  b) decode RLC/MAC message contained in the received DATA.ind,
  c) make sure that it's either GPRS or EGPRS data block,
  d) calculate the last TDMA frame number of RRBP using
     f_rrbp_ack_fn() regardless of its validity,
  e) make sure that the block contains a given LLC payload.

Everything is ok except point d). The problem is that this is
only the case for the first block of RRBP, and not applicable
to the rest having 'rrbp_valid' flag unset. Furthermore, this
function did not match GPRS DL blocks with 'rrbp_valid' flag
unset, for some odd reason.

Let's move RRBP calculation into a separate function called
f_dl_block_ack_fn() and return TDMA frame number of the
received DATA.ind message instead.

Among with that, there are more little changes:

  - simplify matching of (E)GPRS DL data blocks,
  - use 'in' qualifier in parameter list where possible,
  - turn parameter 'data' into a template (present).

Change-Id: I1528408b4399d0a149a23961805277eaab90d407
Signed-off-by: Vadim Yanitskiy <axilirator at gmail.com>
---
M library/RLCMAC_Templates.ttcn
M pcu/PCU_Tests.ttcn
2 files changed, 95 insertions(+), 72 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/86/18186/1

diff --git a/library/RLCMAC_Templates.ttcn b/library/RLCMAC_Templates.ttcn
index 675a081..dd1371f 100644
--- a/library/RLCMAC_Templates.ttcn
+++ b/library/RLCMAC_Templates.ttcn
@@ -497,6 +497,9 @@
 		}
 	}
 
+	/* Either GPRS or EGPRS data block with arbitrary contents */
+	template RlcmacDlBlock tr_RLCMAC_DATA := (tr_RLCMAC_DATA_GPRS, tr_RLCMAC_DATA_EGPRS);
+
 	template RlcmacDlBlock tr_RLCMAC_DATA_GPRS(template (present) boolean rrbp_valid := ?,
 						   template (present) MacRrbp rrbp := ?,
 						   template (present) uint3_t usf := ?) := {
diff --git a/pcu/PCU_Tests.ttcn b/pcu/PCU_Tests.ttcn
index 3d194a9..0a6b410 100644
--- a/pcu/PCU_Tests.ttcn
+++ b/pcu/PCU_Tests.ttcn
@@ -599,78 +599,64 @@
 	}
 }
 
-private function f_rlcmac_dl_block_verify_data_gprs(RlcmacDlBlock dl_block, uint32_t dl_fn,
-						    out uint32_t ack_fn, octetstring data,
+/* This function does what could probably be done with templates */
+private function f_rlcmac_dl_block_verify_data_gprs(in RlcmacDlDataBlock data_block,
+						    template (present) octetstring data := ?,
 						    template (present) uint7_t exp_bsn := ?,
 						    template (present) CodingScheme exp_cs := ?)
 runs on RAW_PCU_Test_CT {
-	log("verifying dl data block (gprs): ", dl_block);
-
-	ack_fn := f_rrbp_ack_fn(dl_fn, dl_block.data.mac_hdr.mac_hdr.rrbp);
-
-	if (not match(dl_block.data.mac_hdr.hdr_ext.bsn, exp_bsn)) {
+	if (not match(data_block.mac_hdr.hdr_ext.bsn, exp_bsn)) {
 		setverdict(fail, "DL block BSN doesn't match: ",
-			   dl_block.data.blocks[0].hdr.length_ind, " vs exp ", exp_bsn);
+			   data_block.mac_hdr.hdr_ext.bsn, " vs exp ", exp_bsn);
 	}
 
-	if (lengthof(dl_block.data.blocks) < 1) {
-		setverdict(fail, "DL block has no LLC payload: ", dl_block);
+	if (lengthof(data_block.blocks) < 1) {
+		setverdict(fail, "DL block has no LLC payload: ", data_block);
 		f_shutdown(__BFILE__, __LINE__);
 	}
 
-	if (ispresent(dl_block.data.blocks[0].hdr) and dl_block.data.blocks[0].hdr.length_ind != lengthof(data)) {
-		setverdict(fail, "DL block has LLC header with wrong expected size: ",
-			   dl_block.data.blocks[0].hdr.length_ind, " vs ", lengthof(data));
-		f_shutdown(__BFILE__, __LINE__);
-	}
-
-	if (dl_block.data.blocks[0].payload != data) {
-		setverdict(fail, "Failed to match content of LLC payload in DL Block: ", dl_block, " vs ", data);
+	if (not match(data_block.blocks[0].payload, data)) {
+		setverdict(fail, "Failed to match content of LLC payload in DL Block: ",
+			   data_block.blocks[0].payload, " vs ", data);
 		f_shutdown(__BFILE__, __LINE__);
 	}
 
 	/* Check next data blocks contain dummy frames */
-	if (lengthof(dl_block.data.blocks) > 1 and substr(dl_block.data.blocks[1].payload, 0, 3) != '43C001'O) {
-		setverdict(fail, "Second data payload is not a dummy frame: ", dl_block.data.blocks[1].payload);
+	if (lengthof(data_block.blocks) > 1 and substr(data_block.blocks[1].payload, 0, 3) != '43C001'O) {
+		setverdict(fail, "Second data payload is not a dummy frame: ",
+			   data_block.blocks[1].payload);
 		f_shutdown(__BFILE__, __LINE__);
 	}
 
 	/* TODO: check exp_cs */
 }
 
-private function f_rlcmac_dl_block_verify_data_egprs(RlcmacDlBlock dl_block, uint32_t dl_fn,
-						     out uint32_t ack_fn, octetstring data,
+/* This function does what could probably be done with templates */
+private function f_rlcmac_dl_block_verify_data_egprs(in RlcmacDlEgprsDataBlock data_block,
+						     template (present) octetstring data := ?,
 						     template (present) uint14_t exp_bsn := ?,
 						     template (present) CodingScheme exp_cs := ?)
 runs on RAW_PCU_Test_CT {
-	log("verifying dl data block (egprs): ", dl_block);
-
-	ack_fn := f_rrbp_ack_fn(dl_fn, dl_block.data_egprs.mac_hdr.rrbp);
-
-	if (not match(dl_block.data_egprs.mac_hdr.bsn1, exp_bsn)) {
+	if (not match(data_block.mac_hdr.bsn1, exp_bsn)) {
 		setverdict(fail, "DL block BSN doesn't match: ",
-			   dl_block.data_egprs.blocks[0].hdr.length_ind, " vs exp ", exp_bsn);
+			   data_block.mac_hdr.bsn1, " vs exp ", exp_bsn);
 	}
 
-	if (lengthof(dl_block.data_egprs.blocks) < 1) {
-		setverdict(fail, "DL block has no LLC payload: ", dl_block);
+	if (lengthof(data_block.blocks) < 1) {
+		setverdict(fail, "DL block has no LLC payload: ", data_block);
 		f_shutdown(__BFILE__, __LINE__);
 	}
 
-	if (ispresent(dl_block.data_egprs.blocks[0].hdr) and dl_block.data_egprs.blocks[0].hdr.length_ind != lengthof(data)) {
-		setverdict(fail, "DL block has LLC header with wrong expected size: ",
-			   dl_block.data_egprs.blocks[0].hdr.length_ind, " vs ", lengthof(data));
-		f_shutdown(__BFILE__, __LINE__);
-	}
-
-	if (dl_block.data_egprs.blocks[0].payload != data) {
-		setverdict(fail, "Failed to match content of LLC payload in DL Block: ", dl_block, " vs ", data);
+	if (not match(data_block.blocks[0].payload, data)) {
+		setverdict(fail, "Failed to match content of LLC payload in DL Block: ",
+			   data_block.blocks[0].payload, " vs ", data);
 		f_shutdown(__BFILE__, __LINE__);
 	}
 
 	/* Check next data blocks contain dummy frames */
-	if (lengthof(dl_block.data_egprs.blocks) > 1 and substr(dl_block.data_egprs.blocks[1].payload, 0, 3) != '43C001'O) {
-		setverdict(fail, "Second data payload is not a dummy frame: ", dl_block.data.blocks[1].payload);
+	if (lengthof(data_block.blocks) > 1 and substr(data_block.blocks[1].payload, 0, 3) != '43C001'O) {
+		setverdict(fail, "Second data payload is not a dummy frame: ",
+			   data_block.blocks[1].payload);
 		f_shutdown(__BFILE__, __LINE__);
 	}
 
@@ -678,36 +664,62 @@
 		See wireshark's egprs_Header_type1_coding_puncturing_scheme_to_mcs. */
 }
 
-private function f_rx_rlcmac_dl_block_exp_data(out RlcmacDlBlock dl_block, out uint32_t ack_fn,
-					       octetstring data,
+/* High level (task specific) helper for receiving and matching GPRS/EGPRS data blocks */
+private function f_rx_rlcmac_dl_block_exp_data(out RlcmacDlBlock dl_block, out uint32_t dl_fn,
+					       template (present) octetstring data := ?,
 					       template (present) uint7_t exp_bsn := ?,
 					       template (present) CodingScheme exp_cs := ?)
 runs on RAW_PCU_Test_CT {
-	var PCUIF_Message pcu_msg;
-	var uint32_t dl_fn;
-	var boolean is_egprs := false;
-	/* FIXME: for some reason, this template expects blocks with 'rrbp_valid' flag set */
-	var template RlcmacDlBlock dl_template := tr_RLCMAC_DATA_GPRS(rrbp_valid := true);
-	dl_template.data.blocks := ?;
-
+	/* FIXME: ideally we should use an alt statement with timeout here, rather than
+	 * having +100500 layers of abstraction. This would facilitate developing the
+	 * multi-TBF/-TRX/-BTS tests, where you cannot expect that the first received
+	 * block is exactly what you need. */
 	f_rx_rlcmac_dl_block(dl_block, dl_fn);
-	if (not match(dl_block, dl_template)) {
-		dl_template := tr_RLCMAC_DATA_EGPRS;
-		dl_template.data_egprs.blocks := ?;
-		if (not match(dl_block, dl_template)) {
-			setverdict(fail, "Failed to match Packet data: ", dl_block, " vs ", dl_template);
-			f_shutdown(__BFILE__, __LINE__);
-		}
-		is_egprs := true;
+
+	/* Make sure it's either GPRS or EGPRS data block */
+	if (not match(dl_block, tr_RLCMAC_DATA)) {
+		setverdict(fail, "Failed to match DL DATA: ", dl_block, " vs ", tr_RLCMAC_DATA);
+		f_shutdown(__BFILE__, __LINE__);
 	}
 
-	if (is_egprs) {
-		f_rlcmac_dl_block_verify_data_egprs(dl_block, dl_fn, ack_fn, data, exp_bsn, exp_cs);
+	if (ischosen(dl_block.data_egprs)) {
+		f_rlcmac_dl_block_verify_data_egprs(dl_block.data_egprs, data, exp_bsn, exp_cs);
+	} else if (ischosen(dl_block.data)) {
+		f_rlcmac_dl_block_verify_data_gprs(dl_block.data, data, exp_bsn, exp_cs);
 	} else {
-		f_rlcmac_dl_block_verify_data_gprs(dl_block, dl_fn, ack_fn, data, exp_bsn, exp_cs);
+		/* Should not happen, but the caller may theoretically give us a template for CTRL */
+		setverdict(fail, "DL block is neither GPRS nor EGPRS data block: ", dl_block);
+		f_shutdown(__BFILE__, __LINE__);
 	}
 }
 
+private function f_dl_block_ack_fn(in RlcmacDlBlock dl_block, uint32_t dl_fn)
+runs on RAW_PCU_Test_CT return uint32_t {
+	var boolean rrbp_valid;
+	var MacRrbp rrbp;
+
+	/* The argument must be either a GPRS or EGPRS data block */
+	if (ischosen(dl_block.data_egprs)) {
+		rrbp_valid := true; /* always valid */
+		rrbp := dl_block.data_egprs.mac_hdr.rrbp;
+	} else if (ischosen(dl_block.data)) {
+		rrbp_valid := dl_block.data.mac_hdr.mac_hdr.rrbp_valid;
+		rrbp := dl_block.data.mac_hdr.mac_hdr.rrbp;
+	} else {
+		/* Should not happen, but the caller may theoretically give us a CTRL block */
+		setverdict(fail, "DL block is neither GPRS nor EGPRS data block: ", dl_block);
+		f_shutdown(__BFILE__, __LINE__);
+	}
+
+	/* Make sure that the given block really needs to be ACKnowledged */
+	if (not rrbp_valid) {
+		setverdict(fail, "DL block shall not be ACKnowledged, field RRBP is not valid");
+		f_shutdown(__BFILE__, __LINE__);
+	}
+
+	return f_rrbp_ack_fn(dl_fn, rrbp);
+}
+
 testcase TC_pcuif_suspend() runs on RAW_PCU_Test_CT {
 	var octetstring ra_id := enc_RoutingAreaIdentification(mp_gb_cfg.cell_id.ra_id);
 	var GprsTlli tlli := 'FFFFFFFF'O;
@@ -1309,6 +1321,7 @@
 	var octetstring data := f_rnd_octstring(10);
 	var boolean ok;
 	var uint32_t sched_fn;
+	var uint32_t dl_fn;
 	var OCT4 tlli := '00000001'O;
 	var AckNackDescription ack_nack_desc := valueof(t_AckNackDescription_init);
 
@@ -1331,11 +1344,12 @@
 
 	/* Wait timer X2002 and DL block is available after CCCH IMM ASS: */
 	f_sleep(X2002);
-	f_rx_rlcmac_dl_block_exp_data(dl_block, sched_fn, data, 0);
+	f_rx_rlcmac_dl_block_exp_data(dl_block, dl_fn, data, 0);
 
 	/* ACK the DL block */
 	f_acknackdesc_ack_block(ack_nack_desc, dl_block, '1'B);
-	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc), 0, sched_fn);
+	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc),
+			     0, f_dl_block_ack_fn(dl_block, dl_fn));
 	/* we are done with the DL-TBF here so far, let's clean up our local state: */
 	ack_nack_desc := valueof(t_AckNackDescription_init)
 
@@ -1347,9 +1361,10 @@
 	f_tx_rlcmac_ul_block(ts_RLCMAC_CTRL_ACK(tlli), 0, sched_fn);
 
 	/* Now that we confirmed the new assignment in the dl-tbf, lets receive the data and ack it */
-	f_rx_rlcmac_dl_block_exp_data(dl_block, sched_fn, data, 0);
+	f_rx_rlcmac_dl_block_exp_data(dl_block, dl_fn, data, 0);
 	f_acknackdesc_ack_block(ack_nack_desc, dl_block, '1'B);
-	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc), 0, sched_fn);
+	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc),
+			     0, f_dl_block_ack_fn(dl_block, dl_fn));
 
 	f_shutdown(__BFILE__, __LINE__, final := true);
 }
@@ -1366,6 +1381,7 @@
 	var octetstring data := f_rnd_octstring(10);
 	var boolean ok;
 	var uint32_t sched_fn;
+	var uint32_t dl_fn;
 	var OCT4 tlli := '00000001'O;
 	var AckNackDescription ack_nack_desc := valueof(t_AckNackDescription_init);
 
@@ -1419,11 +1435,12 @@
 
 	/* Wait timer X2002 and DL block is available after CCCH IMM ASS: */
 	f_sleep(X2002);
-	f_rx_rlcmac_dl_block_exp_data(dl_block, sched_fn, data, 0, exp_cs_mcs);
+	f_rx_rlcmac_dl_block_exp_data(dl_block, dl_fn, data, 0, exp_cs_mcs);
 
 	/* ACK the DL block */
 	f_acknackdesc_ack_block(ack_nack_desc, dl_block, '1'B);
-	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc), 0, sched_fn);
+	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc),
+			     0, f_dl_block_ack_fn(dl_block, dl_fn));
 
 	f_shutdown(__BFILE__, __LINE__, final := true);
 }
@@ -1460,6 +1477,7 @@
 	var octetstring data := f_rnd_octstring(10);
 	var boolean ok;
 	var uint32_t sched_fn;
+	var uint32_t dl_fn;
 	var OCT4 tlli := '00000001'O;
 	var AckNackDescription ack_nack_desc := valueof(t_AckNackDescription_init);
 
@@ -1482,11 +1500,12 @@
 
 	/* Wait timer X2002 and DL block is available after CCCH IMM ASS: */
 	f_sleep(X2002);
-	f_rx_rlcmac_dl_block_exp_data(dl_block, sched_fn, data, 0, exp_cs_mcs);
+	f_rx_rlcmac_dl_block_exp_data(dl_block, dl_fn, data, 0, exp_cs_mcs);
 
 	/* ACK the DL block */
 	f_acknackdesc_ack_block(ack_nack_desc, dl_block, '1'B);
-	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc), 0, sched_fn);
+	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc),
+			     0, f_dl_block_ack_fn(dl_block, dl_fn));
 
 	/* Now MS wants to answer the DL data, Establish an Uplink TBF */
 	ok := f_establish_tbf(rr_imm_ass);
@@ -1536,7 +1555,7 @@
 	var RlcmacDlBlock dl_block;
 	var octetstring data := f_rnd_octstring(10);
 	var boolean ok;
-	var uint32_t sched_fn;
+	var uint32_t dl_fn;
 	var OCT4 tlli := '00000001'O;
 	var AckNackDescription ack_nack_desc := valueof(t_AckNackDescription_init);
 
@@ -1559,7 +1578,7 @@
 
 	/* Wait timer X2002 and DL block is available after CCCH IMM ASS: */
 	f_sleep(X2002);
-	f_rx_rlcmac_dl_block_exp_data(dl_block, sched_fn, data, 0);
+	f_rx_rlcmac_dl_block_exp_data(dl_block, dl_fn, data, 0);
 
 	/* Now we don't ack the dl block (emulate MS failed receiveing IMM ASS
 	 * or GPRS DL, or DL ACK was lost for some reason). As a result, PCU
@@ -1571,11 +1590,12 @@
 
 	/* Wait timer X2002 and DL block is available after CCCH IMM ASS: */
 	f_sleep(X2002);
-	f_rx_rlcmac_dl_block_exp_data(dl_block, sched_fn, data, 0);
+	f_rx_rlcmac_dl_block_exp_data(dl_block, dl_fn, data, 0);
 
 	/* ACK the DL block */
 	f_acknackdesc_ack_block(ack_nack_desc, dl_block, '1'B);
-	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc), 0, sched_fn);
+	f_tx_rlcmac_ul_block(ts_RLCMAC_DL_ACK_NACK(f_rlcmac_dl_block_get_tfi(dl_block), ack_nack_desc),
+			     0, f_dl_block_ack_fn(dl_block, dl_fn));
 
 	f_shutdown(__BFILE__, __LINE__, final := true);
 }

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

Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I1528408b4399d0a149a23961805277eaab90d407
Gerrit-Change-Number: 18186
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <axilirator at gmail.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200510/ef8ad3b7/attachment.htm>


More information about the gerrit-log mailing list