[M] Change in libosmo-gprs[master]: rlcmac: Fix DL_ASS polls removed when UL TBF released - gerrit-log

31 Mar 2023

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/32118 )
Change subject: rlcmac: Fix DL_ASS polls removed when UL TBF released
......................................................................
rlcmac: Fix DL_ASS polls removed when UL TBF released
The DL_TBF for the assignment is not created until later on when the
DL_TBF is first used, only the DL_ASS request is stored as part of the
gpre->tbf_dl_ass_fsm.
Since no DL_TBF existed, the POLL was being stored if the DL_ASS
referred the UL_TBF by means of GlobalTFI->ULTBF.
Instead, let's not require an UL_TBF to be present and simply pass the
GRE along which can be used to send the PKT_CTRL_ACK later on when the
scheduling the poll triggers.
Change-Id: Icdaa30e9ff942fb53cc4bbd801e4542b8885b32a
---
M include/osmocom/gprs/rlcmac/gre.h
M include/osmocom/gprs/rlcmac/pdch_ul_controller.h
M include/osmocom/gprs/rlcmac/tbf.h
M src/rlcmac/gre.c
M src/rlcmac/pdch_ul_controller.c
M src/rlcmac/rlcmac.c
M src/rlcmac/sched.c
M src/rlcmac/tbf.c
M tests/rlcmac/rlcmac_prim_test.err
9 files changed, 79 insertions(+), 56 deletions(-)
Approvals:
  Jenkins Builder: Verified
  fixeria: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved

diff --git a/include/osmocom/gprs/rlcmac/gre.h b/include/osmocom/gprs/rlcmac/gre.h
index 8322cf7..358d0aa 100644
--- a/include/osmocom/gprs/rlcmac/gre.h
+++ b/include/osmocom/gprs/rlcmac/gre.h
@@ -31,5 +31,7 @@
 int gprs_rlcmac_entity_llc_enqueue(struct gprs_rlcmac_entity *gre, uint8_t *ll_pdu, unsigned int ll_pdu_len,
    			   enum osmo_gprs_rlcmac_llc_sapi sapi, uint8_t radio_prio);
+struct msgb *gprs_rlcmac_gre_create_pkt_ctrl_ack(const struct gprs_rlcmac_entity *gre);
+
 #define LOGGRE(gre, level, fmt, args...) \
    LOGRLCMAC(level, "GRE(%08x) " fmt, (gre)->tlli, ## args)
diff --git a/include/osmocom/gprs/rlcmac/pdch_ul_controller.h b/include/osmocom/gprs/rlcmac/pdch_ul_controller.h
index 52970c4..1dcc1db 100644
--- a/include/osmocom/gprs/rlcmac/pdch_ul_controller.h
+++ b/include/osmocom/gprs/rlcmac/pdch_ul_controller.h
@@ -21,7 +21,8 @@
 #include <osmocom/core/linuxrbtree.h>
 #include <osmocom/core/utils.h>
-struct gprs_rlcmac_dl_tbf;
+struct gprs_rlcmac_gre;
+struct gprs_rlcmac_tbf;
struct gprs_rlcmac_pdch_ulc {
    uint8_t ts_nr;
@@ -43,12 +44,16 @@
    struct rb_node node;	/*! entry in gprs_rlcmac_pdch_ulc->tree_root */
    uint32_t fn;
    enum gprs_rlcmac_pdch_ulc_poll_reason reason;
-	struct gprs_rlcmac_tbf *tbf;
+	union {
+		struct gprs_rlcmac_tbf *tbf;
+		struct gprs_rlcmac_entity *gre;
+		void *data;
+	};
 };
struct gprs_rlcmac_pdch_ulc *gprs_rlcmac_pdch_ulc_alloc(void *ctx, uint8_t ts_nr);
-int gprs_rlcmac_pdch_ulc_reserve(struct gprs_rlcmac_pdch_ulc *ulc, uint32_t fn, enum gprs_rlcmac_pdch_ulc_poll_reason reason, struct gprs_rlcmac_tbf *tbf);
+int gprs_rlcmac_pdch_ulc_reserve(struct gprs_rlcmac_pdch_ulc *ulc, uint32_t fn, enum gprs_rlcmac_pdch_ulc_poll_reason reason, void *data);
struct gprs_rlcmac_pdch_ulc_node *gprs_rlcmac_pdch_ulc_get_node(struct gprs_rlcmac_pdch_ulc *ulc, uint32_t fn);
 struct gprs_rlcmac_pdch_ulc_node *gprs_rlcmac_pdch_ulc_pop_node(struct gprs_rlcmac_pdch_ulc *ulc, uint32_t fn);
diff --git a/include/osmocom/gprs/rlcmac/tbf.h b/include/osmocom/gprs/rlcmac/tbf.h
index 83a887d..9130237 100644
--- a/include/osmocom/gprs/rlcmac/tbf.h
+++ b/include/osmocom/gprs/rlcmac/tbf.h
@@ -28,8 +28,6 @@
void gprs_rlcmac_tbf_free(struct gprs_rlcmac_tbf *tbf);
-struct msgb *gprs_rlcmac_tbf_create_pkt_ctrl_ack(const struct gprs_rlcmac_tbf *tbf);
-
 #define LOGPTBF(tbf, lvl, fmt, args...) \
    LOGP(g_rlcmac_log_cat[tbf->direction == GPRS_RLCMAC_TBF_DIR_DL ? \
    		      OSMO_GPRS_RLCMAC_LOGC_TBFDL : \
diff --git a/src/rlcmac/gre.c b/src/rlcmac/gre.c
index 87953c1..2bab3c1 100644
--- a/src/rlcmac/gre.c
+++ b/src/rlcmac/gre.c
@@ -21,6 +21,10 @@
#include <stdbool.h>
+#include <osmocom/core/bitvec.h>
+#include <osmocom/core/msgb.h>
+#include <osmocom/gsm/protocol/gsm_04_08.h>
+
 #include <osmocom/gprs/rlcmac/rlcmac.h>
 #include <osmocom/gprs/rlcmac/rlcmac_prim.h>
 #include <osmocom/gprs/rlcmac/rlcmac_private.h>
@@ -29,6 +33,7 @@
 #include <osmocom/gprs/rlcmac/tbf_ul_fsm.h>
 #include <osmocom/gprs/rlcmac/tbf_ul.h>
 #include <osmocom/gprs/rlcmac/gre.h>
+#include <osmocom/gprs/rlcmac/rlcmac_enc.h>
struct gprs_rlcmac_entity *gprs_rlcmac_entity_alloc(uint32_t tlli)
 {
@@ -131,3 +136,38 @@
return rc;
 }
+
+struct msgb *gprs_rlcmac_gre_create_pkt_ctrl_ack(const struct gprs_rlcmac_entity *gre)
+{
+	struct msgb *msg;
+	struct bitvec bv;
+	RlcMacUplink_t ul_block;
+	int rc;
+
+	OSMO_ASSERT(gre);
+
+	msg = msgb_alloc(GSM_MACBLOCK_LEN, "pkt_ctrl_ack");
+	if (!msg)
+		return NULL;
+
+	/* Initialize a bit vector that uses allocated msgb as the data buffer. */
+	bv = (struct bitvec){
+		.data = msgb_put(msg, GSM_MACBLOCK_LEN),
+		.data_len = GSM_MACBLOCK_LEN,
+	};
+	bitvec_unhex(&bv, GPRS_RLCMAC_DUMMY_VEC);
+
+	gprs_rlcmac_enc_prepare_pkt_ctrl_ack(&ul_block, gre->tlli);
+	rc = osmo_gprs_rlcmac_encode_uplink(&bv, &ul_block);
+	if (rc < 0) {
+		LOGGRE(gre, LOGL_ERROR, "Encoding of Packet Control ACK failed (%d)\n", rc);
+		goto free_ret;
+	}
+	LOGGRE(gre, LOGL_DEBUG, "Tx Packet Control Ack\n");
+
+	return msg;
+
+free_ret:
+	msgb_free(msg);
+	return NULL;
+}
diff --git a/src/rlcmac/pdch_ul_controller.c b/src/rlcmac/pdch_ul_controller.c
index 731caf0..7c5e41a 100644
--- a/src/rlcmac/pdch_ul_controller.c
+++ b/src/rlcmac/pdch_ul_controller.c
@@ -124,11 +124,11 @@
int gprs_rlcmac_pdch_ulc_reserve(struct gprs_rlcmac_pdch_ulc *ulc, uint32_t fn,
    			 enum gprs_rlcmac_pdch_ulc_poll_reason reason,
-				 struct gprs_rlcmac_tbf *tbf)
+				 void *data)
 {
    struct gprs_rlcmac_pdch_ulc_node *item = _alloc_node(ulc, fn);
    item->reason = reason;
-	item->tbf = tbf;
+	item->data = data;
    LOGRLCMAC(LOGL_DEBUG, "Register POLL (TS=%u FN=%u, reason=%s)\n",
    	  ulc->ts_nr, item->fn,
    	  get_value_string(gprs_rlcmac_pdch_ulc_poll_reason_names, item->reason));
diff --git a/src/rlcmac/rlcmac.c b/src/rlcmac/rlcmac.c
index 8ff6c68..574ff5a 100644
--- a/src/rlcmac/rlcmac.c
+++ b/src/rlcmac/rlcmac.c
@@ -400,12 +400,12 @@
    };
    rc = gprs_rlcmac_tbf_start_from_pacch(&gre->dl_tbf_dl_ass_fsm, &ev_data);
-	if (tbf && dl_block->SP) {
+	if (dl_block->SP) {
    	uint32_t poll_fn = rrbp2fn(rlcmac_prim->l1ctl.pdch_data_ind.fn, dl_block->RRBP);
    	gprs_rlcmac_pdch_ulc_reserve(g_ctx->sched.ulc[rlcmac_prim->l1ctl.pdch_data_ind.ts_nr],
    				     poll_fn,
    				     GPRS_RLCMAC_PDCH_ULC_POLL_DL_ASS,
-					     tbf);
+					     gre);
    }
    return rc;
 }
diff --git a/src/rlcmac/sched.c b/src/rlcmac/sched.c
index 46e43ab..50a3910 100644
--- a/src/rlcmac/sched.c
+++ b/src/rlcmac/sched.c
@@ -38,7 +38,7 @@
    struct gprs_rlcmac_ul_tbf *poll_ul_ack_new_ul_tbf; /* 9.3.2.4.2  (answer with PKT RES REQ) */
    struct gprs_rlcmac_ul_tbf *poll_ul_ack; /* 11.2.2 (answer with PKT CTRL ACK) */
    struct gprs_rlcmac_ul_tbf *poll_ul_ass; /* (answer Pkt UL ASS with PKT CTRL ACK) */
-	struct gprs_rlcmac_tbf *poll_dl_ass; /* (answer Pkt DL ASS with PKT CTRL ACK) */
+	struct gprs_rlcmac_entity *poll_dl_ass; /* (answer Pkt DL ASS with PKT CTRL ACK) */
    struct gprs_rlcmac_ul_tbf *ul_ass;	/* PCU grants USF/SBA: transmit Pkt Res Req (2phase access)*/
 };
@@ -80,7 +80,7 @@
    		tbfs->poll_ul_ass = tbf_as_ul_tbf(node->tbf);
    		break;
    	case GPRS_RLCMAC_PDCH_ULC_POLL_DL_ASS:
-			tbfs->poll_dl_ass = node->tbf;
+			tbfs->poll_dl_ass = node->gre;
    		break;
    	case GPRS_RLCMAC_PDCH_ULC_POLL_UL_ACK:
    		/* TS 44.060: 9.3.2.4.2 If the PACKET UPLINK ACK/NACK message
@@ -199,7 +199,7 @@
    if (tbfs->poll_ul_ack) {
    	LOGRLCMAC(LOGL_DEBUG, "(ts=%u,fn=%u,usf=%u) Tx Pkt Control Ack (UL ACK/NACK poll)\n",
    		  bi->ts, bi->fn, bi->usf);
-		msg = gprs_rlcmac_tbf_create_pkt_ctrl_ack(ul_tbf_as_tbf(tbfs->poll_ul_ack));
+		msg = gprs_rlcmac_gre_create_pkt_ctrl_ack(ul_tbf_as_tbf(tbfs->poll_ul_ack)->gre);
    	/* Last UL message, freeing */
    	gprs_rlcmac_ul_tbf_free(tbfs->poll_ul_ack);
    	return msg;
@@ -207,14 +207,14 @@
    if (tbfs->poll_dl_ass) {
    	LOGRLCMAC(LOGL_DEBUG, "(ts=%u,fn=%u,usf=%u) Tx Pkt Control Ack (DL ASS poll)\n",
    		  bi->ts, bi->fn, bi->usf);
-		msg = gprs_rlcmac_tbf_create_pkt_ctrl_ack(tbfs->poll_dl_ass);
+		msg = gprs_rlcmac_gre_create_pkt_ctrl_ack(tbfs->poll_dl_ass);
    	if (msg)
    		return msg;
    }
    if (tbfs->poll_ul_ass) {
    	LOGRLCMAC(LOGL_DEBUG, "(ts=%u,fn=%u,usf=%u) Tx Pkt Control Ack (UL ASS poll)\n",
    		  bi->ts, bi->fn, bi->usf);
-		msg = gprs_rlcmac_tbf_create_pkt_ctrl_ack(ul_tbf_as_tbf(tbfs->poll_ul_ass));
+		msg = gprs_rlcmac_gre_create_pkt_ctrl_ack(ul_tbf_as_tbf(tbfs->poll_ul_ass)->gre);
    	if (msg)
    		return msg;
    }
