trxcon/l1sched: emit DATA.cnf early (on bid=0)

trxcon's scheduler is currently emitting DATA.cnf whenever the last
burst of a DATA.req has been transmitted.  This sounds logical, but
makes the implementation quite complex.  It's even harder to implement
sending of DATA.cnf properly for CSD specific channel modes, which are
to be implemented in a follow-up patch.

The DATA.cnf prims trigger sending of L1CTL DATA.cnf/TRAFFIC.cnf,
which are interpreted as Ready-to-Send by the upper layers (layer23).
Additionally DATA.cnf prims trigger sending of GSMTAP PDUs containing
the respective Uplink frames.

This patch changes the l1sched logic, so that a DATA.cnf primitive
is emitted whenever the respective DATA.req is dequeued and encoded
using the lchan specific channel coding function.  This simplifies
the code a lot and prepares for the upcoming CSD support.

As a bonus, this patch fixes an inconsistency between TDMA FNs reported
in Uplink and Downlink GSMTAP PDUs.  Now we're indicating the first Fn
in both cases, so Uplink is consistent with Downlink.

Change-Id: Ie09a24cd950a93edd871a9fbc5b47ec96c24cceb
Related: OS#4396, OS#1572

10 files changed, 70 insertions, 102 deletions

diff --git a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
index 5385c00d..98282c1c 100644
--- a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
+++ b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h
@@ -223,8 +223,6 @@ struct l1sched_lchan_state {
 
 	/*! Queue of Tx primitives */
 	struct llist_head tx_prims;
-	/*! Tx primitive being sent */
-	struct msgb *prim;
 
 	/*! Mode for TCH channels (see GSM48_CMODE_*) */
 	uint8_t	tch_mode;
diff --git a/src/host/trxcon/include/osmocom/bb/l1sched/prim.h b/src/host/trxcon/include/osmocom/bb/l1sched/prim.h
index af3ac3b6..a9187c2a 100644
--- a/src/host/trxcon/include/osmocom/bb/l1sched/prim.h
+++ b/src/host/trxcon/include/osmocom/bb/l1sched/prim.h
@@ -116,16 +116,17 @@ void l1sched_prim_init(struct msgb *msg,
 struct msgb *l1sched_prim_alloc(enum l1sched_prim_type type,
 				enum osmo_prim_operation op);
 
-bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan, bool is_cmr);
+bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan,
+				     struct msgb *msg, bool is_cmr);
 struct msgb *l1sched_lchan_prim_dequeue_sacch(struct l1sched_lchan_state *lchan);
 struct msgb *l1sched_lchan_prim_dequeue_tch(struct l1sched_lchan_state *lchan, bool facch);
 struct msgb *l1sched_lchan_prim_dummy_lapdm(const struct l1sched_lchan_state *lchan);
-void l1sched_lchan_prim_drop(struct l1sched_lchan_state *lchan);
 
 int l1sched_lchan_emit_data_ind(struct l1sched_lchan_state *lchan,
 				const uint8_t *data, size_t data_len,
 				int n_errors, int n_bits_total, bool traffic);
-int l1sched_lchan_emit_data_cnf(struct l1sched_lchan_state *lchan, uint32_t fn);
+int l1sched_lchan_emit_data_cnf(struct l1sched_lchan_state *lchan,
+				struct msgb *msg, uint32_t fn);
 
 int l1sched_prim_from_user(struct l1sched_state *sched, struct msgb *msg);
 int l1sched_prim_to_user(struct l1sched_state *sched, struct msgb *msg);
diff --git a/src/host/trxcon/src/sched_lchan_common.c b/src/host/trxcon/src/sched_lchan_common.c
index bf318316..2308297d 100644
--- a/src/host/trxcon/src/sched_lchan_common.c
+++ b/src/host/trxcon/src/sched_lchan_common.c
@@ -93,17 +93,18 @@ const char *l1sched_burst_mask2str(const uint8_t *mask, int bits)
 	return buf;
 }
 
-bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan, bool is_cmr)
+bool l1sched_lchan_amr_prim_is_valid(struct l1sched_lchan_state *lchan,
+				     struct msgb *msg, bool is_cmr)
 {
 	enum osmo_amr_type ft_codec;
 	uint8_t cmr_codec;
 	int ft, cmr, len;
 
-	len = osmo_amr_rtp_dec(msgb_l2(lchan->prim), msgb_l2len(lchan->prim),
+	len = osmo_amr_rtp_dec(msgb_l2(msg), msgb_l2len(msg),
 			       &cmr_codec, NULL, &ft_codec, NULL, NULL);
 	if (len < 0) {
 		LOGP_LCHAND(lchan, LOGL_ERROR, "Cannot send invalid AMR payload (%u): %s\n",
-			    msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim));
+			    msgb_l2len(msg), msgb_hexdump_l2(msg));
 		return false;
 	}
 	ft = -1;
diff --git a/src/host/trxcon/src/sched_lchan_pdtch.c b/src/host/trxcon/src/sched_lchan_pdtch.c
index 03d43ce2..001a2d9c 100644
--- a/src/host/trxcon/src/sched_lchan_pdtch.c
+++ b/src/host/trxcon/src/sched_lchan_pdtch.c
@@ -140,19 +140,22 @@ int tx_pdtch_fn(struct l1sched_lchan_state *lchan,
 
 	*mask = *mask << 4;
 
-	lchan->prim = prim_dequeue_pdtch(lchan, br->fn);
-	if (lchan->prim == NULL)
+	struct msgb *msg = prim_dequeue_pdtch(lchan, br->fn);
+	if (msg == NULL)
 		return -ENOENT;
 
 	/* Encode payload */
-	rc = gsm0503_pdtch_encode(bursts_p, msgb_l2(lchan->prim), msgb_l2len(lchan->prim));
+	rc = gsm0503_pdtch_encode(bursts_p, msgb_l2(msg), msgb_l2len(msg));
 	if (rc < 0) {
 		LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n",
-			    msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim));
-		l1sched_lchan_prim_drop(lchan);
+			    msgb_l2len(msg), msgb_hexdump_l2(msg));
+		msgb_free(msg);
 		return -EINVAL;
 	}
 
+	/* Confirm data / traffic sending (pass ownership of the msgb/prim) */
+	l1sched_lchan_emit_data_cnf(lchan, msg, br->fn);
+
 send_burst:
 	/* Determine which burst should be sent */
 	burst = bursts_p + br->bid * 116;
@@ -173,11 +176,5 @@ send_burst:
 
 	LOGP_LCHAND(lchan, LOGL_DEBUG, "Scheduled at fn=%u burst=%u\n", br->fn, br->bid);
 
-	/* If we have sent the last (4/4) burst */
-	if ((*mask & 0x0f) == 0x0f) {
-		/* Confirm data / traffic sending (pass ownership of the prim) */
-		l1sched_lchan_emit_data_cnf(lchan, br->fn);
-	}
-
 	return 0;
 }
diff --git a/src/host/trxcon/src/sched_lchan_rach.c b/src/host/trxcon/src/sched_lchan_rach.c
index 8df7fa25..905f1d57 100644
--- a/src/host/trxcon/src/sched_lchan_rach.c
+++ b/src/host/trxcon/src/sched_lchan_rach.c
@@ -73,28 +73,27 @@ int tx_rach_fn(struct l1sched_lchan_state *lchan,
 	       struct l1sched_burst_req *br)
 {
 	const uint8_t bsic = lchan->ts->sched->bsic;
-	struct l1sched_prim *prim;
 	uint8_t *burst_ptr = br->burst;
 	uint8_t payload[36];
 	int i, rc;
 
-	if (lchan->prim == NULL) {
-		lchan->prim = msgb_dequeue(&lchan->tx_prims);
-		if (lchan->prim == NULL)
-			return 0;
-	}
-	prim = l1sched_prim_from_msgb(lchan->prim);
+	if (llist_empty(&lchan->tx_prims))
+		return 0;
+
+	struct msgb *msg = llist_first_entry(&lchan->tx_prims, struct msgb, list);
+	struct l1sched_prim *prim = l1sched_prim_from_msgb(msg);
 
 	/* Delay sending according to offset value */
 	if (prim->rach_req.offset-- > 0)
 		return 0;
+	llist_del(&msg->list);
 
 	/* Check requested synch. sequence */
 	if (prim->rach_req.synch_seq >= RACH_SYNCH_SEQ_NUM) {
 		LOGP_LCHAND(lchan, LOGL_ERROR,
 			    "Unknown RACH synch. sequence=0x%02x\n",
 			    prim->rach_req.synch_seq);
-		l1sched_lchan_prim_drop(lchan);
+		msgb_free(msg);
 		return -ENOTSUP;
 	}
 
@@ -106,7 +105,7 @@ int tx_rach_fn(struct l1sched_lchan_state *lchan,
 			    "Could not encode %s-bit RACH burst (ra=%u bsic=%u)\n",
 			    prim->rach_req.is_11bit ? "11" : "8",
 			    prim->rach_req.ra, bsic);
-		l1sched_lchan_prim_drop(lchan);
+		msgb_free(msg);
 		return rc;
 	}
 
@@ -130,8 +129,8 @@ int tx_rach_fn(struct l1sched_lchan_state *lchan,
 		    prim->rach_req.is_11bit ? "11" : "8",
 		    get_value_string(rach_synch_seq_names, prim->rach_req.synch_seq), br->fn);
 
-	/* Confirm RACH request (pass ownership of the prim) */
-	l1sched_lchan_emit_data_cnf(lchan, br->fn);
+	/* Confirm RACH request (pass ownership of the msgb/prim) */
+	l1sched_lchan_emit_data_cnf(lchan, msg, br->fn);
 
 	return 0;
 }
diff --git a/src/host/trxcon/src/sched_lchan_tchf.c b/src/host/trxcon/src/sched_lchan_tchf.c
index 0206b610..12bea578 100644
--- a/src/host/trxcon/src/sched_lchan_tchf.c
+++ b/src/host/trxcon/src/sched_lchan_tchf.c
@@ -229,39 +229,39 @@ int tx_tchf_fn(struct l1sched_lchan_state *lchan,
 	memset(&bursts_p[464], 0, 464);
 	*mask = *mask << 4;
 
-	lchan->prim = prim_dequeue_tchf(lchan);
+	struct msgb *msg = prim_dequeue_tchf(lchan);
 
 	/* populate the buffer with bursts */
 	switch (lchan->tch_mode) {
 	case GSM48_CMODE_SIGN:
-		if (lchan->prim == NULL)
-			lchan->prim = l1sched_lchan_prim_dummy_lapdm(lchan);
+		if (msg == NULL)
+			msg = l1sched_lchan_prim_dummy_lapdm(lchan);
 		/* fall-through */
 	case GSM48_CMODE_SPEECH_V1:
 	case GSM48_CMODE_SPEECH_EFR:
-		if (lchan->prim == NULL) {
+		if (msg == NULL) {
 			/* transmit a dummy speech block with inverted CRC3 */
 			gsm0503_tch_fr_encode(bursts_p, NULL, 0, 1);
 			goto send_burst;
 		}
 		rc = gsm0503_tch_fr_encode(bursts_p,
-					   msgb_l2(lchan->prim),
-					   msgb_l2len(lchan->prim), 1);
+					   msgb_l2(msg),
+					   msgb_l2len(msg), 1);
 		break;
 	case GSM48_CMODE_SPEECH_AMR:
 	{
 		bool amr_fn_is_cmr = !sched_tchf_ul_amr_cmi_map[br->fn % 26];
-		const uint8_t *data = lchan->prim ? msgb_l2(lchan->prim) : NULL;
-		size_t data_len = lchan->prim ? msgb_l2len(lchan->prim) : 0;
+		const uint8_t *data = msg ? msgb_l2(msg) : NULL;
+		size_t data_len = msg ? msgb_l2len(msg) : 0;
 
-		if (lchan->prim == NULL) {
+		if (msg == NULL) {
 			/* TODO: It's not clear what to do for TCH/AFS.
 			 * TODO: Send dummy FACCH maybe? */
 			goto send_burst; /* send something */
 		}
 
 		if (data_len != GSM_MACBLOCK_LEN) { /* TCH/AFS: speech */
-			if (!l1sched_lchan_amr_prim_is_valid(lchan, amr_fn_is_cmr))
+			if (!l1sched_lchan_amr_prim_is_valid(lchan, msg, amr_fn_is_cmr))
 				goto free_bad_msg;
 			/* pull the AMR header - sizeof(struct amr_hdr) */
 			data_len -= 2;
@@ -286,12 +286,15 @@ int tx_tchf_fn(struct l1sched_lchan_state *lchan,
 
 	if (rc) {
 		LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n",
-			    msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim));
+			    msgb_l2len(msg), msgb_hexdump_l2(msg));
 free_bad_msg:
-		l1sched_lchan_prim_drop(lchan);
+		msgb_free(msg);
 		return -EINVAL;
 	}
 
+	/* Confirm data / traffic sending (pass ownership of the msgb/prim) */
+	l1sched_lchan_emit_data_cnf(lchan, msg, br->fn);
+
 send_burst:
 	/* Determine which burst should be sent */
 	burst = bursts_p + br->bid * 116;
@@ -312,11 +315,5 @@ send_burst:
 
 	LOGP_LCHAND(lchan, LOGL_DEBUG, "Scheduled fn=%u burst=%u\n", br->fn, br->bid);
 
-	/* If we have sent the last (4/4) burst */
-	if ((*mask & 0x0f) == 0x0f) {
-		/* Confirm data / traffic sending (pass ownership of the prim) */
-		l1sched_lchan_emit_data_cnf(lchan, br->fn);
-	}
-
 	return 0;
 }
diff --git a/src/host/trxcon/src/sched_lchan_tchh.c b/src/host/trxcon/src/sched_lchan_tchh.c
index 3b329cb9..38aef34d 100644
--- a/src/host/trxcon/src/sched_lchan_tchh.c
+++ b/src/host/trxcon/src/sched_lchan_tchh.c
@@ -381,40 +381,40 @@ int tx_tchh_fn(struct l1sched_lchan_state *lchan,
 		goto send_burst;
 	}
 
-	lchan->prim = prim_dequeue_tchh(lchan, br->fn);
+	struct msgb *msg = prim_dequeue_tchh(lchan, br->fn);
 
 	/* populate the buffer with bursts */
 	switch (lchan->tch_mode) {
 	case GSM48_CMODE_SIGN:
 		if (!l1sched_tchh_facch_start(lchan->type, br->fn, 1))
 			goto send_burst; /* XXX: should not happen */
-		if (lchan->prim == NULL)
-			lchan->prim = l1sched_lchan_prim_dummy_lapdm(lchan);
+		if (msg == NULL)
+			msg = l1sched_lchan_prim_dummy_lapdm(lchan);
 		/* fall-through */
 	case GSM48_CMODE_SPEECH_V1:
-		if (lchan->prim == NULL) {
+		if (msg == NULL) {
 			/* transmit a dummy speech block with inverted CRC3 */
 			gsm0503_tch_hr_encode(bursts_p, NULL, 0);
 			goto send_burst;
 		}
 		rc = gsm0503_tch_hr_encode(bursts_p,
-					   msgb_l2(lchan->prim),
-					   msgb_l2len(lchan->prim));
+					   msgb_l2(msg),
+					   msgb_l2len(msg));
 		break;
 	case GSM48_CMODE_SPEECH_AMR:
 	{
 		bool amr_fn_is_cmr = !sched_tchh_ul_amr_cmi_map[br->fn % 26];
-		const uint8_t *data = lchan->prim ? msgb_l2(lchan->prim) : NULL;
-		size_t data_len = lchan->prim ? msgb_l2len(lchan->prim) : 0;
+		const uint8_t *data = msg ? msgb_l2(msg) : NULL;
+		size_t data_len = msg ? msgb_l2len(msg) : 0;
 
-		if (lchan->prim == NULL) {
+		if (msg == NULL) {
 			/* TODO: It's not clear what to do for TCH/AHS.
 			 * TODO: Send dummy FACCH maybe? */
 			goto send_burst; /* send garbage */
 		}
 
 		if (data_len != GSM_MACBLOCK_LEN) { /* TCH/AHS: speech */
-			if (!l1sched_lchan_amr_prim_is_valid(lchan, amr_fn_is_cmr))
+			if (!l1sched_lchan_amr_prim_is_valid(lchan, msg, amr_fn_is_cmr))
 				goto free_bad_msg;
 			/* pull the AMR header - sizeof(struct amr_hdr) */
 			data_len -= 2;
@@ -439,15 +439,18 @@ int tx_tchh_fn(struct l1sched_lchan_state *lchan,
 
 	if (rc) {
 		LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n",
-			    msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim));
+			    msgb_l2len(msg), msgb_hexdump_l2(msg));
 free_bad_msg:
-		l1sched_lchan_prim_drop(lchan);
+		msgb_free(msg);
 		return -EINVAL;
 	}
 
-	if (msgb_l2len(lchan->prim) == GSM_MACBLOCK_LEN)
+	if (msgb_l2len(msg) == GSM_MACBLOCK_LEN)
 		lchan->ul_facch_blocks = 6;
 
+	/* Confirm data / traffic sending (pass ownership of the msgb/prim) */
+	l1sched_lchan_emit_data_cnf(lchan, msg, br->fn);
+
 send_burst:
 	/* Determine which burst should be sent */
 	burst = bursts_p + br->bid * 116;
@@ -472,13 +475,5 @@ send_burst:
 	if (lchan->ul_facch_blocks)
 		lchan->ul_facch_blocks--;
 
-	if ((*mask & 0x0f) == 0x0f) {
-		/* Confirm data / traffic sending (pass ownership of the prim) */
-		if (!lchan->ul_facch_blocks)
-			l1sched_lchan_emit_data_cnf(lchan, br->fn);
-		else /* do not confirm dropped prims */
-			l1sched_lchan_prim_drop(lchan);
-	}
-
 	return 0;
 }
diff --git a/src/host/trxcon/src/sched_lchan_xcch.c b/src/host/trxcon/src/sched_lchan_xcch.c
index e2638362..b3863d57 100644
--- a/src/host/trxcon/src/sched_lchan_xcch.c
+++ b/src/host/trxcon/src/sched_lchan_xcch.c
@@ -139,20 +139,23 @@ int tx_data_fn(struct l1sched_lchan_state *lchan,
 
 	*mask = *mask << 4;
 
-	lchan->prim = prim_dequeue_xcch(lchan);
-	if (lchan->prim == NULL)
-		lchan->prim = l1sched_lchan_prim_dummy_lapdm(lchan);
-	OSMO_ASSERT(lchan->prim != NULL);
+	struct msgb *msg = prim_dequeue_xcch(lchan);
+	if (msg == NULL)
+		msg = l1sched_lchan_prim_dummy_lapdm(lchan);
+	OSMO_ASSERT(msg != NULL);
 
 	/* Encode payload */
-	rc = gsm0503_xcch_encode(bursts_p, msgb_l2(lchan->prim));
+	rc = gsm0503_xcch_encode(bursts_p, msgb_l2(msg));
 	if (rc) {
 		LOGP_LCHAND(lchan, LOGL_ERROR, "Failed to encode L2 payload (len=%u): %s\n",
-			    msgb_l2len(lchan->prim), msgb_hexdump_l2(lchan->prim));
-		l1sched_lchan_prim_drop(lchan);
+			    msgb_l2len(msg), msgb_hexdump_l2(msg));
+		msgb_free(msg);
 		return -EINVAL;
 	}
 
+	/* Confirm data sending (pass ownership of the msgb/prim) */
+	l1sched_lchan_emit_data_cnf(lchan, msg, br->fn);
+
 send_burst:
 	/* Determine which burst should be sent */
 	burst = bursts_p + br->bid * 116;
@@ -173,11 +176,5 @@ send_burst:
 
 	LOGP_LCHAND(lchan, LOGL_DEBUG, "Scheduled fn=%u burst=%u\n", br->fn, br->bid);
 
-	/* If we have sent the last (4/4) burst */
-	if ((*mask & 0x0f) == 0x0f) {
-		/* Confirm data sending (pass ownership of the prim) */
-		l1sched_lchan_emit_data_cnf(lchan, br->fn);
-	}
-
 	return 0;
 }
diff --git a/src/host/trxcon/src/sched_prim.c b/src/host/trxcon/src/sched_prim.c
index 32115608..2597d84d 100644
--- a/src/host/trxcon/src/sched_prim.c
+++ b/src/host/trxcon/src/sched_prim.c
@@ -249,17 +249,6 @@ struct msgb *l1sched_lchan_prim_dequeue_tch(struct l1sched_lchan_state *lchan, b
 }
 
 /**
- * Drops the current primitive of specified logical channel
- *
- * @param lchan a logical channel to drop prim from
- */
-void l1sched_lchan_prim_drop(struct l1sched_lchan_state *lchan)
-{
-	msgb_free(lchan->prim);
-	lchan->prim = NULL;
-}
-
-/**
  * Allocate a DATA.req with dummy LAPDm func=UI frame for the given logical channel.
  * To be used when no suitable DATA.req is present in the Tx queue.
  *
@@ -343,15 +332,12 @@ int l1sched_lchan_emit_data_ind(struct l1sched_lchan_state *lchan,
 	return l1sched_prim_to_user(lchan->ts->sched, msg);
 }
 
-int l1sched_lchan_emit_data_cnf(struct l1sched_lchan_state *lchan, uint32_t fn)
+int l1sched_lchan_emit_data_cnf(struct l1sched_lchan_state *lchan,
+				struct msgb *msg, uint32_t fn)
 {
 	struct l1sched_prim *prim;
-	struct msgb *msg;
 
-	/* take ownership of the prim */
-	if ((msg = lchan->prim) == NULL)
-		return -ENODEV;
-	lchan->prim = NULL;
+	OSMO_ASSERT(msg != NULL);
 
 	/* convert from DATA.req to DATA.cnf */
 	prim = l1sched_prim_from_msgb(msg);
diff --git a/src/host/trxcon/src/sched_trx.c b/src/host/trxcon/src/sched_trx.c
index cac3a219..d07a5791 100644
--- a/src/host/trxcon/src/sched_trx.c
+++ b/src/host/trxcon/src/sched_trx.c
@@ -524,9 +524,6 @@ static void l1sched_reset_lchan(struct l1sched_lchan_state *lchan)
 	lchan->rx_bursts = NULL;
 	lchan->tx_bursts = NULL;
 
-	/* Forget the current prim */
-	l1sched_lchan_prim_drop(lchan);
-
 	/* Flush the queue of pending Tx prims */
 	while ((msg = msgb_dequeue(&lchan->tx_prims)) != NULL)
 		msgb_free(msg);