[M] Change in osmo-hnbgw[master]: rua: validate correct RUA ctx state per RUA message type

2024-01-09 Thread laforge
laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email )

Change subject: rua: validate correct RUA ctx state per RUA message type
..

rua: validate correct RUA ctx state per RUA message type

It helps in a pretty complex situation seen in the field.
A third-party MSC releases SCCP in one fell swoop, not waiting for the
Iu Release Complete to come back from RAN as the specs would suggest.

The result is this odd sequence, where late rx of RUA Disconnect
actually causes a new SCCP connection to be established and torn down
again:

RAN HNBGWMSC
 |--active-RUA-ctx|--active-sccp--|
 ||<--IU-Release-Cmd--|
 |<--IU-Release-Cmd---|   |
 ||<--SCCP-RLSD---| (too soon)
 |...<-RUA-Disconnect-|   x (the consequence)
 |x
 |--RUA-Disconnect--->| (IU Release Complete)
 x  
  |-SCCP-CR-->|
  |-IU-Release-Compl->|
  |<--CREF|
  x   x

This patch is a relatively simple practical improvement of above
situation that is logically obvious:

Validate the correct message type for creating a new RUA-to-SCCP
context: RUA Connect.

That means the IU Release Complete is ignored:

RAN HNBGWMSC
 |--active-RUA-ctx|--active-sccp--|
 ||<--IU-Release-Cmd--|
 |<--IU-Release-Cmd---|   |
 ||<--SCCP-RLSD---| (too soon)
 |...<-RUA-Disconnect-|   x (the consequence)
 |x
 |--RUA-Disconnect--->| (IU Release Complete)
 x 
  x

Related: SYS#6602
Change-Id: Ie359fcada98fb19f56015cf462e6d8c039f5fce5
---
M src/osmo-hnbgw/hnbgw_rua.c
1 file changed, 82 insertions(+), 14 deletions(-)

Approvals:
  fixeria: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve
  daniel: Looks good to me, but someone else must approve
  Jenkins Builder: Verified




diff --git a/src/osmo-hnbgw/hnbgw_rua.c b/src/osmo-hnbgw/hnbgw_rua.c
index 073f3ce..84bd817 100644
--- a/src/osmo-hnbgw/hnbgw_rua.c
+++ b/src/osmo-hnbgw/hnbgw_rua.c
@@ -179,18 +179,12 @@
return get_value_string(rua_procedure_code_names, val);
 }

-static struct hnbgw_context_map *find_or_create_context_map(struct hnb_context 
*hnb, uint32_t rua_ctx_id, bool is_ps,
-   struct msgb 
*ranap_msg)
+static struct hnbgw_context_map *create_context_map(struct hnb_context *hnb, 
uint32_t rua_ctx_id, bool is_ps,
+   struct msgb *ranap_msg)
 {
struct hnbgw_context_map *map;
struct hnbgw_cnlink *cnlink;

-   map = context_map_find_by_rua_ctx_id(hnb, rua_ctx_id, is_ps);
-   if (map) {
-   /* Already established SCCP link. Continue to use that. */
-   return map;
-   }
-
/* Establish a new context map. From the RUA Connect, extract a mobile 
identity, if any, and select a CN link
 * based on an NRI found in the mobile identity, if any. */

@@ -255,12 +249,35 @@
memcpy(ranap_msg->l2h, data, len);
}

-   map = find_or_create_context_map(hnb, context_id, is_ps, ranap_msg);
-   if (!map) {
-   LOGHNB(hnb, DRUA, LOGL_ERROR,
-  "Failed to create context map for %s: rx RUA %s with %u 
bytes RANAP data\n",
-  is_ps ? "IuPS" : "IuCS", 
rua_procedure_code_name(rua_procedure), data ? len : 0);
-   return -1;
+   map = context_map_find_by_rua_ctx_id(hnb, context_id, is_ps);
+
+   switch (rua_procedure) {
+   case RUA_ProcedureCode_id_Connect:
+   /* A Connect message can only be the first message for an 
unused RUA context */
+   if (map) {
+   /* Already established this RUA context. But then how 
can it be a Connect message. */
+   LOGHNB(hnb, DRUA, LOGL_ERROR, "rx RUA %s for already 
active RUA context %u\n",
+  rua_procedure_code_name(rua_procedure), 
context_id);
+   return -EINVAL;
+   }
+   /* ok, this RUA context does not exist yet, so create one. */
+   map = create_context_map(hnb, context_id, is_ps, ranap_msg);
+   if (!map) {
+   LOGHNB(hnb, DRUA, LOGL_ERROR,
+  "Failed to create context map for %s: rx RUA %s 
with %u bytes RANAP data\n",
+  is_ps ? "IuPS" : 

[M] Change in osmo-hnbgw[master]: rua: validate correct RUA ctx state per RUA message type

2023-12-16 Thread laforge
Attention is currently required from: neels.

laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email )

Change subject: rua: validate correct RUA ctx state per RUA message type
..


Patch Set 2: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ie359fcada98fb19f56015cf462e6d8c039f5fce5
Gerrit-Change-Number: 35329
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel 
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-Attention: neels 
Gerrit-Comment-Date: Sat, 16 Dec 2023 19:49:01 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in osmo-hnbgw[master]: rua: validate correct RUA ctx state per RUA message type

2023-12-12 Thread daniel
Attention is currently required from: neels.

daniel has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email )

Change subject: rua: validate correct RUA ctx state per RUA message type
..


Patch Set 2: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ie359fcada98fb19f56015cf462e6d8c039f5fce5
Gerrit-Change-Number: 35329
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel 
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Attention: neels 
Gerrit-Comment-Date: Tue, 12 Dec 2023 13:14:07 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in osmo-hnbgw[master]: rua: validate correct RUA ctx state per RUA message type

2023-12-12 Thread pespin
Attention is currently required from: neels.

pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email )

Change subject: rua: validate correct RUA ctx state per RUA message type
..


Patch Set 2: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ie359fcada98fb19f56015cf462e6d8c039f5fce5
Gerrit-Change-Number: 35329
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Attention: neels 
Gerrit-Comment-Date: Tue, 12 Dec 2023 11:00:04 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in osmo-hnbgw[master]: rua: validate correct RUA ctx state per RUA message type

2023-12-11 Thread fixeria
Attention is currently required from: neels.

fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email )

Change subject: rua: validate correct RUA ctx state per RUA message type
..


Patch Set 2: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ie359fcada98fb19f56015cf462e6d8c039f5fce5
Gerrit-Change-Number: 35329
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Attention: neels 
Gerrit-Comment-Date: Tue, 12 Dec 2023 06:46:35 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


[M] Change in osmo-hnbgw[master]: rua: validate correct RUA ctx state per RUA message type

2023-12-11 Thread neels
Hello Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email

to look at the new patch set (#2).


Change subject: rua: validate correct RUA ctx state per RUA message type
..

rua: validate correct RUA ctx state per RUA message type

It helps in a pretty complex situation seen in the field.
A third-party MSC releases SCCP in one fell swoop, not waiting for the
Iu Release Complete to come back from RAN as the specs would suggest.

The result is this odd sequence, where late rx of RUA Disconnect
actually causes a new SCCP connection to be established and torn down
again:

RAN HNBGWMSC
 |--active-RUA-ctx|--active-sccp--|
 ||<--IU-Release-Cmd--|
 |<--IU-Release-Cmd---|   |
 ||<--SCCP-RLSD---| (too soon)
 |...<-RUA-Disconnect-|   x (the consequence)
 |x
 |--RUA-Disconnect--->| (IU Release Complete)
 x  
  |-SCCP-CR-->|
  |-IU-Release-Compl->|
  |<--CREF|
  x   x

This patch is a relatively simple practical improvement of above
situation that is logically obvious:

Validate the correct message type for creating a new RUA-to-SCCP
context: RUA Connect.

That means the IU Release Complete is ignored:

RAN HNBGWMSC
 |--active-RUA-ctx|--active-sccp--|
 ||<--IU-Release-Cmd--|
 |<--IU-Release-Cmd---|   |
 ||<--SCCP-RLSD---| (too soon)
 |...<-RUA-Disconnect-|   x (the consequence)
 |x
 |--RUA-Disconnect--->| (IU Release Complete)
 x 
  x

Related: SYS#6602
Change-Id: Ie359fcada98fb19f56015cf462e6d8c039f5fce5
---
M src/osmo-hnbgw/hnbgw_rua.c
1 file changed, 82 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/29/35329/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ie359fcada98fb19f56015cf462e6d8c039f5fce5
Gerrit-Change-Number: 35329
Gerrit-PatchSet: 2
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-MessageType: newpatchset


[M] Change in osmo-hnbgw[master]: rua: validate correct RUA ctx state per RUA message type

2023-12-11 Thread neels
neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-hnbgw/+/35329?usp=email )


Change subject: rua: validate correct RUA ctx state per RUA message type
..

