Change in libosmo-sccp[master]: ipa: Move automatic route add/del from ASP to AS level

laforge gerrit-no-reply at lists.osmocom.org
Tue Apr 27 16:44:13 UTC 2021


laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/23894 )

Change subject: ipa: Move automatic route add/del from ASP to AS level
......................................................................

ipa: Move automatic route add/del from ASP to AS level

SS7 routes operate on AS level, not ASP level.  However, the
automatic SS7 route creation/destruction for IPA was implemented
at the ASP level.  This works for single-connection ASs, but
obviously fails in load-share situations:  We attempt to add the
same route several times, and we delete it at the first ASP
disconnect, even while other ASPs still exist.

This patch moves the IPA route creation/deletion from the ASP level
to the AS level.  When the AS becomes active, the route is added;
when it goes to DOWN state, it is removed.

Change-Id: Idb602beae3e9bc19f7bd96355c02ec8dfd9c5d6c
---
M src/xua_as_fsm.c
M src/xua_asp_fsm.c
2 files changed, 64 insertions(+), 55 deletions(-)

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



diff --git a/src/xua_as_fsm.c b/src/xua_as_fsm.c
index 7c791cf..f66ba88 100644
--- a/src/xua_as_fsm.c
+++ b/src/xua_as_fsm.c
@@ -202,6 +202,66 @@
 	} recovery;
 };
 
+/* is the given AS one with a single ASP of IPA type? */
+static bool is_single_ipa_asp(struct osmo_ss7_as *as)
+{
+	unsigned int asp_count = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(as->cfg.asps); i++) {
+		struct osmo_ss7_asp *asp = as->cfg.asps[i];
+		if (!asp)
+			continue;
+		asp_count++;
+		if (asp->cfg.proto != OSMO_SS7_ASP_PROT_IPA)
+			return false;
+	}
+	if (asp_count == 1)
+		return true;
+	return false;
+}
+
+static void ipa_add_route(struct osmo_fsm_inst *fi)
+{
+	struct xua_as_fsm_priv *xafp = (struct xua_as_fsm_priv *) fi->priv;
+	struct osmo_ss7_as *as = xafp->as;
+	struct osmo_ss7_instance *inst = as->inst;
+
+	/* As opposed to M3UA, there is no RKM and we have to implicitly
+	 * automatically add a route once an IPA connection has come up */
+	osmo_ss7_route_create(inst->rtable_system, as->cfg.routing_key.pc, 0xffffff, as->cfg.name);
+}
+
+static void ipa_del_route(struct osmo_fsm_inst *fi)
+{
+	struct xua_as_fsm_priv *xafp = (struct xua_as_fsm_priv *) fi->priv;
+	struct osmo_ss7_as *as = xafp->as;
+	struct osmo_ss7_instance *inst = as->inst;
+	struct osmo_ss7_route *rt;
+
+	/* find the route which we have created if we ever reached ipa_asp_fsm_wait_id_ack2 */
+	rt = osmo_ss7_route_find_dpc_mask(inst->rtable_system, as->cfg.routing_key.pc, 0xffffff);
+	/* no route found, bail out */
+	if (!rt) {
+		LOGPFSML(fi, LOGL_NOTICE, "Attempting to delete route for this IPA AS, but cannot "
+			 "find route for DPC %s. Did you manually delete it?\n",
+			 osmo_ss7_pointcode_print(inst, as->cfg.routing_key.pc));
+		return;
+	}
+
+	/* route points to different AS, bail out */
+	if (rt->dest.as != as) {
+		LOGPFSML(fi, LOGL_NOTICE, "Attempting to delete route for this IPA ASP, but found "
+			 "route for DPC %s points to different AS (%s)\n",
+			 osmo_ss7_pointcode_print(inst, as->cfg.routing_key.pc), rt->dest.as->cfg.name);
+		return;
+	}
+
+	osmo_ss7_route_destroy(rt);
+}
+
+
+
 /* is any other ASP in this AS in state != DOWN? */
 static bool check_any_other_asp_not_down(struct osmo_ss7_as *as, struct osmo_ss7_asp *asp_cmp)
 {
@@ -309,12 +369,16 @@
 		npar.status_info = M3UA_NOTIFY_I_AS_INACT;
 		break;
 	case XUA_AS_S_ACTIVE:
+		if (is_single_ipa_asp(as))
+			ipa_add_route(fi);
 		npar.status_info = M3UA_NOTIFY_I_AS_ACT;
 		break;
 	case XUA_AS_S_PENDING:
 		npar.status_info = M3UA_NOTIFY_I_AS_PEND;
 		break;
 	case XUA_AS_S_DOWN:
+		if (is_single_ipa_asp(as))
+			ipa_del_route(fi);
 		/* RFC4666 sec 4.3.2 AS States:
 		   If we end up here, it means no ASP is ACTIVE or INACTIVE,
 		   meaning no ASP can have already configured the traffic mode
diff --git a/src/xua_asp_fsm.c b/src/xua_asp_fsm.c
index f8683d5..ec85d21 100644
--- a/src/xua_asp_fsm.c
+++ b/src/xua_asp_fsm.c
@@ -939,23 +939,11 @@
 /* Server: We're waiting for an ID ACK */
 static void ipa_asp_fsm_wait_id_ack2(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
-	struct ipa_asp_fsm_priv *iafp = fi->priv;
-	struct osmo_ss7_asp *asp = iafp->asp;
-	struct osmo_ss7_instance *inst = asp->inst;
-	struct osmo_ss7_as *as;
-
-	xua_find_as_for_asp(&as, asp, NULL);
-	OSMO_ASSERT(as);
-
 	switch (event) {
 	case IPA_ASP_E_ID_ACK:
 		/* ACK received, we can go to active state now.  The
 		 * ACTIVE onenter function will inform the AS */
 		osmo_fsm_inst_state_chg(fi, IPA_ASP_S_ACTIVE, 0, 0);
-		/* As opposed to M3UA, there is no RKM and we have to implicitly automatically add
-		 * a route once an IPA connection has come up */
-		osmo_ss7_route_create(inst->rtable_system, as->cfg.routing_key.pc, 0xffffff,
-				      as->cfg.name);
 		break;
 	}
 }
@@ -1011,48 +999,12 @@
 	}
 }
 
