Attention is currently required from: laforge, pespin.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/34905?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
The change is no longer submittable: Verified is unsatisfied now.
Change subject: mgcp-client: MGCP response: pass fmtp to caller
......................................................................
mgcp-client: MGCP response: pass fmtp to caller
When receiving MGCP responses, so far libosmo-mgcp-client completely
ignored a=fmtp: parameters (like 'octet-align'). Add fmtp parsing to
pass the fmtp string to the caller as-is.
Since the responses so far never included the octet_aligned flags, do
not bother to parse fmtp to populate the legacy items. New callers
should use the fmtp string.
Change-Id: If8ca5c3880cad9e41b80e9d1c821439b0d7b7e23
---
M src/libosmo-mgcp-client/mgcp_client.c
1 file changed, 57 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/05/34905/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34905?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: If8ca5c3880cad9e41b80e9d1c821439b0d7b7e23
Gerrit-Change-Number: 34905
Gerrit-PatchSet: 3
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35098?usp=email )
Change subject: bsc: tests recovery from BORKEN after release failed
......................................................................
bsc: tests recovery from BORKEN after release failed
Related: osmo-bsc Ic4728b3efe843ea63e2a0b54b1ea8a925347484a
Related: SYS#6655
Change-Id: I9b4ddfc4a337808d9d5ec538c25fd390b1b2530f
---
M bsc/BSC_Tests.ttcn
1 file changed, 139 insertions(+), 3 deletions(-)
Approvals:
daniel: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/bsc/BSC_Tests.ttcn b/bsc/BSC_Tests.ttcn
index 57b44e0..1fc42ba 100644
--- a/bsc/BSC_Tests.ttcn
+++ b/bsc/BSC_Tests.ttcn
@@ -8529,7 +8529,8 @@
}
friend function f_perform_clear(RSL_DCHAN_PT rsl_pt := RSL, RSLEM_PROC_PT rsl_proc_pt := RSL_PROC,
- template PDU_ML3_NW_MS exp_rr_rel_tmpl := tr_RRM_RR_RELEASE)
+ template PDU_ML3_NW_MS exp_rr_rel_tmpl := tr_RRM_RR_RELEASE,
+ boolean send_rel_ack := true)
runs on MSC_ConnHdlr {
var default ack_dlcx := activate(as_mgcp_ack_all_dlcx());
var default ack_rel_req := activate(as_rsl_ack_all_rel_req());
@@ -8549,8 +8550,10 @@
}
[] rsl_pt.receive(tr_RSL_RF_CHAN_REL(g_chan_nr)) {
f_logp(BSCVTY, "Got RSL RF Chan Rel, sending Rel Ack");
- rsl_pt.send(ts_RSL_RF_CHAN_REL_ACK(g_chan_nr));
- f_rslem_unregister(0, g_chan_nr, PT := rsl_proc_pt);
+ if (send_rel_ack) {
+ rsl_pt.send(ts_RSL_RF_CHAN_REL_ACK(g_chan_nr));
+ f_rslem_unregister(0, g_chan_nr, PT := rsl_proc_pt);
+ }
}
}
deactivate(ack_dlcx);
@@ -12439,6 +12442,125 @@
f_shutdown_helper(ho := true);
}
+private function f_vty_expect_borken(TELNETasp_PT pt, boolean expect_borken)
+{
+ var charstring lchan_summary;
+ lchan_summary := f_vty_transceive_ret(pt, "show lchan summary");
+ var boolean found_borken := (f_strstr(lchan_summary, "State BORKEN") >= 0);
+ if (found_borken != expect_borken) {
+ if (expect_borken) {
+ setverdict(fail, "Expected 'BORKEN' state in ", lchan_summary);
+ } else {
+ setverdict(fail, "Expected no 'BORKEN' state in ", lchan_summary);
+ }
+ mtc.stop;
+ }
+}
+
+private function f_tc_unbreak_lchan_after_missing_rel_ack(integer test_variant) runs on MSC_ConnHdlr {
+
+ /* Establish a channel, so we can mess with the release of it. */
+ var template PDU_BSSAP exp_compl := f_gen_exp_compl();
+ var PDU_BSSAP ass_cmd := f_gen_ass_req();
+
+ ass_cmd.pdu.bssmap.assignmentRequest.channelType := valueof(ts_BSSMAP_IE_ChannelType);
+ ass_cmd.pdu.bssmap.assignmentRequest.codecList := valueof(ts_BSSMAP_IE_CodecList({ts_CodecFR}));
+
+ f_establish_fully(ass_cmd, exp_compl);
+
+ /* shorten X28, the time after which a BORKEN lchan recovers, from 30s to something short */
+ f_vty_enter_cfg_network(BSCVTY);
+ f_vty_transceive(BSCVTY, "timer X28 3");
+ f_vty_transceive(BSCVTY, "end");
+
+ /* Allow us to include handling of Channel Activation in this component, to test recovery from BORKEN */
+ f_rslem_set_auto_chan_act_ack(RSL_PROC, false);
+
+ /* give a moment before releasing */
+ f_sleep(2.0);
+
+ /* Perform a channel release, but omit the RF CHAN REL ACK. The aim is to get the lchan to go BORKEN. */
+ f_perform_clear(send_rel_ack := false);
+
+ f_vty_expect_borken(BSCVTY, false);
+
+ /* After a timeout of X6, the lchan ends up in LCHAN_ST_BORKEN */
+ f_sleep(6.0);
+
+ f_vty_expect_borken(BSCVTY, true);
+
+ /* Now within two seconds, we expect OsmoBSC to attempt recovering from the broken state.
+ *
+ * test_variant == 1:
+ * It will first attempt a Channel Activation. If the BTS accepts that, it means the state is back in sync.
+ * Then OsmoBSC releases the lchan again and makes it available for use.
+ *
+ * test_variant == 2:
+ * It will first attempt a Channel Activation and the BTS NACKs that.
+ * It will then attempt a release, which we ACK, and the channel will be available again.
+ */
+
+ /* Receive the Channel Activation */
+ var ASP_RSL_Unitdata rx_rsl_ud;
+ RSL.receive(tr_ASP_RSL_UD(tr_RSL_MsgTypeD(RSL_MT_CHAN_ACTIV), sid := ?)) -> value rx_rsl_ud;
+
+ var RslChannelNr chan_nr;
+ var RSL_IE_Body ie;
+ if (f_rsl_find_ie(rx_rsl_ud.rsl, RSL_IE_CHAN_NR, ie) == true) {
+ chan_nr := ie.chan_nr;
+ } else {
+ setverdict(fail, "Unable to find RSL_IE_CHAN_NR in ", rx_rsl_ud);
+ return;
+ }
+
+ if (test_variant == 1) {
+ RSL.send(ts_ASP_RSL_UD(ts_RSL_CHAN_ACT_ACK(chan_nr, 23), rx_rsl_ud.streamId));
+
+ /* Back in sync, receive the Channel Release */
+ RSL.receive(tr_RSL_RF_CHAN_REL(chan_nr));
+ RSL.send(ts_RSL_RF_CHAN_REL_ACK(chan_nr));
+ } else if (test_variant == 2) {
+ RSL.send(ts_ASP_RSL_UD(ts_RSL_CHAN_ACT_NACK(chan_nr, RSL_ERR_EQUIPMENT_FAIL), rx_rsl_ud.streamId));
+
+ /* Instead tries a release */
+ RSL.receive(tr_RSL_RF_CHAN_REL(chan_nr));
+ RSL.send(ts_RSL_RF_CHAN_REL_ACK(chan_nr));
+ /* That worked, channel usable again */
+ } else {
+ setverdict(fail, "undefined test_variant ", test_variant);
+ }
+
+ f_vty_expect_borken(BSCVTY, false);
+ setverdict(pass);
+}
+
+/* Test recovery path from a BORKEN lchan */
+private function f_TC_unbreak_lchan_after_missing_rel_ack_1(charstring id) runs on MSC_ConnHdlr {
+ f_tc_unbreak_lchan_after_missing_rel_ack(test_variant := 1);
+}
+testcase TC_unbreak_lchan_after_missing_rel_ack_1() runs on test_CT {
+ var TestHdlrParams pars := f_gen_test_hdlr_pars();
+ var MSC_ConnHdlr vc_conn;
+ f_init(1, true);
+ f_sleep(1.0);
+ vc_conn := f_start_handler(refers(f_TC_unbreak_lchan_after_missing_rel_ack_1), pars);
+ vc_conn.done;
+ f_shutdown_helper();
+}
+
+private function f_TC_unbreak_lchan_after_missing_rel_ack_2(charstring id) runs on MSC_ConnHdlr {
+ f_tc_unbreak_lchan_after_missing_rel_ack(test_variant := 2);
+}
+testcase TC_unbreak_lchan_after_missing_rel_ack_2() runs on test_CT {
+ var TestHdlrParams pars := f_gen_test_hdlr_pars();
+ var MSC_ConnHdlr vc_conn;
+ f_init(1, true);
+ f_sleep(1.0);
+ vc_conn := f_start_handler(refers(f_TC_unbreak_lchan_after_missing_rel_ack_2), pars);
+ vc_conn.done;
+ f_shutdown_helper();
+}
+
control {
/* CTRL interface testing */
execute( TC_ctrl_msc_connection_status() );
@@ -12792,6 +12914,9 @@
/* Run TC_ho_out_of_this_bsc last, because it may trigger a segfault before osmo-bsc's patch
* with change-id I5a3345ab0005a73597f5c27207480912a2f5aae6 */
execute( TC_ho_out_of_this_bsc() );
+
+ execute( TC_unbreak_lchan_after_missing_rel_ack_1() );
+ execute( TC_unbreak_lchan_after_missing_rel_ack_2() );
}
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35098?usp=email
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: I9b4ddfc4a337808d9d5ec538c25fd390b1b2530f
Gerrit-Change-Number: 35098
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: merged
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34997?usp=email )
Change subject: client: replace two assertions with graceful error handling
......................................................................
client: replace two assertions with graceful error handling
A user reports crashes of osmo-bsc upon EV_MDCX. It turns out that there
is a lot of error reporting and a distinct possibility to get a NULL
return value because of external input. Terminate the FSM instead.
FSM termination is the proper way to report a bad error, it signals the
parent_term_evt to the FSM parent, which will then be able to act on the
failed MGCP operation.
Related: SYS#6632
Change-Id: Ia5d8a9aff565399a85a5b116d7029fedcab234e0
---
M src/libosmo-mgcp-client/mgcp_client_fsm.c
1 file changed, 28 insertions(+), 2 deletions(-)
Approvals:
laforge: Looks good to me, but someone else must approve
neels: Looks good to me, approved
fixeria: Looks good to me, but someone else must approve
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
diff --git a/src/libosmo-mgcp-client/mgcp_client_fsm.c b/src/libosmo-mgcp-client/mgcp_client_fsm.c
index 660f0ad..432ad84 100644
--- a/src/libosmo-mgcp-client/mgcp_client_fsm.c
+++ b/src/libosmo-mgcp-client/mgcp_client_fsm.c
@@ -364,13 +364,21 @@
switch (event) {
case EV_MDCX:
msg = make_mdcx_msg(mgcp_ctx);
- OSMO_ASSERT(msg);
+ if (!msg) {
+ /* make_mdcx_msg() should already have logged the error */
+ osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
+ return;
+ }
rc = mgcp_client_tx(mgcp, msg, mgw_mdcx_resp_cb, fi);
new_state = ST_MDCX_RESP;
break;
case EV_DLCX:
msg = make_dlcx_msg(mgcp_ctx);
- OSMO_ASSERT(msg);
+ if (!msg) {
+ /* make_dlcx_msg() should already have logged the error */
+ osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
+ return;
+ }
rc = mgcp_client_tx(mgcp, msg, mgw_dlcx_resp_cb, fi);
new_state = ST_DLCX_RESP;
break;
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34997?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ia5d8a9aff565399a85a5b116d7029fedcab234e0
Gerrit-Change-Number: 34997
Gerrit-PatchSet: 3
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: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: neels.
Hello Jenkins Builder, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-msc/+/35137?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
The change is no longer submittable: Verified is unsatisfied now.
Change subject: msc_vlr_test_call: drop misleading log
......................................................................
msc_vlr_test_call: drop misleading log
It was true once, but not since "do CN CRCX first"
Ie433db1ba0c46d4b97538a969233c155cefac21c
Related: OS#6258
Change-Id: I94e430e4e5b5bf18dbb155258d82f599ada453e6
---
M tests/msc_vlr/msc_vlr_test_call.c
M tests/msc_vlr/msc_vlr_test_call.err
2 files changed, 31 insertions(+), 18 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/37/35137/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35137?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I94e430e4e5b5bf18dbb155258d82f599ada453e6
Gerrit-Change-Number: 35137
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newpatchset
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/35100?usp=email )
Change subject: call_leg: silence some false errors
......................................................................
call_leg: silence some false errors
"[ESTABLISHED] transition to state ESTABLISHED not permitted"
i.e. don't complain when we already are in the established state.
Change-Id: I9b1fd63ed1ee7ed2877a4b2059386354598f4ea4
---
M src/libmsc/call_leg.c
1 file changed, 15 insertions(+), 1 deletion(-)
Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, approved
diff --git a/src/libmsc/call_leg.c b/src/libmsc/call_leg.c
index b797322..5720417 100644
--- a/src/libmsc/call_leg.c
+++ b/src/libmsc/call_leg.c
@@ -158,7 +158,8 @@
}
if (!established)
break;
- call_leg_state_chg(cl, CALL_LEG_ST_ESTABLISHED);
+ if (cl->fi->state != CALL_LEG_ST_ESTABLISHED)
+ call_leg_state_chg(cl, CALL_LEG_ST_ESTABLISHED);
break;
case CALL_LEG_EV_RTP_STREAM_ADDR_AVAILABLE:
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35100?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I9b1fd63ed1ee7ed2877a4b2059386354598f4ea4
Gerrit-Change-Number: 35100
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: merged