dexter has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795 )
Change subject: mgw_fsm: add MGW support to osmo-hnbgw
......................................................................
Patch Set 13:
(5 comments)
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795/7/src/osmo-hnbgw/hnbgw_cn.c
File src/osmo-hnbgw/hnbgw_cn.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795/7/src/osmo-hnbgw/hnbgw_cn.c…
PS7, Line 362: switch (((uint8_t *)msgb_l2(oph->msg))[1]) {
hmm. […]
Done
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795/7/src/osmo-hnbgw/hnbgw_rua.c
File src/osmo-hnbgw/hnbgw_rua.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795/7/src/osmo-hnbgw/hnbgw_rua.…
PS7, Line 273: case RANAP_ProcedureCode_id_RAB_Assignment:
likewise the same here.
Done
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795/1/src/osmo-hnbgw/hnbgw_vty.c
File src/osmo-hnbgw/hnbgw_vty.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795/1/src/osmo-hnbgw/hnbgw_vty.…
PS1, Line 451: g_hnb_gw->config.mgcp_client = talloc_zero(tall_hnb_ctx, struct
mgcp_client_conf);
This doesn't look like the proper place to do
this. Please move this to the g_hnb_gw alloc function.
Done
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795/1/src/osmo-hnbgw/mgw_fsm.c
File src/osmo-hnbgw/mgw_fsm.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795/1/src/osmo-hnbgw/mgw_fsm.c@…
PS1, Line 84: /* We have to store the RANAP RAB AssignmentRequest message, since we can
not take ownership of the osmo prim
why can't you take ownsership? is it freed by the
callback caller? If not, talloc_steal, if yes, msg […]
It is freed by the code that
calls mgw_fsm_alloc_and_handle_rab_ass_req() from hnbgw_cn.c. See also sccp_sap_up() It is
built that way. We could try to change it though its only 4 functions that would have to
be changed so that we can take ownership of the msg buffer.
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795/1/src/osmo-hnbgw/mgw_fsm.c@…
PS1, Line 420: .in_event_mask = S(MGW_EV_MGCP_OK) | S(MGW_EV_MGCP_FAIL),
I find it much more clear having one per line, but OK.
If you give me an example from somewhere in the osmocom codebase I can replicate the
coding style.
--
To view, visit
https://gerrit.osmocom.org/c/osmo-hnbgw/+/26795
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ib9b62e0145184b91c56ce5d8870760bfa49cc5a4
Gerrit-Change-Number: 26795
Gerrit-PatchSet: 13
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Fri, 14 Jan 2022 16:47:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment