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 <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 21 Feb 2023 13:53:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment