cellular-infrastructure
/
osmo-pcu
mirror of https://gerrit.osmocom.org/osmo-pcu
9
0
Fork 1
Code Issues Releases Wiki Activity

Fix MS ending up with assigned imsi 000 

The whole paging path and data structre is cleaned up.
New MS helpers ms_imsi_is_valid() and ms_paging_group() are introduced
to help in the process and keep implementation details inside GprsMs
class.

Related: OS#5303
Change-Id: I4c0838b26ede58e4b711410eee2a8e4f71e9414b
This commit is contained in:
Pau Espin 2021-11-12 18:01:50 +01:00
parent 13866f02a2
commit 5deac1404d
9 changed files with 40 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/bts.cpp
View File

@ -1037,10 +1037,11 @@ int bts_rcv_ptcch_rach(struct gprs_rlcmac_bts *bts, const struct rach_ind_params
return 0;
}
void bts_snd_dl_ass(struct gprs_rlcmac_bts *bts, struct gprs_rlcmac_tbf *tbf, uint16_t pgroup)
void bts_snd_dl_ass(struct gprs_rlcmac_bts *bts, struct gprs_rlcmac_tbf *tbf)
{
uint8_t trx_no = tbf->trx->trx_no;
uint8_t ts_no = tbf->first_ts;
uint16_t pgroup = ms_paging_group(tbf_ms(tbf));
int plen;
LOGPTBF(tbf, LOGL_INFO, "TX: START Immediate Assignment Downlink (PCH)\n");

2
src/bts.h
View File

@ -301,7 +301,7 @@ uint32_t bts_rfn_to_fn(const struct gprs_rlcmac_bts *bts, int32_t rfn);
struct gprs_rlcmac_dl_tbf *bts_dl_tbf_by_tfi(struct gprs_rlcmac_bts *bts, uint8_t tfi, uint8_t trx, uint8_t ts);
struct gprs_rlcmac_ul_tbf *bts_ul_tbf_by_tfi(struct gprs_rlcmac_bts *bts, uint8_t tfi, uint8_t trx, uint8_t ts);
void bts_snd_dl_ass(struct gprs_rlcmac_bts *bts, struct gprs_rlcmac_tbf *tbf, uint16_t pgroup);
void bts_snd_dl_ass(struct gprs_rlcmac_bts *bts, struct gprs_rlcmac_tbf *tbf);
void bts_set_current_frame_number(struct gprs_rlcmac_bts *bts, uint32_t frame_number);
void bts_set_current_block_frame_number(struct gprs_rlcmac_bts *bts, int frame_number);

16
src/gprs_bssgp_pcu.c
View File

@ -108,12 +108,8 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp)
uint8_t egprs_ms_class = 0;
int rc;
MS_Radio_Access_capability_t rac;
/* TODO: is it really necessary to initialize this as a "000" IMSI? It seems, the function should just return an
* error if no IMSI IE was found. */
struct osmo_mobile_identity mi_imsi = {
.type = GSM_MI_TYPE_TMSI,
};
OSMO_STRLCPY_ARRAY(mi_imsi.imsi, "000");
const char *imsi = NULL;
struct osmo_mobile_identity mi_imsi;
budh = (struct bssgp_ud_hdr *)msgb_bssgph(msg);
tlli = ntohl(budh->tlli);
@ -144,6 +140,7 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp)
LOGP(DBSSGP, LOGL_NOTICE, "Failed to parse IMSI IE (rc=%d)\n", rc);
return bssgp_tx_status(BSSGP_CAUSE_COND_IE_ERR, NULL, msg);
}
imsi = &mi_imsi.imsi[0];
}
/* parse ms radio access capability */
@ -180,10 +177,11 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp)
"TLLI (old) IE\n");
}
LOGP(DBSSGP, LOGL_INFO, "LLC [SGSN -> PCU] = TLLI: 0x%08x IMSI: %s len: %d\n", tlli, mi_imsi.imsi, len);
LOGP(DBSSGP, LOGL_INFO, "LLC [SGSN -> PCU] = TLLI: 0x%08x IMSI: %s len: %d\n",
tlli, imsi ? : "none", len);
return dl_tbf_handle(the_pcu->bssgp.bts, tlli, tlli_old, mi_imsi.imsi,
ms_class, egprs_ms_class, delay_csec, data, len);
return dl_tbf_handle(the_pcu->bssgp.bts, tlli, tlli_old, imsi, ms_class,
egprs_ms_class, delay_csec, data, len);
}
/* 3GPP TS 48.018 Table 10.3.2. Returns 0 on success, suggested BSSGP cause otherwise */

