pespin has submitted this change. (
https://gerrit.osmocom.org/c/osmo-pcu/+/30639 )
Change subject: tests/TbfTest: reproduce buggy corner case: MS with TBFs on 2
TRXs
......................................................................
tests/TbfTest: reproduce buggy corner case: MS with TBFs on 2 TRXs
Add a test which showcases a scenario where the PCU ends up with 1
GprsMs object holding a DL-TBF in a weird condition half in one TRX and
half in other due to ms_merge().
This test (slightly adapted) used to cause a crash in osmo-pcu.git
586ddda9bc09d60f2d491158de843825cb7c876a (a few versions behind current
master).
Related: SYS#6231
Change-Id: Ic16b5e96debf91e72684833cd64253687857f3aa
---
M tests/tbf/TbfTest.cpp
M tests/tbf/TbfTest.err
2 files changed, 207 insertions(+), 0 deletions(-)
Approvals:
osmith: Looks good to me, but someone else must approve
fixeria: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index 6d57843..6769165 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -2406,6 +2406,106 @@
TALLOC_FREE(the_pcu);
}
+/* Test DL-TBF first assigned over CCCH ImmAss, then after X2002 timeout, when
MS
+is available to receive from TBF on PACCH, upgrade it to multislot. In the
+middle the MS would request a new UL-TBF and PCU ends up creating 2 MS objects
on
+different TRX, which are eventually merged.
+Hence, new multislot DL-TBF allocation (assigned over PACCH) happens on a
different TRX
+than the one which was assigned over CCCH and where the PktDlAss is sent.
SYS#6231 */
+static void test_ms_merge_dl_tbf_different_trx(void)
+{
+ the_pcu = prepare_pcu();
+ struct gprs_rlcmac_bts *bts = bts_alloc(the_pcu, 0);
+ GprsMs *first_ms, *second_ms;
+ uint8_t ts_no = 1;
+ uint8_t ms_class = 11;
+ struct gprs_rlcmac_trx *trx0 = &bts->trx[0];
+ struct gprs_rlcmac_trx *trx1 = &bts->trx[1];
+ uint32_t old_tlli = 0xa3c2f953;
+ uint32_t new_tlli = 0xecc1f953;
+ const char *imsi = "001001000000001";
+ uint8_t llc_buf[19];
+ int rc;
+ unsigned delay_csec = 1000;
+ struct gprs_rlcmac_dl_tbf *dl_tbf;
+ struct gprs_rlcmac_ul_tbf *ul_tbf;
+ uint32_t fn = 0;
+
+ fprintf(stderr, "=== start %s ===\n", __func__);
+
+ bts->pcu->nsi = gprs_ns2_instantiate(tall_pcu_ctx, gprs_ns_prim_cb,
NULL);
+ if (!bts->pcu->nsi) {
+ LOGP(DBSSGP, LOGL_ERROR, "Failed to create NS instance\n");
+ abort();
+ }
+
+ setup_bts(bts, ts_no);
+ the_pcu->alloc_algorithm = alloc_algorithm_b;
+ /* trx0->pdch[ts_no].enable(); Already enabled during setup_bts()) */
+ trx0->pdch[ts_no].disable();
+ trx1->pdch[4].enable();
+ trx1->pdch[5].enable();
+ trx1->pdch[6].enable();
+ gprs_bssgp_init(bts, 5234, 5234, 1, 1, false, 0, 0, 0);
+
+ /* Handle LLC frame 1. This will create the TBF we want in TRX1 and
+ * we'll have it upgrade to multislot on TRX0 later. */
+ memset(llc_buf, 1, sizeof(llc_buf));
+ rc = dl_tbf_handle(bts, old_tlli, 0, imsi, ms_class, 0,
+ delay_csec, llc_buf, sizeof(llc_buf));
+ OSMO_ASSERT(rc >= 0);
+
+ first_ms = bts_ms_store(bts)->get_ms(GSM_RESERVED_TMSI,
GSM_RESERVED_TMSI, imsi);
+ OSMO_ASSERT(first_ms);
+ dl_tbf = ms_dl_tbf(first_ms);
+ OSMO_ASSERT(dl_tbf);
+ OSMO_ASSERT(tbf_get_trx(dl_tbf) == trx1);
+ OSMO_ASSERT(dl_tbf->control_ts->trx == trx1);
+ struct gprs_rlcmac_pdch *old_dl_control_ts = dl_tbf->control_ts;
+
+ /* Enable PDCHs on TRX0 so that second_ms is allocated on TRX0: */
+ trx0->pdch[1].enable();
+ trx0->pdch[2].enable();
+ trx0->pdch[3].enable();
+
+ second_ms = bts_alloc_ms(bts, 0, 0);
+ ms_set_tlli(second_ms, new_tlli);
+ ul_tbf = ul_tbf_alloc(bts, second_ms, 0, true);
+ OSMO_ASSERT(ul_tbf != NULL);
+ ms_update_announced_tlli(second_ms, new_tlli);
+
+ /* Here PCU gets to know the MS are the same and they get merged. */
+ rc = dl_tbf_handle(bts, new_tlli, old_tlli, imsi, ms_class, 0,
+ delay_csec, llc_buf, sizeof(llc_buf));
+ OSMO_ASSERT(rc >= 0);
+ /* Here we assert DL-TBF is currently moved to the new MS, which is
wrong! */
+ OSMO_ASSERT(dl_tbf == ms_dl_tbf(second_ms));
+
+ /* Here BTS would answer with data_cnf and trigger
+ * bts_rcv_imm_ass_cnf(), which would trigger TBF_EV_ASSIGN_PCUIF_CNF.
+ * That in turn would set up timer X2002. Finally, X2002 timeout
+ * moves it to ASSIGN state for multislot upgrade. We set X2002 timeout
to 0 here to get
+ * immediate trigger through osmo_select_main() */
+ OSMO_ASSERT(osmo_tdef_set(the_pcu->T_defs, -2002, 0, OSMO_TDEF_MS) ==
0);
+ osmo_fsm_inst_dispatch(ms_dl_tbf(second_ms)->state_fi,
TBF_EV_ASSIGN_PCUIF_CNF, NULL);
+ osmo_select_main(0);
+ OSMO_ASSERT(dl_tbf == ms_dl_tbf(second_ms));
+ OSMO_ASSERT(dl_tbf->state_is(TBF_ST_ASSIGN));
+ /* Here we validate DL-TBF was intially allocated in TRX1 but moved to
TRX0 during multislot upgrade: */
+ OSMO_ASSERT(tbf_get_trx(dl_tbf) == trx0);
+ OSMO_ASSERT(old_dl_control_ts != dl_tbf->control_ts);
+
+ /* dl_tbf is still in the list of trx1 so that the PktDlAss on the old
+ * TRX/TS can be scheduled to assign the new TRX/TS allocation: */
+ OSMO_ASSERT(dl_tbf == llist_first_entry_or_null(&trx1->dl_tbfs, struct
llist_item, list)->entry);
+
+ /* get the PACCH PktDlAss for the DL-TBF: */
+ request_dl_rlc_block(dl_tbf->bts, dl_tbf->control_ts, &fn);
+
+ fprintf(stderr, "=== end %s ===\n", __func__);
+ TALLOC_FREE(the_pcu);
+}
+
static void test_tbf_puan_urbb_len(void)
{
the_pcu = prepare_pcu();
@@ -3409,6 +3509,7 @@
test_packet_access_rej_epdan();
test_packet_access_rej_prr();
test_packet_access_rej_prr_no_other_tbfs();
+ test_ms_merge_dl_tbf_different_trx();
if (getenv("TALLOC_REPORT_FULL"))
talloc_report_full(tall_pcu_ctx, stderr);
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index e38e694..ead39bc 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -9429,3 +9429,109 @@
UL_ASS_TBF(UL:TFI-0-0--1:STATE-NEW:GPRS:TLLI-0xffeeddcc){NONE}: Deallocated
DL_ASS_TBF(UL:TFI-0-0--1:STATE-NEW:GPRS:TLLI-0xffeeddcc){NONE}: Deallocated
=== end test_packet_access_rej_prr_no_other_tbfs ===
+=== start test_ms_merge_dl_tbf_different_trx ===
+PDCH(bts=0,trx=0,ts=1) PDCH state: disabled => enabled
+PDCH(bts=0,trx=0,ts=1) PDCH state: enabled => disabled
+PDCH(bts=0,trx=1,ts=4) PDCH state: disabled => enabled
+PDCH(bts=0,trx=1,ts=5) PDCH state: disabled => enabled
+PDCH(bts=0,trx=1,ts=6) PDCH state: disabled => enabled
+Creating MS object, TLLI = 0xffffffff
+Modifying MS object, TLLI = 0xffffffff, MS class 0 -> 11
+Modifying MS object, TLLI = 0xffffffff, IMSI '' -> '001001000000001'
+The MS object cannot fully confirm an unexpected TLLI: 0xa3c2f953, partly
confirmed
+MS(IMSI-001001000000001:TLLI-0xa3c2f953:TA-220:MSCLS-11-0) appending 19 bytes
to DL LLC queue
+MS(IMSI-001001000000001:TLLI-0xa3c2f953:TA-220:MSCLS-11-0) ********** DL-TBF
starts here **********
+MS(IMSI-001001000000001:TLLI-0xa3c2f953:TA-220:MSCLS-11-0) Allocating DL TBF
+UL_ASS_TBF{NONE}: Allocated
+DL_ASS_TBF{NONE}: Allocated
+DL_TBF{NEW}: Allocated
+[DL] algo B <single> (suggested TRX: -1): Alloc start
+Found first unallocated TRX=1 TFI=0
+Slot Allocation (Algorithm B) for class 11
+- Skipping TS 0, because not enabled
+- Skipping TS 1, because not enabled
+- Skipping TS 2, because not enabled
+- Skipping TS 3, because not enabled
+- Skipping TS 7, because not enabled
+- Possible DL/UL slots: (TS=0)"....CCC."(TS=7)
+Rx=4 Tx=3 Sum Rx+Tx=5, Tta=3 Ttb=1, Tra=2 Trb=1, Type=1
+- Skipping TS 6, because num TBFs 0 >= 0
+Selected DL slots: (TS=0)".ddd.D.."(TS=7), single
+[DL] algo B <single> (suggested TRX: -1): using single slot at TS 5
+- Reserved DL/UL slots: (TS=0)"..D..CU."(TS=7)
+- Assigning DL TS 5
+PDCH(bts=0,trx=1,ts=5) Attaching
TBF(DL:TFI-0-1-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953), 1 TBFs,
USFs = 00, TFIs = 00000001.
+TBF(DL:TFI-0-1-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953) Setting
Control TS PDCH(bts=0,trx=1,ts=5)
+TBF(DL:TFI-0-1-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953)
Allocated: trx = 1, ul_slots = 20, dl_slots = 20
+MS(IMSI-001001000000001:TLLI-0xa3c2f953:TA-220:MSCLS-11-0) Attaching DL TBF:
TBF(DL:TFI-0-1-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953)
+TBF(DL:TFI-0-1-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953)
[DOWNLINK] START (PCH)
+TBF(DL:TFI-0-1-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953) Send
downlink assignment on PCH, no TBF exist (IMSI=001001000000001)
+DL_TBF(DL:TFI-0-1-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953){NEW}:
Received Event ASSIGN_ADD_CCCH
+TBF(DL:TFI-0-1-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953) set ass.
type CCCH [prev CCCH:0, PACCH:0]
+DL_TBF(DL:TFI-0-1-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953){NEW}:
state_chg to ASSIGN
+TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953) TX:
START Immediate Assignment Downlink (PCH)
+ - TRX=1 (0) TS=5 TA=220
+PDCH(bts=0,trx=0,ts=1) PDCH state: disabled => enabled
+PDCH(bts=0,trx=0,ts=2) PDCH state: disabled => enabled
+PDCH(bts=0,trx=0,ts=3) PDCH state: disabled => enabled
+Creating MS object, TLLI = 0xffffffff
+Modifying MS object, UL TLLI: 0xffffffff -> 0xecc1f953, not yet confirmed
+MS(TLLI-0xecc1f953:TA-220:MSCLS-0-0) ********** UL-TBF starts here **********
+MS(TLLI-0xecc1f953:TA-220:MSCLS-0-0) Allocating UL TBF
+UL_ASS_TBF{NONE}: Allocated
+DL_ASS_TBF{NONE}: Allocated
+UL_TBF{NEW}: Allocated
+UL_ACK_TBF{NONE}: Allocated
+[UL] algo B <single> (suggested TRX: 0): Alloc start
+Found first unallocated TRX=0 TFI=0
+- Skipping TS 0, because not enabled
+- Skipping TS 4, because not enabled
+- Skipping TS 5, because not enabled
+- Skipping TS 6, because not enabled
+- Skipping TS 7, because not enabled
+- Possible DL/UL slots: (TS=0)".CCC...."(TS=7)
+Rx=4 Tx=4 Sum Rx+Tx=5, Tta=2 Ttb=1, Tra=2 Trb=1, Type=1
+- Skipping TS 3, because num TBFs 0 >= 0
+Selected UL slots: (TS=0)"..U.uu.."(TS=7), single
+[UL] algo B <single> (suggested TRX: 0): using single slot at TS 2
+- Reserved DL/UL slots: (TS=0)"..C.DDD."(TS=7)
+- Assigning UL TS 2
+PDCH(bts=0,trx=0,ts=2) Attaching
TBF(UL:TFI-0-0-0:STATE-NEW:GPRS:TLLI-0xecc1f953), 1 TBFs, USFs = 01, TFIs =
00000001.
+TBF(UL:TFI-0-0-0:STATE-NEW:GPRS:TLLI-0xecc1f953) Setting Control TS
PDCH(bts=0,trx=0,ts=2)
+TBF(UL:TFI-0-0-0:STATE-NEW:GPRS:TLLI-0xecc1f953) Allocated: trx = 0, ul_slots
= 04, dl_slots = 00
+MS(TLLI-0xecc1f953:TA-220:MSCLS-0-0) Attaching UL TBF:
TBF(UL:TFI-0-0-0:STATE-NEW:GPRS:TLLI-0xecc1f953)
+There is a new MS object for the same MS: (0xa3c2f953, '001001000000001') ->
(0xecc1f953, '')
+MS(TLLI-0xecc1f953:TA-220:MSCLS-0-0:UL) Merge MS:
MS(IMSI-001001000000001:TLLI-0xa3c2f953:TA-220:MSCLS-11-0:DL)
+Modifying MS object, TLLI = 0xecc1f953, MS class 0 -> 11
+TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953) Merge
MS: Move DL TBF: MS(IMSI-001001000000001:TLLI-0xa3c2f953:TA-220:MSCLS-11-0:DL)
=> MS(IMSI-001001000000001:TLLI-0xecc1f953:TA-220:MSCLS-11-0:UL)
+MS(IMSI-001001000000001:TLLI-0xa3c2f953:TA-220:MSCLS-11-0) Detaching TBF:
TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xa3c2f953)
+MS(IMSI-001001000000001:TLLI-0xecc1f953:TA-220:MSCLS-11-0:UL) Attaching DL
TBF: TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953)
+MS(IMSI-001001000000001:TLLI-0xa3c2f953:TA-220:MSCLS-11-0) Clearing MS object
+MS(TA-220:MSCLS-11-0) Destroying MS object
+Modifying MS object, TLLI: 0xecc1f953 confirmed
+MS(IMSI-001001000000001:TLLI-0xecc1f953:TA-220:MSCLS-11-0:UL:DL) appending 19
bytes to DL LLC queue
+DL_TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953){ASSIGN}:
Received Event ASSIGN_PCUIF_CNF
+TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953)
Starting timer X2002 [assignment (AGCH)] with 0 sec. 0 microsec
+DL_TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953){ASSIGN}:
Timeout of X2002
+TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953)
Changing Control TS PDCH(bts=0,trx=1,ts=5) -> PDCH(bts=0,trx=0,ts=2)
+TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953)
Upgrade to multislot
+PDCH(bts=0,trx=1,ts=5) Detaching
TBF(DL:TFI-0-1-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953), 1
TBFs, USFs = 00, TFIs = 00000001.
+[DL] algo B <multi> (suggested TRX: -1): Alloc start
+Found first unallocated TRX=0 TFI=0
+Selected DL slots: (TS=0)".DDDddd."(TS=7), multi
+[DL] algo B <multi> (suggested TRX: -1): using 3 slots
+- Assigning DL TS 1
+PDCH(bts=0,trx=0,ts=1) Attaching
TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953), 1
TBFs, USFs = 00, TFIs = 00000001.
+- Assigning DL TS 2
+PDCH(bts=0,trx=0,ts=2) Attaching
TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953), 1
TBFs, USFs = 01, TFIs = 00000001.
+- Assigning DL TS 3
+PDCH(bts=0,trx=0,ts=3) Attaching
TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953), 1
TBFs, USFs = 00, TFIs = 00000001.
+TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953) Send
downlink assignment on PACCH, because
TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953) exists
+DL_ASS_TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953){NONE}:
Received Event SCHED_ASS
+DL_ASS_TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953){NONE}:
state_chg to SEND_ASS
+DL_TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953){ASSIGN}:
Received Event ASSIGN_ADD_PACCH
+TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953) set
ass. type PACCH [prev CCCH:0, PACCH:0]
+=== end test_ms_merge_dl_tbf_different_trx ===
+MS(IMSI-001001000000001:TLLI-0xecc1f953:TA-220:MSCLS-11-0:UL:DL) Destroying MS
object
+MS(IMSI-001001000000001:TLLI-0xecc1f953:TA-220:MSCLS-11-0:DL) Detaching TBF:
TBF(UL:TFI-0-0-0:STATE-NEW:GPRS:IMSI-001001000000001:TLLI-0xecc1f953)
+MS(IMSI-001001000000001:TLLI-0xecc1f953:TA-220:MSCLS-11-0) Detaching TBF:
TBF(DL:TFI-0-0-0:STATE-ASSIGN:GPRS:IMSI-001001000000001:TLLI-0xecc1f953)
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/30639
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: Ic16b5e96debf91e72684833cd64253687857f3aa
Gerrit-Change-Number: 30639
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: lynxis lazus <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged
