From 99e06370517f24f59dbfdfc52acd2a67c34a727a Mon Sep 17 00:00:00 2001 From: Jacob Erlbeck Date: Tue, 7 Oct 2014 14:12:30 +0200 Subject: gprs-ns: Fix reset state handling 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 cf7adaf1..65d04947 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 ad8e6d57..2185ff8a 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 66b1dbb7..1e0bf7f8 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: -- cgit v1.2.3