July 2023 - gerrit-log - lists.osmocom.org

[M] Change in osmo-bts[master]: common: Make socket queue max. length configurable
by arehbein 02 Jul '23

02 Jul '23

Attention is currently required from: laforge, fixeria, msuraev. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31534 ) Change subject: common: Make socket queue max. length configurable ...................................................................... Patch Set 16: (1 comment) File src/common/vty.c: https://gerrit.osmocom.org/c/osmo-bts/+/31534/comment/ce6b2868_49dfab45 PS9, Line 473: vty_out(vty, " pcu-socket-wqueue-length %d%s", bts->pcu.sock_wqueue_len_max, VTY_NEWLINE); > I agree with max. It is an unsigned int, so %d should cause a compiler warning. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31534 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Id6ba6e4eadce9ce82ef2407f4e28346e7fe4abfa Gerrit-Change-Number: 31534 Gerrit-PatchSet: 16 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Sun, 02 Jul 2023 17:46:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-bts[master]: common: Make socket queue max. length configurable
by arehbein 02 Jul '23

02 Jul '23

Attention is currently required from: arehbein, fixeria, msuraev. Hello Jenkins Builder, pespin, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/31534 to look at the new patch set (#16). Change subject: common: Make socket queue max. length configurable ...................................................................... common: Make socket queue max. length configurable Title refers to the maximum length of the osmo_wqueue used for the PCU socket connection. Related: OS#5774 Change-Id: Id6ba6e4eadce9ce82ef2407f4e28346e7fe4abfa --- M include/osmo-bts/bts.h M include/osmo-bts/pcu_if.h M src/common/bts.c M src/common/main.c M src/common/pcu_sock.c M src/common/vty.c M tests/osmo-bts.vty 7 files changed, 59 insertions(+), 25 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/34/31534/16 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31534 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Id6ba6e4eadce9ce82ef2407f4e28346e7fe4abfa Gerrit-Change-Number: 31534 Gerrit-PatchSet: 16 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-pcu[master]: cosmetic: pdch.cpp: Drop wrong comment due to copy-paste error
by pespin 02 Jul '23

02 Jul '23

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33529 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: cosmetic: pdch.cpp: Drop wrong comment due to copy-paste error ...................................................................... cosmetic: pdch.cpp: Drop wrong comment due to copy-paste error Change-Id: I40f93473c86500f655a0a83c7816a065707e2ed9 --- M src/pdch.cpp 1 file changed, 9 insertions(+), 1 deletion(-) Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve diff --git a/src/pdch.cpp b/src/pdch.cpp index 7dfd3c2..0ea4f4b 100644 --- a/src/pdch.cpp +++ b/src/pdch.cpp @@ -734,7 +734,6 @@ * contention resolution is done: */ if ((dl_tbf = ms_dl_tbf(ms))) { - /* Get rid of previous finished UL TBF before providing a new one */ LOGPTBFDL(dl_tbf, LOGL_NOTICE, "Got PACKET RESOURCE REQ while DL-TBF pending, killing it\n"); tbf_free(dl_tbf); -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33529 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I40f93473c86500f655a0a83c7816a065707e2ed9 Gerrit-Change-Number: 33529 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-pcu[master]: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted'
by pespin 02 Jul '23

02 Jul '23

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33530 ) Change subject: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted' ...................................................................... tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted' As seen during manual testing: 20230628140439674 DTBF tbf_dl.cpp:116 MS(IMSI-901700000015256:TLLI-0xd4d57c9d:TA-0:MSCLS-12-12) Allocating DL TBF ... 20230628140439674 DTBFDL tbf_dl.cpp:489 DL_TBF(DL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){NEW}: Received Event ASSIGN_ADD_CCCH 20230628140439674 DTBFDL bts.cpp:1108 TBF(DL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){ASSIGN} Tx CCCH (PCH) Immediate Assignment [PktDlAss=PDCH(bts=0,trx=0,ts=5)] TA=0 ... 20230628140440238 DMS pdch.cpp:681 MS(IMSI-901700000015256:TLLI-0xd4d57c9d:TA-0:MSCLS-12-12:DL): + rcv_resource_request: now used by 2 (tbf,rcv_resource_request) 20230628140440238 DTBFDL pdch.cpp:738 TBF(DL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){ASSIGN} Got PACKET RESOURCE REQ while DL-TBF pending, killing it 20230628140440238 DTBF tbf.cpp:271 TBF(DL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){ASSIGN} free ... 20230628140440377 DTBFUL tbf_ul_fsm.c:169 UL_TBF(UL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){ASSIGN}: state_chg to FLOW 20230628140440377 DTBF tbf_dl.cpp:116 MS(IMSI-901700000015256:TLLI-0xd4d57c9d:TA-0:MSCLS-12-12:UL) Allocating DL TBF 20230628140440377 DTBFDL tbf_dl.cpp:475 DL_TBF(DL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){NEW}: Received Event ASSIGN_ADD_PACCH 20230628140440387 DTBF tbf_dl_ass_fsm.c:105 TBF(UL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){FLOW} start Packet Downlink Assignment (PACCH) for TBF(DL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){ASSIGN} ... 20230628140440816 DTBFDL bts.cpp:737 DL_TBF(DL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){FLOW}: Received Event ASSIGN_PCUIF_CNF 20230628140440816 DTBFDL bts.cpp:737 DL_TBF(DL:TFI-0-0-0:E:IMSI-901700000015256:TLLI-0xd4d57c9d){FLOW}: Event ASSIGN_PCUIF_CNF not permitted Change-Id: I042b0117552acae25c750e762f5cc254399da64f --- M src/tbf_dl_fsm.c 1 file changed, 42 insertions(+), 0 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve diff --git a/src/tbf_dl_fsm.c b/src/tbf_dl_fsm.c index d604f6f..8d95d14 100644 --- a/src/tbf_dl_fsm.c +++ b/src/tbf_dl_fsm.c @@ -213,6 +213,20 @@ struct tbf_dl_fsm_ctx *ctx = (struct tbf_dl_fsm_ctx *)fi->priv; switch (event) { + case TBF_EV_ASSIGN_PCUIF_CNF: + if (!(ctx->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) { + /* This can happen if we initiated a CCCH DlAss from an + * older TBF object (same TLLI) towards BTS, and the DL-TBF + * was recreated (this one) and was successfully assigned over PACCH. + * This is usually the case if MS requests 2phase access + * to get an UL TBF while we were waiting for a DL TBF + * assignment for that same MS over PCH. + */ + LOGPTBFDL(ctx->dl_tbf, LOGL_INFO, + "Ignoring event ASSIGN_PCUIF_CNF from BTS " + "(CCCH was not requested on current assignment)\n"); + } + break; case TBF_EV_DL_ACKNACK_MISS: /* DL TBF: we missed a DL ACK/NACK. If we started assignment * over CCCH and never received any DL ACK/NACK yet, it means we @@ -437,6 +451,7 @@ }, [TBF_ST_FLOW] = { .in_event_mask = + X(TBF_EV_ASSIGN_PCUIF_CNF) | X(TBF_EV_DL_ACKNACK_MISS) | X(TBF_EV_LAST_DL_DATA_SENT) | X(TBF_EV_MAX_N3105), -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33530 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I042b0117552acae25c750e762f5cc254399da64f Gerrit-Change-Number: 33530 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-pcu[master]: Reestore last LLC frames never completely acked when freeing DL TBF
by pespin 02 Jul '23

02 Jul '23

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33525 ) Change subject: Reestore last LLC frames never completely acked when freeing DL TBF ...................................................................... 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 --- M src/gprs_ms.c M src/llc.c M src/llc.h M src/tbf_dl.cpp M src/tbf_dl.h M tests/llc/LlcTest.cpp M tests/tbf/TbfTest.err 7 files changed, 158 insertions(+), 21 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved diff --git a/src/gprs_ms.c b/src/gprs_ms.c index 06d012c..6e6390b 100644 --- a/src/gprs_ms.c +++ b/src/gprs_ms.c @@ -456,6 +456,8 @@ 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 @@ 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 */ diff --git a/src/llc.c b/src/llc.c index 7745bd0..45b8ac4 100644 --- a/src/llc.c +++ b/src/llc.c @@ -35,6 +35,8 @@ { 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 @@ 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 @@ 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 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 @@ 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 @@ 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; } diff --git a/src/llc.h b/src/llc.h index d12daac..594f77f 100644 --- a/src/llc.h +++ b/src/llc.h @@ -50,6 +50,17 @@ 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 @@ 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 @@ 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_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) { diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp index f4439d3..dc35b84 100644 --- a/src/tbf_dl.cpp +++ b/src/tbf_dl.cpp @@ -173,6 +173,8 @@ 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 @@ 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 @@ 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 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 @@ 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 { diff --git a/src/tbf_dl.h b/src/tbf_dl.h index dabe9a9..7223fa0 100644 --- a/src/tbf_dl.h +++ b/src/tbf_dl.h @@ -42,6 +42,11 @@ 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 @@ /* 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 @@ 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; diff --git a/tests/llc/LlcTest.cpp b/tests/llc/LlcTest.cpp index 554c749..f5bc54a 100644 --- a/tests/llc/LlcTest.cpp +++ b/tests/llc/LlcTest.cpp @@ -75,11 +75,10 @@ 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 @@ 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); } diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err index e582807..02cefe2 100644 --- a/tests/tbf/TbfTest.err +++ b/tests/tbf/TbfTest.err @@ -2852,7 +2852,7 @@ 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) -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33525 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I9be4035fb2cf2b3ee56e91dcc12cc8c24028b4aa Gerrit-Change-Number: 33525 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-mgw[master]: mgcp_client: tweak extract_codec_name() implementation
by fixeria 01 Jul '23

01 Jul '23

Attention is currently required from: neels. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/33535 ) Change subject: mgcp_client: tweak extract_codec_name() implementation ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/33535 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifc7302b6c5f9288a622e33c3e8b5fe0e7037dbdc Gerrit-Change-Number: 33535 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Sat, 01 Jul 2023 20:43:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted'
by fixeria 01 Jul '23

01 Jul '23

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33530 ) Change subject: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted' ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33530 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I042b0117552acae25c750e762f5cc254399da64f Gerrit-Change-Number: 33530 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sat, 01 Jul 2023 20:42:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: cosmetic: pdch.cpp: Drop wrong comment due to copy-paste error
by fixeria 01 Jul '23

01 Jul '23

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33529 ) Change subject: cosmetic: pdch.cpp: Drop wrong comment due to copy-paste error ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33529 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I40f93473c86500f655a0a83c7816a065707e2ed9 Gerrit-Change-Number: 33529 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sat, 01 Jul 2023 20:41:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-pcu[master]: Reestore last LLC frames never completely acked when freeing DL TBF
by fixeria 01 Jul '23

01 Jul '23

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33525 ) Change subject: Reestore last LLC frames never completely acked when freeing DL TBF ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33525 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I9be4035fb2cf2b3ee56e91dcc12cc8c24028b4aa Gerrit-Change-Number: 33525 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sat, 01 Jul 2023 20:41:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted'
by laforge 01 Jul '23

01 Jul '23

Attention is currently required from: pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33530 ) Change subject: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted' ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33530 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I042b0117552acae25c750e762f5cc254399da64f Gerrit-Change-Number: 33530 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sat, 01 Jul 2023 07:25:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0