pespin submitted this change.
mtp: Improve mtp_sap/ss7_user APIs
* The ss7_user API was so far never used externally, and internally only
by SCCP and a unit test. It has several bad practices like first
passing a inst parent ptr to allocate it, and then require a inst
pointer again to operate on it (eg. register/unregister, send a
primitive, etc.). This only makes the API more complex than needed and
creates potential problems where a isnt differnet than the one ss7_user
was allocated on is passed. Instead, the ss7_user is pinned to its
parent inst during alloction, and all MTP-SAP always happens uniquely
through the ss7_user. This follows much more closely the already
sanitized and much more used sccp_sap/sccp_user APIs.
* Move more primitive/SAP related code to mtp_sap.c, to make it easier to
spot where the inter-layer communication happens. This way we also have
the same disposition as with sccp_sap/sccp_user.
* Clean up some primitive related APIs to unify naming with other usual
SAP/primitive osmocom code.
Change-Id: If320db2bcec7ccbbc2cdac0cbf018fd8be7bde22
---
M TODO-RELEASE
M include/osmocom/sigtran/osmo_ss7.h
M src/mtp_sap.c
M src/sccp_instance.c
M src/sccp_scrc.c
M src/ss7_hmrt.c
M src/ss7_instance.h
M src/ss7_user.c
M src/ss7_user.h
M src/xua_internal.h
M tests/ss7/ss7_test.c
11 files changed, 133 insertions(+), 115 deletions(-)
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 2ecfd57..dd3b333 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -10,3 +10,4 @@
libosmo-netif >1.6.0 stream OSMO_STREAM_{CLI,SRV}_TCP_SOCKOPT_KEEP*
libosmo-sigtran add enum mtp_network_indicator, mtp_network_indicator_vals
libosmo-sigtran add osmo_ss7_instance_get_network_indicator()
+libosmo-sigtran add osmo_ss7_user_mtp_sap_prim_down()
diff --git a/include/osmocom/sigtran/osmo_ss7.h b/include/osmocom/sigtran/osmo_ss7.h
index 5a51ba3..b560caf 100644
--- a/include/osmocom/sigtran/osmo_ss7.h
+++ b/include/osmocom/sigtran/osmo_ss7.h
@@ -92,15 +92,13 @@
void osmo_ss7_user_set_priv(struct osmo_ss7_user *user, void *priv);
void *osmo_ss7_user_get_priv(const struct osmo_ss7_user *user);
-int osmo_ss7_user_register(struct osmo_ss7_instance *inst, uint8_t service_ind,
- struct osmo_ss7_user *user);
-int osmo_ss7_user_unregister(struct osmo_ss7_instance *inst, uint8_t service_ind,
- struct osmo_ss7_user *user);
+int osmo_ss7_user_register(struct osmo_ss7_user *user, uint8_t service_ind);
+int osmo_ss7_user_unregister(struct osmo_ss7_user *user, uint8_t service_ind);
-/* SS7 User wants to issue MTP-TRANSFER.req */
-int osmo_ss7_user_mtp_xfer_req(struct osmo_ss7_instance *inst,
- struct osmo_mtp_prim *omp);
+/* MTP User wants to submit a primitive down to MTP SAP */
+int osmo_ss7_user_mtp_sap_prim_down(struct osmo_ss7_user *user,
+ struct osmo_mtp_prim *omp);
/***********************************************************************
* SCCP Instance
diff --git a/src/mtp_sap.c b/src/mtp_sap.c
index b6b6eeb..cef0497 100644
--- a/src/mtp_sap.c
+++ b/src/mtp_sap.c
@@ -27,6 +27,10 @@
#include <osmocom/sigtran/osmo_ss7.h>
#include <osmocom/sigtran/mtp_sap.h>
+#include "ss7_user.h"
+#include "ss7_internal.h"
+#include "xua_internal.h"
+
const struct value_string osmo_mtp_prim_type_names[] = {
{ OSMO_MTP_PRIM_TRANSFER, "MTP-TRANSFER" },
{ OSMO_MTP_PRIM_PAUSE, "MTP-PAUSE" },
@@ -57,3 +61,46 @@
mtp_prim_hdr_name_buf(prim_name_buf, sizeof(prim_name_buf), oph);
return prim_name_buf;
}
+
+/*! \brief Send a MTP SAP Primitive up to the MTP User
+ * \param[in] osu MTP User to whom to send the primitive
+ * \param[in] prim Primitive to send to the user
+ * \returns return value of the MTP User's prim_cb() function
+ *
+ * Ownership of prim->oph->msg is passed to the user of the registered callback
+ */
+int ss7_user_mtp_sap_prim_up(const struct osmo_ss7_user *osu, struct osmo_mtp_prim *omp)
+{
+ LOGPSS7U(osu, LOGL_DEBUG, "Delivering to MTP User: %s\n",
+ osmo_mtp_prim_name(&omp->oph));
+ return osu->prim_cb(&omp->oph, (void *) osu->priv);
+}
+
+/* MTP-User requests to send a MTP-TRANSFER.req via the stack
+ * \param[in] osu MTP User sending us the primitive
+ * \param[in] oph Osmocom primitive sent by the user
+ *
+ * The oph->msg ownership is transferred to this function, which will free it.
+ */
+int osmo_ss7_user_mtp_sap_prim_down(struct osmo_ss7_user *osu, struct osmo_mtp_prim *omp)
+{
+ struct msgb *msg = omp->oph.msg;
+ int rc;
+
+ OSMO_ASSERT(omp->oph.sap == MTP_SAP_USER);
+
+ switch (OSMO_PRIM_HDR(&omp->oph)) {
+ case OSMO_PRIM(OSMO_MTP_PRIM_TRANSFER, PRIM_OP_REQUEST):
+ rc = hmrt_mtp_xfer_request_l4_to_l3(osu->inst, &omp->u.transfer,
+ msgb_l2(msg), msgb_l2len(msg));
+ break;
+ default:
+ LOGPSS7U(osu, LOGL_ERROR, "Ignoring unknown primitive %u:%u\n",
+ omp->oph.primitive, omp->oph.operation);
+ rc = -1;
+ break;
+ }
+
+ msgb_free(msg);
+ return rc;
+}
diff --git a/src/sccp_instance.c b/src/sccp_instance.c
index f5b056f..0d67e4d 100644
--- a/src/sccp_instance.c
+++ b/src/sccp_instance.c
@@ -326,7 +326,7 @@
inst->ss7_user = osmo_ss7_user_create(ss7, "SCCP");
osmo_ss7_user_set_prim_cb(inst->ss7_user, mtp_user_prim_cb);
osmo_ss7_user_set_priv(inst->ss7_user, inst);
- osmo_ss7_user_register(ss7, MTP_SI_SCCP, inst->ss7_user);
+ osmo_ss7_user_register(inst->ss7_user, MTP_SI_SCCP);
llist_add_tail(&inst->list, &sccp_instances);
@@ -341,7 +341,7 @@
return;
inst->ss7->sccp = NULL;
- osmo_ss7_user_unregister(inst->ss7, MTP_SI_SCCP, inst->ss7_user);
+ osmo_ss7_user_unregister(inst->ss7_user, MTP_SI_SCCP);
osmo_ss7_user_destroy(inst->ss7_user);
inst->ss7_user = NULL;
diff --git a/src/sccp_scrc.c b/src/sccp_scrc.c
index ad474aa..9cf71a3 100644
--- a/src/sccp_scrc.c
+++ b/src/sccp_scrc.c
@@ -134,7 +134,7 @@
param->sio = MTP_SIO(MTP_SI_SCCP, s7i->cfg.network_indicator);
/* 3) send via MTP-SAP (osmo_ss7_instance) */
- return osmo_ss7_user_mtp_xfer_req(s7i, omp);
+ return osmo_ss7_user_mtp_sap_prim_down(inst->ss7_user, omp);
}
/* Generate MTP-TRANSFER.req from xUA message */
diff --git a/src/ss7_hmrt.c b/src/ss7_hmrt.c
index bd5d480..b915481 100644
--- a/src/ss7_hmrt.c
+++ b/src/ss7_hmrt.c
@@ -44,7 +44,7 @@
#include "ss7_user.h"
/* convert from M3UA message to MTP-TRANSFER.ind osmo_mtp_prim */
-struct osmo_mtp_prim *m3ua_to_xfer_ind(struct xua_msg *xua)
+static struct osmo_mtp_prim *m3ua_to_xfer_ind(struct xua_msg *xua)
{
struct osmo_mtp_prim *prim;
struct osmo_mtp_transfer_param *param;
@@ -74,23 +74,10 @@
return prim;
}
-/* convert from MTP-TRANSFER.req to osmo_mtp_prim */
-static struct xua_msg *mtp_prim_to_m3ua(struct osmo_mtp_prim *prim)
-{
- struct msgb *msg = prim->oph.msg;
- struct osmo_mtp_transfer_param *param = &prim->u.transfer;
- struct m3ua_data_hdr data_hdr;
-
- mtp_xfer_param_to_m3ua_dh(&data_hdr, param);
-
- return m3ua_xfer_from_data(&data_hdr, msgb_l2(msg), msgb_l2len(msg));
-}
-
/* delivery given XUA message to given SS7 user
* Ownership of xua_msg passed is transferred to this function.
*/
-static int deliver_to_mtp_user(const struct osmo_ss7_user *osu,
- struct xua_msg *xua)
+static int deliver_to_mtp_user(const struct osmo_ss7_user *osu, struct xua_msg *xua)
{
struct osmo_mtp_prim *prim;
int rc;
@@ -103,7 +90,7 @@
}
prim->u.transfer = xua->mtp;
- rc = osu->prim_cb(&prim->oph, (void *) osu->priv);
+ rc = ss7_user_mtp_sap_prim_up(osu, prim);
ret_free:
xua_msg_free(xua);
@@ -145,17 +132,18 @@
return -1;
}
- /* Check for local SSN registered for this DPC/SSN */
- osu = inst->user[service_ind];
- if (osu) {
- return deliver_to_mtp_user(osu, xua);
- } else {
+ /* "User Part Available?" */
+ osu = ss7_user_find(inst, service_ind);
+ if (!osu) {
+ /* "Discard Message" */
LOGSS7(inst, LOGL_NOTICE, "No MTP-User for SI %u\n", service_ind);
- /* Discard Message */
/* FIXME: User Part Unavailable HMDT -> HMRT */
xua_msg_free(xua);
return -1;
}
+
+ /* "MTP Transfer indication HMDT→L4" */
+ return deliver_to_mtp_user(osu, xua);
}
/* HMDC->HMRT Msg For Routing; Figure 26/Q.704 */
@@ -251,35 +239,22 @@
}
}
-/* MTP-User requests to send a MTP-TRANSFER.req via the stack */
-int osmo_ss7_user_mtp_xfer_req(struct osmo_ss7_instance *inst,
- struct osmo_mtp_prim *omp)
+/* Figure 26/Q.704 (sheet 1 of 5) "MTP Transfer request L4→L3" */
+int hmrt_mtp_xfer_request_l4_to_l3(struct osmo_ss7_instance *inst, const struct osmo_mtp_transfer_param *param, uint8_t *user_data, size_t user_data_len)
{
+ struct m3ua_data_hdr data_hdr;
struct xua_msg *xua;
- int rc;
- OSMO_ASSERT(omp->oph.sap == MTP_SAP_USER);
+ /* convert from osmo_mtp_prim MTP-TRANSFER.req to xua_msg */
+ mtp_xfer_param_to_m3ua_dh(&data_hdr, param);
+ xua = m3ua_xfer_from_data(&data_hdr, user_data, user_data_len);
+ OSMO_ASSERT(xua);
+ xua->mtp = *param;
- switch (OSMO_PRIM_HDR(&omp->oph)) {
- case OSMO_PRIM(OSMO_MTP_PRIM_TRANSFER, PRIM_OP_REQUEST):
- xua = mtp_prim_to_m3ua(omp);
- xua->mtp = omp->u.transfer;
- /* normally we would call hmrt_message_for_routing()
- * here, if we were to follow the state diagrams of the
- * ITU-T Q.70x specifications. However, what if a local
- * MTP user sends a MTP-TRANSFER.req to a local SSN?
- * This wouldn't work as per the spec, but I believe it
- * is a very useful feature (aka "loopback device" in
- * IPv4). So we call m3ua_hmdc_rx_from_l2() just like
- * the MTP-TRANSFER had been received from L2. */
- rc = m3ua_hmdc_rx_from_l2(inst, xua);
- break;
- default:
- LOGSS7(inst, LOGL_ERROR, "Ignoring unknown primitive %u:%u\n",
- omp->oph.primitive, omp->oph.operation);
- rc = -1;
- }
-
- msgb_free(omp->oph.msg);
- return rc;
+ /* normally we would call hmrt_message_for_routing() here, if we were to follow the state
+ * diagrams of the ITU-T Q.70x specifications. However, what if a local MTP user sends a
+ * MTP-TRANSFER.req to a local SSN? This wouldn't work as per the spec, but I believe it
+ * is a very useful feature (aka "loopback device" in IPv4). So we call
+ * m3ua_hmdc_rx_from_l2() just like the MTP-TRANSFER had been received from L2. */
+ return m3ua_hmdc_rx_from_l2(inst, xua);
}
diff --git a/src/ss7_instance.h b/src/ss7_instance.h
index d4d0b3d..3045403 100644
--- a/src/ss7_instance.h
+++ b/src/ss7_instance.h
@@ -43,7 +43,7 @@
struct llist_head xua_servers;
/* array for faster lookup of user (indexed by service
* indicator) */
- const struct osmo_ss7_user *user[16];
+ struct osmo_ss7_user *user[16];
struct osmo_ss7_route_table *rtable_system;
diff --git a/src/ss7_user.c b/src/ss7_user.c
index e7d0483..0ec42ff 100644
--- a/src/ss7_user.c
+++ b/src/ss7_user.c
@@ -26,9 +26,11 @@
#include <osmocom/core/talloc.h>
#include <osmocom/core/prim.h>
#include <osmocom/sigtran/osmo_ss7.h>
+#include <osmocom/sigtran/mtp_sap.h>
#include "ss7_user.h"
#include "ss7_internal.h"
+#include "xua_internal.h"
/***********************************************************************
* MTP Users (Users of MTP, such as SCCP or ISUP)
@@ -48,9 +50,17 @@
void osmo_ss7_user_destroy(struct osmo_ss7_user *user)
{
+ ss7_user_unregister_all(user);
talloc_free(user);
}
+struct osmo_ss7_user *ss7_user_find(struct osmo_ss7_instance *inst, uint8_t service_indicator)
+{
+ if (service_indicator >= ARRAY_SIZE(inst->user))
+ return NULL;
+ return inst->user[service_indicator];
+}
+
struct osmo_ss7_instance *osmo_ss7_user_get_instance(const struct osmo_ss7_user *user)
{
return user->inst;
@@ -72,75 +82,57 @@
}
/*! \brief Register a MTP user for a given service indicator
- * \param[in] inst SS7 instance for which we register the user
+ * \param[in] user SS7 user to register (including primitive call-back)
* \param[in] service_ind Service (ISUP, SCCP, ...)
- * \param[in] user SS7 user (including primitive call-back)
* \returns 0 on success; negative on error */
-int osmo_ss7_user_register(struct osmo_ss7_instance *inst, uint8_t service_ind,
- struct osmo_ss7_user *user)
+int osmo_ss7_user_register(struct osmo_ss7_user *user, uint8_t service_ind)
{
+ struct osmo_ss7_instance *inst = user->inst;
+
if (service_ind >= ARRAY_SIZE(inst->user))
return -EINVAL;
if (inst->user[service_ind])
return -EBUSY;
- DEBUGP(DLSS7, "registering user=%s for SI %u with priv %p\n",
- user->name, service_ind, user->priv);
+ LOGPSS7U(user, LOGL_DEBUG, "registering for SI %u with priv %p\n",
+ service_ind, user->priv);
- user->inst = inst;
inst->user[service_ind] = user;
return 0;
}
/*! \brief Unregister a MTP user for a given service indicator
- * \param[in] inst SS7 instance for which we register the user
+ * \param[in] user SS7 user to unregister.
* \param[in] service_ind Service (ISUP, SCCP, ...)
- * \param[in] user (optional) SS7 user. If present, we will not
- * unregister other users
* \returns 0 on success; negative on error */
-int osmo_ss7_user_unregister(struct osmo_ss7_instance *inst, uint8_t service_ind,
- struct osmo_ss7_user *user)
+int osmo_ss7_user_unregister(struct osmo_ss7_user *user, uint8_t service_ind)
{
+ struct osmo_ss7_instance *inst = user->inst;
+
if (service_ind >= ARRAY_SIZE(inst->user))
return -EINVAL;
if (!inst->user[service_ind])
return -ENODEV;
- if (user && (inst->user[service_ind] != user))
+ if (inst->user[service_ind] != user)
return -EINVAL;
- if (user)
- user->inst = NULL;
+ LOGPSS7U(user, LOGL_DEBUG, "unregistering from SI %u with priv %p\n",
+ service_ind, user->priv);
+
inst->user[service_ind] = NULL;
return 0;
}
-/* deliver to a local MTP user */
-int ss7_mtp_to_user(struct osmo_ss7_instance *inst, struct osmo_mtp_prim *omp)
+void ss7_user_unregister_all(struct osmo_ss7_user *user)
{
- uint32_t service_ind;
- const struct osmo_ss7_user *osu;
-
- if (omp->oph.sap != MTP_SAP_USER ||
- omp->oph.primitive != OSMO_MTP_PRIM_TRANSFER ||
- omp->oph.operation != PRIM_OP_INDICATION) {
- LOGP(DLSS7, LOGL_ERROR, "Unsupported Primitive\n");
- return -EINVAL;
+ struct osmo_ss7_instance *inst = user->inst;
+ for (unsigned int i = 0; i < ARRAY_SIZE(inst->user); i++) {
+ if (inst->user[i] == user)
+ inst->user[i] = NULL;
}
-
- service_ind = omp->u.transfer.sio & 0xF;
- osu = inst->user[service_ind];
-
- if (!osu) {
- LOGP(DLSS7, LOGL_NOTICE, "No MTP-User for SI %u\n", service_ind);
- return -ENODEV;
- }
-
- DEBUGP(DLSS7, "delivering MTP-TRANSFER.ind to user %s, priv=%p\n",
- osu->name, osu->priv);
- return osu->prim_cb(&omp->oph, (void *) osu->priv);
}
diff --git a/src/ss7_user.h b/src/ss7_user.h
index 6021330..280fc82 100644
--- a/src/ss7_user.h
+++ b/src/ss7_user.h
@@ -5,7 +5,7 @@
#include <osmocom/sigtran/mtp_sap.h>
/***********************************************************************
- * SS7 Linksets
+ * SS7 User (MTP SAP)
***********************************************************************/
struct osmo_ss7_instance;
@@ -21,4 +21,11 @@
void *priv;
};
-int ss7_mtp_to_user(struct osmo_ss7_instance *inst, struct osmo_mtp_prim *omp);
+struct osmo_ss7_user *ss7_user_find(struct osmo_ss7_instance *inst, uint8_t service_indicator);
+void ss7_user_unregister_all(struct osmo_ss7_user *user);
+int ss7_user_mtp_sap_prim_up(const struct osmo_ss7_user *osu, struct osmo_mtp_prim *omp);
+
+#define _LOGPSS7U(osu, subsys, level, fmt, args ...) \
+ _LOGSS7((osu)->inst, subsys, level, "ss7_user(%s) " fmt, osu->name, ## args)
+#define LOGPSS7U(osu, level, fmt, args ...) \
+ _LOGPSS7U(osu, DLSS7, level, fmt, ## args)
diff --git a/src/xua_internal.h b/src/xua_internal.h
index 36393f3..90a68be 100644
--- a/src/xua_internal.h
+++ b/src/xua_internal.h
@@ -1,5 +1,8 @@
#pragma once
+#include <unistd.h>
+#include <stdint.h>
+
#include <osmocom/core/tdef.h>
#include <osmocom/sigtran/osmo_ss7.h>
#include "xua_msg.h"
@@ -34,7 +37,6 @@
void sua_tx_dupu(struct osmo_ss7_asp *asp, const uint32_t *rctx, unsigned int num_rctx,
uint32_t dpc, uint16_t user, uint16_t cause, const char *info_str);
-struct osmo_mtp_prim *m3ua_to_xfer_ind(struct xua_msg *xua);
int m3ua_hmdc_rx_from_l2(struct osmo_ss7_instance *inst, struct xua_msg *xua);
struct msgb *m3ua_to_msg(struct xua_msg *xua);
int m3ua_tx_xua_as(struct osmo_ss7_as *as, struct xua_msg *xua);
@@ -69,6 +71,9 @@
const struct m3ua_data_hdr *mdh);
void mtp_xfer_param_to_m3ua_dh(struct m3ua_data_hdr *mdh,
const struct osmo_mtp_transfer_param *param);
+int hmrt_mtp_xfer_request_l4_to_l3(struct osmo_ss7_instance *inst,
+ const struct osmo_mtp_transfer_param *param,
+ uint8_t *user_data, size_t user_data_len);
extern const struct xua_msg_class m3ua_msg_class_mgmt;
diff --git a/tests/ss7/ss7_test.c b/tests/ss7/ss7_test.c
index c8e1c67..739d786 100644
--- a/tests/ss7/ss7_test.c
+++ b/tests/ss7/ss7_test.c
@@ -124,25 +124,18 @@
osmo_ss7_user_set_priv(user, (void *) 0x1234);
/* registration */
- OSMO_ASSERT(osmo_ss7_user_register(s7i, 1, user) == 0);
- OSMO_ASSERT(osmo_ss7_user_register(s7i, 1, NULL) == -EBUSY);
- OSMO_ASSERT(osmo_ss7_user_register(s7i, 255, NULL) == -EINVAL);
+ OSMO_ASSERT(osmo_ss7_user_register(user, 1) == 0);
+ OSMO_ASSERT(osmo_ss7_user_register(user, 1) == -EBUSY);
+ OSMO_ASSERT(osmo_ss7_user_register(user, 255) == -EINVAL);
/* primitive delivery */
- OSMO_ASSERT(ss7_mtp_to_user(s7i, &omp) == 23);
+ OSMO_ASSERT(ss7_user_mtp_sap_prim_up(user, &omp) == 23);
/* cleanup */
- OSMO_ASSERT(osmo_ss7_user_unregister(s7i, 255, NULL) == -EINVAL);
- OSMO_ASSERT(osmo_ss7_user_unregister(s7i, 10, NULL) == -ENODEV);
- OSMO_ASSERT(osmo_ss7_user_unregister(s7i, 1, user2) == -EINVAL);
- OSMO_ASSERT(osmo_ss7_user_unregister(s7i, 1, user) == 0);
-
- /* primitive delivery should fail now */
- OSMO_ASSERT(ss7_mtp_to_user(s7i, &omp) == -ENODEV);
-
- /* wrong primitive delivery should also fail */
- omp.oph.primitive = OSMO_MTP_PRIM_PAUSE;
- OSMO_ASSERT(ss7_mtp_to_user(s7i, &omp) == -EINVAL);
+ OSMO_ASSERT(osmo_ss7_user_unregister(user, 255) == -EINVAL);
+ OSMO_ASSERT(osmo_ss7_user_unregister(user, 10) == -ENODEV);
+ OSMO_ASSERT(osmo_ss7_user_unregister(user2, 1) == -EINVAL);
+ OSMO_ASSERT(osmo_ss7_user_unregister(user, 1) == 0);
osmo_ss7_user_destroy(user);
osmo_ss7_user_destroy(user2);
To view, visit change 41411. To unsubscribe, or for help writing mail filters, visit settings.