Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431 )
Change subject: context map: introduce RUA and SCCP FSMs to fix leaks ...................................................................... Patch Set 5: (3 comments) Patchset: PS5: > 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: https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/6418a045_cd84cdc8 PS5, Line 90: /* RUA contxt ID used in RUA messages to/from the hnb_gw. */ > context thx File src/osmo-hnbgw/hnbgw_rua.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/945c7fef_a77231fe PS5, 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 https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I6ff7e36532ff57c6f2d3e7e419dd22ef27dafd19 Gerrit-Change-Number: 31431 Gerrit-PatchSet: 5 Gerrit-Owner: neels <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Tue, 21 Feb 2023 13:53:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]> Gerrit-MessageType: comment
