osmith has uploaded a new patch set (#20) to the change originally created by neels. ( https://gerrit.osmocom.org/c/osmo-hlr/+/16205 )
Change subject: refactor: add and use lu_fsm, osmo_gsup_req, osmo_gt ...................................................................... refactor: add and use lu_fsm, osmo_gsup_req, osmo_gt These are seemingly orthogonal changes in one patch, because they are in fact sufficiently intertwined that we are not willing to spend the time to separate them. They are also refactoring changes, unlikely to make sense on their own. ** lu_fsm: Attempting to make luop.c keep state about incoming GSUP requests made me find shortcomings in several places: - since it predates osmo_fsm, it is a state machine that does not strictly enforce the order of state transitions or the right sequence of incoming events. - several places OSMO_ASSERT() on data received from the network. - modifies the subscriber state before a LU is accepted. - dead code about canceling a subscriber in a previous VLR. That would be a good thing to actually do, which should also be trivial now that we record vlr_name and sgsn_name, but I decided to remove the dead code for now. To both step up the LU game *and* make it easier for me to integrate osmo_gsup_req handling, I decided to create a lu_fsm, drawing from my, by now, ample experience of writing osmo_fsms. ** osmo_gsup_req: Prepare for D-GSM, where osmo-hlr will do proxy routing for remote HLRs / communicate with remote MSCs via a proxy: a) It is important that a response that osmo-hlr generates and that is sent back to a requesting MSC contains all IEs that are needed to route it back to the requester. Particularly source_name must become destination_name in the response to be able to even reach the requesting MSC. Other fields are also necessary to match, which were so far taken care of in individual numerous code paths. b) For some operations, the response to a GSUP request is generated asynchronously (like Update Location Request -> Response, or taking the response from an EUSE, or the upcoming proxying to a remote HLR). To be able to feed a request message's information back into the response, we must thus keep the request data around. Since struct osmo_gsup_message references a lot of external data, usually with pointers directly into the received msgb, it is not so trivial to pass GSUP message data around asynchronously, on its own. osmo_gsup_req is the combined solution for both a and b: it keeps all data for a GSUP message by taking ownership of the incoming msgb, and it provides an explicit API "forcing" callers to respond with osmo_gsup_req_respond(), so that all code paths trivially are definitely responding with the correct IEs set to match the request's routing (by using osmo_gsup_make_response() recently added to libosmocore). Adjust all osmo-hlr code paths to use *only* osmo_gsup_req to respond to incoming requests received on the GSUP server (above LU code being one of them). In fact, the same should be done on the client side. Hence osmo_gsup_req is implemented in a server/client agnostic way, and is placed in libosmo-gsupclient. As soon as we see routing errors in complex GSUP setups, using osmo_gsup_req in the related GSUP client is likely to resolve those problems without much thinking required beyond making all code paths use it. libosmo-gsupclient is hence added to osmo-hlr binary's own library dependencies. It would have been added by the D-GSM proxy routing anyway, we are just doing it a little sooner. ** global_title.c / osmo_gt: We so far handle a Global Title a.k.a. an IPA unit name as pointer + size, or as just pointer with implicit talloc size. To ease working with GSUP peer identification data, I require: - a non-allocated storage of a Global Title. It brings the drawback of being size limited, but our current implementation is anyway only able to handle MSC and SGSN names of 31 characters (see struct hlr_subscriber). - a single-argument handle for Global Title, - easy to use utility functions like osmo_gt_name(), osmo_gt_cmp(), and copying by simple assignment, a = b. Hence this patch adds a osmo_gt in global_title.h and global_title.c. Heavily used in LU and osmo_gsup_req. Depends: libosmocore Id9692880079ea0f219f52d81b1923a76fc640566 Change-Id: I3a8dff3d4a1cbe10d6ab08257a0138d6b2a082d9 --- M configure.ac M include/Makefile.am A include/osmocom/gsupclient/global_title.h A include/osmocom/gsupclient/gsup_req.h M include/osmocom/hlr/Makefile.am M include/osmocom/hlr/db.h M include/osmocom/hlr/gsup_router.h M include/osmocom/hlr/gsup_server.h M include/osmocom/hlr/hlr.h M include/osmocom/hlr/hlr_ussd.h M include/osmocom/hlr/logging.h A include/osmocom/hlr/lu_fsm.h D include/osmocom/hlr/luop.h M src/Makefile.am M src/db.c M src/db_hlr.c M src/gsup_router.c M src/gsup_send.c M src/gsup_server.c M src/gsupclient/Makefile.am A src/gsupclient/global_title.c M src/gsupclient/gsup_client.c A src/gsupclient/gsup_req.c M src/hlr.c M src/hlr_ussd.c M src/logging.c A src/lu_fsm.c D src/luop.c M tests/Makefile.am M tests/db/Makefile.am M tests/db/db_test.c M tests/db/db_test.err A tests/gsup/Makefile.am A tests/gsup/gsup_test.c A tests/gsup/gsup_test.err A tests/gsup/gsup_test.ok M tests/gsup_server/Makefile.am M tests/test_nodes.vty M tests/testsuite.at 39 files changed, 1,626 insertions(+), 905 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/05/16205/20 -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/16205 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I3a8dff3d4a1cbe10d6ab08257a0138d6b2a082d9 Gerrit-Change-Number: 16205 Gerrit-PatchSet: 20 Gerrit-Owner: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osm...@sysmocom.de> Gerrit-CC: laforge <lafo...@osmocom.org> Gerrit-MessageType: newpatchset