Neels Hofmeyr has submitted this change and it was merged. (
https://gerrit.osmocom.org/11667 )
Change subject: lchan release: always Deact SACCH
......................................................................
lchan release: always Deact SACCH
If an lchan is being released and had a SACCH active, there is no reason to
omit the Deact SACCH message ever. All of the callers that passed
do_deact_sacch = false did so for no good reason.
Drop the do_deact_sacch flag everywhere and, when the lchan type matches and
SAPI[0] is still active, simply always send a Deact SACCH message.
The do_deact_sacch flag was carried over from legacy code, by me, mainly
because I never really understood why it was there. I do hope I'm correct now,
asserting that having this flag makes no sense.
Change-Id: Id3301df059582da2377ef82feae554e94fa42035
---
M include/osmocom/bsc/bsc_subscr_conn_fsm.h
M include/osmocom/bsc/gsm_data.h
M include/osmocom/bsc/lchan_fsm.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/assignment_fsm.c
M src/osmo-bsc/bsc_subscr_conn_fsm.c
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/gsm_04_08_rr.c
M src/osmo-bsc/gsm_08_08.c
M src/osmo-bsc/handover_fsm.c
M src/osmo-bsc/lchan_fsm.c
M tests/gsm0408/gsm0408_test.c
M tests/handover/handover_test.c
13 files changed, 32 insertions(+), 32 deletions(-)
Approvals:
Neels Hofmeyr: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/include/osmocom/bsc/bsc_subscr_conn_fsm.h
b/include/osmocom/bsc/bsc_subscr_conn_fsm.h
index fcdba50..f5ed7bd 100644
--- a/include/osmocom/bsc/bsc_subscr_conn_fsm.h
+++ b/include/osmocom/bsc/bsc_subscr_conn_fsm.h
@@ -71,7 +71,7 @@
struct assignment_request *req);
void gscon_change_primary_lchan(struct gsm_subscriber_connection *conn, struct
gsm_lchan *new_lchan);
-void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool
do_sacch_deact, bool do_rr_release);
+void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool
do_rr_release);
void gscon_lchan_releasing(struct gsm_subscriber_connection *conn, struct
gsm_lchan *lchan);
void gscon_forget_lchan(struct gsm_subscriber_connection *conn, struct
gsm_lchan *lchan);
diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 5805a5f..3712b97 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -513,7 +513,6 @@
/* If an event to release the lchan comes in while still waiting for
responses, just mark this
* flag, so that the lchan will gracefully release at the next sensible
junction. */
bool release_requested;
- bool deact_sacch;
bool do_rr_release;
char *last_error;
diff --git a/include/osmocom/bsc/lchan_fsm.h b/include/osmocom/bsc/lchan_fsm.h
index d3315a6..48cd383 100644
--- a/include/osmocom/bsc/lchan_fsm.h
+++ b/include/osmocom/bsc/lchan_fsm.h
@@ -49,7 +49,7 @@
void lchan_fsm_init();
void lchan_fsm_alloc(struct gsm_lchan *lchan);
-void lchan_release(struct gsm_lchan *lchan, bool do_deact_sacch, bool
do_rr_release,
+void lchan_release(struct gsm_lchan *lchan, bool do_rr_release,
bool err, enum gsm48_rr_cause cause_rr);
struct lchan_activate_info {
diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index 0ec8fbc..c16f044 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -896,7 +896,7 @@
* the connection will presumably be torn down and lead to an lchan
release. During initial
* Channel Request from the MS, an lchan has no conn yet, so in that
case release now. */
if (!lchan->conn) {
- lchan_release(lchan, false, false, true, *cause_p);
+ lchan_release(lchan, false, true, *cause_p);
} else
osmo_fsm_inst_dispatch(lchan->conn->fi, GSCON_EV_RSL_CONN_FAIL,
(void*)cause_p);
diff --git a/src/osmo-bsc/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c
index 653681e..93362f8 100644
--- a/src/osmo-bsc/assignment_fsm.c
+++ b/src/osmo-bsc/assignment_fsm.c
@@ -98,7 +98,7 @@
if (conn->assignment.new_lchan) {
struct gsm_lchan *lchan = conn->assignment.new_lchan;
conn->assignment.new_lchan = NULL;
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true, RSL_ERR_EQUIPMENT_FAIL);
}
if (conn->assignment.created_ci_for_msc) {
@@ -213,7 +213,7 @@
if (!conn->assignment.fi) {
/* The lchan was ready, and we failed to tell the MSC about it.
By releasing this lchan,
* the conn will notice that its primary lchan is gone and
should clean itself up. */
- lchan_release(conn->lchan, false, true, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(conn->lchan, true, true, RSL_ERR_EQUIPMENT_FAIL);
return;
}
diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c
b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index 5856d7a..ec06e9b 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -152,7 +152,7 @@
/* Release an lchan in such a way that it doesn't fire events back to the
conn. */
static void gscon_release_lchan(struct gsm_subscriber_connection *conn, struct
gsm_lchan *lchan,
- bool do_sacch_deact, bool do_rr_release, bool
err, enum gsm48_rr_cause cause_rr)
+ bool do_rr_release, bool err, enum
gsm48_rr_cause cause_rr)
{
if (!lchan || !conn)
return;
@@ -164,17 +164,17 @@
conn->ho.new_lchan = NULL;
if (conn->assignment.new_lchan == lchan)
conn->assignment.new_lchan = NULL;
- lchan_release(lchan, do_sacch_deact, do_rr_release, err, cause_rr);
+ lchan_release(lchan, do_rr_release, err, cause_rr);
}
-void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool
do_sacch_deact, bool do_rr_release)
+void gscon_release_lchans(struct gsm_subscriber_connection *conn, bool
do_rr_release)
{
if (conn->ho.fi)
handover_end(conn, HO_RESULT_CONN_RELEASE);
assignment_reset(conn);
- gscon_release_lchan(conn, conn->lchan, do_sacch_deact, do_rr_release,
false, 0);
+ gscon_release_lchan(conn, conn->lchan, do_rr_release, false, 0);
}
static void handle_bssap_n_connect(struct osmo_fsm_inst *fi, struct
osmo_scu_prim *scu_prim)
@@ -620,7 +620,7 @@
osmo_fsm_inst_dispatch(conn->lchan->fi_rtp,
LCHAN_RTP_EV_ESTABLISHED, 0);
if (old_lchan && (old_lchan != new_lchan))
- gscon_release_lchan(conn, old_lchan, false, false, false, 0);
+ gscon_release_lchan(conn, old_lchan, false, false, 0);
}
void gscon_lchan_releasing(struct gsm_subscriber_connection *conn, struct
gsm_lchan *lchan)
@@ -716,7 +716,7 @@
if (conn->fi->state != ST_CLEARING)
osmo_fsm_inst_state_chg(fi, ST_CLEARING, 60, 999);
LOGPFSML(fi, LOGL_DEBUG, "Releasing all lchans (if any) after
BSSMAP Clear Command\n");
- gscon_release_lchans(conn, true, true);
+ gscon_release_lchans(conn, true);
/* FIXME: Release all terestrial resources in ST_CLEARING */
/* According to 3GPP 48.008 3.1.9.1. "The BSS need not wait for
the radio channel
* release to be completed or for the guard timer to expire
before returning the
@@ -792,7 +792,7 @@
}
LOGPFSML(fi, LOGL_DEBUG, "Releasing all lchans (if any) because this
conn is terminating\n");
- gscon_release_lchans(conn, false, true);
+ gscon_release_lchans(conn, true);
/* drop pending messages */
gscon_dtap_queue_flush(conn, 0);
@@ -806,7 +806,7 @@
switch (fi->T) {
case 993210:
- gscon_release_lchan(conn, conn->lchan, false, true, true,
RSL_ERR_INTERWORKING);
+ gscon_release_lchan(conn, conn->lchan, true, true,
RSL_ERR_INTERWORKING);
/* MSC has not responded/confirmed connection with CC, this
* could indicate a bad SCCP connection. We now inform the the
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index 9f42d8a..8eb0692 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -4588,7 +4588,7 @@
}
vty_out(vty, "%% Asking for release of %s in state %s%s",
gsm_lchan_name(lchan),
osmo_fsm_inst_state_name(lchan->fi), VTY_NEWLINE);
- lchan_release(lchan, false, !!(lchan->conn), false, 0);
+ lchan_release(lchan, !!(lchan->conn), false, 0);
}
return CMD_SUCCESS;
diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c
index c3dd307..4659c1a 100644
--- a/src/osmo-bsc/gsm_04_08_rr.c
+++ b/src/osmo-bsc/gsm_04_08_rr.c
@@ -942,7 +942,7 @@
/* allocate a new connection */
lchan->conn =
bsc_subscr_con_allocate(msg->lchan->ts->trx->bts->network);
if (!lchan->conn) {
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
return -1;
}
lchan->conn->lchan = lchan;
diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c
index 48e87fc..062c878 100644
--- a/src/osmo-bsc/gsm_08_08.c
+++ b/src/osmo-bsc/gsm_08_08.c
@@ -525,7 +525,7 @@
/* FIXME: I have not the slightest idea what move_to_msc()
intends to do; during lchan
* FSM introduction, I changed this and hope it is the
appropriate action. I actually
* assume this is unused legacy code for osmo-bsc_nat?? */
- gscon_release_lchans(_conn, false, false);
+ gscon_release_lchans(_conn, false);
return 1;
}
diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c
index 8076eeb..f2836cf 100644
--- a/src/osmo-bsc/handover_fsm.c
+++ b/src/osmo-bsc/handover_fsm.c
@@ -243,7 +243,7 @@
struct mgwep_ci *ci;
if (conn->ho.new_lchan)
/* New lchan was activated but never passed to a conn */
- lchan_release(conn->ho.new_lchan, true, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(conn->ho.new_lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
ci = conn->ho.created_ci_for_msc;
if (ci) {
@@ -725,7 +725,7 @@
/* 3GPP TS 48.008 3.1.5.3.3 "Abnormal
Conditions": if neither MS reports
* HO Failure nor the MSC sends a Clear
Command, we should release the
* dedicated radio resources and send a Clear
Request to the MSC. */
- lchan_release(conn->lchan, false, true, true,
GSM48_RR_CAUSE_ABNORMAL_TIMER);
+ lchan_release(conn->lchan, true, true,
GSM48_RR_CAUSE_ABNORMAL_TIMER);
/* Once the channel release is through, the
BSSMAP Clear will follow. */
break;
}
@@ -756,7 +756,7 @@
/* Detach the new_lchan last, so we can still see it in above logging */
if (ho->new_lchan) {
/* Release new lchan, it didn't work out */
- lchan_release(ho->new_lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(ho->new_lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
ho->new_lchan = NULL;
}
diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c
index f432644..5428d97 100644
--- a/src/osmo-bsc/lchan_fsm.c
+++ b/src/osmo-bsc/lchan_fsm.c
@@ -156,14 +156,14 @@
LOG_LCHAN(lchan, LOGL_ERROR,
"lchan activation for assignment succeeded,
but lchan has no conn:"
" cannot trigger appropriate actions.
Release.\n");
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
break;
}
if (!lchan->conn->assignment.fi) {
LOG_LCHAN(lchan, LOGL_ERROR,
"lchan activation for assignment succeeded,
but lchan has no"
" assignment ongoing: cannot trigger
appropriate actions. Release.\n");
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
break;
}
osmo_fsm_inst_dispatch(lchan->conn->assignment.fi,
ASSIGNMENT_EV_LCHAN_ESTABLISHED,
@@ -177,14 +177,14 @@
if (!lchan->conn) {
LOG_LCHAN(lchan, LOGL_ERROR,
"lchan activation for handover succeeded, but
lchan has no conn\n");
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
break;
}
if (!lchan->conn->ho.fi) {
LOG_LCHAN(lchan, LOGL_ERROR,
"lchan activation for handover succeeded, but
lchan has no"
" handover ongoing\n");
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
break;
}
osmo_fsm_inst_dispatch(lchan->conn->ho.fi,
HO_EV_LCHAN_ESTABLISHED, lchan);
@@ -720,14 +720,14 @@
LOG_LCHAN(lchan, LOGL_ERROR,
"lchan activation for assignment succeeded,
but lchan has no conn:"
" cannot trigger appropriate actions.
Release.\n");
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
break;
}
if (!lchan->conn->assignment.fi) {
LOG_LCHAN(lchan, LOGL_ERROR,
"lchan activation for assignment succeeded,
but lchan has no"
" assignment ongoing: cannot trigger
appropriate actions. Release.\n");
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
break;
}
/* After the Chan Activ Ack, the MS expects to receive an RR
Assignment Command.
@@ -740,14 +740,14 @@
LOG_LCHAN(lchan, LOGL_ERROR,
"lchan activation for handover succeeded, but
lchan has no conn:"
" cannot trigger appropriate actions.
Release.\n");
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
break;
}
if (!lchan->conn->ho.fi) {
LOG_LCHAN(lchan, LOGL_ERROR,
"lchan activation for handover succeeded, but
lchan has no"
" handover ongoing: cannot trigger
appropriate actions. Release.\n");
- lchan_release(lchan, false, false, true,
RSL_ERR_EQUIPMENT_FAIL);
+ lchan_release(lchan, false, true,
RSL_ERR_EQUIPMENT_FAIL);
break;
}
/* After the Chan Activ Ack of the new lchan, send the MS an RR
Handover Command on the
@@ -935,7 +935,7 @@
if (lchan->fi_rtp)
osmo_fsm_inst_dispatch(lchan->fi_rtp, LCHAN_RTP_EV_RELEASE, 0);
- if (lchan->deact_sacch && should_sacch_deact(lchan))
+ if (should_sacch_deact(lchan))
rsl_deact_sacch(lchan);
}
@@ -1296,7 +1296,7 @@
}
}
-void lchan_release(struct gsm_lchan *lchan, bool sacch_deact, bool
do_rr_release,
+void lchan_release(struct gsm_lchan *lchan, bool do_rr_release,
bool err, enum gsm48_rr_cause cause_rr)
{
if (!lchan || !lchan->fi)
@@ -1309,7 +1309,6 @@
struct osmo_fsm_inst *fi = lchan->fi;
lchan->release_in_error = err;
lchan->rsl_error_cause = cause_rr;
- lchan->deact_sacch = sacch_deact;
lchan->do_rr_release = do_rr_release;
/* States waiting for events will notice the desire to release when
done waiting, so it is enough
diff --git a/tests/gsm0408/gsm0408_test.c b/tests/gsm0408/gsm0408_test.c
index 1e6a097..d15e149 100644
--- a/tests/gsm0408/gsm0408_test.c
+++ b/tests/gsm0408/gsm0408_test.c
@@ -986,7 +986,7 @@
const char *bsc_subscr_name(struct bsc_subscr *bsub) { return NULL; }
-void lchan_release(struct gsm_lchan *lchan, bool do_deact_sacch, bool
do_rr_release,
+void lchan_release(struct gsm_lchan *lchan, bool do_rr_release,
bool err, enum gsm48_rr_cause cause_rr) {}
int rsl_data_request(struct msgb *msg, uint8_t link_id) { return 0; }
diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c
index 7cb4086..f728c5b 100644
--- a/tests/handover/handover_test.c
+++ b/tests/handover/handover_test.c
@@ -462,6 +462,8 @@
break;
case RSL_MT_IPAC_CRCX:
break;
+ case RSL_MT_DEACTIVATE_SACCH:
+ break;
default:
printf("unknown rsl message=0x%x\n", dh->c.msg_type);
}
--
To view, visit https://gerrit.osmocom.org/11667
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id3301df059582da2377ef82feae554e94fa42035
Gerrit-Change-Number: 11667
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
