Reestore last LLC frames never completely acked when freeing DL TBF

Scenario: A DL TBF is assigned over PCH (CCCH) and we start transmitting
DL data blocks blindly after X2002, but at the same time the MS start
packet-access-procedure to request an UL TBF.
Right now osmo-pcu correctly detects the MS is available in PDCH and
re-assigns a DL TBF using PACCH, but the LLC frames it transmitted in
the old PCH-assigned DL TBF get lost when that older TBF is freed
(because the DL blocks were removed from the GprsMs llc_queue).

This issue is now more frequent since X2002 timer was added recently to
delay starting requesting USF for a UL TBF, hence the contention
resolution in general takes more time and hence the PACCH assignment of
the DL TBF takes more time too, so more DL data blocks are transmitted
to the DL TBF assigned over PCH during that time.

This patch improves the situation to at least recover the DL blocks
transmitted if the DL TBF is freed (due to MS merge trigger by scenario
mentioned above), where no DL ACK/NACK was ever received by the MS.

Ideal solution would be to have complete tracking of which LLC PDUs from
the llc_queues were completely ACKed at RLC/MAC level, but that really
requires a lot of work and major refactoring, which are left as a future
improvement.

Change-Id: I9be4035fb2cf2b3ee56e91dcc12cc8c24028b4aa
This commit is contained in:
Pau Espin 2023-06-27 18:02:33 +02:00
parent 11627ffaae
commit 37c2f84d53
7 changed files with 126 additions and 21 deletions

View File