diff --git a/src/rlcmac/tbf.c b/src/rlcmac/tbf.c
index 524740d..c82f0ea 100644
--- a/src/rlcmac/tbf.c
+++ b/src/rlcmac/tbf.c
@@ -19,14 +19,9 @@
  *
  */
-#include <osmocom/core/bitvec.h>
-#include <osmocom/core/msgb.h>
-#include <osmocom/gsm/protocol/gsm_04_08.h>
-
 #include <osmocom/gprs/rlcmac/tbf.h>
 #include <osmocom/gprs/rlcmac/tbf_ul.h>
 #include <osmocom/gprs/rlcmac/gre.h>
-#include <osmocom/gprs/rlcmac/rlcmac_enc.h>
 #include <osmocom/gprs/rlcmac/pdch_ul_controller.h>
void gprs_rlcmac_tbf_constructor(struct gprs_rlcmac_tbf *tbf,
@@ -50,38 +45,3 @@
    	gprs_rlcmac_ul_tbf_free(tbf_as_ul_tbf(tbf));
    /* else: TODO dl_tbf not yet implemented */
 }
-
-struct msgb *gprs_rlcmac_tbf_create_pkt_ctrl_ack(const struct gprs_rlcmac_tbf *tbf)
-{
-	struct msgb *msg;
-	struct bitvec bv;
-	RlcMacUplink_t ul_block;
-	int rc;
-
-	OSMO_ASSERT(tbf);
-
-	msg = msgb_alloc(GSM_MACBLOCK_LEN, "pkt_ctrl_ack");
-	if (!msg)
-		return NULL;
-
-	/* Initialize a bit vector that uses allocated msgb as the data buffer. */
-	bv = (struct bitvec){
-		.data = msgb_put(msg, GSM_MACBLOCK_LEN),
-		.data_len = GSM_MACBLOCK_LEN,
-	};
-	bitvec_unhex(&bv, GPRS_RLCMAC_DUMMY_VEC);
-
-	gprs_rlcmac_enc_prepare_pkt_ctrl_ack(&ul_block, tbf->gre->tlli);
-	rc = osmo_gprs_rlcmac_encode_uplink(&bv, &ul_block);
-	if (rc < 0) {
-		LOGPTBF(tbf, LOGL_ERROR, "Encoding of Packet Control ACK failed (%d)\n", rc);
-		goto free_ret;
-	}
-	LOGPTBF(tbf, LOGL_DEBUG, "Tx Packet Control Ack\n");
-
-	return msg;
-
-free_ret:
-	msgb_free(msg);
-	return NULL;
-}
diff --git a/tests/rlcmac/rlcmac_prim_test.err b/tests/rlcmac/rlcmac_prim_test.err
index f0b55d0..e7edbca 100644
--- a/tests/rlcmac/rlcmac_prim_test.err
+++ b/tests/rlcmac/rlcmac_prim_test.err
@@ -61,7 +61,7 @@
 DLGLOBAL DEBUG Register POLL (TS=7 FN=21, reason=UL_ACK)
 DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_RTS.indication
 DLGLOBAL DEBUG (ts=7,fn=21,usf=0) Tx Pkt Control Ack (UL ACK/NACK poll)
-DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00002342) Tx Packet Control Ack
+DLGLOBAL DEBUG GRE(00002342) Tx Packet Control Ack
 DLGLOBAL INFO UL_TBF_ASS{IDLE}: Deallocated
 DLGLOBAL INFO UL_TBF{RELEASING}: Deallocated
 DLGLOBAL DEBUG Tx to lower layers: L1CTL-PDCH_DATA.request
@@ -755,7 +755,7 @@
 DLGLOBAL INFO UL_TBF_ASS{COMPLETED}: state_chg to IDLE
 DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_RTS.indication
 DLGLOBAL DEBUG (ts=7,fn=43,usf=2) Tx Pkt Control Ack (UL ASS poll)
-DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00000001) Tx Packet Control Ack
+DLGLOBAL DEBUG GRE(00000001) Tx Packet Control Ack
 DLGLOBAL DEBUG Tx to lower layers: L1CTL-PDCH_DATA.request
 DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_RTS.indication
 DLGLOBAL DEBUG TBF(UL:NR-0:TLLI-00000001) Sending new block at BSN 0, CS=CS-2
-- 
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32118
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: Icdaa30e9ff942fb53cc4bbd801e4542b8885b32a
Gerrit-Change-Number: 32118
Gerrit-PatchSet: 1
Gerrit-Owner: pespin pespin@sysmocom.de
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel dwillmann@sysmocom.de
Gerrit-Reviewer: fixeria vyanitskiy@sysmocom.de
Gerrit-Reviewer: pespin pespin@sysmocom.de
Gerrit-MessageType: merged



    