make sure early lchan act failure resets the lchan

Fix crash after AMR configuration fails.

The crash is due to an assertion that finds a non-NULL conn in the lchan, when
re-using an lchan that has failed in AMR configuration earlier on. That is
because the AMR config still happens in state UNUSED.

  DCHAN ERROR lchan(0-0-2-TCH_F_TCH_H_PDCH-0)[0x6120000066a0]{UNUSED}: (type=TCH_F) lchan allocation failed in state UNUSED: Can not generate multirate configuration IE
  ...
  DCHAN DEBUG lchan(0-0-2-TCH_F_TCH_H_PDCH-0)[0x6120000066a0]{UNUSED}: (type=TCH_F) After failure handling, already in state UNUSED
  ...
  ...
  DCHAN DEBUG lchan(0-0-2-TCH_F_TCH_H_PDCH-0)[0x6120000066a0]{UNUSED}: Received Event LCHAN_EV_ACTIVATE (lchan_fsm.c:324)
  Assert failed !lchan->conn ../../../../src/osmo-bsc/src/osmo-bsc/lchan_fsm.c:491

The FSM design idea is that when returning to the UNUSED state, all lchan state
is cleared. However, when calling lchan_activate(), a failure may happen still
in state UNUSED, so that we don't transition *back* to UNUSED properly.

So, first transition out of UNUSED before failures can happen. (Other ways to
solve this would be to invoke lchan clearing even if already in UNUSED, but
semantically, transitioning first makes more sense.)

Upon LCHAN_EV_ACTIVATE, just remember the lchan_activate_info and transition to
WAIT_TS_READY, so that on lchan_fail(), we can normally transition back to
UNUSED and clear the lchan.

Move the initial lchan activation code to lchan_fsm_wait_ts_ready_onenter().

Also, there is a bit of duplication of members of the lchan->activate (lchan
state) and the lchan_activate_info (passed to lchan_activate()) structs. The
fix for this also removes the dup:

Add struct lchan_activate_info as child struct at lchan->activate.info, drop
the other lchan->activate members that would dup .info.*. Move struct
lchan_activate_info declaration to gsm_data.h.

Apply the new '.info' member struct throughout the code.

Related: OS#3737
Change-Id: Ide665b10fa3f4583059c55346db8da833959e3cc

2 files changed, 14 insertions, 18 deletions

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index cf34c6f16..20901420e 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -503,6 +503,19 @@ extern const struct value_string lchan_activate_mode_names[];
 static inline const char *lchan_activate_mode_name(enum lchan_activate_mode activ_for)
 { return get_value_string(lchan_activate_mode_names, activ_for); }
 
+struct lchan_activate_info {
+	enum lchan_activate_mode activ_for;
+	struct gsm_subscriber_connection *for_conn;
+	/* This always is for a specific lchan, so its lchan->type indicates full or half rate.
+	 * When a dyn TS was selected, the lchan->type has been set to the desired rate. */
+	enum gsm48_chan_mode chan_mode;
+	uint16_t s15_s0;
+	bool requires_voice_stream;
+	bool wait_before_switching_rtp; /*< true = requires LCHAN_EV_READY_TO_SWITCH_RTP */
+	uint16_t msc_assigned_cic;
+	struct gsm_lchan *re_use_mgw_endpoint_from_lchan;
+};
+
 struct gsm_lchan {
 	/* The TS that we're part of */
 	struct gsm_bts_trx_ts *ts;
@@ -517,18 +530,14 @@ struct gsm_lchan {
 	struct mgwep_ci *mgw_endpoint_ci_bts;
 
 	struct {
-		enum lchan_activate_mode activ_for;
+		struct lchan_activate_info info;
 		bool activ_ack; /*< true as soon as RSL Chan Activ Ack is received */
 		bool immediate_assignment_sent;
 		/*! This flag ensures that when an lchan activation has succeeded, and we have already
 		 * sent ACKs like Immediate Assignment or BSSMAP Assignment Complete, and if other errors
 		 * occur later, e.g. during release, that we don't send a NACK out of context. */
 		bool concluded;
-		bool requires_voice_stream;
-		bool wait_before_switching_rtp; /*< true = requires LCHAN_EV_READY_TO_SWITCH_RTP */
-		uint16_t msc_assigned_cic;
 		enum gsm0808_cause gsm0808_error_cause;
-		struct gsm_lchan *re_use_mgw_endpoint_from_lchan;
 	} activate;
 
 	struct {
diff --git a/include/osmocom/bsc/lchan_fsm.h b/include/osmocom/bsc/lchan_fsm.h
index 48cd3836a..55ab02400 100644
--- a/include/osmocom/bsc/lchan_fsm.h
+++ b/include/osmocom/bsc/lchan_fsm.h
@@ -52,19 +52,6 @@ void lchan_fsm_alloc(struct gsm_lchan *lchan);
 void lchan_release(struct gsm_lchan *lchan, bool do_rr_release,
 		   bool err, enum gsm48_rr_cause cause_rr);
 
-struct lchan_activate_info {
-	enum lchan_activate_mode activ_for;
-	struct gsm_subscriber_connection *for_conn;
-	/* This always is for a specific lchan, so its lchan->type indicates full or half rate.
-	 * When a dyn TS was selected, the lchan->type has been set to the desired rate. */
-	enum gsm48_chan_mode chan_mode;
-	uint16_t s15_s0;
-	bool requires_voice_stream;
-	bool wait_before_switching_rtp;
-	uint16_t msc_assigned_cic;
-	struct gsm_lchan *old_lchan;
-};
-
 void lchan_activate(struct gsm_lchan *lchan, struct lchan_activate_info *info);
 void lchan_ready_to_switch_rtp(struct gsm_lchan *lchan);