pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39396?usp=email ) ( 10 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: ASP loadsharing: Pass ownership of Tx xua_msg down the stack [1/2] ...................................................................... ASP loadsharing: 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. Related: SYS#7112 Change-Id: I49e718e624da08510a9c3d28bdb360c59f9a65d3 --- M src/ipa.c M src/m3ua.c M src/osmo_ss7_hmrt.c M src/sua.c 4 files changed, 48 insertions(+), 19 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/ipa.c b/src/ipa.c index 4e0180c..47ddc0d 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 2f180c3..cdbfc48 100644 --- a/src/m3ua.c +++ b/src/m3ua.c @@ -506,23 +506,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; @@ -535,6 +543,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; @@ -794,10 +803,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); @@ -900,7 +907,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 @@ -934,7 +940,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. @@ -950,7 +955,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 675b231..654692a 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,13 +155,16 @@ 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; } } /* 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) { @@ -215,12 +231,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; @@ -257,7 +275,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 6d023cb..4d90f9b 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; -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39396?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I49e718e624da08510a9c3d28bdb360c59f9a65d3 Gerrit-Change-Number: 39396 Gerrit-PatchSet: 11 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>