From eddf339871559f282fe615f9c618a1220bd97743 Mon Sep 17 00:00:00 2001 From: Holger Hans Peter Freyther Date: Wed, 15 Nov 2017 12:25:47 +0100 Subject: mobile: Instead of putting semantic in a comment, use an enum The enum was created to understand the different states during the shutdown and find places where it is used. The normal transitions are like. Idle -> Imsi Detach -> L1 Reset -> Done Idle -> L1 Reset -> Done The shutdown can get stuck in case: * Out of memory situation while handling IMSI detach (timeout) * Never receiving l1 reset acknnowledgment. The code could benefit from the move to osmo fsm to deal with proper timeouts. Change-Id: Iee1140e4848923c7270495c381bf87b7e3fddee1 --- .../include/osmocom/bb/common/osmocom_data.h | 7 +++++++ src/host/layer23/src/mobile/app_mobile.c | 22 +++++++++++----------- src/host/layer23/src/mobile/gsm411_sms.c | 2 +- src/host/layer23/src/mobile/gsm480_ss.c | 2 +- src/host/layer23/src/mobile/gsm48_cc.c | 2 +- src/host/layer23/src/mobile/gsm48_mm.c | 4 ++-- src/host/layer23/src/mobile/vty_interface.c | 22 +++++++++++----------- 7 files changed, 34 insertions(+), 27 deletions(-) (limited to 'src/host/layer23') diff --git a/src/host/layer23/include/osmocom/bb/common/osmocom_data.h b/src/host/layer23/include/osmocom/bb/common/osmocom_data.h index 21b28805..7a935f97 100644 --- a/src/host/layer23/include/osmocom/bb/common/osmocom_data.h +++ b/src/host/layer23/include/osmocom/bb/common/osmocom_data.h @@ -54,6 +54,13 @@ struct rx_meas_stat { int16_t s, rl_fail; }; +enum { + MS_SHUTDOWN_NONE = 0, + MS_SHUTDOWN_IMSI_DETACH = 1, + MS_SHUTDOWN_WAIT_RESET = 2, + MS_SHUTDOWN_COMPL = 3, +}; + /* One Mobilestation for osmocom */ struct osmocom_ms { struct llist_head entity; diff --git a/src/host/layer23/src/mobile/app_mobile.c b/src/host/layer23/src/mobile/app_mobile.c index 1905010e..d28af00e 100644 --- a/src/host/layer23/src/mobile/app_mobile.c +++ b/src/host/layer23/src/mobile/app_mobile.c @@ -96,9 +96,9 @@ int mobile_signal_cb(unsigned int subsys, unsigned int signal, set = &ms->settings; /* waiting for reset after shutdown */ - if (ms->shutdown == 2) { + if (ms->shutdown == MS_SHUTDOWN_WAIT_RESET) { LOGP(DMOB, LOGL_NOTICE, "MS '%s' has been resetted\n", ms->name); - ms->shutdown = 3; + ms->shutdown = MS_SHUTDOWN_COMPL; break; } @@ -142,13 +142,13 @@ int mobile_exit(struct osmocom_ms *ms, int force) struct gsm48_mmlayer *mm = &ms->mmlayer; /* if shutdown is already performed */ - if (ms->shutdown >= 2) + if (ms->shutdown >= MS_SHUTDOWN_WAIT_RESET) return 0; if (!force && ms->started) { struct msgb *nmsg; - ms->shutdown = 1; /* going down */ + ms->shutdown = MS_SHUTDOWN_IMSI_DETACH; nmsg = gsm48_mmevent_msgb_alloc(GSM48_MM_EVENT_IMSI_DETACH); if (!nmsg) return -ENOMEM; @@ -168,10 +168,10 @@ int mobile_exit(struct osmocom_ms *ms, int force) lapdm_channel_exit(&ms->lapdm_channel); if (ms->started) { - ms->shutdown = 2; /* being down, wait for reset */ + ms->shutdown = MS_SHUTDOWN_WAIT_RESET; /* being down, wait for reset */ l1ctl_tx_reset_req(ms, L1CTL_RES_T_FULL); } else { - ms->shutdown = 3; /* being down */ + ms->shutdown = MS_SHUTDOWN_COMPL; /* being down */ } vty_notify(ms, NULL); vty_notify(ms, "Power off!\n"); @@ -230,7 +230,7 @@ int mobile_init(struct osmocom_ms *ms) gsm_random_imei(&ms->settings); - ms->shutdown = 0; + ms->shutdown = MS_SHUTDOWN_NONE; ms->started = false; if (!strcmp(ms->settings.imei, "000000000000000")) { @@ -268,7 +268,7 @@ struct osmocom_ms *mobile_new(char *name) gsm_support_init(ms); gsm_settings_init(ms); - ms->shutdown = 3; /* being down */ + ms->shutdown = MS_SHUTDOWN_COMPL; if (mncc_recv_app) { mncc_name = talloc_asprintf(ms, "/tmp/ms_mncc_%s", ms->name); @@ -298,7 +298,7 @@ int mobile_delete(struct osmocom_ms *ms, int force) ms->mncc_entity.sock_state = NULL; } - if (ms->shutdown == 0 || (ms->shutdown == 1 && force)) { + if (ms->shutdown == MS_SHUTDOWN_NONE || (ms->shutdown == MS_SHUTDOWN_IMSI_DETACH && force)) { rc = mobile_exit(ms, force); if (rc < 0) return rc; @@ -339,9 +339,9 @@ int l23_app_work(int *_quit) int work = 0; llist_for_each_entry_safe(ms, ms2, &ms_list, entity) { - if (ms->shutdown != 3) + if (ms->shutdown != MS_SHUTDOWN_COMPL) work |= mobile_work(ms); - if (ms->shutdown == 3) { + if (ms->shutdown == MS_SHUTDOWN_COMPL) { if (ms->l2_wq.bfd.fd > -1) { layer2_close(ms); ms->l2_wq.bfd.fd = -1; diff --git a/src/host/layer23/src/mobile/gsm411_sms.c b/src/host/layer23/src/mobile/gsm411_sms.c index 22db8590..1b102622 100644 --- a/src/host/layer23/src/mobile/gsm411_sms.c +++ b/src/host/layer23/src/mobile/gsm411_sms.c @@ -633,7 +633,7 @@ static int gsm411_tx_sms_submit(struct osmocom_ms *ms, const char *sms_sca, LOGP(DLSMS, LOGL_INFO, "..._sms_submit()\n"); /* no running, no transaction */ - if (!ms->started || ms->shutdown) { + if (!ms->started || ms->shutdown != MS_SHUTDOWN_COMPL) { LOGP(DLSMS, LOGL_ERROR, "Phone is down\n"); gsm411_sms_report(ms, sms, GSM411_RP_CAUSE_MO_TEMP_FAIL); sms_free(sms); diff --git a/src/host/layer23/src/mobile/gsm480_ss.c b/src/host/layer23/src/mobile/gsm480_ss.c index ff90faaf..ee2c9439 100644 --- a/src/host/layer23/src/mobile/gsm480_ss.c +++ b/src/host/layer23/src/mobile/gsm480_ss.c @@ -603,7 +603,7 @@ int ss_send(struct osmocom_ms *ms, const char *code, int new_trans) } /* no running, no transaction */ - if (!ms->started || ms->shutdown) { + if (!ms->started || ms->shutdown != MS_SHUTDOWN_NONE) { gsm480_ss_result(ms, "", 0); return -EIO; } diff --git a/src/host/layer23/src/mobile/gsm48_cc.c b/src/host/layer23/src/mobile/gsm48_cc.c index d398c765..f1e81098 100644 --- a/src/host/layer23/src/mobile/gsm48_cc.c +++ b/src/host/layer23/src/mobile/gsm48_cc.c @@ -1923,7 +1923,7 @@ int mncc_tx_to_cc(void *inst, int msg_type, void *arg) struct gsm_trans *trans; int i, rc; - if (!ms->started || ms->shutdown) { + if (!ms->started || ms->shutdown != MS_SHUTDOWN_NONE) { LOGP(DCC, LOGL_NOTICE, "Phone is down!\n"); if (ms->mncc_entity.mncc_recv && msg_type != MNCC_REL_REQ) { struct gsm_mncc rel; diff --git a/src/host/layer23/src/mobile/gsm48_mm.c b/src/host/layer23/src/mobile/gsm48_mm.c index 4b1db1e5..f32d57ab 100644 --- a/src/host/layer23/src/mobile/gsm48_mm.c +++ b/src/host/layer23/src/mobile/gsm48_mm.c @@ -1849,7 +1849,7 @@ static int gsm48_mm_imsi_detach_end(struct osmocom_ms *ms, struct msgb *msg) subscr->sim_valid = 0; /* wait for RR idle and then power off when IMSI is detached */ - if (ms->shutdown) { + if (ms->shutdown != MS_SHUTDOWN_NONE) { if (mm->state == GSM48_MM_ST_MM_IDLE) { mobile_exit(ms, 1); return 0; @@ -1944,7 +1944,7 @@ static int gsm48_mm_imsi_detach_release(struct osmocom_ms *ms, struct msgb *msg) new_mm_state(mm, GSM48_MM_ST_WAIT_NETWORK_CMD, 0); /* power off */ - if (ms->shutdown) { + if (ms->shutdown != MS_SHUTDOWN_NONE) { mobile_exit(ms, 1); return 0; } diff --git a/src/host/layer23/src/mobile/vty_interface.c b/src/host/layer23/src/mobile/vty_interface.c index 3703ac5a..d11f625a 100644 --- a/src/host/layer23/src/mobile/vty_interface.c +++ b/src/host/layer23/src/mobile/vty_interface.c @@ -123,7 +123,7 @@ static void vty_restart(struct vty *vty, struct osmocom_ms *ms) { if (vty_reading) return; - if (ms->shutdown != 0) + if (ms->shutdown != MS_SHUTDOWN_NONE) return; vty_out(vty, "You must restart MS '%s' ('shutdown / no shutdown') for " "change to take effect!%s", ms->name, VTY_NEWLINE); @@ -142,7 +142,7 @@ static struct osmocom_ms *get_ms(const char *name, struct vty *vty) llist_for_each_entry(ms, &ms_list, entity) { if (!strcmp(ms->name, name)) { - if (ms->shutdown) { + if (ms->shutdown != MS_SHUTDOWN_NONE) { vty_out(vty, "MS '%s' is admin down.%s", name, VTY_NEWLINE); return NULL; @@ -189,9 +189,9 @@ static void gsm_ms_dump(struct osmocom_ms *ms, struct vty *vty) service = ", MM connection active"; vty_out(vty, "MS '%s' is %s%s%s%s", ms->name, - (ms->shutdown) ? "administratively " : "", - (ms->shutdown || !ms->started) ? "down" : "up", - (!ms->shutdown) ? service : "", + (ms->shutdown != MS_SHUTDOWN_NONE) ? "administratively " : "", + (ms->shutdown != MS_SHUTDOWN_NONE || !ms->started) ? "down" : "up", + (ms->shutdown == MS_SHUTDOWN_NONE) ? service : "", VTY_NEWLINE); vty_out(vty, " IMEI: %s%s", set->imei, VTY_NEWLINE); vty_out(vty, " IMEISV: %s%s", set->imeisv, VTY_NEWLINE); @@ -201,7 +201,7 @@ static void gsm_ms_dump(struct osmocom_ms *ms, struct vty *vty) else vty_out(vty, " IMEI generation: fixed%s", VTY_NEWLINE); - if (ms->shutdown) + if (ms->shutdown != MS_SHUTDOWN_NONE) return; if (set->plmn_mode == PLMN_MODE_AUTO) @@ -303,7 +303,7 @@ DEFUN(show_subscr, show_subscr_cmd, "show subscriber [MS_NAME]", gsm_subscr_dump(&ms->subscr, print_vty, vty); } else { llist_for_each_entry(ms, &ms_list, entity) { - if (!ms->shutdown) { + if (ms->shutdown == MS_SHUTDOWN_NONE) { gsm_subscr_dump(&ms->subscr, print_vty, vty); vty_out(vty, "%s", VTY_NEWLINE); } @@ -1529,7 +1529,7 @@ static void config_write_ms(struct vty *vty, struct osmocom_ms *ms) (set->test_always) ? "everywhere" : "foreign-country", VTY_NEWLINE); /* no shutdown must be written to config, because shutdown is default */ - vty_out(vty, " %sshutdown%s", (ms->shutdown) ? "" : "no ", + vty_out(vty, " %sshutdown%s", (ms->shutdown != MS_SHUTDOWN_NONE) ? "" : "no ", VTY_NEWLINE); vty_out(vty, "!%s", VTY_NEWLINE); } @@ -2699,7 +2699,7 @@ DEFUN(cfg_no_shutdown, cfg_ms_no_shutdown_cmd, "no shutdown", struct osmocom_ms *ms = vty->index, *tmp; int rc; - if (ms->shutdown != 3) + if (ms->shutdown != MS_SHUTDOWN_COMPL) return CMD_SUCCESS; llist_for_each_entry(tmp, &ms_list, entity) { @@ -2738,7 +2738,7 @@ DEFUN(cfg_shutdown, cfg_ms_shutdown_cmd, "shutdown", { struct osmocom_ms *ms = vty->index; - if (ms->shutdown == 0) + if (ms->shutdown == MS_SHUTDOWN_NONE) mobile_exit(ms, 0); return CMD_SUCCESS; @@ -2749,7 +2749,7 @@ DEFUN(cfg_shutdown_force, cfg_ms_shutdown_force_cmd, "shutdown force", { struct osmocom_ms *ms = vty->index; - if (ms->shutdown <= 1) + if (ms->shutdown <= MS_SHUTDOWN_IMSI_DETACH) mobile_exit(ms, 1); return CMD_SUCCESS; -- cgit v1.2.3