fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-iuh/+/26725 )

Change subject: mgw_fsm: add MGW support to osmo-hnbgw
......................................................................


Patch Set 2:

(7 comments)

https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/configure.ac
File configure.ac:

https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/configure.ac@52
PS2, Line 52: PKG_CHECK_MODULES(LIBOSMOMGCPCLIENT, libosmo-mgcp-client >= 1.9.0)
Please also make sure that both:

* 'contrib/osmo-iuh.spec.in' and
* debian/control

are updated to reflect the new dependency.


https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/hnbgw.c
File src/hnbgw.c:

https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/hnbgw.c@376
PS2, Line 376: LOGL_DEBUG
Why debug? All other categories are set to LOGL_NOTICE.


https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/hnbgw.c@379
PS2, Line 379:
ws


https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/hnbgw_cn.c
File src/hnbgw_cn.c:

https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/hnbgw_cn.c@349
PS2, Line 349:  uint8_t *msg_data;
const


https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/hnbgw_cn.c@362
PS2, Line 362:  /* Intercept RAB Assignment Request, Setup MGW FSM */
I am not saying the current approach is wrong, just an alternative variant:

  if (!map->is_ps && msgb_l2len(oph->msg) > 2) {
    switch (msgb_l2(oph->msg)[1]) {
    case RANAP_ProcedureCode_id_RAB_Assignment:
      return mgw_fsm_alloc_and_handle_rab_ass_req(map, oph);
    case RANAP_ProcedureCode_id_Iu_Release:
      /* Any IU Release will terminate the MGW FSM */
      mgw_fsm_handle_iu_release(map);
    }
  }

IMO, it's shorter and easier to read.


https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/hnbgw_rua.c
File src/hnbgw_rua.c:

https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/hnbgw_rua.c@192
PS2, Line 192:  uint8_t *msg_data;
const


https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/mgw_fsm.c
File src/mgw_fsm.c:

https://gerrit.osmocom.org/c/osmo-iuh/+/26725/2/src/mgw_fsm.c@189
PS2, Line 189:  switch (event) {
weird formatting



--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/26725
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: Ib9b62e0145184b91c56ce5d8870760bfa49cc5a4
Gerrit-Change-Number: 26725
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-CC: fixeria <[email protected]>
Gerrit-CC: laforge <[email protected]>
Gerrit-CC: pespin <[email protected]>
Gerrit-Comment-Date: Wed, 05 Jan 2022 07:21:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to