Change in osmo-hlr[master]: fix USSD routing to multiple MSC

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Sat Apr 6 15:14:56 UTC 2019


Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/13479 )

Change subject: fix USSD routing to multiple MSC
......................................................................


Patch Set 5: Code-Review-1

(6 comments)

current patch set has more holes than a swiss cheese, but I have a better idea...

https://gerrit.osmocom.org/#/c/13479/5/src/hlr_ussd.c
File src/hlr_ussd.c:

https://gerrit.osmocom.org/#/c/13479/5/src/hlr_ussd.c@175
PS5, Line 175: 	struct osmo_timer_list subscr_timeout;
let's take this ss->subscr from the top. I had a very simple idea in mind, but it wasn't thought through properly, confused you guys and has now mutated to a timeouted cache of stale data... Let's drop this again.

Here is a probably much better approach:

- store only the MSC name in ss->.

- For all MO USSD sessions, store the originating MSC's vlr_name in ss-> on the first message coming in from the MSC/VLR. Whatever happens, always route back to that MSC name: all errors and replies. This should then prevent all additional database lookups.

- For MT USSD sessions, there will be no MSC's vlr_name stored in ss-> at first. Do a db lookup for routing to the right vlr_name, store it in ss->vlr_name.

Hence we will hit the db for routing at most once per ss_session, will not keep any other stale subscr state, don't need timers.


https://gerrit.osmocom.org/#/c/13479/5/src/hlr_ussd.c@251
PS5, Line 251: return -EINVAL;
> memleak: msg was allocated dynamically, so here you need to free it.
thx


https://gerrit.osmocom.org/#/c/13479/5/src/hlr_ussd.c@254
PS5, Line 254: 		osmo_timer_schedule(&ss->subscr_timeout, OSMO_SESSION_SUBSCRIBER_CACHE_TIMEOUT, 0);
(this was not even assigning anything to ss->subscr)


https://gerrit.osmocom.org/#/c/13479/5/src/hlr_ussd.c@263
PS5, Line 263: USSD
> 'SS/USSD' would be more correct, as we would also deal with "structured" Supplementary Services some […]
(ack, I have no good idea about naming here)


https://gerrit.osmocom.org/#/c/13479/5/src/hlr_ussd.c@286
PS5, Line 286: resp_msg = gsm0480_msgb_alloc_name(__func__);
> So here we allocate a message buffer on heap... […]
agree, but separate patch.


https://gerrit.osmocom.org/#/c/13479/5/src/hlr_ussd.c@343
PS5, Line 343: 		osmo_timer_schedule(&ss->subscr_timeout, OSMO_SESSION_SUBSCRIBER_CACHE_TIMEOUT, 0);
(setting ss->subscr to point at local struct subscr becomes invalid memory as soon as the function exits)



-- 
To view, visit https://gerrit.osmocom.org/13479
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I18067bfadd33a6bc59a9ee336b6937313826fce3
Gerrit-Change-Number: 13479
Gerrit-PatchSet: 5
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Comment-Date: Sat, 06 Apr 2019 15:14:56 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190406/e8309a44/attachment.html>


More information about the gerrit-log mailing list