@ -456,6 +456,8 @@ void ms_update_announced_tlli(struct GprsMs *ms, uint32_t tlli)
void ms_merge_and_clear_ms(struct GprsMs *ms, struct GprsMs *old_ms)
{
char old_ms_name[128];
struct gprs_rlcmac_dl_tbf *dl_tbf;
OSMO_ASSERT(old_ms != ms);
ms_ref(old_ms, __func__);
@ -472,6 +474,12 @@ void ms_merge_and_clear_ms(struct GprsMs *ms, struct GprsMs *old_ms)
if (!ms_egprs_ms_class(ms) && ms_egprs_ms_class(old_ms))
ms_set_egprs_ms_class(ms, ms_egprs_ms_class(old_ms));
if ((dl_tbf = ms_dl_tbf(old_ms))) {
/* Move the last partially/totally unacked LLC PDU back to the LLC queue: */
dl_tbf_copy_unacked_pdus_to_llc_queue(dl_tbf);
}
/* Now merge the old_ms queue into the new one: */
llc_queue_move_and_merge(&ms->llc_queue, &old_ms->llc_queue);
/* Clean up the old MS object */

View File

@ -35,6 +35,8 @@ void llc_reset(struct gprs_llc *llc)
{
llc->index = 0;
llc->length = 0;
llc->prio = 0;
llc->meta_info = (struct MetaInfo){0};
memset(llc->frame, 0x42, sizeof(llc->frame));
}
@ -231,6 +233,26 @@ void llc_queue_move_and_merge(struct gprs_llc_queue *q, struct gprs_llc_queue *o
q->queue_octets = queue_octets;
}
/* Prepend / Put back a previously dequeued LLC frame (llc_queue_dequeue()) */
void llc_queue_merge_prepend(struct gprs_llc_queue *q, struct gprs_llc *llc)
{
struct MetaInfo *meta_storage;
unsigned int len = llc_frame_length(llc);
struct msgb *llc_msg = msgb_alloc(len, "llc_pdu_queue");
OSMO_ASSERT(llc_msg);
memcpy(msgb_put(llc_msg, len), llc->frame, len);
q->queue_size += 1;
q->queue_octets += msgb_length(llc_msg);
meta_storage = (struct MetaInfo *)&llc_msg->cb[0];
memcpy(meta_storage, &llc->meta_info, sizeof(struct MetaInfo));
/* Prepend: */
llist_add(&llc_msg->list, &q->pq[llc->prio].queue);
}
#define ALPHA 0.5f
static struct msgb *llc_queue_pick_msg(struct gprs_llc_queue *q, enum gprs_llc_queue_prio *prio)
@ -265,7 +287,7 @@ static struct msgb *llc_queue_pick_msg(struct gprs_llc_queue *q, enum gprs_llc_q
return msg;
}
struct msgb *llc_queue_dequeue(struct gprs_llc_queue *q)
struct msgb *llc_queue_dequeue(struct gprs_llc_queue *q, enum gprs_llc_queue_prio *out_prio, struct MetaInfo *out_info)
{
struct msgb *msg;
struct timespec tv_now, tv_now2;
@ -274,6 +296,7 @@ struct msgb *llc_queue_dequeue(struct gprs_llc_queue *q)
struct gprs_pcu *pcu = bts->pcu;
struct timespec hyst_delta = {0, 0};
enum gprs_llc_queue_prio prio;
const struct MetaInfo *info = NULL;
if (pcu->vty.llc_discard_csec)
csecs_to_timespec(pcu->vty.llc_discard_csec, &hyst_delta);
@ -282,7 +305,7 @@ struct msgb *llc_queue_dequeue(struct gprs_llc_queue *q)
timespecadd(&tv_now, &hyst_delta, &tv_now2);
while ((msg = llc_queue_pick_msg(q, &prio))) {
const struct MetaInfo *info = (const struct MetaInfo *)&msg->cb[0];
info = (const struct MetaInfo *)&msg->cb[0];
const struct timespec *tv_disc = &info->expire_time;
const struct timespec *tv_recv = &info->recv_time;
@ -335,6 +358,17 @@ drop_frame:
bssgp_tx_llc_discarded(pcu->bssgp.bctx, ms_tlli(q->ms), frames, octets);
}
if (!msg)
return NULL;
if (out_prio)
*out_prio = prio;
if (out_info) {
OSMO_ASSERT(info);
*out_info = *info;
}
return msg;
}

View File

@ -50,6 +50,17 @@ struct gprs_llc_hdr {
uint8_t control[0];
} __attribute__ ((packed));
struct MetaInfo {
struct timespec recv_time;
struct timespec expire_time;
};
enum gprs_llc_queue_prio { /* lowest value has highest prio */
LLC_QUEUE_PRIO_GMM = 0, /* SAPI 1 */
LLC_QUEUE_PRIO_TOM_SMS, /* SAPI 2,7,8 */
LLC_QUEUE_PRIO_OTHER, /* Other SAPIs */
_LLC_QUEUE_PRIO_SIZE /* used to calculate size of enum */
};
/**
* I represent the LLC data to a MS
*/
@ -57,6 +68,10 @@ struct gprs_llc {
uint8_t frame[LLC_MAX_LEN]; /* current DL or UL frame */
uint16_t index; /* current write/read position of frame */
uint16_t length; /* len of current DL LLC_frame, 0 == no frame */
/* Saved when dequeue from llc_queue; allows re-enqueing in the queue if Tx fails */
enum gprs_llc_queue_prio prio;
struct MetaInfo meta_info;
};
void llc_init(struct gprs_llc *llc);
@ -99,19 +114,9 @@ static inline bool llc_fits_in_current_frame(const struct gprs_llc *llc, uint8_t
return llc->length + chunk_size <= LLC_MAX_LEN;
}
struct MetaInfo {
struct timespec recv_time;
struct timespec expire_time;
};
/**
* I store the LLC frames that come from the SGSN.
*/
enum gprs_llc_queue_prio { /* lowest value has highest prio */
LLC_QUEUE_PRIO_GMM = 0, /* SAPI 1 */
LLC_QUEUE_PRIO_TOM_SMS, /* SAPI 2,7,8 */
LLC_QUEUE_PRIO_OTHER, /* Other SAPIs */
_LLC_QUEUE_PRIO_SIZE /* used to calculate size of enum */
};
struct gprs_llc_prio_queue {
struct gprs_codel codel_state;
struct llist_head queue; /* queued LLC DL data. See enum gprs_llc_queue_prio. */
@ -133,8 +138,9 @@ void llc_queue_init(struct gprs_llc_queue *q, struct GprsMs *ms);
void llc_queue_clear(struct gprs_llc_queue *q, struct gprs_rlcmac_bts *bts);
void llc_queue_set_codel_interval(struct gprs_llc_queue *q, unsigned int interval);
void llc_queue_move_and_merge(struct gprs_llc_queue *q, struct gprs_llc_queue *o);
void llc_queue_merge_prepend(struct gprs_llc_queue *q, struct gprs_llc *llc);
void llc_queue_enqueue(struct gprs_llc_queue *q, struct msgb *llc_msg, const struct timespec *expire_time);
struct msgb *llc_queue_dequeue(struct gprs_llc_queue *q);
struct msgb *llc_queue_dequeue(struct gprs_llc_queue *q, enum gprs_llc_queue_prio *out_prio, struct MetaInfo *out_info);
static inline size_t llc_queue_size(const struct gprs_llc_queue *q)
{

View File

@ -173,6 +173,8 @@ gprs_rlcmac_dl_tbf::gprs_rlcmac_dl_tbf(struct gprs_rlcmac_bts *bts_, GprsMs *ms)
state_fi = osmo_fsm_inst_alloc(&tbf_dl_fsm, this, &state_fsm, LOGL_INFO, NULL);
OSMO_ASSERT(state_fi);
INIT_LLIST_HEAD(&this->tx_llc_until_first_dl_ack_rcvd);
/* This has to be called in child constructor because enable_egprs()
* uses the window() virtual function which is dependent on subclass. */
if (ms_mode(m_ms) != GPRS)
@ -542,7 +544,7 @@ void gprs_rlcmac_dl_tbf::schedule_next_frame()
return;
/* dequeue next LLC frame, if any */
msg = llc_queue_dequeue(llc_queue());
msg = llc_queue_dequeue(llc_queue(), &m_llc.prio, &m_llc.meta_info);
if (!msg)
return;
@ -661,6 +663,15 @@ int gprs_rlcmac_dl_tbf::create_new_bsn(const uint32_t fn, enum CodingScheme cs)
LOGPTBFDL(this, LOGL_DEBUG, "Complete DL frame, len=%d\n", llc_frame_length(&m_llc));
gprs_rlcmac_dl_bw(this, llc_frame_length(&m_llc));
bts_do_rate_ctr_add(bts, CTR_LLC_DL_BYTES, llc_frame_length(&m_llc));
/* Keep transmitted LLC PDUs until first ACK to avoid lossing them if MS is not there. */
if (!this->m_first_dl_ack_rcvd) {
struct gprs_dl_llc_llist_item *llc_it = talloc(this, struct gprs_dl_llc_llist_item);
memcpy(&llc_it->llc, &m_llc, sizeof(llc_it->llc));
/* Prepend to list to store them in inverse order of transmission, see
* dl_tbf_copy_unacked_pdus_to_llc_queue() for the complete picture. */
llist_add(&llc_it->list, &this->tx_llc_until_first_dl_ack_rcvd);
}
llc_reset(&m_llc);
if (is_final) {
@ -1073,7 +1084,15 @@ int gprs_rlcmac_dl_tbf::rcvd_dl_ack(bool final_ack, unsigned first_bsn,
int rc;
LOGPTBFDL(this, LOGL_DEBUG, "downlink acknowledge\n");
m_first_dl_ack_rcvd = true;
if (m_first_dl_ack_rcvd == false) {
/* MS is there, free temporarily stored transmitted LLC PDUs */
struct gprs_dl_llc_llist_item *llc_it;
while ((llc_it = llist_first_entry_or_null(&this->tx_llc_until_first_dl_ack_rcvd, struct gprs_dl_llc_llist_item, list))) {
llist_del(&llc_it->list);
talloc_free(llc_it);
}
m_first_dl_ack_rcvd = true;
}
m_last_dl_poll_ack_lost = false;
/* reset N3105 */
@ -1112,6 +1131,33 @@ bool dl_tbf_first_dl_ack_rcvd(const struct gprs_rlcmac_dl_tbf *tbf)
return tbf->m_first_dl_ack_rcvd;
}
/* Copy back to GprsMs' llc_queue the LLC PDUs previously dequeued and never
* fully ACKED at the MS side.
* FIXME: For now, only blocks transmitted and without first ever DL ACK are
* copied, because we have no way yet to track LLC PDUs once they are converted
* to RLC blocks. This is however enough to cover the case where a DL TBF is
* assigned over PCH and the MS never answers.
*/
void dl_tbf_copy_unacked_pdus_to_llc_queue(struct gprs_rlcmac_dl_tbf *tbf)
{
struct GprsMs *ms = tbf_ms(dl_tbf_as_tbf(tbf));
struct gprs_dl_llc_llist_item *llc_it;
/* If we have LLC PDU still being transmitted, prepend it first to the queue: */
if (llc_frame_length(&tbf->m_llc) > 0)
llc_queue_merge_prepend(&ms->llc_queue, &tbf->m_llc);
/* Iterate over the list of totally transmitted LLC PDUs and merge them
* into the queue. The items in the list are in inverse order of
* transmission, hence when popping from here and enqueueing (prepending)
* back to the llc_queue it ends up in the exact same initial order. */
while ((llc_it = llist_first_entry_or_null(&tbf->tx_llc_until_first_dl_ack_rcvd, struct gprs_dl_llc_llist_item, list))) {
llist_del(&llc_it->list);
llc_queue_merge_prepend(&ms->llc_queue, &llc_it->llc);
talloc_free(llc_it);
}
}
/* Does this DL TBF require to poll the MS for DL ACK/NACK? */
bool gprs_rlcmac_dl_tbf::need_poll_for_dl_ack_nack() const
{

View File

@ -42,6 +42,11 @@ enum tbf_dl_prio {
DL_PRIO_CONTROL, /* a control block needs to be sent */
};
struct gprs_dl_llc_llist_item {
struct llist_head list; /* this item added in dl_tbf->tx_llc_until_first_dl_ack_rcvd */
struct gprs_llc llc;
};
struct gprs_rlcmac_dl_tbf : public gprs_rlcmac_tbf {
gprs_rlcmac_dl_tbf(struct gprs_rlcmac_bts *bts, GprsMs *ms);
~gprs_rlcmac_dl_tbf();
@ -78,6 +83,11 @@ struct gprs_rlcmac_dl_tbf : public gprs_rlcmac_tbf {
/* Whether we receive at least one PKT DL ACK/NACK from MS since this DL TBF was assigned: */
bool m_first_dl_ack_rcvd;
/* Keep transmitted LLC PDUs until first ACK to avoid losing them if MS is not there.
* list of gprs_dl_llc_llist_item, stored in inverse order of transmission (last transmitted
* is first in the list ) */
struct llist_head tx_llc_until_first_dl_ack_rcvd;
struct BandWidth {
struct timespec dl_bw_tv; /* timestamp for dl bw calculation */
uint32_t dl_bw_octets; /* number of octets since bw_tv */
@ -161,6 +171,8 @@ void dl_tbf_request_dl_ack(struct gprs_rlcmac_dl_tbf *tbf);
bool dl_tbf_first_dl_ack_rcvd(const struct gprs_rlcmac_dl_tbf *tbf);
int dl_tbf_upgrade_to_multislot(struct gprs_rlcmac_dl_tbf *tbf);
void dl_tbf_copy_unacked_pdus_to_llc_queue(struct gprs_rlcmac_dl_tbf *tbf);
static inline struct gprs_rlcmac_tbf *dl_tbf_as_tbf(struct gprs_rlcmac_dl_tbf *dl_tbf)
{
return (struct gprs_rlcmac_tbf *)dl_tbf;

View File

@ -75,11 +75,10 @@ static void dequeue_and_check(gprs_llc_queue *queue, const uint8_t *exp_data,
size_t len, const MetaInfo *exp_info)
{
struct msgb *llc_msg;
const MetaInfo *info_res;
MetaInfo info_res;
llc_msg = llc_queue_dequeue(queue);
llc_msg = llc_queue_dequeue(queue, NULL, &info_res);
OSMO_ASSERT(llc_msg != NULL);
info_res = (struct MetaInfo *)&llc_msg->cb[0];
fprintf(stderr, "dequeued msg, length %u (expected %zu), data %s\n",
msgb_length(llc_msg), len, msgb_hexdump(llc_msg));
@ -88,8 +87,8 @@ static void dequeue_and_check(gprs_llc_queue *queue, const uint8_t *exp_data,
fprintf(stderr, "check failed!\n");
if (exp_info) {
OSMO_ASSERT(memcmp(&exp_info->recv_time, &info_res->recv_time, sizeof(info_res->recv_time)) == 0);
OSMO_ASSERT(memcmp(&exp_info->expire_time, &info_res->expire_time, sizeof(info_res->expire_time)) == 0);
OSMO_ASSERT(memcmp(&exp_info->recv_time, &info_res.recv_time, sizeof(info_res.recv_time)) == 0);
OSMO_ASSERT(memcmp(&exp_info->expire_time, &info_res.expire_time, sizeof(info_res.expire_time)) == 0);
}
msgb_free(llc_msg);
}

View File

@ -2852,7 +2852,7 @@ DL_TBF(DL:TFI-0-0-1:G:IMSI-0011223344:TLLI-0xf5667788){NEW}: Received Event ASSI
TBF(DL:TFI-0-0-1:G:IMSI-0011223344:TLLI-0xf5667788){NEW} set ass. type PACCH [prev CCCH:0, PACCH:0]
DL_TBF(DL:TFI-0-0-1:G:IMSI-0011223344:TLLI-0xf5667788){NEW}: state_chg to ASSIGN
TBF(DL:TFI-0-0-1:G:IMSI-0011223344:TLLI-0xf5667788){ASSIGN} Starting timer X2001 [assignment (PACCH)] with 2 sec. 0 microsec
New MS: TLLI = 0xf5667788, TA = 7, IMSI = 0011223344, LLC = 1
New MS: TLLI = 0xf5667788, TA = 7, IMSI = 0011223344, LLC = 2
MS(IMSI-0011223344:TLLI-0xf5667788:TA-7:MSCLS-1-0:UL:DL) Destroying MS object
MS(IMSI-0011223344:TLLI-0xf5667788:TA-7:MSCLS-1-0:UL:DL) Detaching TBF: TBF(UL:TFI-0-0-1:G:IMSI-0011223344:TLLI-0xf5667788){FLOW}
MS(IMSI-0011223344:TLLI-0xf5667788:TA-7:MSCLS-1-0:DL): - tbf: now used by 1 (tbf)