12
src/gprs_ms.c
View File

@ -522,6 +522,18 @@ void ms_set_imsi(struct GprsMs *ms, const char *imsi)
osmo_strlcpy(ms->imsi, imsi, sizeof(ms->imsi));
}
uint16_t ms_paging_group(struct GprsMs *ms)
{
uint16_t pgroup;
if (!ms_imsi_is_valid(ms))
return 0; /* 000 is the special "all paging" group */
if ((pgroup = imsi2paging_group(ms_imsi(ms))) > 999) {
LOGPMS(ms, DRLCMAC, LOGL_ERROR, "IMSI to paging group failed!\n");
return 0;
}
return pgroup;
}
void ms_set_ta(struct GprsMs *ms, uint8_t ta_)
{
if (ta_ == ms->ta)

6
src/gprs_ms.h
View File

@ -132,6 +132,7 @@ void ms_detach_tbf(struct GprsMs *ms, struct gprs_rlcmac_tbf *tbf);
void ms_set_tlli(struct GprsMs *ms, uint32_t tlli);
bool ms_confirm_tlli(struct GprsMs *ms, uint32_t tlli);
void ms_set_imsi(struct GprsMs *ms, const char *imsi);
uint16_t ms_paging_group(struct GprsMs *ms);
void ms_update_l1_meas(struct GprsMs *ms, const struct pcu_l1_meas *meas);
@ -186,6 +187,11 @@ static inline const char *ms_imsi(const struct GprsMs *ms)
return ms->imsi;
}
static inline bool ms_imsi_is_valid(const struct GprsMs *ms)
{
return ms->imsi[0] != '\0';
}
static inline uint8_t ms_ta(const struct GprsMs *ms)
{
return ms->ta;

6
src/gprs_ms_storage.cpp
View File

@ -29,8 +29,6 @@ extern "C" {
#include <osmocom/gsm/gsm48.h>
}
#define GPRS_UNDEFINED_IMSI "000"
static void ms_storage_ms_idle_cb(struct GprsMs *ms)
{
llist_del(&ms->list);
@ -89,10 +87,10 @@ GprsMs *GprsMsStorage::get_ms(uint32_t tlli, uint32_t old_tlli, const char *imsi
/* not found by TLLI */
if (imsi && imsi[0] && strcmp(imsi, GPRS_UNDEFINED_IMSI) != 0) {
if (imsi && imsi[0] != '\0') {
llist_for_each(tmp, &m_list) {
ms = llist_entry(tmp, typeof(*ms), list);
if (strcmp(imsi, ms_imsi(ms)) == 0)
if (ms_imsi_is_valid(ms) && strcmp(imsi, ms_imsi(ms)) == 0)
return ms;
}
}

14
src/tbf_dl.cpp
View File

@ -296,7 +296,9 @@ int dl_tbf_handle(struct gprs_rlcmac_bts *bts,
/* check for existing TBF */
ms = bts_ms_store(bts)->get_ms(tlli, tlli_old, imsi);
if (ms && strlen(ms_imsi(ms)) == 0) {
/* If we got MS by TLLI above let's see if we already have another MS
* object identified by IMSI and merge them */
if (ms && !ms_imsi_is_valid(ms) && imsi) {
ms_old = bts_ms_store(bts)->get_ms(0, 0, imsi);
if (ms_old && ms_old != ms) {
/* The TLLI has changed (RAU), so there are two MS
@ -310,7 +312,7 @@ int dl_tbf_handle(struct gprs_rlcmac_bts *bts,
if (!ms_dl_tbf(ms) && ms_dl_tbf(ms_old)) {
LOGP(DTBF, LOGL_NOTICE,
"IMSI %s, old TBF %s: moving DL TBF to new MS object\n",
imsi, ms_dl_tbf(ms_old)->name());
imsi ? : "unknown", ms_dl_tbf(ms_old)->name());
dl_tbf = ms_dl_tbf(ms_old);
/* Move the DL TBF to the new MS */
dl_tbf->set_ms(ms);
@ -323,7 +325,8 @@ int dl_tbf_handle(struct gprs_rlcmac_bts *bts,
if (!ms)
ms = bts_alloc_ms(bts, ms_class, egprs_ms_class);
ms_set_imsi(ms, imsi);
if (imsi)
ms_set_imsi(ms, imsi);
ms_confirm_tlli(ms, tlli);
if (!ms_ms_class(ms) && ms_class) {
ms_set_ms_class(ms, ms_class);
@ -599,7 +602,6 @@ struct msgb *gprs_rlcmac_dl_tbf::create_dl_acked_block(uint32_t fn, uint8_t ts,
/* depending on the current TBF, we assign on PACCH or AGCH */
void gprs_rlcmac_dl_tbf::trigger_ass(struct gprs_rlcmac_tbf *old_tbf)
{
uint16_t pgroup;
/* stop pending timer */
stop_timers("assignment (DL-TBF)");
@ -618,9 +620,7 @@ void gprs_rlcmac_dl_tbf::trigger_ass(struct gprs_rlcmac_tbf *old_tbf)
osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_ASSIGN_ADD_CCCH, NULL);
/* send immediate assignment */
if ((pgroup = imsi2paging_group(imsi())) > 999)
LOGPTBFDL(this, LOGL_ERROR, "IMSI to paging group failed! (%s)\n", imsi());
bts_snd_dl_ass(bts, this, pgroup);
bts_snd_dl_ass(bts, this);
}
}

8
src/tbf_fsm.c
View File

@ -214,15 +214,11 @@ static void st_flow(struct osmo_fsm_inst *fi, uint32_t event, void *data)
if ((ctx->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))
&& !(ctx->state_flags & (1 << GPRS_RLCMAC_FLAG_DL_ACK))) {
struct GprsMs *ms = tbf_ms(ctx->tbf);
const char *imsi = ms_imsi(ms);
uint16_t pgroup;
LOGPTBF(ctx->tbf, LOGL_DEBUG, "Re-send downlink assignment on PCH (IMSI=%s)\n",
imsi);
ms_imsi_is_valid(ms) ? ms_imsi(ms) : "");
tbf_fsm_state_chg(fi, TBF_ST_ASSIGN);
/* send immediate assignment */
if ((pgroup = imsi2paging_group(imsi)) > 999)
LOGPTBF(ctx->tbf, LOGL_ERROR, "IMSI to paging group failed! (%s)\n", imsi);
bts_snd_dl_ass(ms->bts, ctx->tbf, pgroup);
bts_snd_dl_ass(ms->bts, ctx->tbf);
}
break;
case TBF_EV_LAST_DL_DATA_SENT:

2
tests/tbf/TbfTest.cpp
View File

@ -1718,7 +1718,7 @@ static void send_dl_data(struct gprs_rlcmac_bts *bts, uint32_t tlli, const char
OSMO_ASSERT(ms != NULL);
OSMO_ASSERT(ms_dl_tbf(ms) != NULL);
if (imsi[0] && strcmp(imsi, "000") != 0) {
if (imsi[0] != '\0') {
ms2 = bts_ms_by_tlli(bts, tlli, GSM_RESERVED_TMSI);
OSMO_ASSERT(ms == ms2);
}