-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
Hi! First a small update: we finally decided to _not_ use token mode, as it confuses a lot of phones, after receiving the token message the SIM goes into an error state and the only way to recover is to reboot the phone. Now we just read the IMSI straight from the SIM (using the card python library, just like osmo-sim-auth does) and register them. Now, the issue we see is that 1-2 users every day reach a point where they can make phone calls and send messages, but they can't receive neither of them. Sometimes the problem can be fixed by running:
subscriber extension <EXT> clear-requests
but this only work when there are pending requests. In other cases, the phone has to be rebooted. Did anyone encounter the same issue? Cheers
Ciaby
On Sat, Nov 29, 2014 at 11:07:40AM -0600, Ciaby wrote:
Now, the issue we see is that 1-2 users every day reach a point where they can make phone calls and send messages, but they can't receive neither of them. Sometimes the problem can be fixed by running:
subscriber extension <EXT> clear-requests
but this only work when there are pending requests. In other cases, the phone has to be rebooted. Did anyone encounter the same issue?
Rebooting the phone? The sourcefile name "bsc_hack.c" is still kind of true for the NITB. This means that at somepoint some part of the code has not called subscr_put_channel.
We might add some string parameters to the logic to see which part of the code is to blame.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
On 11/30/2014 09:42 AM, Holger Hans Peter Freyther wrote: [...]
Rebooting the phone? The sourcefile name "bsc_hack.c" is still kind of true for the NITB. This means that at somepoint some part of the code has not called subscr_put_channel.
Yes, we're aware of that :)
We might add some string parameters to the logic to see which part of the code is to blame.
That would be great. Can you point me in the right direction? Which file/library is responsible for that? Cheers
Ciaby
On Sun, Nov 30, 2014 at 01:57:58PM -0600, Ciaby wrote:
That would be great. Can you point me in the right direction? Which file/library is responsible for that? Cheers
Hi,
something like the below would be a start. You could add a VTY command to dump the queue including the state and the reason string. And also show the last reason as this shows the item that is currently being dispatched.
Going through the "FIXME"/HACKs notes. Depending on how much time/budget you have this whole code could be cleaned up. In real systems the voice and SMS services are independent. They shouldn't share the same "queue".. but I am drifting away.
diff --git a/openbsc/include/openbsc/gsm_subscriber.h b/openbsc/include/openbsc/gsm_subscriber.h index 7e0a419..207c010 100644 --- a/openbsc/include/openbsc/gsm_subscriber.h +++ b/openbsc/include/openbsc/gsm_subscriber.h @@ -60,6 +60,9 @@ struct gsm_subscriber {
/* GPRS/SGSN related fields */ struct sgsn_mm_ctx *mm; + + /* debugging */ + const char *last_reason; };
enum gsm_subscriber_field { @@ -92,7 +95,7 @@ struct gsm_subscriber *subscr_get_or_create(struct gsm_network *net, int subscr_update(struct gsm_subscriber *s, struct gsm_bts *bts, int reason); void subscr_put_channel(struct gsm_subscriber *subscr); void subscr_get_channel(struct gsm_subscriber *subscr, - int type, gsm_cbfn *cbfn, void *param); + int type, gsm_cbfn *cbfn, void *param, const char *reason); struct gsm_subscriber *subscr_active_by_tmsi(struct gsm_network *net, uint32_t tmsi); struct gsm_subscriber *subscr_active_by_imsi(struct gsm_network *net, @@ -114,4 +117,38 @@ int subscr_update_expire_lu(struct gsm_subscriber *subscr, struct gsm_bts *bts); struct gsm_subscriber *subscr_alloc(void); extern struct llist_head active_subscribers;
+/* + * Struct for pending channel requests. This is managed in the + * llist_head requests of each subscriber. The reference counting + * should work in such a way that a subscriber with a pending request + * remains in memory. + */ +struct subscr_request { + struct llist_head entry; + + /* back reference */ + struct gsm_subscriber *subscr; + + /* the requested channel type */ + int channel_type; + + /* what did we do */ + int state; + + /* the callback data */ + gsm_cbfn *cbfn; + void *param; + + /* debugging */ + const char *reason; +}; + +enum { + REQ_STATE_INITIAL, + REQ_STATE_QUEUED, + REQ_STATE_PAGED, + REQ_STATE_FAILED_START, + REQ_STATE_DISPATCHED, +}; + #endif /* _GSM_SUBSCR_H */ diff --git a/openbsc/src/libbsc/bsc_ctrl_commands.c b/openbsc/src/libbsc/bsc_ctrl_commands.c index 7c8636c..a806670 100644 --- a/openbsc/src/libbsc/bsc_ctrl_commands.c +++ b/openbsc/src/libbsc/bsc_ctrl_commands.c @@ -237,7 +237,6 @@ static int verify_trx_max_power(struct ctrl_cmd *cmd, const char *value, void *_
return 0; } -CTRL_CMD_DEFINE_RANGE(trx_arfcn, "arfcn", struct gsm_bts_trx, arfcn, 0, 1023);
static int set_trx_max_power(struct ctrl_cmd *cmd, void *_data) { @@ -259,6 +258,7 @@ static int set_trx_max_power(struct ctrl_cmd *cmd, void *_data) return get_trx_max_power(cmd, _data); } CTRL_CMD_DEFINE(trx_max_power, "max-power-reduction"); +CTRL_CMD_DEFINE_RANGE(trx_arfcn, "arfcn", struct gsm_bts_trx, arfcn, 0, 1023);
int bsc_base_ctrl_cmds_install(void) { diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index db6fc5f..dbe1393 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -3115,7 +3115,7 @@ int mncc_tx_to_cc(struct gsm_network *net, int msg_type, void *arg) }
*trans->paging_request = subscr->net; - subscr_get_channel(subscr, RSL_CHANNEED_TCH_F, setup_trig_pag_evt, trans->paging_request); + subscr_get_channel(subscr, RSL_CHANNEED_TCH_F, setup_trig_pag_evt, trans->paging_request, "MNCC RX call");
subscr_put(subscr); return 0; diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c index b2100d8..802ec17 100644 --- a/openbsc/src/libmsc/gsm_04_11.c +++ b/openbsc/src/libmsc/gsm_04_11.c @@ -944,7 +944,7 @@ int gsm411_send_sms_subscr(struct gsm_subscriber *subscr, }
/* if not, we have to start paging */ - subscr_get_channel(subscr, RSL_CHANNEED_SDCCH, paging_cb_send_sms, sms); + subscr_get_channel(subscr, RSL_CHANNEED_SDCCH, paging_cb_send_sms, sms, "SEND SMS"); return 0; }
diff --git a/openbsc/src/libmsc/gsm_subscriber.c b/openbsc/src/libmsc/gsm_subscriber.c index bc6f3cf..881604b 100644 --- a/openbsc/src/libmsc/gsm_subscriber.c +++ b/openbsc/src/libmsc/gsm_subscriber.c @@ -47,37 +47,6 @@ int gsm48_secure_channel(struct gsm_subscriber_connection *conn, int key_seq, gsm_cbfn *cb, void *cb_data);
-/* - * Struct for pending channel requests. This is managed in the - * llist_head requests of each subscriber. The reference counting - * should work in such a way that a subscriber with a pending request - * remains in memory. - */ -struct subscr_request { - struct llist_head entry; - - /* back reference */ - struct gsm_subscriber *subscr; - - /* the requested channel type */ - int channel_type; - - /* what did we do */ - int state; - - /* the callback data */ - gsm_cbfn *cbfn; - void *param; -}; - -enum { - REQ_STATE_INITIAL, - REQ_STATE_QUEUED, - REQ_STATE_PAGED, - REQ_STATE_FAILED_START, - REQ_STATE_DISPATCHED, -}; - static struct gsm_subscriber *get_subscriber(struct gsm_network *net, int type, const char *ident) { @@ -125,6 +94,7 @@ static int subscr_paging_dispatch(unsigned int hooknum, unsigned int event, request->state = REQ_STATE_DISPATCHED; llist_del(&request->entry); subscr->in_callback = 1; + subscr->last_reason = request->reason; request->cbfn(hooknum, event, msg, data, request->param); subscr->in_callback = 0;
@@ -216,7 +186,7 @@ static void subscr_send_paging_request(struct gsm_subscriber *subscr) }
void subscr_get_channel(struct gsm_subscriber *subscr, - int type, gsm_cbfn *cbfn, void *param) + int type, gsm_cbfn *cbfn, void *param, const char *reason) { struct subscr_request *request;
@@ -234,6 +204,7 @@ void subscr_get_channel(struct gsm_subscriber *subscr, request->cbfn = cbfn; request->param = param; request->state = REQ_STATE_INITIAL; + request->reason = reason;
/* * FIXME: We might be able to assign more than one diff --git a/openbsc/tests/channel/channel_test.c b/openbsc/tests/channel/channel_test.c index 9fb1e9e..75b441a 100644 --- a/openbsc/tests/channel/channel_test.c +++ b/openbsc/tests/channel/channel_test.c @@ -77,7 +77,7 @@ int main(int argc, char **argv) subscr->net = network;
/* Ask for a channel... */ - subscr_get_channel(subscr, RSL_CHANNEED_TCH_F, subscr_cb, (void*)0x2342L); + subscr_get_channel(subscr, RSL_CHANNEED_TCH_F, subscr_cb, (void*)0x2342L, "dummy");
while (!s_end) { osmo_select_main(0);
On Tue, Dec 02, 2014 at 08:59:29AM +0100, Holger Hans Peter Freyther wrote:
something like the below would be a start. You could add a VTY command to dump the queue including the state and the reason string. And also show the last reason as this shows the item that is currently being dispatched.
My current thought on this system is that we should remove subscr_put_channel. It is based on some assumptions/goals that does not make sense anymore.
MT-SMS:
We need a way to enforce that only one SMS is delivered at a time. This should be done for SMS that arrive through the MO-SMS (SubmitSM), SMPP and the SMS queue.
MT-Call:
Just page if no call is present yet. We can nowadays upgrade a "SDCCH" to a TCH (late assignment)
Paging:
We need to change the way we handle the paging response. Instead of a single callback we should introduce a signal that will be handled by both SMS and Call after the channel has been authenticated.
This should avoid the whole kind of problem. In terms of time my estimate would be a week of work.
On Wed, Feb 04, 2015 at 10:10:31AM +0100, Holger Hans Peter Freyther wrote:
My current thought on this system is that we should remove subscr_put_channel. It is based on some assumptions/goals that does not make sense anymore.
I started the removal zecke/features/no-queue... it still needs integration with the SMPP/sms_queue... and I have not used it at all yet..
On Mon, Apr 06, 2015 at 12:08:19PM +0200, Holger Hans Peter Freyther wrote:
I started the removal zecke/features/no-queue... it still needs integration with the SMPP/sms_queue... and I have not used it at all yet..
I fixed the first round of crashes/defects. It should be feature equal to the current master branch. What I noticed is that if there is a SDCCH open and we place a call.. it is not using the same audio codec as the other leg of the call.
I have to monitor the tch_mode of the logical channel.
On Mon, Apr 06, 2015 at 05:17:53PM +0200, Holger Hans Peter Freyther wrote:
I fixed the first round of crashes/defects. It should be feature equal to the current master branch. What I noticed is that if there is a SDCCH open and we place a call.. it is not using the same audio codec as the other leg of the call.
I have to monitor the tch_mode of the logical channel.
Okay, we enter into the field of MNCC not being up to the job. The issue was that the channel type was compared to TCH_F and then the "TCH_H" codec was assigned...
I know the whole topic of codec selection is something to look into. I have put a band-aid on it to have gsm_04_08.c and the mncc_builtin.c to behave the same.
The next topic (besides more testing) is sms_queue and SMPP integration to enforce the one SMS per direction rule and have the two systems coordinate each other.
-
Ciaby -
Holger Hans Peter Freyther