rua: validate correct RUA ctx state per RUA message type

It helps in a pretty complex situation seen in the field.
A third-party MSC releases SCCP in one fell swoop, not waiting for the
Iu Release Complete to come back from RAN as the specs would suggest.

The result is this odd sequence, where late rx of RUA Disconnect
actually causes a new SCCP connection to be established and torn down
again:

RAN HNBGWMSC
 |--active-RUA-ctx|--active-sccp--|
 ||<--IU-Release-Cmd--|
 ||<--SCCP-RLSD---| (this should have waited)
 |...<-RUA-Disconnect-| (this is the consequence)
 |x
 |--RUA-Disconnect--->| (IU Release Complete)
 |  
 ||-SCCP-CR-->|
 ||-IU-Release-Compl->|
 ||<--CREF|
  x

This patch is a relatively simple practical improvement of above
situation that is logically obvious:

Validate the correct message type for creating a new RUA-to-SCCP
context: RUA Connect.

That means the IU Release Complete is ignored:

RAN HNBGWMSC
 |--active-RUA-ctx|--active-sccp--|
 ||<--IU-Release-Cmd--|
 ||<--SCCP-RLSD---| (this should have waited)
 |...<-RUA-Disconnect-| (this is the consequence)
 |x
 |--RUA-Disconnect--->| (IU Release Complete)

  x

Related: SYS#6602
Change-Id: Ie359fcada98fb19f56015cf462e6d8c039f5fce5
---
M src/osmo-hnbgw/hnbgw_rua.c
1 file changed, 80 insertions(+), 14 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/29/35329/1

diff --git a/src/osmo-hnbgw/hnbgw_rua.c b/src/osmo-hnbgw/hnbgw_rua.c
index 073f3ce..84bd817 100644
--- a/src/osmo-hnbgw/hnbgw_rua.c
+++ b/src/osmo-hnbgw/hnbgw_rua.c
@@ -179,18 +179,12 @@
return get_value_string(rua_procedure_code_names, val);
 }

-static struct hnbgw_context_map *find_or_create_context_map(struct hnb_context 
*hnb, uint32_t rua_ctx_id, bool is_ps,
-   struct msgb 
*ranap_msg)
+static struct hnbgw_context_map *create_context_map(struct hnb_context *hnb, 
uint32_t rua_ctx_id, bool is_ps,
+   struct msgb *ranap_msg)
 {
struct hnbgw_context_map *map;
struct hnbgw_cnlink *cnlink;

-   map = context_map_find_by_rua_ctx_id(hnb, rua_ctx_id, is_ps);
-   if (map) {
-   /* Already established SCCP link. Continue to use that. */
-   return map;
-   }
-
/* Establish a new context map. From the RUA Connect, extract a mobile 
identity, if any, and select a CN link
 * based on an NRI found in the mobile identity, if any. */

@@ -255,12 +249,35 @@
memcpy(ranap_msg->l2h, data, len);
}

-   map = find_or_create_context_map(hnb, context_id, is_ps, ranap_msg);
-   if (!map) {
-   LOGHNB(hnb, DRUA, LOGL_ERROR,
-  "Failed to create context map for %s: rx RUA %s with %u 
bytes RANAP data\n",
-  is_ps ? "IuPS" : "IuCS", 
rua_procedure_code_name(rua_procedure), data ? len : 0);
-   return -1;
+   map = context_map_find_by_rua_ctx_id(hnb, context_id, is_ps);
+
+   switch (rua_procedure) {
+   case RUA_ProcedureCode_id_Connect:
+   /* A Connect message can only be the first message for an 
unused RUA context */
+   if (map) {
+   /* Already established this RUA context. But then how 
can it be a Connect message. */
+   LOGHNB(hnb, DRUA, LOGL_ERROR, "rx RUA %s for already 
active RUA context %u\n",
+  rua_procedure_code_name(rua_procedure), 
context_id);
+   return -EINVAL;
+   }
+   /* ok, this RUA context does not exist yet, so create one. */
+   map = create_context_map(hnb, context_id, is_ps, ranap_msg);
+   if (!map) {
+   LOGHNB(hnb, DRUA, LOGL_ERROR,
+  "Failed to create context map for %s: rx RUA %s 
with %u bytes RANAP data\n",
+  is_ps ? "IuPS" : "IuCS", 
rua_procedure_code_name(rua_procedure), data ? len : 0);
+   return -EINVAL;
+   }
+   break;
+
+   default:
+   /* Any message other than Connect must have a valid