pespin has submitted this change. (
https://gerrit.osmocom.org/c/libosmo-gprs/+/34313?usp=email )
Change subject: tbf_ul_ass_fsm: Listen only on 1 TS for PKT UL ASS when
assignment done from DL TBF
......................................................................
tbf_ul_ass_fsm: Listen only on 1 TS for PKT UL ASS when assignment done from DL
TBF
This also fixes a memcpy writing out of bounds reported by Coverity
CID#323120 in gprs_rlcmac_tbf_ul_ass_start_from_dl_tbf_ack_nack(), due
to the difference of size between struct gprs_rlcmac_ul_tbf_allocation
and struct gprs_rlcmac_dl_tbf_allocation.
While fixing it, actually properly implement passing of the 1 only
interesting TS to the tbf_ul_ass_fsm at that point in time.
Change-Id: I89b15982b73f00599183981142495d7b9befbb78
---
M include/osmocom/gprs/rlcmac/rlcmac_enc.h
M include/osmocom/gprs/rlcmac/tbf_dl.h
M include/osmocom/gprs/rlcmac/tbf_ul_ass_fsm.h
M src/rlcmac/rlcmac_enc.c
M src/rlcmac/sched.c
M src/rlcmac/tbf_dl.c
M src/rlcmac/tbf_ul_ass_fsm.c
M tests/rlcmac/rlcmac_prim_test.err
8 files changed, 47 insertions(+), 24 deletions(-)
Approvals:
fixeria: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/include/osmocom/gprs/rlcmac/rlcmac_enc.h
b/include/osmocom/gprs/rlcmac/rlcmac_enc.h
index 298d364..1ed02be 100644
--- a/include/osmocom/gprs/rlcmac/rlcmac_enc.h
+++ b/include/osmocom/gprs/rlcmac/rlcmac_enc.h
@@ -47,6 +47,6 @@
void gprs_rlcmac_enc_prepare_pkt_resource_req(RlcMacUplink_t *block, struct
gprs_rlcmac_ul_tbf *ul_tbf, enum gprs_rlcmac_access_type acc_type);
-void gprs_rlcmac_enc_prepare_pkt_downlink_ack_nack(RlcMacUplink_t *block,
const struct gprs_rlcmac_dl_tbf *dl_tbf);
+void gprs_rlcmac_enc_prepare_pkt_downlink_ack_nack(RlcMacUplink_t *block,
const struct gprs_rlcmac_dl_tbf *dl_tbf, bool chan_req);
void gprs_rlcmac_enc_prepare_pkt_ctrl_ack(RlcMacUplink_t *block, uint32_t
tlli);
diff --git a/include/osmocom/gprs/rlcmac/tbf_dl.h
b/include/osmocom/gprs/rlcmac/tbf_dl.h
index fe0b2ef..fc75972 100644
--- a/include/osmocom/gprs/rlcmac/tbf_dl.h
+++ b/include/osmocom/gprs/rlcmac/tbf_dl.h
@@ -45,7 +45,7 @@
int gprs_rlcmac_dl_tbf_configure_l1ctl(struct gprs_rlcmac_dl_tbf *dl_tbf);
-struct msgb *gprs_rlcmac_dl_tbf_create_pkt_dl_ack_nack(struct
gprs_rlcmac_dl_tbf *dl_tbf);
+struct msgb *gprs_rlcmac_dl_tbf_create_pkt_dl_ack_nack(struct
gprs_rlcmac_dl_tbf *dl_tbf, uint8_t tn);
int gprs_rlcmac_dl_tbf_rcv_data_block(struct gprs_rlcmac_dl_tbf *dl_tbf,
const struct gprs_rlcmac_rlc_data_info
*rlc,
diff --git a/include/osmocom/gprs/rlcmac/tbf_ul_ass_fsm.h
b/include/osmocom/gprs/rlcmac/tbf_ul_ass_fsm.h
index ea9e521..ad32471 100644
--- a/include/osmocom/gprs/rlcmac/tbf_ul_ass_fsm.h
+++ b/include/osmocom/gprs/rlcmac/tbf_ul_ass_fsm.h
@@ -82,7 +82,7 @@
int gprs_rlcmac_tbf_ul_ass_start(struct gprs_rlcmac_ul_tbf *ul_tbf, enum
gprs_rlcmac_tbf_ul_ass_type type);
int gprs_rlcmac_tbf_ul_ass_start_from_releasing_ul_tbf(struct
gprs_rlcmac_ul_tbf *ul_tbf, struct gprs_rlcmac_ul_tbf *old_ul_tbf);
-int gprs_rlcmac_tbf_ul_ass_start_from_dl_tbf_ack_nack(struct
gprs_rlcmac_ul_tbf *ul_tbf, const struct gprs_rlcmac_dl_tbf *dl_tbf);
+int gprs_rlcmac_tbf_ul_ass_start_from_dl_tbf_ack_nack(struct
gprs_rlcmac_ul_tbf *ul_tbf, const struct gprs_rlcmac_dl_tbf *dl_tbf, uint8_t
tn);
bool gprs_rlcmac_tbf_ul_ass_pending(struct gprs_rlcmac_ul_tbf *ul_tbf);
bool gprs_rlcmac_tbf_ul_ass_match_rach_req(struct gprs_rlcmac_ul_tbf *ul_tbf,
uint8_t ra);
diff --git a/src/rlcmac/rlcmac_enc.c b/src/rlcmac/rlcmac_enc.c
index d8cd61e..dec2fa6 100644
--- a/src/rlcmac/rlcmac_enc.c
+++ b/src/rlcmac/rlcmac_enc.c
@@ -384,11 +384,9 @@
/* TODO: fill cqr from info stored probably in the gre object. */
}
-void gprs_rlcmac_enc_prepare_pkt_downlink_ack_nack(RlcMacUplink_t *block,
const struct gprs_rlcmac_dl_tbf *dl_tbf)
+void gprs_rlcmac_enc_prepare_pkt_downlink_ack_nack(RlcMacUplink_t *block,
const struct gprs_rlcmac_dl_tbf *dl_tbf, bool chan_req)
{
Packet_Downlink_Ack_Nack_t *ack = &block->u.Packet_Downlink_Ack_Nack;
- struct gprs_rlcmac_entity *gre = dl_tbf->tbf.gre;
- int rc;
memset(block, 0, sizeof(*block));
ack->MESSAGE_TYPE = OSMO_GPRS_RLCMAC_UL_MSGT_PACKET_DOWNLINK_ACK_NACK;
@@ -398,9 +396,7 @@
ack->DOWNLINK_TFI = dl_tbf->cur_alloc.dl_tfi;
gprs_rlcmac_enc_prepare_pkt_ack_nack_desc_gprs(&ack->Ack_Nack_Description,
dl_tbf);
- /* Channel Request Description. Request a UL-TBF if we have UL data
- * queued to send and no active UL BF (TS 44.060 8.1.2.5) */
- if (!gre->ul_tbf && gprs_rlcmac_entity_have_tx_data_queued(gre)) {
+ if (chan_req) {
Channel_Request_Description_t *chan_req =
&ack->Channel_Request_Description;
ack->Exist_Channel_Request_Description = 1;
chan_req->PEAK_THROUGHPUT_CLASS = 0; /* TODO */
@@ -408,11 +404,6 @@
chan_req->RLC_MODE = GPRS_RLCMAC_RLC_MODE_ACKNOWLEDGED;
chan_req->LLC_PDU_TYPE = GPRS_RLCMAC_LLC_PDU_TYPE_ACKNOWLEDGED;
chan_req->RLC_OCTET_COUNT = 0; /* TODO */
-
- gre->ul_tbf = gprs_rlcmac_ul_tbf_alloc(gre);
- rc =
gprs_rlcmac_tbf_ul_ass_start_from_dl_tbf_ack_nack(gre->ul_tbf, dl_tbf);
- if (rc < 0)
- LOGPTBFDL(dl_tbf, LOGL_ERROR, "Failed starting
assignment of requested UL TBF (%d)\n", rc);
} else {
ack->Exist_Channel_Request_Description = 0;
}
diff --git a/src/rlcmac/sched.c b/src/rlcmac/sched.c
index 6af2b6a..faa9ff1 100644
--- a/src/rlcmac/sched.c
+++ b/src/rlcmac/sched.c
@@ -175,7 +175,7 @@
if (tbfs->poll_dl_ack_final_ack) {
LOGRLCMAC(LOGL_DEBUG, "(ts=%u,fn=%u,usf=%u) Tx DL ACK/NACK
FinalAck=1\n",
bi->ts, bi->fn, bi->usf);
- msg =
gprs_rlcmac_dl_tbf_create_pkt_dl_ack_nack(tbfs->poll_dl_ack_final_ack);
+ msg =
gprs_rlcmac_dl_tbf_create_pkt_dl_ack_nack(tbfs->poll_dl_ack_final_ack, bi->ts);
if (msg)
return msg;
}
@@ -237,7 +237,7 @@
if (tbfs->poll_dl_ack) {
LOGRLCMAC(LOGL_DEBUG, "(ts=%u,fn=%u,usf=%u) Tx DL ACK/NACK\n",
bi->ts, bi->fn, bi->usf);
- msg =
gprs_rlcmac_dl_tbf_create_pkt_dl_ack_nack(tbfs->poll_dl_ack);
+ msg =
gprs_rlcmac_dl_tbf_create_pkt_dl_ack_nack(tbfs->poll_dl_ack, bi->ts);
if (msg)
return msg;
}
diff --git a/src/rlcmac/tbf_dl.c b/src/rlcmac/tbf_dl.c
index a7f0ea9..b8d37a8 100644
--- a/src/rlcmac/tbf_dl.c
+++ b/src/rlcmac/tbf_dl.c
@@ -160,11 +160,13 @@
return gprs_rlcmac_prim_call_down_cb(rlcmac_prim);
}
-struct msgb *gprs_rlcmac_dl_tbf_create_pkt_dl_ack_nack(struct
gprs_rlcmac_dl_tbf *dl_tbf)
+struct msgb *gprs_rlcmac_dl_tbf_create_pkt_dl_ack_nack(struct
gprs_rlcmac_dl_tbf *dl_tbf, uint8_t tn)
{
struct msgb *msg;
struct bitvec bv;
RlcMacUplink_t ul_block;
+ bool chan_req = false;
+ struct gprs_rlcmac_entity *gre = dl_tbf->tbf.gre;
int rc;
OSMO_ASSERT(dl_tbf);
@@ -173,6 +175,17 @@
if (!msg)
return NULL;
+ /* Channel Request Description. Request a UL-TBF if we have UL data
+ * queued to send and no active UL TBF (TS 44.060 8.1.2.5) */
+ if (!gre->ul_tbf && gprs_rlcmac_entity_have_tx_data_queued(gre)) {
+ gre->ul_tbf = gprs_rlcmac_ul_tbf_alloc(gre);
+ rc =
gprs_rlcmac_tbf_ul_ass_start_from_dl_tbf_ack_nack(gre->ul_tbf, dl_tbf, tn);
+ if (rc < 0)
+ LOGPTBFDL(dl_tbf, LOGL_ERROR, "Failed starting
assignment of requested UL TBF (%d)\n", rc);
+ else
+ chan_req = true;
+ }
+
/* Initialize a bit vector that uses allocated msgb as the data buffer.
*/
bv = (struct bitvec){
.data = msgb_put(msg, GSM_MACBLOCK_LEN),
@@ -180,7 +193,7 @@
};
bitvec_unhex(&bv, GPRS_RLCMAC_DUMMY_VEC);
- gprs_rlcmac_enc_prepare_pkt_downlink_ack_nack(&ul_block, dl_tbf);
+ gprs_rlcmac_enc_prepare_pkt_downlink_ack_nack(&ul_block, dl_tbf,
chan_req);
rc = osmo_gprs_rlcmac_encode_uplink(&bv, &ul_block);
if (rc < 0) {
LOGPTBFDL(dl_tbf, LOGL_ERROR, "Encoding of Packet Downlink
ACK/NACK failed (%d)\n", rc);
diff --git a/src/rlcmac/tbf_ul_ass_fsm.c b/src/rlcmac/tbf_ul_ass_fsm.c
index 0f0f9ee..5587ddb 100644
--- a/src/rlcmac/tbf_ul_ass_fsm.c
+++ b/src/rlcmac/tbf_ul_ass_fsm.c
@@ -680,14 +680,17 @@
/* A DL-TBF requested a UL TBF over DL ACK/NACK, wait to receive Pkt Ul Ass for
* it, aka switch the FSM to trigger the 2hpase directly (tx Pkt Res Req) */
-int gprs_rlcmac_tbf_ul_ass_start_from_dl_tbf_ack_nack(struct
gprs_rlcmac_ul_tbf *ul_tbf, const struct gprs_rlcmac_dl_tbf *dl_tbf)
+int gprs_rlcmac_tbf_ul_ass_start_from_dl_tbf_ack_nack(struct
gprs_rlcmac_ul_tbf *ul_tbf, const struct gprs_rlcmac_dl_tbf *dl_tbf, uint8_t tn)
{
int rc;
ul_tbf->ul_ass_fsm.dl_tbf = dl_tbf;
- /* FIXME: Ideally this should only be the TS where the PKT UL ASS was
received... */
- ul_tbf->ul_ass_fsm.phase1_alloc.num_ts = dl_tbf->cur_alloc.num_ts;
- memcpy(&ul_tbf->ul_ass_fsm.phase1_alloc.ts[0], &dl_tbf->cur_alloc.ts[0],
- sizeof(ul_tbf->ul_ass_fsm.phase1_alloc.ts));
+
+ /* The TS where the PKT UL ASS is to be received is the one where the DL
+ * ACK/NACK was sent in the DL TBF (control TS): */
+ ul_tbf->ul_ass_fsm.phase1_alloc.num_ts = 1;
+ ul_tbf->ul_ass_fsm.phase1_alloc.ts[tn].allocated = true;
+ ul_tbf->ul_ass_fsm.phase1_alloc.ts[tn].usf = 0xff;
+
rc = osmo_fsm_inst_dispatch(ul_tbf->ul_ass_fsm.fi,
GPRS_RLCMAC_TBF_UL_ASS_EV_START_FROM_DL_TBF,
NULL);
diff --git a/tests/rlcmac/rlcmac_prim_test.err
b/tests/rlcmac/rlcmac_prim_test.err
index 95dc475..41773b2 100644
--- a/tests/rlcmac/rlcmac_prim_test.err
+++ b/tests/rlcmac/rlcmac_prim_test.err
@@ -950,13 +950,13 @@
DLGLOBAL DEBUG GRE(00000001) Enqueueing LLC-PDU len=14 SAPI=GMM radio_prio=1
DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_RTS.indication
DLGLOBAL DEBUG (ts=7,fn=21,usf=0) Tx DL ACK/NACK FinalAck=1
-DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) - SSN 1, V(N):
"IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIR" R=Received
I=Invalid, FINAL_ACK=1
DLGLOBAL INFO UL_TBF{NEW}: Allocated
DLGLOBAL INFO UL_TBF_ASS{IDLE}: Allocated
DLGLOBAL INFO UL_TBF_ASS{IDLE}: Received Event START_FROM_DL_TBF
DLGLOBAL INFO UL_TBF{NEW}: Received Event UL_ASS_START
DLGLOBAL INFO UL_TBF{NEW}: state_chg to ASSIGN
DLGLOBAL INFO UL_TBF_ASS{IDLE}: state_chg to WAIT_PKT_UL_ASS
+DLGLOBAL DEBUG TBF(DL:NR-0:TLLI-00000001) - SSN 1, V(N):
"IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIR" R=Received
I=Invalid, FINAL_ACK=1
DLGLOBAL INFO TBF(DL:NR-0:TLLI-00000001) Starting T3192 (0 ms)
DLGLOBAL DEBUG Tx to lower layers: L1CTL-PDCH_DATA.request
DLGLOBAL DEBUG Rx from lower layers: L1CTL-PDCH_DATA.indication
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/34313?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: I89b15982b73f00599183981142495d7b9befbb78
Gerrit-Change-Number: 34313
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged
