Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
gerrit-no-reply at lists.osmocom.org
Tue May 7 13:38:52 UTC 2019
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/13006 )
Change subject: hlr.c: forward GSUP messages between clients
Patch Set 13:
PS13, Line 469: session_id
> AFAIR, libosmocore doesn't encode the OSMO_GSUP_SESSION_ID_IE if gsup_msg->session_state == 0. […]
(So far none of the forwarded messages use a session id or session state.
For MAP/DIAMETER compatibility, that might be necessary/nice-to-have in the future though)
PS13, Line 485: if (!msgb_l2(msg) || !msgb_l2len(msg))
> This check is redundant and not actually needed, since we do osmo_gsup_decode(msgb_l2(msg), msgb_l2l […]
I see it as an initial sanity check. If msgb_l2() is NULL, we may run into segfaults, if the len is 0, we should reject it. I'd rather keep this check. This comes from buggy code actually killing osmo-hlr, instead it should merely complain and continue to run.
PS13, Line 501: LOGP_GSUP_FWD
> Here we would see 'OSMO_GSUP_MSGT_E_ROUTING_ERROR' as the message name, because we don't store the o […]
Ok, you are saying the logging fails to log the initial message, and instead logs the Routing Error reply.
The problem is that at this point the initial msg has already been discarded...
maybe LOG_GSUP_FWD should have a separate arg passing the original message type?
To view, visit https://gerrit.osmocom.org/13006
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Owner: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-CC: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Comment-Date: Tue, 07 May 2019 13:38:52 +0000
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the gerrit-log