-static void ipa_asp_fsm_del_route(struct osmo_fsm_inst *fi)
-{
-	struct ipa_asp_fsm_priv *iafp = fi->priv;
-	struct osmo_ss7_asp *asp = iafp->asp;
-	struct osmo_ss7_instance *inst = asp->inst;
-	struct osmo_ss7_as *as;
-	struct osmo_ss7_route *rt;
-
-	xua_find_as_for_asp(&as, asp, NULL);
-	OSMO_ASSERT(as);
-
-	/* find the route which we have created if we ever reached ipa_asp_fsm_wait_id_ack2 */
-	rt = osmo_ss7_route_find_dpc_mask(inst->rtable_system, as->cfg.routing_key.pc, 0xffffff);
-	/* no route found, bail out */
-	if (!rt) {
-		LOGPFSML(fi, LOGL_NOTICE, "Attempting to delete route for this IPA ASP, but cannot "
-			 "find route for DPC %s. Did you manually delete it?\n",
-			 osmo_ss7_pointcode_print(inst, as->cfg.routing_key.pc));
-		return;
-	}
-
-	/* route points to different AS, bail out */
-	if (rt->dest.as != as) {
-		LOGPFSML(fi, LOGL_NOTICE, "Attempting to delete route for this IPA ASP, but found "
-			 "route for DPC %s points to different AS (%s)\n",
-			 osmo_ss7_pointcode_print(inst, as->cfg.routing_key.pc), rt->dest.as->cfg.name);
-		return;
-	}
-
-	osmo_ss7_route_destroy(rt);
-	/* FIXME: Why don't we also delete this timer if we return early above?
-	 * FIXME: Where is this timer even scheduled? */
-	osmo_timer_del(&iafp->pong_timer);
-}
-
 /* Server + Client: We're actively transmitting user data */
 static void ipa_asp_fsm_active(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
 	switch (event) {
 	case XUA_ASP_E_M_ASP_DOWN_REQ:
 	case XUA_ASP_E_M_ASP_INACTIVE_REQ:
-		ipa_asp_fsm_del_route(fi);
 		osmo_fsm_inst_state_chg(fi, IPA_ASP_S_DOWN, 0, 0);
 		break;
 	}
@@ -1062,7 +1014,6 @@
 {
 	switch (event) {
 	case XUA_ASP_E_M_ASP_DOWN_REQ:
-		ipa_asp_fsm_del_route(fi);
 		osmo_fsm_inst_state_chg(fi, IPA_ASP_S_DOWN, 0, 0);
 		break;
 	}
@@ -1187,11 +1138,6 @@
 	},
 };
 
-static void ipa_asp_fsm_cleanup(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause cause)
-{
-	ipa_asp_fsm_del_route(fi);
-}
-
 struct osmo_fsm ipa_asp_fsm = {
 	.name = "IPA_ASP",
 	.states = ipa_asp_states,
@@ -1204,7 +1150,6 @@
 			       S(XUA_ASP_E_ASPSM_BEAT) |
 			       S(XUA_ASP_E_ASPSM_BEAT_ACK),
 	.allstate_action = ipa_asp_allstate,
-	.cleanup = ipa_asp_fsm_cleanup,
 };
 
 

-- 
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/23894
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: Idb602beae3e9bc19f7bd96355c02ec8dfd9c5d6c
Gerrit-Change-Number: 23894
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210427/7206b336/attachment.htm>


More information about the gerrit-log mailing list