Attention is currently required from: pespin.

View Change

3 comments:

• Patchset:

  • Patch Set #5:

    I find this all quite hard to review. […]

    Maybe I can spend yet another hour to see if I can split off very simple things.
    But to split is essentially asking to re-implement this from scratch. Not sure if we have enough hours left to spend on this topic for that.

    Another idea is maybe i can place comments here in gerrit for each patch chunk?

    i did initially have two patches, one adding the FSMs and one moving from oph to ranap msgb, but that was a bunch of new code being added, then changed again *a lot* in the patch following right after that. Squashing them made the patch volume shrink *a lot*, and makes *a lot* more sense when reading the code. I spent many hours on keeping the two patches separate, and it was a complete waste of time in the end, so I'm a bit frustrated with trying to do big refactoring in small patches now.
    It's like asking to build a new house while some rotten moldy house is still standing in its place -- the new doors and windows will simply not fit in the old walls.

    I would encourage to review by following the message pipeline:

    • See that rua_to_scu() is now extremely simple.
    • See that the osmo_scu_prim composition for SCCP messages has moved from hnbgw_rua.c to context_map_sccp.c, and follow the path of rua_to_scu() to osmo_sccp_user_sap_down().
    • Notice that everything connection-oriented feeds into FSM events, and that their data arg is in OTC_SELECT.

    The FSM charts are looking pretty complex too, not sure if it helps but I uploaded them for convenience:
    https://people.osmocom.org/neels/veY1eiNg/hnbgw_context_map.png
    https://people.osmocom.org/neels/veY1eiNg/hnbgw_context_map_fsm.png

• File include/osmocom/hnbgw/context_map.h:

  • Patch Set #5, Line 90: /* RUA contxt ID used in RUA messages to/from the hnb_gw. */

    context

    thx

• File src/osmo-hnbgw/hnbgw_rua.c:

  • Patch Set #5, Line 183: { RUA_ProcedureCode_id_Connect, "id-Connect" },

    Drop "id-" prefix

    AFAIK the messages are called "id-Connect" in RUA, including the "id-", whatever the meaning may be

To view, visit change 31431. To unsubscribe, or for help writing mail filters, visit settings.