- gerrit-log - lists.osmocom.org

Change in libosmo-netif[master]: export osmo_amr_bits

by pespin

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/26716 ) Change subject: export osmo_amr_bits ...................................................................... Patch Set 1: > Patch Set 1: Code-Review+1 > > it's not really exporting anything, it's just adding a declaration for an existing export? Yes well, but usually no one knows about a function being exported unless you check the header file :) -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/26716 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ic03653b2a885c5d9419c6c75a718b550fc30b86e Gerrit-Change-Number: 26716 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 04 Jan 2022 13:41:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

3 years, 6 months

1
0
0 0

Change in osmo-mgw[master]: mgcp_client: add new codec IUFP as VND.3GPP.IUFP

by pespin

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/26651 ) Change subject: mgcp_client: add new codec IUFP as VND.3GPP.IUFP ...................................................................... Patch Set 5: (1 comment) https://gerrit.osmocom.org/c/osmo-mgw/+/26651/5/src/libosmo-mgcp-client/mgc… File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/26651/5/src/libosmo-mgcp-client/mgc… PS5, Line 62: { CODEC_IUFP, "VND.3GPP.IUFP/16000" }, > So far we use upper case everywhere. however in theory this code is called IuFP. […] Why do you say it's called IuFP? See 3GPP TS 29.414 6.3.3.5 "Media Attributes": """ The following media attribute shall be supplied: “a=rtpmap:<dynamic payload number> VND.3GPP.IUFP/16000”, where :<dynamic payload number> is the same dynamic payload type number as in the above media announcement <fmt list>. """ It's true though that 6.3 says: """ NOTE: The IuFP is registered with IANA as the MIME type ”VND.3GPP.IuFP”of the ”audio” category, however, this registration does not preclude the use of IuFP to transport ”data”. """ But the one in case in section 6.3.3.5 seems to be more similar to what's needed by SDP, so I'd go for all caps. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/26651 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifd1210a897743396899f34457c96e6fd2109c6b3 Gerrit-Change-Number: 26651 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 04 Jan 2022 13:39:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: lynxis lazus <lynxis(a)fe80.eu> Gerrit-MessageType: comment

3 years, 6 months

1
0
0 0

Change in osmo-iuh[master]: mgw_fsm: add MGW support to osmo-hnbgw

