aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPau Espin Pedrol <pespin@sysmocom.de>2021-11-08 17:38:47 +0100
committerpespin <pespin@sysmocom.de>2021-11-08 18:54:12 +0000
commit13961c9b7a204970240c0d56079a4961e7e69140 (patch)
tree5f5695fc317675acd315df58a21bce314c074f5f
parentf8a93cb8829cb102206e3ec8111458b131301efa (diff)
bts_pch_timer: Fix timer working only for MI type IMSI
This commit actually addresses 2 errors: 1- gprs_bssgp_pcu_rx_paging_ps() called gprs_rlcmac_paging_request() with MI which can be either TMSI or IMSI, and the later always called bts_pch_timer_start() passing mi->imsi regardless of the MI type. Hence, trash was being accessed & stored into bts_pch_timer structures if MI type used for paging was TMSI. 2- When the MS received the PS paging on CCCH and requests an UL TBF, it will send some data. If one phase access is used for whatever reason, the IMSI may not be yet available in the GprsMs object since we never received it (and we'd only have it by means of PktResourceReq). Hence, let's better first try to match the paging by TLLI/TMSI if set in both places, and otherwise use the IMSI. Related: OS#5297 Change-Id: Iedffb7c6978a3faf0fc26ce2181dde9791a8b6f4
-rw-r--r--src/bts_pch_timer.c51
-rw-r--r--src/bts_pch_timer.h8
-rw-r--r--src/gprs_bssgp_pcu.c5
-rw-r--r--src/gprs_rlcmac.cpp2
-rw-r--r--src/tbf_ul.cpp2
-rw-r--r--tests/alloc/AllocTest.cpp10
-rw-r--r--tests/alloc/AllocTest.err4
7 files changed, 60 insertions, 22 deletions
diff --git a/src/bts_pch_timer.c b/src/bts_pch_timer.c
index 20373ac8..312d85a9 100644
--- a/src/bts_pch_timer.c
+++ b/src/bts_pch_timer.c
@@ -26,8 +26,22 @@
#include <gprs_debug.h>
#include <gprs_pcu.h>
#include <bts_pch_timer.h>
+#include <gprs_ms.h>
-static struct bts_pch_timer *bts_pch_timer_get(struct gprs_rlcmac_bts *bts, const char *imsi)
+static struct bts_pch_timer *bts_pch_timer_get_by_ptmsi(struct gprs_rlcmac_bts *bts, uint32_t ptmsi)
+{
+ struct bts_pch_timer *p;
+ OSMO_ASSERT(ptmsi != GSM_RESERVED_TMSI);
+
+ llist_for_each_entry(p, &bts->pch_timer, entry) {
+ if (p->ptmsi != GSM_RESERVED_TMSI && p->ptmsi == ptmsi)
+ return p;
+ }
+
+ return NULL;
+}
+
+static struct bts_pch_timer *bts_pch_timer_get_by_imsi(struct gprs_rlcmac_bts *bts, const char *imsi)
{
struct bts_pch_timer *p;
@@ -57,29 +71,46 @@ static void T3113_callback(void *data)
bts_pch_timer_remove(p);
}
-void bts_pch_timer_start(struct gprs_rlcmac_bts *bts, const char *imsi)
+void bts_pch_timer_start(struct gprs_rlcmac_bts *bts, const struct osmo_mobile_identity *mi_paging,
+ const char *imsi)
{
- if (bts_pch_timer_get(bts, imsi))
+ struct bts_pch_timer *p;
+ struct osmo_tdef *tdef;
+
+ /* We already have a timer running for this IMSI */
+ if (bts_pch_timer_get_by_imsi(bts, imsi))
return;
- struct bts_pch_timer *p;
p = talloc_zero(bts, struct bts_pch_timer);
llist_add_tail(&p->entry, &bts->pch_timer);
- osmo_strlcpy(p->imsi, imsi, sizeof(p->imsi));
p->bts = bts;
+ OSMO_STRLCPY_ARRAY(p->imsi, imsi);
+ p->ptmsi = (mi_paging->type == GSM_MI_TYPE_TMSI) ? mi_paging->tmsi : GSM_RESERVED_TMSI;
- struct osmo_tdef *tdef = osmo_tdef_get_entry(the_pcu->T_defs, 3113);
+ tdef = osmo_tdef_get_entry(the_pcu->T_defs, 3113);
OSMO_ASSERT(tdef);
osmo_timer_setup(&p->T3113, T3113_callback, p);
osmo_timer_schedule(&p->T3113, tdef->val, 0);
- LOGP(DPCU, LOGL_DEBUG, "PCH paging timer started for IMSI=%s\n", p->imsi);
+ if (log_check_level(DPCU, LOGL_DEBUG)) {
+ char str[64];
+ osmo_mobile_identity_to_str_buf(str, sizeof(str), mi_paging);
+ LOGP(DPCU, LOGL_DEBUG, "PCH paging timer started for MI=%s IMSI=%s\n", str, p->imsi);
+ }
}
-void bts_pch_timer_stop(struct gprs_rlcmac_bts *bts, const char *imsi)
+void bts_pch_timer_stop(struct gprs_rlcmac_bts *bts, const struct GprsMs *ms)
{
- struct bts_pch_timer *p = bts_pch_timer_get(bts, imsi);
-
+ struct bts_pch_timer *p = NULL;
+ uint32_t tlli = ms_tlli(ms);
+ const char *imsi = ms_imsi(ms);
+
+ /* First try matching by TMSI if available in MS */
+ if (tlli != GSM_RESERVED_TMSI)
+ p = bts_pch_timer_get_by_ptmsi(bts, tlli);
+ /* Otherwise try matching by IMSI if available in MS */
+ if (!p && imsi[0] != '\0')
+ p = bts_pch_timer_get_by_imsi(bts, imsi);
if (p)
bts_pch_timer_remove(p);
}
diff --git a/src/bts_pch_timer.h b/src/bts_pch_timer.h
index 26b89c80..3e471618 100644
--- a/src/bts_pch_timer.h
+++ b/src/bts_pch_timer.h
@@ -32,11 +32,15 @@ struct bts_pch_timer {
struct llist_head entry;
struct gprs_rlcmac_bts *bts;
struct osmo_timer_list T3113;
+ uint32_t ptmsi; /* GSM_RESERVED_TMSI if not available */
char imsi[OSMO_IMSI_BUF_SIZE];
};
-void bts_pch_timer_start(struct gprs_rlcmac_bts *bts, const char *imsi);
-void bts_pch_timer_stop(struct gprs_rlcmac_bts *bts, const char *imsi);
+struct GprsMs;
+
+void bts_pch_timer_start(struct gprs_rlcmac_bts *bts, const struct osmo_mobile_identity *mi_paging,
+ const char *imsi);
+void bts_pch_timer_stop(struct gprs_rlcmac_bts *bts, const struct GprsMs *ms);
void bts_pch_timer_stop_all(struct gprs_rlcmac_bts *bts);
#ifdef __cplusplus
diff --git a/src/gprs_bssgp_pcu.c b/src/gprs_bssgp_pcu.c
index 0dd6cdcd..424a3814 100644
--- a/src/gprs_bssgp_pcu.c
+++ b/src/gprs_bssgp_pcu.c
@@ -39,6 +39,7 @@
#include "tbf_dl.h"
#include "llc.h"
#include "gprs_rlcmac.h"
+#include "bts_pch_timer.h"
/* Tuning parameters for BSSGP flow control */
#define FC_DEFAULT_LIFE_TIME_SECS 10 /* experimental value, 10s */
@@ -319,7 +320,9 @@ static int gprs_bssgp_pcu_rx_paging_ps(struct msgb *msg, const struct tlv_parsed
/* FIXME: look if MS is attached a specific BTS and then only page on that one? */
llist_for_each_entry(bts, &the_pcu->bts_list, list) {
- gprs_rlcmac_paging_request(bts, &paging_mi, pgroup);
+ if (gprs_rlcmac_paging_request(bts, &paging_mi, pgroup) < 0)
+ continue;
+ bts_pch_timer_start(bts, &paging_mi, mi_imsi.imsi);
}
return 0;
}
diff --git a/src/gprs_rlcmac.cpp b/src/gprs_rlcmac.cpp
index 22b12dfc..ffa656c3 100644
--- a/src/gprs_rlcmac.cpp
+++ b/src/gprs_rlcmac.cpp
@@ -26,7 +26,6 @@ extern "C" {
#include <pcu_l1_if.h>
#include <gprs_rlcmac.h>
#include <bts.h>
-#include <bts_pch_timer.h>
#include <encoding.h>
#include <tbf.h>
#include <gprs_debug.h>
@@ -49,7 +48,6 @@ int gprs_rlcmac_paging_request(struct gprs_rlcmac_bts *bts, const struct osmo_mo
return -1;
}
bts_do_rate_ctr_inc(bts, CTR_PCH_REQUESTS);
- bts_pch_timer_start(bts, mi->imsi);
pcu_l1if_tx_pch(bts, paging_request, plen, pgroup);
bitvec_free(paging_request);
diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp
index 1d06e531..02821226 100644
--- a/src/tbf_ul.cpp
+++ b/src/tbf_ul.cpp
@@ -443,7 +443,7 @@ int gprs_rlcmac_ul_tbf::rcv_data_block_acknowledged(
"Decoded premier TLLI=0x%08x of UL DATA TFI=%d.\n",
new_tlli, rlc->tfi);
update_ms(new_tlli, GPRS_RLCMAC_UL_TBF);
- bts_pch_timer_stop(bts, ms_imsi(ms()));
+ bts_pch_timer_stop(bts, ms());
} else if (new_tlli != GSM_RESERVED_TMSI && new_tlli != tlli()) {
LOGPTBFUL(this, LOGL_NOTICE,
"Decoded TLLI=%08x mismatch on UL DATA TFI=%d. (Ignoring due to contention resolution)\n",
diff --git a/tests/alloc/AllocTest.cpp b/tests/alloc/AllocTest.cpp
index 35bbfc4d..cd9c7bc8 100644
--- a/tests/alloc/AllocTest.cpp
+++ b/tests/alloc/AllocTest.cpp
@@ -806,15 +806,17 @@ static void test_2_consecutive_dl_tbfs()
static void test_bts_pch_timer(void)
{
struct gprs_rlcmac_bts *bts = bts_alloc(the_pcu, 0);
- const char *imsi1 = "1234";
- const char *imsi2 = "5678";
+ struct osmo_mobile_identity mi_imsi1, mi_imsi2;
+ mi_imsi1.type = mi_imsi2.type = GSM_MI_TYPE_IMSI;
+ OSMO_STRLCPY_ARRAY(mi_imsi1.imsi, "1234");
+ OSMO_STRLCPY_ARRAY(mi_imsi2.imsi, "5678");
fprintf(stderr, "Testing bts_pch_timer dealloc on bts dealloc\n");
log_set_category_filter(osmo_stderr_target, DPCU, 1, LOGL_DEBUG);
fprintf(stderr, "Starting PCH timer for 2 IMSI\n");
- bts_pch_timer_start(bts, imsi1);
- bts_pch_timer_start(bts, imsi2);
+ bts_pch_timer_start(bts, &mi_imsi1, mi_imsi1.imsi);
+ bts_pch_timer_start(bts, &mi_imsi2, mi_imsi2.imsi);
fprintf(stderr, "Deallocating BTS, expecting the PCH timer to be stopped and deallocated\n");
talloc_free(bts);
diff --git a/tests/alloc/AllocTest.err b/tests/alloc/AllocTest.err
index cb98332e..53e2edde 100644
--- a/tests/alloc/AllocTest.err
+++ b/tests/alloc/AllocTest.err
@@ -501219,8 +501219,8 @@ UL_ASS_TBF(DL-TFI_1){NONE}: Deallocated
DL_ASS_TBF(DL-TFI_1){NONE}: Deallocated
Testing bts_pch_timer dealloc on bts dealloc
Starting PCH timer for 2 IMSI
-PCH paging timer started for IMSI=1234
-PCH paging timer started for IMSI=5678
+PCH paging timer started for MI=IMSI-1234 IMSI=1234
+PCH paging timer started for MI=IMSI-5678 IMSI=5678
Deallocating BTS, expecting the PCH timer to be stopped and deallocated
PCH paging timer stopped for IMSI=1234
PCH paging timer stopped for IMSI=5678