[M] Change in libosmo-sccp[master]: ipa: Use ASP name as ipa_unit_name on dynamic ASPs
pespin has submitted this change. (
https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email )
Change subject: ipa: Use ASP name as ipa_unit_name on dynamic ASPs
..
ipa: Use ASP name as ipa_unit_name on dynamic ASPs
A recent patch fixed a long problem where the ASP name (instead of
expected AS name) was used as ipa_unit_name in IPA based ASPs.
For server side it doesn't matter much, sense anyway the ipa_unit_name
is actually restored on the struct with what's received in IPA GET_RESP
message (see ipa_asp_fsm_wait_id_resp()). So the fix was actually for
the client side in the scenario where a non-dynamic ASP with an assigned
AS was configured in the VTY.
However, dynamic ASPs usually have no assigned AS (because in general it
is really not created/configured, as the ASP is created on the flight).
As a result, the recent patch (see "Fixes" below), broke dynamic ASPs
case because from then one ipa_asp_fsm_start() would fail and terminate
the FSM because ipa_find_as_for_asp() was returning NULL.
So improve the recent patch by applying the previous logic for dynamic
ASPs:
* On the server side, it really doesn't matter since as mentioned, the
field will be repopulated later on, but allows the code to avoid
terminating the FSM and hence be brought up and be ready to receive
clients.
* On the client case, this is how dynamic IPA ASPs were ment to be used
when they were introduced anyway (use ASP as ipa_unit_id, meaning
"AS name" == "ASP name").
Furthermore, on the client side, the non-dynamic IPA ASPs need their
bring up be delayed until assigned to an AS, because the AS name is sent
in ipa_unit_name field in Tx IPA ID RESP.
This usually happens at a later point than ASP (FSM) creation, because
first the ASP object is created (through VTY or API) and then assigned
to an AS through osmo_ss7_as_add_asp() usually from a later "asp" vty
command in the "as" node.
Fixes: 65741dca056e3a16973ad156dd4c09760a6a945b
Change-Id: I0a741449450c998253b1e44a76a3b7fc224e0903
Related: SYS#5914
---
M src/osmo_ss7_as.c
M src/xua_asp_fsm.c
M src/xua_asp_fsm.h
3 files changed, 86 insertions(+), 8 deletions(-)
Approvals:
laforge: Looks good to me, but someone else must approve
Jenkins Builder: Verified
fixeria: Looks good to me, approved
diff --git a/src/osmo_ss7_as.c b/src/osmo_ss7_as.c
index 34baf52..9d78897 100644
--- a/src/osmo_ss7_as.c
+++ b/src/osmo_ss7_as.c
@@ -35,6 +35,7 @@
#include "ss7_internal.h"
#include "xua_as_fsm.h"
+#include "xua_asp_fsm.h"
/***
* SS7 Application Server
@@ -114,6 +115,7 @@
for (i = 0; i < ARRAY_SIZE(as->cfg.asps); i++) {
if (!as->cfg.asps[i]) {
as->cfg.asps[i] = asp;
+ osmo_fsm_inst_dispatch(asp->fi, XUA_ASP_E_AS_ASSIGNED,
as);
return 0;
}
}
diff --git a/src/xua_asp_fsm.c b/src/xua_asp_fsm.c
index cc9a13a..701e081 100644
--- a/src/xua_asp_fsm.c
+++ b/src/xua_asp_fsm.c
@@ -65,6 +65,8 @@
{ XUA_ASP_E_ASPSM_BEAT, "ASPSM_BEAT" },
{ XUA_ASP_E_ASPSM_BEAT_ACK, "ASPSM_BEAT_ACK" },
+ { XUA_ASP_E_AS_ASSIGNED,"AS_ASSIGNED" },
+
{ IPA_ASP_E_ID_RESP,"IPA_CCM_ID_RESP" },
{ IPA_ASP_E_ID_GET, "IPA_CCM_ID_GET" },
{ IPA_ASP_E_ID_ACK, "IPA_CCM_ID_ACK" },
@@ -687,6 +689,9 @@
case XUA_ASP_E_ASPSM_BEAT_ACK:
/* FIXME: stop timer, if any */
break;
+ case XUA_ASP_E_AS_ASSIGNED:
+ /* Ignore, only used in IPA asps so far. */
+ break;
default:
break;
}
@@ -1058,6 +1063,7 @@
static void ipa_asp_allstate(struct osmo_fsm_inst *fi, uint32_t event, void
*data)
{
struct ipa_asp_fsm_priv *iafp = fi->priv;
+ struct osmo_ss7_as *as;
int fd;
switch (event) {
@@ -1077,6 +1083,15 @@
/* stop timer, if any */
osmo_timer_del(>pong_timer);
break;
+ case XUA_ASP_E_AS_ASSIGNED:
+ as = data;
+ osmo_talloc_replace_string(iafp->ipa_unit,
>ipa_unit->unit_name, as->cfg.name);
+ /* Now that the AS is known, start the client side: */
+ if (iafp->role == OSMO_SS7_ASP_ROLE_ASP && fi->state ==
IPA_ASP_S_DOWN) {
+ LOGPFSML(fi, LOGL_NOTICE, "Bringing up ASP now once it
has been assigned to an AS\n");
+ osmo_fsm_inst_dispatch(fi, XUA_ASP_E_M_ASP_UP_REQ,
NULL);
+ }
+ break;
default:
break;
}
@@ -1175,7 +1190,8 @@
.allstate_event_mask = S(XUA_ASP_E_SCTP_COMM_DOWN_IND) |
S(XUA_ASP_E_SCTP_RESTART_IND) |
S(XUA_ASP_E_ASPSM_BEAT) |
-
[M] Change in libosmo-sccp[master]: ipa: Use ASP name as ipa_unit_name on dynamic ASPs
Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email ) Change subject: ipa: Use ASP name as ipa_unit_name on dynamic ASPs .. Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: I0a741449450c998253b1e44a76a3b7fc224e0903 Gerrit-Change-Number: 35348 Gerrit-PatchSet: 2 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 13 Dec 2023 02:36:10 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmo-sccp[master]: ipa: Use ASP name as ipa_unit_name on dynamic ASPs
Attention is currently required from: pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email ) Change subject: ipa: Use ASP name as ipa_unit_name on dynamic ASPs .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: I0a741449450c998253b1e44a76a3b7fc224e0903 Gerrit-Change-Number: 35348 Gerrit-PatchSet: 2 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 12 Dec 2023 23:32:15 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmo-sccp[master]: ipa: Use ASP name as ipa_unit_name on dynamic ASPs
Attention is currently required from: laforge, pespin. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review-1 by pespin, Verified+1 by Jenkins Builder Change subject: ipa: Use ASP name as ipa_unit_name on dynamic ASPs .. ipa: Use ASP name as ipa_unit_name on dynamic ASPs A recent patch fixed a long problem where the ASP name (instead of expected AS name) was used as ipa_unit_name in IPA based ASPs. For server side it doesn't matter much, sense anyway the ipa_unit_name is actually restored on the struct with what's received in IPA GET_RESP message (see ipa_asp_fsm_wait_id_resp()). So the fix was actually for the client side in the scenario where a non-dynamic ASP with an assigned AS was configured in the VTY. However, dynamic ASPs usually have no assigned AS (because in general it is really not created/configured, as the ASP is created on the flight). As a result, the recent patch (see "Fixes" below), broke dynamic ASPs case because from then one ipa_asp_fsm_start() would fail and terminate the FSM because ipa_find_as_for_asp() was returning NULL. So improve the recent patch by applying the previous logic for dynamic ASPs: * On the server side, it really doesn't matter since as mentioned, the field will be repopulated later on, but allows the code to avoid terminating the FSM and hence be brought up and be ready to receive clients. * On the client case, this is how dynamic IPA ASPs were ment to be used when they were introduced anyway (use ASP as ipa_unit_id, meaning "AS name" == "ASP name"). Furthermore, on the client side, the non-dynamic IPA ASPs need their bring up be delayed until assigned to an AS, because the AS name is sent in ipa_unit_name field in Tx IPA ID RESP. This usually happens at a later point than ASP (FSM) creation, because first the ASP object is created (through VTY or API) and then assigned to an AS through osmo_ss7_as_add_asp() usually from a later "asp" vty command in the "as" node. Fixes: 65741dca056e3a16973ad156dd4c09760a6a945b Change-Id: I0a741449450c998253b1e44a76a3b7fc224e0903 Related: SYS#5914 --- M src/osmo_ss7_as.c M src/xua_asp_fsm.c M src/xua_asp_fsm.h 3 files changed, 86 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/48/35348/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: I0a741449450c998253b1e44a76a3b7fc224e0903 Gerrit-Change-Number: 35348 Gerrit-PatchSet: 2 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[M] Change in libosmo-sccp[master]: ipa: Use ASP name as ipa_unit_name on dynamic ASPs
Attention is currently required from: laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email ) Change subject: ipa: Use ASP name as ipa_unit_name on dynamic ASPs .. Patch Set 1: Code-Review-1 (1 comment) Patchset: PS1: This is still not fixing the entire problem, because the ASP FSM is started during ASP creation in VTY, that means before the AS is created and VTY and hence gets assigned to ASP... We should ideally delay ASP UP until the whole config is read. -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: I0a741449450c998253b1e44a76a3b7fc224e0903 Gerrit-Change-Number: 35348 Gerrit-PatchSet: 1 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: laforge Gerrit-Comment-Date: Tue, 12 Dec 2023 17:21:34 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmo-sccp[master]: ipa: Use ASP name as ipa_unit_name on dynamic ASPs
pespin has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email )
Change subject: ipa: Use ASP name as ipa_unit_name on dynamic ASPs
..
ipa: Use ASP name as ipa_unit_name on dynamic ASPs
A recent patch fixed a long problem where the ASP name (instead of
expected AS name) was used as ipa_unit_name in IPA based ASPs.
For server side it doesn't matter much, sense anyway the ipa_unit_name
is actually restored on the struct with what's received in IPA GET_RESP
message (see ipa_asp_fsm_wait_id_resp()). So the fix was actually for
the client side in the scenario where a non-dynamic ASP with an assigned
AS was configured in the VTY.
However, dynamic ASPs usually have no assigned AS (because in general it
is really not created/configured, as the ASP is created on the flight).
As a result, the recent patch (see "Fixes" below), broke dynamic ASPs
case because from then one ipa_asp_fsm_start() would fail and terminate
the FSM because ipa_find_as_for_asp() was returning NULL.
So improve the recent patch by applying the previous logic for dynamic
ASPs:
* On the server side, it really doesn't matter since as mentioned, the
field will be repopulated later on, but allows the code to avoid
terminating the FSM.
* On the client case, this is how dynamic IPA ASPs were mento be used
when they were introduced anyway.
Fixes: 65741dca056e3a16973ad156dd4c09760a6a945b
Change-Id: I0a741449450c998253b1e44a76a3b7fc224e0903
Related: SYS#5914
---
M src/xua_asp_fsm.c
1 file changed, 48 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/48/35348/1
diff --git a/src/xua_asp_fsm.c b/src/xua_asp_fsm.c
index cc9a13a..6d81ce2 100644
--- a/src/xua_asp_fsm.c
+++ b/src/xua_asp_fsm.c
@@ -1191,24 +1191,33 @@
struct osmo_fsm_inst *fi;
struct ipa_asp_fsm_priv *iafp;
struct osmo_ss7_as *as = ipa_find_as_for_asp(asp);
+ const char *unit_name;
/* allocate as child of AS? */
fi = osmo_fsm_inst_alloc(_asp_fsm, asp, NULL, log_level,
asp->cfg.name);
- if (!as) {
- osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
- return NULL;
- }
-
iafp = talloc_zero(fi, struct ipa_asp_fsm_priv);
if (!iafp) {
osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
return NULL;
}
+
+ if (as) {
+ unit_name = as->cfg.name;
+ } else if (asp->dyn_allocated) {
+ LOGPFSML(fi, LOGL_INFO, "Dynamic ASP is not assigned to any AS,
"
+"using ASP name instead of AS name as
ipa_unit_name\n");
+ unit_name = asp->cfg.name;
+ } else {
+ LOGPFSML(fi, LOGL_ERROR, "ASP is not assigned to any AS, fix
your config!\n");
+ osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
+ return NULL;
+ }
+
iafp->role = role;
iafp->asp = asp;
iafp->ipa_unit = talloc_zero(iafp, struct ipaccess_unit);
- iafp->ipa_unit->unit_name = talloc_strdup(iafp->ipa_unit, as->cfg.name);
+ iafp->ipa_unit->unit_name = talloc_strdup(iafp->ipa_unit, unit_name);
iafp->pong_timer.cb = ipa_pong_timer_cb;
iafp->pong_timer.data = fi;
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/35348?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: I0a741449450c998253b1e44a76a3b7fc224e0903
Gerrit-Change-Number: 35348
Gerrit-PatchSet: 1
Gerrit-Owner: pespin
Gerrit-MessageType: newchange
