February 2023 - gerrit-log - lists.osmocom.org

[S] Change in osmo-hnbgw[master]: ranap_rab_ass_test: fix use of deprecated ranap_cn_rx_co_decode()

by neels

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31432 to look at the new patch set (#4). Change subject: ranap_rab_ass_test: fix use of deprecated ranap_cn_rx_co_decode() ...................................................................... ranap_rab_ass_test: fix use of deprecated ranap_cn_rx_co_decode() Change-Id: Iea41b027d047b2da52d0733fce4c9d75bb74fe48 --- M tests/ranap_rab_ass/ranap_rab_ass_test.c 1 file changed, 13 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/32/31432/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31432 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Iea41b027d047b2da52d0733fce4c9d75bb74fe48 Gerrit-Change-Number: 31432 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 4 months

1
0
0 0

[XL] Change in osmo-hnbgw[master]: context map: introduce RUA and SCCP FSMs to fix leaks

by neels

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431 to look at the new patch set (#5). Change subject: context map: introduce RUA and SCCP FSMs to fix leaks ...................................................................... context map: introduce RUA and SCCP FSMs to fix leaks Refactor the entire RUA <-> SCCP connection-oriented message forwarding: - conquer confusion about hnbgw_context_map release behavior, and - eradicate SCCP connection leaks. Finer points: == Context map state == So far, we had a single context map state and some flags to keep track of both the RUA and the SCCP connections. It was easy to miss connection cleanup steps, especially on the SCCP side. Instead, the two FSMs clearly define the RUA and SCCP conn states separately, and each side takes care of its own release needs for all possible scenarios. - When both RUA and SCCP are released, the context map is discarded. - A context map can stay around to wait for proper SCCP release, even if the RUA side has lost the HNB connection. - Completely drop the async "context mapper garbage collection", because the FSMs clarify the release and free steps, synchronously. - We still keep a (clarified) enum for global context map state, but this is only used for VTY reporting to remain mostly unchanged. == Context map cleanup confusion == The function context_map_deactivate() looked like a general cleanup function, but it is in fact only called when a HNB disconnects. Rename the function to context_map_hnb_released(), and move the general cleanup steps to the never-skipped context_map_free(). == Free context maps separately from HNB == When a HNB releases, talloc_steal() the context maps out of the HNB specific hnb_ctx, so that they are not freed along with the HNB state, possibly leaving SCCP connections afloat. (It is still nice to normally keep context maps as talloc children of their respective hnb_ctx, so talloc reports show which belongs to which.) So far, context map handling found the global hnb_gw pointer via map->hnb_ctx->gw. But in fact, a HNB may disappear at any point in time. Instead, use a separate hnb_gw pointer in map->gw. == RUA procedure codes vs. SCCP prims == So far, the RUA rx side composed SCCP prims to pass on: RUA rx ---SCCP-prim--> RANAP handling ---SCCP-prim--> SCCP tx That is a source of confusion: a RUA procedure code should not translate 1:1 to SCCP prims, especially for RUA id-Disconnect (see release charts below). Instead, move SCCP prim composition over to the SCCP side, using FSM events to forward: RUA rx --event--> RUA FSM --event--> SCCP FSM --SCCP-prim--> SCCP tx +RANAP +RANAP +RANAP RUA tx <--RUA---- RUA FSM <--event-- SCCP FSM <--event-- SCCP rx +RANAP +RANAP +RANAP Hence choose the correct prim according to the SCCP FSM state. - in hnbgw_rua.c, use RUA procedure codes, not prim types. - via the new FSM events' data args, pass msgb containing RANAP PDUs. == Fix SCCP Release behavior == So far, the normal conn release behavior was HNB HNBGW CN | --id-Disconnect--> | ---SCCP-Released--> | Iu-ReleaseComplete | | <--SCCP-RLC-------- | (no data) Instead, the SCCP release is now in accordance with 3GPP TS 48.006 9.2 'Connection release': The MSC sends a SCCP released message. This message shall not contain any user data field. i.e.: HNB HNBGW CN | --id-Disconnect--> | ---Data-Form-1(!)--> | Iu-ReleaseComplete | <--SCCP-Released---- | (no data) | | ---SCCP-RLC--------> | (no data) (Side note, the final SCCP Release Confirm step is taken care of by libosmo-sigtran's sccp_scoc.c FSM.) If the CN fails to respond with SCCP-Released, on new X31 timeout, osmo-hnbgw will send an SCCP Released to the CN as fallback. == Memory model for message dispatch == So far, an osmo_scu_prim aka "oph" was passed between RUA and SCCP handling code, and the final dispatch freed it. Every error path had to take care not to leak any oph. Instead, use a much easier and much more leakage proof memory model, inspired by fixeria: - on rx, dispatch RANAP msgb that live in OTC_SELECT. - no code path needs to msgb_free() -- the msgb is discarded via OTC_SELECT when handling is done, error or no error. - any code path may also choose to store the msgb for async dispatch, using talloc_steal(). The user plane mapping via MGW and UPF do that. - if any code path does msgb_free(), that would be no problem either (but none do so now, for simplicity). == Layer separation == Dispatch *all* connection-oriented RUA tx via the RUA FSM and SCCP tx via the SCCP FSM, do not call rua_tx_dt() or osmo_sccp_user_sap_down() directly. == Memory model for decoded ranap_message IEs == Use a talloc destructor to make sure that the ranap_message IEs are always implicitly freed upon talloc_free(), so that no code path can possibly forget to do so. == Implicit cleanup by talloc == Use talloc scoping to remove a bunch of explicit cleanup code. For example: talloc_steal(mgw_fsm_priv, message); mgw_fsm_priv->ranap_rab_ass_req_message = message; and later implicitly free 'message' by only doing: talloc_free(mgw_fsm_priv) == Connection-less dead code == The function rua_to_scu() contained some dead code for connection-less messages. In fact, rua_to_scu() was and is only used for connection-oriented messages. Clarify that. Related: SYS#6297 Change-Id: I6ff7e36532ff57c6f2d3e7e419dd22ef27dafd19 --- M include/osmocom/hnbgw/context_map.h M include/osmocom/hnbgw/hnbgw.h M include/osmocom/hnbgw/hnbgw_cn.h M include/osmocom/hnbgw/mgw_fsm.h M include/osmocom/hnbgw/ps_rab_ass_fsm.h M include/osmocom/hnbgw/tdefs.h M src/osmo-hnbgw/Makefile.am M src/osmo-hnbgw/context_map.c A src/osmo-hnbgw/context_map_rua.c A src/osmo-hnbgw/context_map_sccp.c M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/hnbgw_cn.c M src/osmo-hnbgw/hnbgw_rua.c M src/osmo-hnbgw/hnbgw_vty.c M src/osmo-hnbgw/mgw_fsm.c M src/osmo-hnbgw/ps_rab_ass_fsm.c M src/osmo-hnbgw/ps_rab_fsm.c M src/osmo-hnbgw/tdefs.c 18 files changed, 1,490 insertions(+), 405 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/31/31431/5 -- 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-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 4 months

1
0
0 0

[S] Change in osmo-hnbgw[master]: sccp_sap_up(): ignore PCSTATE.ind

by neels

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31429 to look at the new patch set (#4). Change subject: sccp_sap_up(): ignore PCSTATE.ind ...................................................................... sccp_sap_up(): ignore PCSTATE.ind Silence the error log about "unknown" PCSTATE prim. Todo / coming up: instead of ignoring, detect a Destination Unavailable from prim->u.pcstate and disconnect all conns with that particular CN link. Depends: libosmo-sccp If381f537ab91af1feef7f0e51921217f27e18e6a Change-Id: I547387a5cc14ccb506be04ac785e6807fc4e6a96 --- M TODO-RELEASE M src/osmo-hnbgw/hnbgw_cn.c 2 files changed, 20 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/29/31429/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31429 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I547387a5cc14ccb506be04ac785e6807fc4e6a96 Gerrit-Change-Number: 31429 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 4 months

1
0
0 0

[S] Change in libosmo-sccp[master]: fixup for: add osmo_scu_prim_name_c() / _buf()

by neels

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-sccp/+/31434 to look at the new patch set (#3). Change subject: fixup for: add osmo_scu_prim_name_c() / _buf() ...................................................................... fixup for: add osmo_scu_prim_name_c() / _buf() I would like to tweak the names for the recently added functions, so that they are more clear -- quickly before we release it or anyone uses these. Depends: libosmocore I9f43428af654a5674ac3035fe4db1394aac7a7af Related: I4c1998fd7fee7282d107846dae2cff4b5ceb3a7b Change-Id: If381f537ab91af1feef7f0e51921217f27e18e6a --- M TODO-RELEASE M include/osmocom/sigtran/sccp_sap.h M src/sccp_sap.c 3 files changed, 31 insertions(+), 9 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/34/31434/3 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/31434 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: If381f537ab91af1feef7f0e51921217f27e18e6a Gerrit-Change-Number: 31434 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 4 months

1
0
0 0

[S] Change in osmo-hnbgw[master]: sccp_sap_up(): ignore PCSTATE.ind

by neels

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31429 to look at the new patch set (#3). Change subject: sccp_sap_up(): ignore PCSTATE.ind ...................................................................... sccp_sap_up(): ignore PCSTATE.ind Silence the error log about "unknown" PCSTATE prim. Todo / coming up: instead of ignoring, detect a Destination Unavailable from prim->u.pcstate and disconnect all conns with that particular CN link. Depends: libosmo-sccp If381f537ab91af1feef7f0e51921217f27e18e6a Change-Id: I547387a5cc14ccb506be04ac785e6807fc4e6a96 --- M TODO-RELEASE M src/osmo-hnbgw/hnbgw_cn.c 2 files changed, 20 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/29/31429/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31429 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I547387a5cc14ccb506be04ac785e6807fc4e6a96 Gerrit-Change-Number: 31429 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 4 months

1
0
0 0

[XL] Change in osmo-hnbgw[master]: context map: introduce RUA and SCCP FSMs to fix leaks

by neels

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431 to look at the new patch set (#4). Change subject: context map: introduce RUA and SCCP FSMs to fix leaks ...................................................................... context map: introduce RUA and SCCP FSMs to fix leaks Refactor the entire RUA <-> SCCP connection-oriented message forwarding: - conquer confusion about hnbgw_context_map release behavior, and - eradicate SCCP connection leaks. Finer points: == Context map state == So far, we had a single context map state and some flags to keep track of both the RUA and the SCCP connections. It was easy to miss connection cleanup steps, especially on the SCCP side. Instead, the two FSMs clearly define the RUA and SCCP conn states separately, and each side takes care of its own release needs for all possible scenarios. - When both RUA and SCCP are released, the context map is discarded. - A context map can stay around to wait for proper SCCP release, even if the RUA side has lost the HNB connection. - Completely drop the async "context mapper garbage collection", because the FSMs clarify the release and free steps, synchronously. - We still keep a (clarified) enum for global context map state, but this is only used for VTY reporting to remain mostly unchanged. == Context map cleanup confusion == The function context_map_deactivate() looked like a general cleanup function, but it is in fact only called when a HNB disconnects. Rename the function to context_map_hnb_released(), and move the general cleanup steps to the never-skipped context_map_free(). == Free context maps separately from HNB == When a HNB releases, talloc_steal() the context maps out of the HNB specific hnb_ctx, so that they are not freed along with the HNB state, possibly leaving SCCP connections afloat. (It is still nice to normally keep context maps as talloc children of their respective hnb_ctx, so talloc reports show which belongs to which.) So far, context map handling found the global hnb_gw pointer via map->hnb_ctx->gw. But in fact, a HNB may disappear at any point in time. Instead, use a separate hnb_gw pointer in map->gw. == RUA procedure codes vs. SCCP prims == So far, the RUA rx side composed SCCP prims to pass on: RUA rx ---SCCP-prim--> RANAP handling ---SCCP-prim--> SCCP tx That is a source of confusion: a RUA procedure code should not translate 1:1 to SCCP prims, especially for RUA id-Disconnect (see release charts below). Instead, move SCCP prim composition over to the SCCP side, using FSM events to forward: RUA rx --event--> RUA FSM --event--> SCCP FSM --SCCP-prim--> SCCP tx +RANAP +RANAP RUA tx <--event-- RUA FSM <--event-- SCCP FSM <--event-- SCCP rx +RANAP +RANAP +RANAP Hence choose the correct prim according to the SCCP FSM state. - in hnbgw_rua.c, use RUA procedure codes, not prim types. - via the new FSM events' data args, pass msgb containing RANAP PDUs. == Fix SCCP Release behavior == So far, the normal conn release behavior was HNB HNBGW CN | --id-Disconnect-------> | ---SCCP-Released----> | Iu-ReleaseComplete | | <--SCCP-RLC---------- | (no data) Instead, the SCCP release is now in accordance with 3GPP TS 48.006 9.2 'Connection release': The MSC sends a SCCP released message. This message shall not contain any user data field. i.e.: HNB HNBGW CN | --id-Disconnect-------> | ---Data-Form-1(!)---> | Iu-ReleaseComplete | <--SCCP-Released----- | (no data) | | ---SCCP-RLC---------> | (no data) (Side note, the final SCCP Release Confirm step is taken care of by libosmo-sigtran's sccp_scoc.c FSM.) If the CN fails to respond with SCCP-Released, on new X31 timeout, osmo-hnbgw will send an SCCP Released to the CN as fallback. == Memory model for message dispatch == So far, an osmo_scu_prim aka "oph" was passed between RUA and SCCP handling code, and the final dispatch freed it. Every error path had to take care not to leak any oph. Instead, use a much easier and much more leakage proof memory model, inspired by fixeria: - on rx, dispatch RANAP msgb that live in OTC_SELECT. - no code path needs to msgb_free() -- the msgb is discarded via OTC_SELECT when handling is done, error or no error. - any code path may also choose to store the msgb for async dispatch, using talloc_steal(). The user plane mapping via MGW and UPF do that. - if any code path does msgb_free(), that would be no problem either (but none do so now, for simplicity). == Layer separation == Dispatch *all* connection-oriented RUA tx via the RUA FSM and SCCP tx via the SCCP FSM, do not call rua_tx_dt() or osmo_sccp_user_sap_down() directly. == Memory model for decoded ranap_message IEs == Use a talloc destructor to make sure that the ranap_message IEs are always implicitly freed upon talloc_free(), so that no code path can possibly forget to do so. == Implicit cleanup by talloc == Use talloc scoping to remove a bunch of explicit cleanup code. For example: talloc_steal(mgw_fsm_priv, message); mgw_fsm_priv->ranap_rab_ass_req_message = message; and later implicitly free 'message' by only doing: talloc_free(mgw_fsm_priv) == Connection-less dead code == The function rua_to_scu() contained some dead code for connection-less messages. In fact, rua_to_scu() was and is only used for connection-oriented messages. Clarify that. Related: SYS#6297 Change-Id: I6ff7e36532ff57c6f2d3e7e419dd22ef27dafd19 --- M include/osmocom/hnbgw/context_map.h M include/osmocom/hnbgw/hnbgw.h M include/osmocom/hnbgw/hnbgw_cn.h M include/osmocom/hnbgw/mgw_fsm.h M include/osmocom/hnbgw/ps_rab_ass_fsm.h M include/osmocom/hnbgw/tdefs.h M src/osmo-hnbgw/Makefile.am M src/osmo-hnbgw/context_map.c A src/osmo-hnbgw/context_map_rua.c A src/osmo-hnbgw/context_map_sccp.c M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/hnbgw_cn.c M src/osmo-hnbgw/hnbgw_rua.c M src/osmo-hnbgw/hnbgw_vty.c M src/osmo-hnbgw/mgw_fsm.c M src/osmo-hnbgw/ps_rab_ass_fsm.c M src/osmo-hnbgw/ps_rab_fsm.c M src/osmo-hnbgw/tdefs.c 18 files changed, 1,490 insertions(+), 405 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/31/31431/4 -- 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: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 4 months

1
0
0 0

[S] Change in libosmo-sccp[master]: fixup for: add osmo_scu_prim_name_c() / _buf()

by neels

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-sccp/+/31434 to look at the new patch set (#2). Change subject: fixup for: add osmo_scu_prim_name_c() / _buf() ...................................................................... fixup for: add osmo_scu_prim_name_c() / _buf() I would like to tweak the names for the recently added functions, so that they are more clear -- quickly before we release it or anyone uses these. Depends: libosmocore I9f43428af654a5674ac3035fe4db1394aac7a7af Related: I4c1998fd7fee7282d107846dae2cff4b5ceb3a7b Change-Id: If381f537ab91af1feef7f0e51921217f27e18e6a --- M TODO-RELEASE M include/osmocom/sigtran/sccp_sap.h M src/sccp_sap.c 3 files changed, 31 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/34/31434/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/31434 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: If381f537ab91af1feef7f0e51921217f27e18e6a Gerrit-Change-Number: 31434 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 4 months

1
0
0 0

[S] Change in libosmocore[master]: add osmo_prim_operation_name()

by neels

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/31435 ) Change subject: add osmo_prim_operation_name() ...................................................................... add osmo_prim_operation_name() Also remove the explicit array dimension from the .h file. Change-Id: I9f43428af654a5674ac3035fe4db1394aac7a7af --- M include/osmocom/core/prim.h 1 file changed, 16 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/35/31435/1 diff --git a/include/osmocom/core/prim.h b/include/osmocom/core/prim.h index 99eabff..8e6b436 100644 --- a/include/osmocom/core/prim.h +++ b/include/osmocom/core/prim.h @@ -30,7 +30,11 @@ PRIM_OP_CONFIRM, /*!< confirm */ }; -extern const struct value_string osmo_prim_op_names[5]; +extern const struct value_string osmo_prim_op_names[]; +static inline const char *osmo_prim_operation_name(enum osmo_prim_operation val) +{ + return get_value_string(osmo_prim_op_names, val); +} /*!< The upper 8 byte of the technology, the lower 24 bits for the SAP */ #define _SAP_GSM_SHIFT 24 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31435 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I9f43428af654a5674ac3035fe4db1394aac7a7af Gerrit-Change-Number: 31435 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 4 months

1
0
0 0

[XL] Change in osmo-hnbgw[master]: context map: introduce RUA and SCCP FSMs to fix leaks

by neels

Attention is currently required from: neels. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431 to look at the new patch set (#3). Change subject: context map: introduce RUA and SCCP FSMs to fix leaks ...................................................................... context map: introduce RUA and SCCP FSMs to fix leaks Refactor the entire RUA <-> SCCP connection-oriented message forwarding: - conquer confusion about hnbgw_context_map release behavior, and - eradicate SCCP connection leaks. Finer points: == Context map state == So far, we had a single context map state and some flags to keep track of both the RUA and the SCCP connections. It was easy to miss connection cleanup steps, especially on the SCCP side. Instead, the two FSMs clearly define the RUA and SCCP conn states separatelt, and each side takes care of its own release needs for all possible scenarios. - When both RUA and SCCP are released, the context map is discarded. - A context map can stay around to wait for proper SCCP release, even if the RUA side has lost the HNB connection. - Completely drop the async "context mapper garbage collection", because the FSMs clarify the release and free steps, synchronously. - We still keep a (clarified) enum for global context map state, but this is only used for VTY reporting to remain mostly unchanged. == Context map cleanup confusion == The function context_map_deactivate() looked like a general cleanup function, but it is in fact only called when a HNB disconnects. Rename the function to context_map_hnb_released(), and move the general cleanup steps to the never-skipped context_map_free(). == Free context maps separately from HNB == When a HNB releases, talloc_steal() the context maps out of the HNB specific hnb_ctx, so that they are not freed along with the HNB state, possibly leaving SCCP connections afloat. (It is still nice to normally keep context maps as talloc children of their respective hnb_ctx, so talloc reports show which belongs to which.) So far, context map handling found the global hnb_gw pointer via map->hnb_ctx->gw. But in fact, a HNB may disappear at any point in time. Instead, use a separate hnb_gw pointer in map->gw. == RUA procedure codes vs. SCCP prims == So far, the RUA rx side composed SCCP prims to pass on: RUA rx ---SCCP-prim--> RANAP handling ---SCCP-prim--> SCCP tx That is a source of confusion: a RUA procedure code should not translate 1:1 to SCCP prims, especially for RUA id-Disconnect (see release charts below). Instead, move SCCP prim composition over to the SCCP side, using FSM events to forward: RUA rx --event--> RUA FSM --event--> SCCP FSM --SCCP-prim--> SCCP tx +RANAP +RANAP RUA tx <--event-- RUA FSM <--event-- SCCP FSM <--event-- SCCP rx +RANAP +RANAP +RANAP Hence choose the correct prim according to the SCCP FSM state. - in hnbgw_rua.c, use RUA procedure codes, not prim types. - via the new FSM events' data args, pass msgb containing RANAP PDUs. == Fix SCCP Release behavior == So far, the normal conn release behavior was HNB HNBGW CN | --id-Disconnect-------> | ---SCCP-Released----> | Iu-ReleaseComplete | | <--SCCP-RLC---------- | (no data) Instead, the SCCP release is now in accordance with 3GPP TS 48.006 9.2 'Connection release': The MSC sends a SCCP released message. This message shall not contain any user data field. i.e.: HNB HNBGW CN | --id-Disconnect-------> | ---Data-Form-1(!)---> | Iu-ReleaseComplete | <--SCCP-Released----- | (no data) | | ---SCCP-RLC---------> | (no data) (Side note, the final SCCP Release Confirm step is taken care of by libosmo-sigtran's sccp_scoc.c FSM.) If the CN fails to respond with SCCP-Released, on new X31 timeout, osmo-hnbgw will send an SCCP Released to the CN as fallback. == Memory model for message dispatch == So far, an osmo_scu_prim aka "oph" was passed between RUA and SCCP handling code, and the final dispatch freed it. Every error path had to take care not to leak any oph. Instead, use a much easier and much more leakage proof memory model, inspired by fixeria: - on rx, dispatch RANAP msgb that live in OTC_SELECT. - no code path needs to msgb_free() -- the msgb is discarded via OTC_SELECT when handling is done, error or no error. - any code path may also choose to store the msgb for async dispatch, using talloc_steal(). The user plane mapping via MGW and UPF do that. - if any code path does msgb_free(), that would be no problem either (but none do so now, for simplicity). == Layer separation == Dispatch *all* connection-oriented RUA tx via the RUA FSM and SCCP tx via the SCCP FSM, do not call rua_tx_dt() or osmo_sccp_user_sap_down() directly. == Memory model for decoded ranap_message IEs == Use a talloc destructor to make sure that the ranap_message IEs are always implicitly freed upon talloc_free(), so that no code path can possibly forget to do so. == Implicit cleanup by talloc == Use talloc scoping to remove a bunch of explicit cleanup code. For example: talloc_steal(mgw_fsm_priv, message); mgw_fsm_priv->ranap_rab_ass_req_message = message; and later implicitly free 'message' by only doing: talloc_free(mgw_fsm_priv) == Connection-less dead code == The function rua_to_scu() contained some dead code for connection-less messages. In fact, rua_to_scu() was and is only used for connection-oriented messages. Clarify that. Related: SYS#6297 Change-Id: I6ff7e36532ff57c6f2d3e7e419dd22ef27dafd19 --- M include/osmocom/hnbgw/context_map.h M include/osmocom/hnbgw/hnbgw.h M include/osmocom/hnbgw/hnbgw_cn.h M include/osmocom/hnbgw/mgw_fsm.h M include/osmocom/hnbgw/ps_rab_ass_fsm.h M include/osmocom/hnbgw/tdefs.h M src/osmo-hnbgw/Makefile.am M src/osmo-hnbgw/context_map.c A src/osmo-hnbgw/context_map_rua.c A src/osmo-hnbgw/context_map_sccp.c M src/osmo-hnbgw/hnbgw.c M src/osmo-hnbgw/hnbgw_cn.c M src/osmo-hnbgw/hnbgw_rua.c M src/osmo-hnbgw/hnbgw_vty.c M src/osmo-hnbgw/mgw_fsm.c M src/osmo-hnbgw/ps_rab_ass_fsm.c M src/osmo-hnbgw/ps_rab_fsm.c M src/osmo-hnbgw/tdefs.c 18 files changed, 1,490 insertions(+), 405 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/31/31431/3 -- 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: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 4 months

1
0
0 0

[XL] Change in osmo-hnbgw[master]: context map: introduce RUA and SCCP FSMs to fix leaks

by Jenkins Builder

Attention is currently required from: neels. Jenkins Builder 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 2: (3 comments) File include/osmocom/hnbgw/context_map.h: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3842): https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/208a1a7e_8ff21503 PS2, Line 35: /* Something disruptive occured elsewhere than RUA, we want to clean up the RUA conn and be gone. */ 'occured' may be misspelled - perhaps 'occurred'? Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3842): https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/7e5582fc_476f930c PS2, Line 58: /* Something disruptive occured elsewhere than SCCP, we want to clean up the SCCP conn and be gone. */ 'occured' may be misspelled - perhaps 'occurred'? File src/osmo-hnbgw/hnbgw_vty.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-3842): https://gerrit.osmocom.org/c/osmo-hnbgw/+/31431/comment/a5921446_3183c582 PS2, Line 212: state_count[map->is_ps? 1 : 0][context_map_get_state(map)]++; spaces required around that '?' (ctx:VxW) -- 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: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 21 Feb 2023 03:54:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

2025

2024

2023

2022

gerrit-log February 2023