by pespin

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-iuh/+/26725 ) Change subject: mgw_fsm: add MGW support to osmo-hnbgw ...................................................................... Patch Set 1: (15 comments) https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/hnbgw_cn.c File src/hnbgw_cn.c: https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/hnbgw_cn.c@367 PS1, Line 367: * its own copy of the message contents, so it is safe to free oph->msg() in sccp_sap_up() below */ when you say "sccp_sap_up() below", you mean the caller of handle_cn_data_ind freed the msgb? https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/hnbgw_cn.c@369 PS1, Line 369: } else if (msg_len > 2 && msg_data[1] == RANAP_ProcedureCode_id_Iu_Release && !map->is_ps && map->mgw_fi) { Probably several levels here look far more structured and extensible: if (msg_len > 2) { switch (msg_data[1]): case RANAP_ProcedureCode_id_RAB_Assignment: if (!map->is_ps) ... break; } https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/hnbgw_rua.c File src/hnbgw_rua.c: https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/hnbgw_rua.c@232 PS1, Line 232: /* TODO: Check for map and if this is a RAB Assignment response what about this? https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/hnbgw_rua.c@278 PS1, Line 278: if (map && !release_context_map && msg_len > 2 && msg_data[1] == RANAP_ProcedureCode_id_RAB_Assignment same, probably splitting into several levels makes it more easy to avoid future issues when extending this. https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/hnbgw_vty.c File src/hnbgw_vty.c: https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/hnbgw_vty.c@409 PS1, Line 409: mgcp_client_config_write(vty, " "); are you sure this prefix is correct? aren't you missing an extra space? There should probably be two of them. https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c File src/mgw_fsm.c: https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c@69 PS1, Line 69: MGW_ST_CRCX_HNB, WAIT_CRCX? https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c@71 PS1, Line 71: MGW_ST_MDCX_HNB, WAIT_MDCX? https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c@86 PS1, Line 86: uint8_t rab_ass_req[IUH_MSGB_SIZE]; Use "struct *msgb" here and mgcp_copy()? https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c@87 PS1, Line 87: unsigned int rab_ass_req_len; You don't need this field if you copy the msgb. https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c@190 PS1, Line 190: unneeded space https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c@243 PS1, Line 243: osmo_tdef_fsm_inst_state_chg(fi, MGW_ST_CRCX_MSC, mgw_fsm_timeouts, mgw_fsm_T_defs, -1); You can probably use your own define macro here to only pass fi and new state, see other FSMs https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c@414 PS1, Line 414: .in_event_mask = 0 I find this "0 |" way quite strange and confusing. I think we usually do: .in_event_mask = S(MGW_EV_MGCP_OK) | S(MGW_EV_MGCP_FAIL), https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c@485 PS1, Line 485: .allstate_event_mask = 0 | S(MGW_EV_MGCP_TERM) Please clean up this line(s). https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/mgw_fsm.c@500 PS1, Line 500: struct osmo_fsm_inst *osmo_fsm_inst; You can probably drop this variable. https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/tdefs.c File src/tdefs.c: https://gerrit.osmocom.org/c/osmo-iuh/+/26725/1/src/tdefs.c@20 PS1, Line 20: {.T = -1001,.default_val = 5,.desc = "Timeout for HNB side call-leg (to-HNB) creation" }, missing space after comma in all lines -- To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/26725 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-iuh Gerrit-Branch: master Gerrit-Change-Id: Ib9b62e0145184b91c56ce5d8870760bfa49cc5a4 Gerrit-Change-Number: 26725 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 04 Jan 2022 13:34:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

3 years, 6 months

1
0
0 0

Change in osmo-mgw[master]: Add multithreading for the virtual trunk

by laforge

laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/25432 ) Change subject: Add multithreading for the virtual trunk ...................................................................... Patch Set 37: > Patch Set 37: > > no follow-up for two weeks? ping? -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/25432 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I31be8253600c8af0a43c967d0d128f5ba7b16260 Gerrit-Change-Number: 25432 Gerrit-PatchSet: 37 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Tue, 04 Jan 2022 13:34:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

3 years, 6 months

1
0
0 0

Change in osmo-ttcn3-hacks[master]: bsc: make perform_clear() work when VTY is not initialized

by laforge

laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26617 ) Change subject: bsc: make perform_clear() work when VTY is not initialized ...................................................................... Patch Set 1: (1 comment) https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26617/1/bsc/BSC_Tests.ttcn File bsc/BSC_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26617/1/bsc/BSC_Tests.ttcn@… PS1, Line 1069: if (g_vty_initialized) { > Can we simply have this check in f_logp()? good idea! -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/26617 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I8116075f32937bd06ba14b426010bf6fec2ef402 Gerrit-Change-Number: 26617 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 04 Jan 2022 13:33:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 6 months

1
0
0 0

Change in osmo-bsc[master]: log: always include timeouts in FSM transition logging

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/26609 ) Change subject: log: always include timeouts in FSM transition logging ...................................................................... log: always include timeouts in FSM transition logging Before: state_chg to ACTIVE state_chg to WAIT_RLL_RTP_RELEASED state_chg to WAIT_SCCP_RLSD After: State change to ACTIVE (no timeout) State change to WAIT_RLL_RTP_RELEASED (T3109, 5s) State change to WAIT_SCCP_RLSD (X4, 60s) Change-Id: I94b7dc4d9e5e45dc731bcb3a843ede9fb6cc0839 --- M src/osmo-bsc/osmo_bsc_main.c 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/osmo-bsc/osmo_bsc_main.c b/src/osmo-bsc/osmo_bsc_main.c index b7e2616..167b0a3 100644 --- a/src/osmo-bsc/osmo_bsc_main.c +++ b/src/osmo-bsc/osmo_bsc_main.c @@ -891,6 +891,7 @@ rate_ctr_init(tall_bsc_ctx); osmo_fsm_set_dealloc_ctx(OTC_SELECT); + osmo_fsm_log_timeouts(true); /* Allocate global gsm_network struct */ rc = bsc_network_alloc(); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/26609 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I94b7dc4d9e5e45dc731bcb3a843ede9fb6cc0839 Gerrit-Change-Number: 26609 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years, 6 months

1
0
0 0

Change in osmo-bsc[master]: lchan/gscon: always clear both cross ref pointers

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/26610 ) Change subject: lchan/gscon: always clear both cross ref pointers ...................................................................... lchan/gscon: always clear both cross ref pointers During specific release scenarios, it became clear that an lchan still pointed at a conn even after it had been deallocated. That was due to setting conn->lchan = NULL but not lchan->conn = NULL. Fix that. Do lchan_forget_conn() first, because during gscon_forget_lchan() we may enter the gscon clearing dance, which in case of no SCCP conn being present will soon / should immediately deallocate the conn. Related: OS#5337 Related: I8c8537acf6b47b121903197608636c43ae601a57 (osmo-bsc) Change-Id: Idbfe4672233ba8105eff5ba77ee07fd871358255 --- M src/osmo-bsc/lchan_fsm.c 1 file changed, 7 insertions(+), 3 deletions(-) Approvals: laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c index 5ae6df6..84f8dc5 100644 --- a/src/osmo-bsc/lchan_fsm.c +++ b/src/osmo-bsc/lchan_fsm.c @@ -478,8 +478,11 @@ { LOG_LCHAN(lchan, LOGL_DEBUG, "Clearing lchan state\n"); - if (lchan->conn) - gscon_forget_lchan(lchan->conn, lchan); + if (lchan->conn) { + struct gsm_subscriber_connection *conn = lchan->conn; + lchan_forget_conn(lchan); + gscon_forget_lchan(conn, lchan); + } if (lchan->rqd_ref) { talloc_free(lchan->rqd_ref); @@ -1392,8 +1395,9 @@ * lchan_reset(), we make sure it does. But in case of releases from error handling, the * conn might as well notice now already that its lchan is becoming unusable. */ if (lchan->conn) { - gscon_forget_lchan(lchan->conn, lchan); + struct gsm_subscriber_connection *conn = lchan->conn; lchan_forget_conn(lchan); + gscon_forget_lchan(conn, lchan); } rc = rsl_tx_rf_chan_release(lchan); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/26610 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Idbfe4672233ba8105eff5ba77ee07fd871358255 Gerrit-Change-Number: 26610 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years, 6 months

