[PATCH 7/7] gb: Fix gprs_ns_rx_reset to not create NS-VC duplicates

Under special circumstances receiving a NS-RESET leads to duplicated NS-VC entries. This patch changes gprs_ns_rx_reset() to check for this case and to eventually delete the old entry. This happens when the source port of a NS-VC changes to a new one that has already been used by another NS-VC. --- include/osmocom/gprs/gprs_ns.h | 3 +++ src/gb/gprs_ns.c | 45 +++++++++++++++++++++++++++++++--------- tests/gb/gprs_ns_test.ok | 2 -- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/include/osmocom/gprs/gprs_ns.h b/include/osmocom/gprs/gprs_ns.h index c709312..562894a 100644 --- a/include/osmocom/gprs/gprs_ns.h +++ b/include/osmocom/gprs/gprs_ns.h @@ -117,8 +117,11 @@ struct gprs_nsvc { enum nsvc_timer_mode timer_mode; int alive_retries; + int dup_deletion_ctr; + unsigned int remote_end_is_sgsn:1; unsigned int persistent:1; + unsigned int nsvci_is_valid:1; struct rate_ctr_group *ctrg; diff --git a/src/gb/gprs_ns.c b/src/gb/gprs_ns.c index 4414bd5..8bd1df2 100644 --- a/src/gb/gprs_ns.c +++ b/src/gb/gprs_ns.c @@ -83,6 +83,8 @@ #include "common_vty.h" +const char *gprs_ns_format_peer(struct gprs_nsvc *nsvc); + static const struct tlv_definition ns_att_tlvdef = { .def = { [NS_IE_CAUSE] = { TLV_TYPE_TvLV, 0 }, @@ -651,8 +653,8 @@ static int gprs_ns_rx_reset(struct gprs_nsvc *nsvc, struct msgb *msg) { struct gprs_ns_hdr *nsh = (struct gprs_ns_hdr *) msg->l2h; struct tlv_parsed tp; - uint8_t *cause; - uint16_t *nsvci, *nsei; + uint8_t cause; + uint16_t nsvci, nsei; int rc; rc = tlv_parse(&tp, &ns_att_tlvdef, nsh->data, @@ -671,22 +673,45 @@ static int gprs_ns_rx_reset(struct gprs_nsvc *nsvc, struct msgb *msg) return -EINVAL; } - cause = (uint8_t *) TLVP_VAL(&tp, NS_IE_CAUSE); - nsvci = (uint16_t *) TLVP_VAL(&tp, NS_IE_VCI); - nsei = (uint16_t *) TLVP_VAL(&tp, NS_IE_NSEI); + cause = *(uint8_t *) TLVP_VAL(&tp, NS_IE_CAUSE); + nsvci = ntohs(*(uint16_t *) TLVP_VAL(&tp, NS_IE_VCI)); + nsei = ntohs(*(uint16_t *) TLVP_VAL(&tp, NS_IE_NSEI)); + + LOGP(DNS, LOGL_INFO, "NSVCI=%u%s Rx NS RESET (NSEI=%u, NSVCI=%u, cause=%s)\n", + nsvc->nsvci, nsvc->nsvci_is_valid ? "" : "(invalid)", + nsei, nsvci, gprs_ns_cause_str(cause)); - LOGP(DNS, LOGL_INFO, "NSEI=%u Rx NS RESET (NSVCI=%u, cause=%s)\n", - nsvc->nsvci, nsvc->nsei, gprs_ns_cause_str(*cause)); + if (nsvc->nsvci_is_valid && nsvc->nsvci != nsvci) { + /* NS-VCI has changed */ + struct gprs_nsvc *other_nsvc; + other_nsvc = gprs_nsvc_by_nsvci(nsvc->nsi, nsvci); + + if (other_nsvc) { + /* The NS-VCI is already used by this NS-VC */ + + char *old_peer = strdup(gprs_ns_format_peer(other_nsvc)); + LOGP(DNS, LOGL_INFO, + "NS-VC changed link (NSVCI=%u) from %s to %s\n", + nsvci, old_peer, gprs_ns_format_peer(nsvc)); + free(old_peer); + + nsvc->dup_deletion_ctr = 1 + other_nsvc->dup_deletion_ctr; + + /* Delete the 'old' object */ + gprs_nsvc_delete(other_nsvc); + } + } /* Mark NS-VC as blocked and alive */ nsvc->state = NSE_S_BLOCKED | NSE_S_ALIVE; - nsvc->nsei = ntohs(*nsei); - nsvc->nsvci = ntohs(*nsvci); + nsvc->nsei = nsei; + nsvc->nsvci = nsvci; + nsvc->nsvci_is_valid = 1; /* inform interested parties about the fact that this NSVC * has received RESET */ - ns_osmo_signal_dispatch(nsvc, S_NS_RESET, *cause); + ns_osmo_signal_dispatch(nsvc, S_NS_RESET, cause); rc = gprs_ns_tx_reset_ack(nsvc); diff --git a/tests/gb/gprs_ns_test.ok b/tests/gb/gprs_ns_test.ok index 578b905..7416f15 100644 --- a/tests/gb/gprs_ns_test.ok +++ b/tests/gb/gprs_ns_test.ok @@ -102,7 +102,6 @@ result (RESET) = 9 Current NS-VCIs: VCI 0x1122, NSEI 0x1122, peer 0x01020304:3333 - VCI 0x1122, NSEI 0x3344, peer 0x01020304:4444 --- Peer port 4444, RESET, NSEI is changed back --- @@ -118,7 +117,6 @@ RESPONSE, msg length 1 result (RESET) = 9 Current NS-VCIs: - VCI 0x1122, NSEI 0x1122, peer 0x01020304:3333 VCI 0x1122, NSEI 0x1122, peer 0x01020304:4444 ===== NS protocol test END -- 1.7.9.5