Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients

Neels Hofmeyr 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:

(3 comments)

https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c
File src/hlr.c:

https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@469
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)


https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@485
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.


https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@501
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.
Nice catch.

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-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
Gerrit-Change-Number: 13006
Gerrit-PatchSet: 13
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
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190507/55ec6f4b/attachment.html>


More information about the gerrit-log mailing list