[PATCH 6/6] gprs-ns: Fix reset state handling

[Jacob Erlbeck]

Currently the NS-VC's state is updated from within gprs_ns_tx_reset,
which can lead to an inconsistent state when the RESET_ACK is lost.
In this state, the NSE_S_RESET bit is set but the Tns-reset timer is
not started.

This patch moves the state update into gprs_nsvc_reset. This way, the
state flags are consistent with the timer.

Addresses:
  SGSN -> BSS       NS_ALIVE
  BSS -> SGSN       NS_ALIVE_ACK
  BSS -> SGSN       BVC_RESET
  SGSN -> BSS       NS_STATUS, Cause: NS-VC blocked, NS VCI: 0x65
  and there is no BSS->SGSN NS_ALIVE

Ticket: OW#1213
Sponsored-by: On-Waves ehf
---
 src/gb/gprs_ns.c         |  9 ++++-----
 tests/gb/gprs_ns_test.c  |  2 --
 tests/gb/gprs_ns_test.ok | 27 +++++++++++++++++++++++----
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/src/gb/gprs_ns.c b/src/gb/gprs_ns.c
index cf7adaf..65d0494 100644
--- a/src/gb/gprs_ns.c
+++ b/src/gb/gprs_ns.c
@@ -348,8 +348,6 @@ int gprs_ns_tx_reset(struct gprs_nsvc *nsvc, uint8_t cause)
 	LOGP(DNS, LOGL_INFO, "NSEI=%u Tx NS RESET (NSVCI=%u, cause=%s)\n",
 		nsvc->nsei, nsvc->nsvci, gprs_ns_cause_str(cause));
 
-	nsvc->state |= NSE_S_RESET;
-
 	msg->l2h = msgb_put(msg, sizeof(*nsh));
 	nsh = (struct gprs_ns_hdr *) msg->l2h;
 	nsh->pdu_type = NS_PDUT_RESET;
@@ -1250,8 +1248,8 @@ int gprs_ns_process_msg(struct gprs_ns_inst *nsi, struct msgb *msg,
 		 * and should send a NS-RESET to make sure everything recovers
 		 * fine. */
 		if ((*nsvc)->state == NSE_S_BLOCKED)
-			rc = gprs_ns_tx_reset((*nsvc), NS_CAUSE_PDU_INCOMP_PSTATE);
-		else
+			rc = gprs_nsvc_reset((*nsvc), NS_CAUSE_PDU_INCOMP_PSTATE);
+		else if (!((*nsvc)->state & NSE_S_RESET))
 			rc = gprs_ns_tx_alive_ack(*nsvc);
 		break;
 	case NS_PDUT_ALIVE_ACK:
@@ -1503,7 +1501,8 @@ int gprs_nsvc_reset(struct gprs_nsvc *nsvc, uint8_t cause)
 		nsvc->nsei);
 
 	/* Mark NS-VC locally as blocked and dead */
-	nsvc->state = NSE_S_BLOCKED;
+	nsvc->state = NSE_S_BLOCKED | NSE_S_RESET;
+
 	/* Send NS-RESET PDU */
 	rc = gprs_ns_tx_reset(nsvc, cause);
 	if (rc < 0) {
diff --git a/tests/gb/gprs_ns_test.c b/tests/gb/gprs_ns_test.c
index ad8e6d5..2185ff8 100644
--- a/tests/gb/gprs_ns_test.c
+++ b/tests/gb/gprs_ns_test.c
@@ -839,14 +839,12 @@ static void test_sgsn_reset_invalid_state()
 
 	sent_pdu_type = -1;
 	send_ns_alive(nsi, &sgsn_peer);
-	/* Disabled, since it is not yet fixed
 	OSMO_ASSERT(sent_pdu_type == -1);
 	send_ns_reset_ack(nsi, &sgsn_peer, SGSN_NSEI+1, SGSN_NSEI);
 	OSMO_ASSERT(sent_pdu_type == NS_PDUT_ALIVE);
 	send_ns_alive_ack(nsi, &sgsn_peer);
 	OSMO_ASSERT(sent_pdu_type == NS_PDUT_UNBLOCK);
 	send_ns_unblock_ack(nsi, &sgsn_peer);
-	*/
 
 	send_ns_unitdata(nsi, &sgsn_peer, 0x1234, dummy_sdu, sizeof(dummy_sdu));
 
diff --git a/tests/gb/gprs_ns_test.ok b/tests/gb/gprs_ns_test.ok
index 66b1dbb..1e0bf7f 100644
--- a/tests/gb/gprs_ns_test.ok
+++ b/tests/gb/gprs_ns_test.ok
@@ -806,18 +806,37 @@ result (ALIVE) = 12
 PROCESSING ALIVE from 0x05060708:32000
 0a 
 
+result (ALIVE) = 0
+
+PROCESSING RESET_ACK from 0x05060708:32000
+03 01 82 01 01 04 82 01 00 
+
 MESSAGE to SGSN, msg length 1
+0a 
+
+result (RESET_ACK) = 1
+
+PROCESSING ALIVE_ACK from 0x05060708:32000
 0b 
 
-result (ALIVE) = 1
+MESSAGE to SGSN, msg length 1
+06 
+
+result (ALIVE_ACK) = 1
+
+PROCESSING UNBLOCK_ACK from 0x05060708:32000
+07 
+
+==> got signal NS_UNBLOCK, NS-VC 0x0101/5.6.7.8:32000
+result (UNBLOCK_ACK) = 0
 
 PROCESSING UNITDATA from 0x05060708:32000
 00 00 12 34 01 02 03 04 
 
-MESSAGE to SGSN, msg length 8
-08 00 81 03 01 82 01 01 
+CALLBACK, event 0, msg length 4, bvci 0x1234
+01 02 03 04 
 
-result (UNITDATA) = 8
+result (UNITDATA) = 0
 
 Current NS-VCIs:
 
-- 
1.9.1