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
This commit is contained in:
Pau Espin 2021-11-08 17:38:47 +01:00 committed by pespin
parent f8a93cb882
commit 13961c9b7a
7 changed files with 59 additions and 21 deletions

View File

@ -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);
}

View File

@ -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

View File

@ -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;
}

View File

@ -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);

View File

@ -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",

View File

@ -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);

View File

@ -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