1
0
0 0

Change in osmo-bsc[master]: lcs: fix bsc_subscr use_count leak

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/26611 ) Change subject: lcs: fix bsc_subscr use_count leak ...................................................................... lcs: fix bsc_subscr use_count leak In lcs_ta_req_wait_ta_onenter(), fix use count leak of 'start-paging': get() the use count only after the early exits. osmo-ttcn3-hacks patch I69d4c5c6f8d499bb7f0b96a48af045361433c57b introduces testing against this leak in various LCS tests (e.g. BSC_Tests.TC_lcs_loc_req_for_active_ms_ta_req). Related: OS#5355 Change-Id: Ibbfbfe766eafe42c78048ec5b3b503a11ef5535d --- M src/osmo-bsc/lcs_ta_req.c 1 file changed, 11 insertions(+), 11 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/osmo-bsc/lcs_ta_req.c b/src/osmo-bsc/lcs_ta_req.c index 6252d46..f0bb02e 100644 --- a/src/osmo-bsc/lcs_ta_req.c +++ b/src/osmo-bsc/lcs_ta_req.c @@ -122,17 +122,6 @@ return; } - paging = (struct bsc_paging_params){ - .reason = BSC_PAGING_FOR_LCS, - .msc = loc_req->conn->sccp.msc, - .bsub = loc_req->conn->bsub, - .tmsi = GSM_RESERVED_TMSI, - .imsi = loc_req->req.imsi, - .chan_needed = RSL_CHANNEED_ANY, - }; - if (paging.bsub) - bsc_subscr_get(paging.bsub, BSUB_USE_PAGING_START); - /* Do we already have an active lchan with knowledge of TA? */ lchan = loc_req->conn->lchan; if (lchan) { @@ -147,6 +136,17 @@ return; } + paging = (struct bsc_paging_params){ + .reason = BSC_PAGING_FOR_LCS, + .msc = loc_req->conn->sccp.msc, + .bsub = loc_req->conn->bsub, + .tmsi = GSM_RESERVED_TMSI, + .imsi = loc_req->req.imsi, + .chan_needed = RSL_CHANNEED_ANY, + }; + if (paging.bsub) + bsc_subscr_get(paging.bsub, BSUB_USE_PAGING_START); + if (!loc_req->req.cell_id_present) { LOG_LCS_TA_REQ(lcs_ta_req, LOGL_DEBUG, "No Cell Identity in BSSMAP Location Request, paging entire BSS\n"); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/26611 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ibbfbfe766eafe42c78048ec5b3b503a11ef5535d Gerrit-Change-Number: 26611 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years, 6 months

1
0
0 0

Change in osmo-bsc[master]: fix gscon clear 1/n: store clear cause in gscon

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/26612 ) Change subject: fix gscon clear 1/n: store clear cause in gscon ...................................................................... fix gscon clear 1/n: store clear cause in gscon Allow returning a context sensitive cause instead of a hardcoded one in gscon pre_term(). Also, the conn->cause is needed to move message dispatch to an "onenter" function in patch I234b2a754d0c98031056981823cdbc187e977741. I Split this part off as a separate patch for better readability. Related: OS#5337 Change-Id: Ib6432746040899129d1d73ae8dc59add2d88a915 --- M include/osmocom/bsc/gsm_data.h M src/osmo-bsc/bsc_subscr_conn_fsm.c 2 files changed, 7 insertions(+), 3 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h index 238b2f7..4e26ec1 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -404,6 +404,8 @@ bool last_eutran_plmn_valid; /* Is information stored in field below available? */ struct osmo_plmn_id last_eutran_plmn; } fast_return; + + enum gsm0808_cause clear_cause; }; diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c index 2140c9b..ff742a2 100644 --- a/src/osmo-bsc/bsc_subscr_conn_fsm.c +++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c @@ -144,6 +144,7 @@ struct msgb *resp; int rc; + conn->clear_cause = cause; if (conn->rx_clear_command) { LOGPFSML(conn->fi, LOGL_DEBUG, "Not sending BSSMAP CLEAR REQUEST, already got CLEAR COMMAND from MSC\n"); @@ -976,9 +977,7 @@ } LOGPFSML(fi, LOGL_DEBUG, "Releasing all lchans (if any) because this conn is terminating\n"); - /* when things go smoothly, the lchan should have been released before FSM instance termination. So if this is - * necessary it's probably "abnormal". */ - gscon_release_lchans(conn, true, GSM48_RR_CAUSE_ABNORMAL_UNSPEC); + gscon_release_lchans(conn, true, bsc_gsm48_rr_cause_from_gsm0808_cause(conn->clear_cause)); /* drop pending messages */ gscon_dtap_queue_flush(conn, 0); @@ -1057,6 +1056,9 @@ INIT_LLIST_HEAD(&conn->hodec2.penalty_timers); conn->sccp.conn_id = -1; + /* Default clear cause (on RR translates to GSM48_RR_CAUSE_ABNORMAL_UNSPEC) */ + conn->clear_cause = GSM0808_CAUSE_EQUIPMENT_FAILURE; + /* don't allocate from 'conn' context, as gscon_cleanup() will call talloc_free(conn) before * libosmocore will call talloc_free(conn->fi), i.e. avoid use-after-free during cleanup */ conn->fi = osmo_fsm_inst_alloc(&gscon_fsm, net, conn, LOGL_DEBUG, NULL); -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/26612 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ib6432746040899129d1d73ae8dc59add2d88a915 Gerrit-Change-Number: 26612 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years, 6 months

1
0
0 0

Change in osmo-bsc[master]: fix gscon clear 2/n: proper state transition to ST_CLEARING

by laforge

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/26613 ) Change subject: fix gscon clear 2/n: proper state transition to ST_CLEARING ...................................................................... fix gscon clear 2/n: proper state transition to ST_CLEARING The way the ST_CLEARING is entered before this patch has various symptoms of how I / we used osmo_fsm when we were still FSM amateurs in Osmocom. Patch that up: - In gscon_bssmap_clear(), ask for a state transition to ST_CLEARING first. Go ahead only if it is allowed. - move the Clear Request messaging to ST_CLEARING's onenter function. - Fix the timeout behavior: by using conn_fsm_state_chg(), use the actual proper X4 timer value for ST_CLEARING from VTY configuration instead of hardcoded magic numbers. Related: OS#5337 Change-Id: I234b2a754d0c98031056981823cdbc187e977741 --- M src/osmo-bsc/bsc_subscr_conn_fsm.c 1 file changed, 20 insertions(+), 15 deletions(-) Approvals: fixeria: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c index ff742a2..e33fe43 100644 --- a/src/osmo-bsc/bsc_subscr_conn_fsm.c +++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c @@ -141,10 +141,24 @@ static void gscon_bssmap_clear(struct gsm_subscriber_connection *conn, enum gsm0808_cause cause) { + /* already clearing? */ + switch (conn->fi->state) { + case ST_CLEARING: + return; + default: + break; + } + conn->clear_cause = cause; + conn_fsm_state_chg(ST_CLEARING); +} + +static void gscon_fsm_clearing_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state) +{ struct msgb *resp; int rc; - conn->clear_cause = cause; + struct gsm_subscriber_connection *conn = fi->priv; + enum gsm0808_cause cause = conn->clear_cause; if (conn->rx_clear_command) { LOGPFSML(conn->fi, LOGL_DEBUG, "Not sending BSSMAP CLEAR REQUEST, already got CLEAR COMMAND from MSC\n"); @@ -152,14 +166,14 @@ } if (!conn->sccp.msc) { - LOGPFSML(conn->fi, LOGL_ERROR, "Unable to deliver BSSMAP Clear Request message, no MSC for this conn\n"); + LOGPFSML(fi, LOGL_ERROR, "Unable to deliver BSSMAP Clear Request message, no MSC for this conn\n"); return; } - LOGPFSML(conn->fi, LOGL_DEBUG, "Tx BSSMAP CLEAR REQUEST(%s) to MSC\n", gsm0808_cause_name(cause)); + LOGPFSML(fi, LOGL_DEBUG, "Tx BSSMAP CLEAR REQUEST(%s) to MSC\n", gsm0808_cause_name(cause)); resp = gsm0808_create_clear_rqst(cause); if (!resp) { - LOGPFSML(conn->fi, LOGL_ERROR, "Unable to compose BSSMAP Clear Request message\n"); + LOGPFSML(fi, LOGL_ERROR, "Unable to compose BSSMAP Clear Request message\n"); return; } @@ -355,8 +369,6 @@ osmo_fsm_inst_state_name(conn->fi)); } gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE); - if (conn->fi->state != ST_CLEARING) - osmo_fsm_inst_state_chg(fi, ST_CLEARING, 60, -4); return; default: OSMO_ASSERT(false); @@ -376,7 +388,6 @@ confirmed connection, then instead simply drop the connection */ LOGPFSML(fi, LOGL_INFO, "Connection confirmed but lchan was dropped previously, clearing conn\n"); - osmo_fsm_inst_state_chg(conn->fi, ST_CLEARING, 60, -4); gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE); break; } @@ -705,6 +716,7 @@ }, [ST_CLEARING] = { .name = "CLEARING", + .onenter = gscon_fsm_clearing_onenter, /* dead end state */ }, }; @@ -763,11 +775,6 @@ yet so we cannot release it. First wait for the CC, and release in gscon_fsm_wait_cc(). */ break; default: - /* Ensure that the FSM is in ST_CLEARING. */ - osmo_fsm_inst_state_chg(conn->fi, ST_CLEARING, 60, -4); - /* fall thru, omit an error log if already in ST_CLEARING */ - case ST_CLEARING: - /* Request a Clear Command from the MSC. */ gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE); break; } @@ -806,8 +813,7 @@ osmo_fsm_inst_name(conn->fi), detach_label); } - if ((conn->fi && conn->fi->state != ST_CLEARING) - && !conn->lchan + if (!conn->lchan && !conn->ho.new_lchan && !conn->assignment.new_lchan && !conn->lcs.loc_req) @@ -1205,7 +1211,6 @@ failed_to_send: LOGPFSML(conn->fi, LOGL_ERROR, "Tx BSSMAP CLEAR REQUEST to MSC\n"); gscon_bssmap_clear(conn, GSM0808_CAUSE_EQUIPMENT_FAILURE); - osmo_fsm_inst_state_chg(conn->fi, ST_ACTIVE, 0, 0); } void gscon_submit_rsl_dtap(struct gsm_subscriber_connection *conn, -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/26613 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I234b2a754d0c98031056981823cdbc187e977741 Gerrit-Change-Number: 26613 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: ipse <Alexander.Chemeris(a)gmail.com> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

3 years, 6 months

1
0
0 0

2025

2024

2023

2022

gerrit-log