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 <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged
