pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39396?usp=email )
Change subject: Pass ownership of Tx xua_msg down the stack [1/2] ......................................................................
Pass ownership of Tx xua_msg down the stack [1/2]
This is a first step towards getting MTP OPC & SLS fields down the stack so that they can be used to implement loadsharing at ASP level. So far, we always encode the xua_msg (which holds the OPC and SLS info into xua_msg->mtp) into a msgb before passing it to as->fi through event XUA_AS_E_TRANSFER_REQ, which in turn calls xua_as_transmit_msg(as, msg).
This patch is part 1 of 2 patches, which only modifies mostly the Tx path, but still requires modifications on Rx path (done in patch 2/2) to properly handle the forwarding path from Rx to Tx without double freeing. It is submitted in 2 parts to ease code reviewing.
Future patches will modify that code to pass a xua_msg and only encode it into a msgb deeper into the stack after an ASP has been selected and msg needs to be transmitted.
All previous calls to sua_tx_xua_asp() actually had a memory leak before this change, hence why no xua_msg_free() is removed from callers in this patch. Since now ownership is taken by sua_tx_xua_asp() which always frees the xua msg, the memleak is fixed.
Change-Id: I49e718e624da08510a9c3d28bdb360c59f9a65d3 --- M src/ipa.c M src/m3ua.c M src/osmo_ss7_hmrt.c M src/sua.c 4 files changed, 49 insertions(+), 19 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/96/39396/1
diff --git a/src/ipa.c b/src/ipa.c index 6ca5b50..944fbce 100644 --- a/src/ipa.c +++ b/src/ipa.c @@ -94,13 +94,17 @@ /*! \brief Send a given xUA message via a given IPA "Application Server" * \param[in] as Application Server through which to send \a xua * \param[in] xua xUA message to be sent - * \return 0 on success; negative on error */ + * \return 0 on success; negative on error + * + * This function takes ownership of xua msg passed to it. + */ int ipa_tx_xua_as(struct osmo_ss7_as *as, struct xua_msg *xua) { struct msgb *msg; OSMO_ASSERT(as->cfg.proto == OSMO_SS7_ASP_PROT_IPA);
msg = ipa_to_msg(xua); + xua_msg_free(xua); if (!msg) { LOGPAS(as, DLSS7, LOGL_ERROR, "Error encoding IPA Msg\n"); return -1; diff --git a/src/m3ua.c b/src/m3ua.c index 5f94674..dbdd4e2 100644 --- a/src/m3ua.c +++ b/src/m3ua.c @@ -493,23 +493,31 @@ return msg; }
-/* transmit given xua_msg via given ASP */ +/* transmit given xua_msg via given ASP + * This function takes ownership of xua msg passed to it. + */ static int m3ua_tx_xua_asp(struct osmo_ss7_asp *asp, struct xua_msg *xua) { - struct msgb *msg = m3ua_to_msg(xua); + struct msgb *msg;
OSMO_ASSERT(asp->cfg.proto == OSMO_SS7_ASP_PROT_M3UA);
+ msg = m3ua_to_msg(xua); + xua_msg_free(xua); if (!msg) return -1;
+ /* msg becomes owned by osmo_ss7_asp_send here: */ return osmo_ss7_asp_send(asp, msg); }
/*! \brief Send a given xUA message via a given M3UA Application Server * \param[in] as Application Server through which to send \ref xua * \param[in] xua xUA message to be sent - * \return 0 on success; negative on error */ + * \return 0 on success; negative on error + * + * This function takes ownership of xua msg passed to it. + */ int m3ua_tx_xua_as(struct osmo_ss7_as *as, struct xua_msg *xua) { struct msgb *msg; @@ -522,6 +530,7 @@ xua_msg_add_u32(xua, M3UA_IEI_ROUTE_CTX, as->cfg.routing_key.context);
msg = m3ua_to_msg(xua); + xua_msg_free(xua); if (!msg) { LOGPAS(as, DLM3UA, LOGL_ERROR, "Error encoding M3UA Msg\n"); return -1; @@ -792,10 +801,8 @@ err = m3ua_gen_error_msg(rc, msg);
out: - if (err) { + if (err) m3ua_tx_xua_asp(asp, err); - xua_msg_free(err); - }
xua_msg_free(xua);
@@ -898,7 +905,6 @@ xua = m3ua_encode_duna(rctx, num_rctx, aff_pc, num_aff_pc, info_string);
m3ua_tx_xua_asp(asp, xua); - xua_msg_free(xua); }
/*! Transmit SSNM SCON message indicating congestion @@ -932,7 +938,6 @@ xua_msg_add_data(xua, M3UA_IEI_INFO_STRING, strlen(info_string)+1, (const uint8_t *) info_string);
m3ua_tx_xua_asp(asp, xua); - xua_msg_free(xua); }
/*! Transmit SSNM DUPU message indicating user unavailability. @@ -948,7 +953,6 @@ { struct xua_msg *xua = m3ua_encode_dupu(rctx, num_rctx, dpc, user, cause, info_str); m3ua_tx_xua_asp(asp, xua); - xua_msg_free(xua); }
/* received SNM message on ASP side */ diff --git a/src/osmo_ss7_hmrt.c b/src/osmo_ss7_hmrt.c index 5fca2d8..5200bb9 100644 --- a/src/osmo_ss7_hmrt.c +++ b/src/osmo_ss7_hmrt.c @@ -86,24 +86,35 @@ return m3ua_xfer_from_data(&data_hdr, msgb_l2(msg), msgb_l2len(msg)); }
-/* delivery given XUA message to given SS7 user */ +/* 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) { struct osmo_mtp_prim *prim; + int rc;
/* Create MTP-TRANSFER.ind and feed to user */ prim = m3ua_to_xfer_ind(xua); - if (!prim) - return -1; + if (!prim) { + rc = -1; + goto ret_free; + } prim->u.transfer = xua->mtp;
- return osu->prim_cb(&prim->oph, (void *) osu->priv); + rc = osu->prim_cb(&prim->oph, (void *) osu->priv); + +ret_free: + xua_msg_free(xua); + return rc; }
/* HMDC -> HMDT: Message for distribution; Figure 25/Q.704 */ /* This means it is a message we received from remote/L2, and it is to - * be routed to a local user part */ + * be routed to a local user part. + * Ownership of xua_msg passed is transferred to this function. + */ static int hmdt_message_for_distribution(struct osmo_ss7_instance *inst, struct xua_msg *xua) { struct m3ua_data_hdr *mdh; @@ -120,6 +131,7 @@ default: LOGP(DLSS7, LOGL_ERROR, "Unknown M3UA XFER Message " "Type %u\n", xua->hdr.msg_type); + xua_msg_free(xua); return -1; } break; @@ -131,6 +143,7 @@ /* Discard Message */ LOGP(DLSS7, LOGL_ERROR, "Unknown M3UA Message Class %u\n", xua->hdr.msg_class); + xua_msg_free(xua); return -1; }
@@ -142,6 +155,7 @@ LOGP(DLSS7, 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; } } @@ -207,7 +221,9 @@
/* HMDC->HMRT Msg For Routing; Figure 26/Q.704 */ /* local message was receive d from L4, SRM, SLM, STM or SLTC, or - * remote message received from L2 and HMDC determined msg for routing */ + * remote message received from L2 and HMDC determined msg for routing + * Ownership of xua_msg passed is transferred to this function. + */ static int hmrt_message_for_routing(struct osmo_ss7_instance *inst, struct xua_msg *xua) { @@ -237,6 +253,7 @@ if (osmo_ss7_as_down(as)) { LOGP(DLSS7, LOGL_ERROR, "Unable to route HMRT message: the AS %s is down\n", as->cfg.name); + xua_msg_free(xua); return -ENETDOWN; }
@@ -272,12 +289,14 @@ /* Message Received for inaccesible SP HMRT ->RTPC */ /* Discard Message */ } + xua_msg_free(xua); return -1; }
/* HMDC: Received Message L2 -> L3; Figure 24/Q.704 */ /* This means a message was received from L2 and we have to decide if it - * is for the local stack (HMDT) or for routng (HMRT) */ + * is for the local stack (HMDT) or for routng (HMRT) + * Ownership of xua_msg passed is transferred to this function. */ int m3ua_hmdc_rx_from_l2(struct osmo_ss7_instance *inst, struct xua_msg *xua) { uint32_t dpc = xua->mtp.dpc; @@ -314,7 +333,6 @@ * 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); - xua_msg_free(xua); break; default: LOGP(DLSS7, LOGL_ERROR, "Ignoring unknown primitive %u:%u\n", diff --git a/src/sua.c b/src/sua.c index e81c471..79e8567 100644 --- a/src/sua.c +++ b/src/sua.c @@ -291,13 +291,16 @@
static int sua_tx_xua_asp(struct osmo_ss7_asp *asp, struct xua_msg *xua) { - struct msgb *msg = sua_to_msg(xua); + struct msgb *msg;
OSMO_ASSERT(asp->cfg.proto == OSMO_SS7_ASP_PROT_SUA);
+ msg = sua_to_msg(xua); + xua_msg_free(xua); if (!msg) return -1;
+ /* msg becomes owned by osmo_ss7_asp_send here: */ return osmo_ss7_asp_send(asp, msg); }
@@ -317,6 +320,7 @@ xua_msg_add_u32(xua, SUA_IEI_ROUTE_CTX, as->cfg.routing_key.context);
msg = sua_to_msg(xua); + xua_msg_free(xua); if (!msg) { LOGPAS(as, DLSUA, LOGL_ERROR, "Error encoding SUA Msg\n"); return -1;