pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39761?usp=email ) Change subject: mgw: Move endpoint/conn updating logic to helper function ...................................................................... mgw: Move endpoint/conn updating logic to helper function Move code logic doing actions based on endpoint state (and its conns) out of the CRCX/MDCX MGCP handlers. Most of that logic present in CRCX and MDCX is actually the same and can be merged. This way we can also make the updating logic more complex without extending the CRCX/MDCX handlers for and more adding spaghetti code. Change-Id: I24dd15ec76bd6e949a178a3b998b76a44ddbab50 --- M include/osmocom/mgcp/mgcp_e1.h M include/osmocom/mgcp/mgcp_endp.h M src/libosmo-mgcp/mgcp_e1.c M src/libosmo-mgcp/mgcp_endp.c M src/libosmo-mgcp/mgcp_protocol.c 5 files changed, 77 insertions(+), 48 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/61/39761/1 diff --git a/include/osmocom/mgcp/mgcp_e1.h b/include/osmocom/mgcp/mgcp_e1.h index fbd594c..84615b5 100644 --- a/include/osmocom/mgcp/mgcp_e1.h +++ b/include/osmocom/mgcp/mgcp_e1.h @@ -22,6 +22,6 @@ static const uint8_t e1_offsets[] = { 0, 0, 4, 0, 2, 4, 6, 0, 1, 2, 3, 4, 5, 6, 7 }; int mgcp_e1_endp_equip(struct mgcp_endpoint *endp, uint8_t ts, uint8_t ss, uint8_t offs); -void mgcp_e1_endp_update(struct mgcp_endpoint *endp); +int mgcp_e1_endp_update(struct mgcp_endpoint *endp); void mgcp_e1_endp_release(struct mgcp_endpoint *endp, uint8_t ts); int mgcp_e1_send_rtp(struct mgcp_endpoint *endp, struct mgcp_rtp_codec *codec, struct msgb *msg); diff --git a/include/osmocom/mgcp/mgcp_endp.h b/include/osmocom/mgcp/mgcp_endp.h index 8a2c8a2..66fbf21 100644 --- a/include/osmocom/mgcp/mgcp_endp.h +++ b/include/osmocom/mgcp/mgcp_endp.h @@ -131,7 +131,7 @@ struct mgcp_endpoint *mgcp_endp_alloc(struct mgcp_trunk *trunk, unsigned int index); int mgcp_endp_claim(struct mgcp_endpoint *endp, const char *callid); -void mgcp_endp_update(struct mgcp_endpoint *endp); +int mgcp_endp_update(struct mgcp_endpoint *endp, struct mgcp_conn *conn, enum mgcp_verb verb); bool mgcp_endp_is_wildcarded(const char *epname); bool mgcp_endp_is_null(const char *epname); struct mgcp_endpoint *mgcp_endp_by_name_trunk(int *cause, const char *epname, diff --git a/src/libosmo-mgcp/mgcp_e1.c b/src/libosmo-mgcp/mgcp_e1.c index 3a7a450..33fe98f 100644 --- a/src/libosmo-mgcp/mgcp_e1.c +++ b/src/libosmo-mgcp/mgcp_e1.c @@ -661,7 +661,7 @@ /*! Update E1 related parameters (codec and sync pattern). * \param[in] endp endpoint to update. */ -void mgcp_e1_endp_update(struct mgcp_endpoint *endp) +int mgcp_e1_endp_update(struct mgcp_endpoint *endp) { struct mgcp_conn *conn; struct mgcp_conn_rtp *conn_rtp; @@ -685,6 +685,7 @@ /* Update sync pattern */ sync_pat_id = determine_trau_sync_pat(codec->subtype_name, endp->e1.scd.rate, endp->e1.last_amr_ft, endp); osmo_trau_sync_set_pat(endp->e1.trau_sync_fi, sync_pat_id); + return 0; } /*! Remove E1 resources from endpoint diff --git a/src/libosmo-mgcp/mgcp_endp.c b/src/libosmo-mgcp/mgcp_endp.c index a4ab860..324f67d 100644 --- a/src/libosmo-mgcp/mgcp_endp.c +++ b/src/libosmo-mgcp/mgcp_endp.c @@ -640,17 +640,78 @@ /*! update endpoint, updates internal endpoint specific data, should be * after when MDCX or CRCX has been executed successuflly. - * \param[in] endp endpoint to update. */ -void mgcp_endp_update(struct mgcp_endpoint *endp) + * \param[in] endp endpoint to update. + * \returns zero on success, mgcp negative error on failure. */ +static int mgcp_endp_update_virtual(struct mgcp_endpoint *endp, struct mgcp_conn *conn, enum mgcp_verb verb) +{ + OSMO_ASSERT(conn); + OSMO_ASSERT(conn->type == MGCP_CONN_TYPE_RTP); + struct mgcp_trunk *trunk = endp->trunk; + struct mgcp_conn_rtp *conn_rtp = mgcp_conn_get_conn_rtp(conn); + char new_local_addr[INET6_ADDRSTRLEN]; + + /* CRCX: Find a local address for conn based on policy and initial SDP remote + * information, then find a free port for it. + * MDCX: msg may have provided a new remote address, which means we may need + * to update our announced IP addr and re-bind our local end. This can + * happen for instance if MGW initially provided an IPv4 during CRCX + * ACK, and now MDCX tells us the remote has an IPv6 address. + */ + if (mgcp_get_local_addr(new_local_addr, conn_rtp) < 0) + goto fail_bind_port_ret; + + if (strcmp(new_local_addr, conn_rtp->end.local_addr)) { + osmo_strlcpy(conn_rtp->end.local_addr, new_local_addr, sizeof(conn_rtp->end.local_addr)); + mgcp_rtp_end_free_port(&conn_rtp->end); + if (mgcp_trunk_allocate_conn_rtp_ports(trunk, conn_rtp) != 0) + goto fail_bind_port_ret; + } + + switch (conn_rtp->type) { + case MGCP_RTP_DEFAULT: + break; + case MGCP_RTP_OSMUX: + if (conn_osmux_event_rx_crcx_mdcx(conn_rtp) < 0) { + LOGPCONN(conn, DLMGCP, LOGL_ERROR, "CRCX: Osmux handling failed!\n"); + return -500; + } + break; + case MGCP_RTP_IUUP: + break; + default: + return -523; + } + + return 0; + +fail_bind_port_ret: + switch (verb) { + case MGCP_VERB_CRCX: + rate_ctr_inc(rate_ctr_group_get_ctr(trunk->ratectr.mgcp_crcx_ctr_group, + MGCP_CRCX_FAIL_BIND_PORT)); + break; + case MGCP_VERB_MDCX: + rate_ctr_inc(rate_ctr_group_get_ctr(trunk->ratectr.mgcp_mdcx_ctr_group, + MGCP_MDCX_FAIL_BIND_PORT)); + break; + default: + break; + } + return -500; +} + +/*! update endpoint, updates internal endpoint specific data, should be + * after when MDCX or CRCX has been executed successuflly. + * \param[in] endp endpoint to update. + * \returns zero on success, mgcp negative error on failure. */ +int mgcp_endp_update(struct mgcp_endpoint *endp, struct mgcp_conn *conn, enum mgcp_verb verb) { /* Allocate resources */ switch (endp->trunk->trunk_type) { case MGCP_TRUNK_VIRTUAL: - /* No updating initaliziation required for virtual endpoints. */ - break; + return mgcp_endp_update_virtual(endp, conn, verb); case MGCP_TRUNK_E1: - mgcp_e1_endp_update(endp); - break; + return mgcp_e1_endp_update(endp); default: OSMO_ASSERT(false); } diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index bd2120f..e33ce35 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -763,26 +763,11 @@ rc = mgcp_conn_iuup_init(conn_rtp); } - /* Find a local address for conn based on policy and initial SDP remote - information, then find a free port for it */ - if (mgcp_get_local_addr(conn_rtp->end.local_addr, conn_rtp) < 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BIND_PORT)); + rc = mgcp_endp_update(endp, conn, rq->verb); + if (rc < 0) { + error_code = -rc; goto error2; } - if (mgcp_trunk_allocate_conn_rtp_ports(trunk, conn_rtp) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BIND_PORT)); - goto error2; - } - - /* Notify Osmux conn that CRCX was received */ - if (mgcp_conn_rtp_is_osmux(conn_rtp)) { - if (conn_osmux_event_rx_crcx_mdcx(conn_rtp) < 0) { - LOGPCONN(conn, DLMGCP, LOGL_ERROR, "CRCX: Osmux handling failed!\n"); - goto error2; - } - } - - mgcp_endp_update(endp); LOGPCONN(conn, DLMGCP, LOGL_NOTICE, "CRCX: connection successfully created: %s\n", mgcp_conn_dump(conn)); @@ -885,7 +870,6 @@ struct mgcp_endpoint *endp = rq->endp; struct mgcp_parse_hdr_pars *hpars = &pdata->hpars; struct rate_ctr_group *rate_ctrs; - char new_local_addr[INET6_ADDRSTRLEN]; int error_code = 500; struct mgcp_conn *conn = NULL; struct mgcp_conn_rtp *conn_rtp = NULL; @@ -981,30 +965,13 @@ if (mgcp_conn_rtp_is_osmux(conn_rtp)) { conn_rtp->osmux.remote_cid_present = true; conn_rtp->osmux.remote_cid = hpars->remote_osmux_cid; - if (conn_osmux_event_rx_crcx_mdcx(conn_rtp) < 0) { - LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: Osmux handling failed!\n"); - goto error3; - } } - /* MDCX may have provided a new remote address, which means we may need - to update our announced IP addr and re-bind our local end. This can - happen for instance if MGW initially provided an IPv4 during CRCX - ACK, and now MDCX tells us the remote has an IPv6 address. */ - if (mgcp_get_local_addr(new_local_addr, conn_rtp) < 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_BIND_PORT)); + rc = mgcp_endp_update(endp, conn, rq->verb); + if (rc < 0) { + error_code = -rc; goto error3; } - if (strcmp(new_local_addr, conn_rtp->end.local_addr)) { - osmo_strlcpy(conn_rtp->end.local_addr, new_local_addr, sizeof(conn_rtp->end.local_addr)); - mgcp_rtp_end_free_port(&conn_rtp->end); - if (mgcp_trunk_allocate_conn_rtp_ports(trunk, conn_rtp) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_BIND_PORT)); - goto error3; - } - } - - mgcp_endp_update(endp); LOGPCONN(conn, DLMGCP, LOGL_NOTICE, "MDCX: connection successfully modified: %s\n", mgcp_conn_dump(conn)); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39761?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I24dd15ec76bd6e949a178a3b998b76a44ddbab50 Gerrit-Change-Number: 39761 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt;