March 2025 - gerrit-log - lists.osmocom.org

[M] Change in osmo-mgw[master]: mgw: Split DLCX read-only validations into its own function

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39740?usp=email ) Change subject: mgw: Split DLCX read-only validations into its own function ...................................................................... mgw: Split DLCX read-only validations into its own function Similar to what's already done in CRCX and MDXC. Change-Id: I3d86f0cbeacc7c0cd69809f898714a76d3b86ed0 --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 50 insertions(+), 28 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 6a731a3..237f2bf 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -1073,6 +1073,49 @@ return create_err_response(endp, endp, error_code, "MDCX", pdata->trans); } +/* Read-only checks for parsed DLCX request, applied to existing found conn. + * Returns negative MGCP error code on failure, 0 on scucess. + * NOTE: rq->endp may be NULL here! */ +static int validate_parsed_dlcx(struct mgcp_request_data *rq) +{ + struct mgcp_parse_data *pdata = rq->pdata; + struct mgcp_parse_hdr_pars *hpars = &pdata->hpars; + struct rate_ctr_group *rate_ctrs = rq->trunk->ratectr.mgcp_dlcx_ctr_group; + int rc; + + if (hpars->callid) { + /* If we have no endpoint, but a call id in the request, then this request cannot be handled */ + if (!rq->endp) { + LOGPTRUNK(rq->trunk, DLMGCP, LOGL_NOTICE, + "cannot handle requests with call-id (C) without endpoint -- abort!"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); + return -539; + } + if (!(rq->endp->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID) && + mgcp_verify_call_id(rq->endp, hpars->callid) != 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CALLID)); + return -516; + } + } + + if (hpars->connid) { + /* If we have no endpoint, but a connection id in the request, then this request cannot be handled */ + if (!rq->endp) { + LOGPTRUNK(rq->trunk, DLMGCP, LOGL_NOTICE, + "cannot handle requests with conn-id (I) without endpoint -- abort!"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); + return -539; + } + if ((rc = mgcp_verify_ci(rq->endp, hpars->connid)) != 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID)); + return -rc; + } + } + + /* Everything fine, continue */ + return 0; +} + /* DLCX command handler, processes the received command */ static struct msgb *handle_delete_con(struct mgcp_request_data *rq) { @@ -1130,39 +1173,18 @@ if (rc < 0) return create_err_response(rq->trunk, NULL, -rc, "DLCX", pdata->trans); - if (hpars->callid) { - /* If we have no endpoint, but a call id in the request, then this request cannot be handled */ - if (!endp) { - LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, - "cannot handle requests with call-id (C) without endpoint -- abort!"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); - return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans); - } - if (!(endp->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID) && - mgcp_verify_call_id(endp, hpars->callid) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CALLID)); - return create_err_response(endp, endp, 516, "DLCX", pdata->trans); - } - } - - if (hpars->connid) { - /* If we have no endpoint, but a connection id in the request, then this request cannot be handled */ - if (!endp) { - LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, - "cannot handle requests with conn-id (I) without endpoint -- abort!"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); - return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans); - } - if ((rc = mgcp_verify_ci(endp, hpars->connid)) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID)); - return create_err_response(endp, endp, rc, "DLCX", pdata->trans); - } - } + rc = validate_parsed_dlcx(rq); + if (rc < 0) + return create_err_response(rq->trunk, NULL, -rc, "DLCX", pdata->trans); /* The logic does not permit to go past this point without having the * the endp pointer populated. */ OSMO_ASSERT(endp); + /***************************************************************************** + * From here on below we start, delete conn and potentially release endpoint. + *****************************************************************************/ + /* When no connection id is supplied, we will interpret this as a * wildcarded DLCX that refers to the selected endpoint. This means * that we drop all connections on that specific endpoint at once. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39740?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I3d86f0cbeacc7c0cd69809f898714a76d3b86ed0 Gerrit-Change-Number: 39740 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgw: CRCX: Handle codec information before initializing osmux

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39754?usp=email ) Change subject: mgw: CRCX: Handle codec information before initializing osmux ...................................................................... mgw: CRCX: Handle codec information before initializing osmux Same order as done during MDCX. This way we can also eg. check configured codec is AMR before successfully entering configuring osmux. Change-Id: If291db0c048196e6e0eee2c38e648e5a25438078 --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 10 insertions(+), 8 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 237f2bf..ef82796 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -768,6 +768,15 @@ conn_rtp = mgcp_conn_get_conn_rtp(conn); OSMO_ASSERT(conn_rtp); + /* Handle codec information and decide for a suitable codec */ + rc = handle_codec_info(conn_rtp, rq); + mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); + if (rc) { + error_code = rc; + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_CODEC_NEGOTIATION)); + goto error2; + } + /* If X-Osmux (remote CID) was received, alloc next avail CID as local CID */ if (hpars->remote_osmux_cid != MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET) { /* Make sure osmux is setup: */ @@ -785,14 +794,6 @@ } /* else: -1 (wildcard) */ } - /* Handle codec information and decide for a suitable codec */ - rc = handle_codec_info(conn_rtp, rq); - mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); - if (rc) { - error_code = rc; - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_CODEC_NEGOTIATION)); - goto error2; - } /* Upgrade the conn type RTP_DEFAULT->RTP_IUUP if needed based on requested codec: */ if (conn_rtp->type == MGCP_RTP_DEFAULT && strcmp(conn_rtp->end.cset.codec->subtype_name, "VND.3GPP.IUFP") == 0) { @@ -1017,6 +1018,7 @@ error_code = rc; goto error3; } + /* Upgrade the conn type RTP_DEFAULT->RTP_IUUP if needed based on requested codec: */ if (conn_rtp->type == MGCP_RTP_DEFAULT && strcmp(conn_rtp->end.cset.codec->subtype_name, "VND.3GPP.IUFP") == 0) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39754?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: If291db0c048196e6e0eee2c38e648e5a25438078 Gerrit-Change-Number: 39754 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgw: Drop rtp_processing transcoding stubs

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39755?usp=email ) Change subject: mgw: Drop rtp_processing transcoding stubs ...................................................................... mgw: Drop rtp_processing transcoding stubs This has never been used afaik, and it's only clogging the code and adding extra indirect function pointer calls to hot paths for no good reason. Drop all of it, and re-ad whatever makes sense once someone really wants to implement transcoding. Change-Id: Id16af528d94612e771f94333ad5404e5cd1c3318 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/mgcp_network.h M include/osmocom/mgcp/mgcp_ratectr.h M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_ratectr.c 6 files changed, 1 insertion(+), 102 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h index f72807e..edc3cb6 100644 --- a/include/osmocom/mgcp/mgcp.h +++ b/include/osmocom/mgcp/mgcp.h @@ -130,10 +130,6 @@ char source_addr[INET6_ADDRSTRLEN]; char call_agent_addr[INET6_ADDRSTRLEN]; - /* RTP processing */ - mgcp_processing rtp_processing_cb; - mgcp_processing_setup setup_rtp_processing_cb; - struct osmo_wqueue gw_fd; struct mgcp_port_range net_ports; diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h index 8a1fc47..f30b788 100644 --- a/include/osmocom/mgcp/mgcp_network.h +++ b/include/osmocom/mgcp/mgcp_network.h @@ -98,18 +98,6 @@ struct osmo_sockaddr *addr, struct msgb *msg); int mgcp_get_local_addr(char *addr, struct mgcp_conn_rtp *conn); -/* payload processing default functions */ -int mgcp_rtp_processing_default(struct mgcp_endpoint *endp, struct mgcp_rtp_end *dst_end, struct msgb *msg); - -int mgcp_setup_rtp_processing_default(struct mgcp_endpoint *endp, - struct mgcp_conn_rtp *conn_dst, - struct mgcp_conn_rtp *conn_src); - -void mgcp_get_net_downlink_format_default(struct mgcp_endpoint *endp, - const struct mgcp_rtp_codec **codec, - const char **fmtp_extra, - struct mgcp_conn_rtp *conn); - /* internal RTP Annex A counting */ void mgcp_rtp_annex_count(const struct mgcp_endpoint *endp, struct mgcp_rtp_state *state, const uint16_t seq, const int32_t transit, diff --git a/include/osmocom/mgcp/mgcp_ratectr.h b/include/osmocom/mgcp/mgcp_ratectr.h index f566625..1e3365b 100644 --- a/include/osmocom/mgcp/mgcp_ratectr.h +++ b/include/osmocom/mgcp/mgcp_ratectr.h @@ -43,7 +43,6 @@ MGCP_MDCX_FAIL_INVALID_MODE, MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS, MGCP_MDCX_FAIL_NO_REMOTE_CONN_DESC, - MGCP_MDCX_FAIL_START_RTP, MGCP_MDCX_FAIL_AVAIL, }; diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index de837c1..abd095b 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -408,31 +408,6 @@ return ts_error; } -/*! dummy callback to disable transcoding (see also cfg->rtp_processing_cb). - * \param[in] associated endpoint. - * \param[in] destination RTP end. - * \param[in,out] msg message bufffer containing data. Function might change length. - * \returns ignores input parameters, return always 0. */ -int mgcp_rtp_processing_default(struct mgcp_endpoint *endp, - struct mgcp_rtp_end *dst_end, - struct msgb *msg) -{ - return 0; -} - -/*! dummy callback to disable transcoding (see also cfg->setup_rtp_processing_cb). - * \param[in] associated endpoint. - * \param[in] destination RTP connnection. - * \param[in] source RTP connection. - * \returns ignores input parameters, return always 0. */ -int mgcp_setup_rtp_processing_default(struct mgcp_endpoint *endp, - struct mgcp_conn_rtp *conn_dst, - struct mgcp_conn_rtp *conn_src) -{ - LOGPENDP(endp, DRTP, LOGL_DEBUG, "transcoding disabled\n"); - return 0; -} - void mgcp_rtp_annex_count(const struct mgcp_endpoint *endp, struct mgcp_rtp_state *state, const uint16_t seq, const int32_t transit, const uint32_t ssrc, @@ -1187,13 +1162,7 @@ * header is present, we will generate one. */ gen_rtp_header(msg, rtp_end, rtp_state); - /* Run transcoder */ - rc = endp->trunk->cfg->rtp_processing_cb(endp, rtp_end, msg); - if (rc < 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, "Error %d during transcoding\n", rc); - msgb_free(msg); - return rc; - } + /* TODO: Run transcoder */ if (addr) mgcp_patch_and_count(endp, rtp_state, rtp_end, addr, msg); diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index ef82796..7c1a715 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -96,43 +96,6 @@ static struct msgb *handle_rsip(struct mgcp_request_data *data); static struct msgb *handle_noti_req(struct mgcp_request_data *data); -/* Initalize transcoder */ -static int setup_rtp_processing(struct mgcp_endpoint *endp, - struct mgcp_conn_rtp *conn) -{ - struct mgcp_config *cfg = endp->trunk->cfg; - struct mgcp_conn_rtp *conn_src = NULL; - struct mgcp_conn_rtp *conn_dst = conn; - struct mgcp_conn *_conn; - - switch (conn->type) { - case MGCP_RTP_DEFAULT: - case MGCP_RTP_OSMUX: - case MGCP_RTP_IUUP: - break; - default: - LOGPENDP(endp, DLMGCP, LOGL_NOTICE, - "RTP-setup: Endpoint is not configured as RTP default, stopping here!\n"); - return 0; - } - - if (conn->conn->mode == MGCP_CONN_LOOPBACK) { - LOGPENDP(endp, DLMGCP, LOGL_NOTICE, - "RTP-setup: Endpoint is in loopback mode, stopping here!\n"); - return 0; - } - - /* Find the "sister" connection */ - llist_for_each_entry(_conn, &endp->conns, entry) { - if (_conn->id != conn->conn->id) { - conn_src = mgcp_conn_get_conn_rtp(_conn); - break; - } - } - - return cfg->setup_rtp_processing_cb(endp, conn_dst, conn_src); -} - /* Helper function to allocate some memory for responses and retransmissions */ static struct msgb *mgcp_msgb_alloc(void *ctx) { @@ -811,13 +774,6 @@ goto error2; } - if (setup_rtp_processing(endp, conn_rtp) != 0) { - LOGPCONN(conn, DLMGCP, LOGL_ERROR, - "CRCX: could not start RTP processing!\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_START_RTP)); - 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) { @@ -1050,11 +1006,6 @@ } } - if (setup_rtp_processing(endp, conn_rtp) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_START_RTP)); - goto error3; - } - /* modify */ LOGPCONN(conn, DLMGCP, LOGL_DEBUG, "MDCX: modified conn:%s\n", mgcp_conn_dump(conn)); @@ -1394,9 +1345,6 @@ cfg->source_port = 2427; osmo_strlcpy(cfg->source_addr, "0.0.0.0", sizeof(cfg->source_addr)); - cfg->rtp_processing_cb = &mgcp_rtp_processing_default; - cfg->setup_rtp_processing_cb = &mgcp_setup_rtp_processing_default; - INIT_LLIST_HEAD(&cfg->trunks); /* Allocate virtual trunk */ diff --git a/src/libosmo-mgcp/mgcp_ratectr.c b/src/libosmo-mgcp/mgcp_ratectr.c index abf3883..1f06117 100644 --- a/src/libosmo-mgcp/mgcp_ratectr.c +++ b/src/libosmo-mgcp/mgcp_ratectr.c @@ -93,7 +93,6 @@ [MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS] = { "mdcx:conn_opt", "connection options invalid." }, [MGCP_MDCX_FAIL_NO_REMOTE_CONN_DESC] = { "mdcx:no_remote_conn_desc", "no opposite end specified for connection." }, - [MGCP_MDCX_FAIL_START_RTP] = { "mdcx:start_rtp_failure", "failure to start RTP processing." }, [MGCP_MDCX_FAIL_AVAIL] = { "mdcx:unavailable", "endpoint unavailable." }, }; -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39755?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Id16af528d94612e771f94333ad5404e5cd1c3318 Gerrit-Change-Number: 39755 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgw: Rearrange last steps of CRCX and MDCX

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39756?usp=email ) Change subject: mgw: Rearrange last steps of CRCX and MDCX ...................................................................... mgw: Rearrange last steps of CRCX and MDCX Make code in CRCX and MDCX look closer, since it is really doing the same. Move the dummy ping code after everything has been done correctly, since the ping is just a side effect of the conn being created. Change-Id: I42acda16cb2d59a9b7aae06b7584d4dfc1e91f9e --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 10 insertions(+), 15 deletions(-) Approvals: pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 7c1a715..e4386b2 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -782,8 +782,11 @@ } } - LOGPCONN(conn, DLMGCP, LOGL_DEBUG, - "CRCX: Creating connection: port: %u\n", conn_rtp->end.local_port); + mgcp_endp_update(endp); + + LOGPCONN(conn, DLMGCP, LOGL_NOTICE, + "CRCX: connection successfully created: %s\n", mgcp_conn_dump(conn)); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_SUCCESS)); /* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */ OSMO_ASSERT(trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE); @@ -791,11 +794,6 @@ trunk->keepalive_interval != MGCP_KEEPALIVE_NEVER) send_dummy(endp, conn_rtp); - LOGPCONN(conn, DLMGCP, LOGL_NOTICE, - "CRCX: connection successfully created\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_SUCCESS)); - mgcp_endp_update(endp); - /* NOTE: Only in the virtual trunk we allow dynamic endpoint names */ bool add_epname = rq->wildcarded && trunk->trunk_type == MGCP_TRUNK_VIRTUAL; return create_response_with_sdp(endp, conn_rtp, "CRCX", pdata->trans, add_epname, true); @@ -1006,9 +1004,11 @@ } } - /* modify */ - LOGPCONN(conn, DLMGCP, LOGL_DEBUG, - "MDCX: modified conn:%s\n", mgcp_conn_dump(conn)); + mgcp_endp_update(endp); + + LOGPCONN(conn, DLMGCP, LOGL_NOTICE, + "MDCX: connection successfully modified: %s\n", mgcp_conn_dump(conn)); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_SUCCESS)); /* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */ OSMO_ASSERT(trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE); @@ -1016,11 +1016,6 @@ trunk->keepalive_interval != MGCP_KEEPALIVE_NEVER) send_dummy(endp, conn_rtp); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_SUCCESS)); - - LOGPCONN(conn, DLMGCP, LOGL_NOTICE, - "MDCX: connection successfully modified\n"); - mgcp_endp_update(endp); return create_response_with_sdp(endp, conn_rtp, "MDCX", pdata->trans, false, false); error3: return create_err_response(endp, endp, error_code, "MDCX", pdata->trans); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39756?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I42acda16cb2d59a9b7aae06b7584d4dfc1e91f9e Gerrit-Change-Number: 39756 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[XS] Change in osmo-mgw[master]: mgw: mdcx: Fix inc of non-existing ctr in mdcx counter group

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39758?usp=email ) Change subject: mgw: mdcx: Fix inc of non-existing ctr in mdcx counter group ...................................................................... mgw: mdcx: Fix inc of non-existing ctr in mdcx counter group Change-Id: If849e3922c7ae864771c41ff7c0fac37ef0280de --- M include/osmocom/mgcp/mgcp_ratectr.h M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_ratectr.c 3 files changed, 4 insertions(+), 2 deletions(-) Approvals: pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve diff --git a/include/osmocom/mgcp/mgcp_ratectr.h b/include/osmocom/mgcp/mgcp_ratectr.h index 1e3365b..d013894 100644 --- a/include/osmocom/mgcp/mgcp_ratectr.h +++ b/include/osmocom/mgcp/mgcp_ratectr.h @@ -43,6 +43,7 @@ MGCP_MDCX_FAIL_INVALID_MODE, MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS, MGCP_MDCX_FAIL_NO_REMOTE_CONN_DESC, + MGCP_MDCX_FAIL_BIND_PORT, MGCP_MDCX_FAIL_AVAIL, }; diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index e4386b2..bd2120f 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -992,14 +992,14 @@ 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_CRCX_FAIL_BIND_PORT)); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_BIND_PORT)); 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_CRCX_FAIL_BIND_PORT)); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_BIND_PORT)); goto error3; } } diff --git a/src/libosmo-mgcp/mgcp_ratectr.c b/src/libosmo-mgcp/mgcp_ratectr.c index 1f06117..6c2c97f 100644 --- a/src/libosmo-mgcp/mgcp_ratectr.c +++ b/src/libosmo-mgcp/mgcp_ratectr.c @@ -93,6 +93,7 @@ [MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS] = { "mdcx:conn_opt", "connection options invalid." }, [MGCP_MDCX_FAIL_NO_REMOTE_CONN_DESC] = { "mdcx:no_remote_conn_desc", "no opposite end specified for connection." }, + [MGCP_MDCX_FAIL_BIND_PORT] = { "mdcx:bind_port", "port bind failure." }, [MGCP_MDCX_FAIL_AVAIL] = { "mdcx:unavailable", "endpoint unavailable." }, }; -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39758?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: If849e3922c7ae864771c41ff7c0fac37ef0280de Gerrit-Change-Number: 39758 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgw: Increment rate_ctr MGCP_*_FAIL_UNHANDLED_PARAM during parsing

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39737?usp=email ) Change subject: mgw: Increment rate_ctr MGCP_*_FAIL_UNHANDLED_PARAM during parsing ...................................................................... mgw: Increment rate_ctr MGCP_*_FAIL_UNHANDLED_PARAM during parsing We could not do it before because we had no easy access to the command verb being parsed. Now that we have access to it, bettter increase it where we have exact path where stuff happens. Change-Id: I3ea45fc1d25284b253ac9b7f0c0a925c10c994ca --- M src/libosmo-mgcp/mgcp_msg.c M src/libosmo-mgcp/mgcp_protocol.c 2 files changed, 19 insertions(+), 24 deletions(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved laforge: Looks good to me, but someone else must approve diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c index afeadd5..f7cba70 100644 --- a/src/libosmo-mgcp/mgcp_msg.c +++ b/src/libosmo-mgcp/mgcp_msg.c @@ -422,6 +422,22 @@ goto mgcp_header_done; default: LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "unhandled option: '%c'/%d\n", *line, *line); + switch (pdata->rq->verb) { + case MGCP_VERB_CRCX: + rate_ctr_inc(rate_ctr_group_get_ctr(pdata->rq->trunk->ratectr.mgcp_crcx_ctr_group, + MGCP_CRCX_FAIL_UNHANDLED_PARAM)); + break; + case MGCP_VERB_MDCX: + rate_ctr_inc(rate_ctr_group_get_ctr(pdata->rq->trunk->ratectr.mgcp_mdcx_ctr_group, + MGCP_MDCX_FAIL_UNHANDLED_PARAM)); + break; + case MGCP_VERB_DLCX: + rate_ctr_inc(rate_ctr_group_get_ctr(pdata->rq->trunk->ratectr.mgcp_dlcx_ctr_group, + MGCP_DLCX_FAIL_UNHANDLED_PARAM)); + break; + default: + break; + } return -539; } } diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 0e0f9dd..ef0aad4 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -689,15 +689,8 @@ /* parse CallID C: and LocalParameters L: */ rc = mgcp_parse_hdr_pars(pdata); - switch (rc) { - case 0: - break; /* all good, continue below */ - case -539: - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNHANDLED_PARAM)); + if (rc < 0) return create_err_response(rq->trunk, NULL, -rc, "CRCX", pdata->trans); - default: - return create_err_response(rq->trunk, NULL, -rc, "CRCX", pdata->trans); - } /* Parse SDP if found: */ if (hpars->have_sdp) { @@ -901,15 +894,8 @@ } rc = mgcp_parse_hdr_pars(pdata); - switch (rc) { - case 0: - break; /* all good, continue below */ - case -539: - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_UNHANDLED_PARAM)); + if (rc < 0) return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans); - default: - return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans); - } /* If a CallID is provided during MDCX, validate (unless endp was explicitly configured to ignore it * through "X-Osmo-IGN: C") that it matches the one previously set. */ @@ -1094,15 +1080,8 @@ } rc = mgcp_parse_hdr_pars(pdata); - switch (rc) { - case 0: - break; /* all good, continue below */ - case -539: - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); + if (rc < 0) return create_err_response(rq->trunk, NULL, -rc, "DLCX", pdata->trans); - default: - return create_err_response(rq->trunk, NULL, -rc, "DLCX", pdata->trans); - } if (hpars->callid) { /* If we have no endpoint, but a call id in the request, then this request cannot be handled */ -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39737?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I3ea45fc1d25284b253ac9b7f0c0a925c10c994ca Gerrit-Change-Number: 39737 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgw: Split CRCX read-only validations into its own function

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39736?usp=email ) Change subject: mgw: Split CRCX read-only validations into its own function ...................................................................... mgw: Split CRCX read-only validations into its own function The SDP parsing is moved to a step beforehand, so that validation of full message can be done in a subsequent step. That validation step is moved into a function to give some air to handle_create_con() and easily spot the logic. The logic based on force_realloc is split so that code modifying the object is moved to the later update step. Change-Id: I639ad0a25a0af4a4637045ca8bf61e436a789426 --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 108 insertions(+), 80 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 49a4e83..0e0f9dd 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -567,6 +567,87 @@ return 534; } +/* Read-only checks for parsed CRCX request. + * Returns negative MGCP error code on failure, 0 on scucess. */ +static int validate_parsed_crcx(struct mgcp_request_data *rq) +{ + struct mgcp_parse_data *pdata = rq->pdata; + struct mgcp_trunk *trunk = rq->trunk; + struct mgcp_endpoint *endp = rq->endp; + struct mgcp_parse_hdr_pars *hpars = &pdata->hpars; + struct rate_ctr_group *rate_ctrs = trunk->ratectr.mgcp_crcx_ctr_group; + + /* Check parameters */ + if (!hpars->callid) { + LOGPENDP(endp, DLMGCP, LOGL_ERROR, + "CRCX: insufficient parameters, missing callid\n"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_MISSING_CALLID)); + return -516; + } + + if (hpars->mode == MGCP_CONN_NONE) { + LOGPENDP(endp, DLMGCP, LOGL_ERROR, + "CRCX: insufficient parameters, invalid mode\n"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE)); + return -517; + } + + /* It is illegal to send a connection identifier + * together with a CRCX, the MGW will assign the + * connection identifier by itself on CRCX */ + if (hpars->connid) { + LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: 'I: %s' not expected!\n", hpars->connid); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BAD_ACTION)); + return -523; + } + + /* Reject osmux if disabled by config */ + if (trunk->cfg->osmux.usage == OSMUX_USAGE_OFF && + hpars->remote_osmux_cid != MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET) { + LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: Request with Osmux but it is disabled by config!\n"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_NO_OSMUX)); + return -511; + } + /* Reject non-osmux if required by config */ + if (trunk->cfg->osmux.usage == OSMUX_USAGE_ONLY && + hpars->remote_osmux_cid == MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET) { + LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: Request without Osmux but it is required by config!\n"); + return -517; + } + + /* Read-only checks here, force_realloc case done out of there afterwards.*/ + if (!trunk->force_realloc) { + /* Check if we are able to accept the creation of another connection */ + if (mgcp_endp_is_full(endp)) { + /* There is no more room for a connection, leave + * everything as it is and return with an error */ + LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: endpoint full, max. %d connections allowed!\n", + endp->type->max_conns); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_LIMIT_EXCEEDED)); + return -540; + } + + /* Check if this endpoint already serves a call, if so, check if the + * callids match up so that we are sure that this is our call. + * Do check only if endpoint was (or is by current CRCX) configured + * to explicitly ignore it ("X-Osmo-IGN: C"). + */ + if (endp->callid && + !((endp->x_osmo_ign | hpars->x_osmo_ign) & MGCP_X_OSMO_IGN_CALLID) && + mgcp_verify_call_id(endp, hpars->callid)) { + /* This is not our call, leave everything as it is and + * return with an error. */ + LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: already seized by other call (%s)\n", + endp->callid); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNKNOWN_CALLID)); + return -400; + } + } + + /* Everything fine, continue */ + return 0; +} + /* CRCX command handler, processes the received command */ static struct msgb *handle_create_con(struct mgcp_request_data *rq) { @@ -589,6 +670,7 @@ return create_err_response(rq->cfg, NULL, 502, "CRCX", pdata->trans); } + /* rq->trunk is available (non-null) from here on. */ rate_ctrs = trunk->ratectr.mgcp_crcx_ctr_group; /* we must have a free ep */ @@ -617,30 +699,6 @@ return create_err_response(rq->trunk, NULL, -rc, "CRCX", pdata->trans); } - /* Check parameters */ - if (!hpars->callid) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, - "CRCX: insufficient parameters, missing callid\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_MISSING_CALLID)); - return create_err_response(endp, endp, 516, "CRCX", pdata->trans); - } - - if (hpars->mode == MGCP_CONN_NONE) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, - "CRCX: insufficient parameters, invalid mode\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE)); - return create_err_response(endp, endp, 517, "CRCX", pdata->trans); - } - - /* It is illegal to send a connection identifier - * together with a CRCX, the MGW will assign the - * connection identifier by itself on CRCX */ - if (hpars->connid) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: 'I: %s' not expected!\n", hpars->connid); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BAD_ACTION)); - return create_err_response(endp, endp, 523, "CRCX", pdata->trans); - } - /* Parse SDP if found: */ if (hpars->have_sdp) { rc = mgcp_parse_sdp_data(pdata); @@ -650,67 +708,37 @@ } } - /* Reject osmux if disabled by config */ - if (trunk->cfg->osmux.usage == OSMUX_USAGE_OFF && - hpars->remote_osmux_cid != MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: Request with Osmux but it is disabled by config!\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_NO_OSMUX)); - return create_err_response(endp, endp, 511, "CRCX", pdata->trans); - } - /* Reject non-osmux if required by config */ - if (trunk->cfg->osmux.usage == OSMUX_USAGE_ONLY && - hpars->remote_osmux_cid == MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: Request without Osmux but it is required by config!\n"); - return create_err_response(endp, endp, 517, "CRCX", pdata->trans); - } - - /* Check if we are able to accept the creation of another connection */ - if (mgcp_endp_is_full(endp)) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, - "CRCX: endpoint full, max. %i connections allowed!\n", - endp->type->max_conns); - if (trunk->force_realloc) { - /* There is no more room for a connection, make some - * room by blindly tossing the oldest of the two two - * connections */ - mgcp_endp_free_conn_oldest(endp); - OSMO_ASSERT(!mgcp_endp_is_full(endp)); - } else { - /* There is no more room for a connection, leave - * everything as it is and return with an error */ - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_LIMIT_EXCEEDED)); - return create_err_response(endp, endp, 540, "CRCX", pdata->trans); - } - } - - /* Check if this endpoint already serves a call, if so, check if the - * callids match up so that we are sure that this is our call. - * Do check only if endpoint was (or is by current CRCX) configured - * to explicitly ignore it ("X-Osmo-IGN: C"). - */ - if (endp->callid && - !((endp->x_osmo_ign | hpars->x_osmo_ign) & MGCP_X_OSMO_IGN_CALLID) && - mgcp_verify_call_id(endp, hpars->callid)) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, - "CRCX: already seized by other call (%s)\n", - endp->callid); - if (trunk->force_realloc) - /* This is not our call, toss everything by releasing - * the entire endpoint. (rude!) */ - mgcp_endp_release(endp); - else { - /* This is not our call, leave everything as it is and - * return with an error. */ - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNKNOWN_CALLID)); - return create_err_response(endp, endp, 400, "CRCX", pdata->trans); - } - } + rc = validate_parsed_crcx(rq); + if (rc < 0) + return create_err_response(endp, endp, -rc, "CRCX", pdata->trans); /******************************************************************* * Allocate and update endpoint and conn. - * From here on below we start updating endpoing and creating conn: + * From here on below we start updating endpoint and creating conn: *******************************************************************/ + if (trunk->force_realloc) { + /* Check if we are able to accept the creation of another connection */ + if (mgcp_endp_is_full(endp)) { + /* There is no more room for a connection, make some + * room by blindly tossing the oldest of the two two + * connections */ + LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: endpoint full, max. %d connections allowed!\n", + endp->type->max_conns); + mgcp_endp_free_conn_oldest(endp); + OSMO_ASSERT(!mgcp_endp_is_full(endp)); + } + + /* Check if this endpoint already serves a call and then check if the callids match up */ + if (endp->callid && + !((endp->x_osmo_ign | hpars->x_osmo_ign) & MGCP_X_OSMO_IGN_CALLID) && + mgcp_verify_call_id(endp, hpars->callid)) { + /* This is not our call, toss everything by releasing + * the entire endpoint. (rude!) */ + mgcp_endp_release(endp); + } + } + /* Update endp->x_osmo_ign: */ endp->x_osmo_ign |= hpars->x_osmo_ign; -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39736?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I639ad0a25a0af4a4637045ca8bf61e436a789426 Gerrit-Change-Number: 39736 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgw: Split MDCX read-only validations into its own function

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39738?usp=email ) Change subject: mgw: Split MDCX read-only validations into its own function ...................................................................... mgw: Split MDCX read-only validations into its own function The SDP parsing is moved to a step beforehand, so that validation of full message can be done in a subsequent step. That validation step is moved into a function to give some air to handle_modify_con() and easily spot the logic. Change-Id: I469413dbe89cc4795deb50d76b434655cba9b776 --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 47 insertions(+), 26 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index ef0aad4..79a647a 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -849,6 +849,48 @@ return create_err_response(endp, endp, error_code, "CRCX", pdata->trans); } +/* Read-only checks for parsed MDCX request. + * Returns negative MGCP error code on failure, 0 on scucess. */ +static int validate_parsed_mdcx(struct mgcp_request_data *rq) +{ + struct mgcp_parse_data *pdata = rq->pdata; + struct mgcp_trunk *trunk = rq->trunk; + struct mgcp_endpoint *endp = rq->endp; + struct mgcp_parse_hdr_pars *hpars = &pdata->hpars; + struct rate_ctr_group *rate_ctrs = trunk->ratectr.mgcp_mdcx_ctr_group; + int error_code; + + if (mgcp_endp_num_conns(endp) <= 0) { + LOGPENDP(endp, DLMGCP, LOGL_ERROR, + "MDCX: endpoint is not holding a connection.\n"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_NO_CONN)); + return -400; + } + + /* If a CallID is provided during MDCX, validate (unless endp was explicitly configured to ignore + * it through "X-Osmo-IGN: C") that it matches the one previously set. */ + if (hpars->callid && + !(endp->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID) && + mgcp_verify_call_id(endp, hpars->callid) < 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CALLID)); + return -516; + } + + if (!hpars->connid) { + LOGPENDP(endp, DLMGCP, LOGL_ERROR, + "MDCX: insufficient parameters, missing ci (connectionIdentifier)\n"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_NO_CONNID)); + return -515; + } + if ((error_code = mgcp_verify_ci(endp, hpars->connid)) != 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CONNID)); + return -error_code; + } + + /* Everything fine, continue */ + return 0; +} + /* MDCX command handler, processes the received command */ static struct msgb *handle_modify_con(struct mgcp_request_data *rq) { @@ -870,7 +912,7 @@ LOGP(DLMGCP, LOGL_ERROR, "MDCX: Not allowed in 'null' endpoint!\n"); return create_err_response(rq->cfg, NULL, 502, "MDCX", pdata->trans); } - + /* rq->trunk is available (non-null) from here on. */ rate_ctrs = trunk->ratectr.mgcp_mdcx_ctr_group; /* Prohibit wildcarded requests */ @@ -886,36 +928,11 @@ LOGPENDP(endp, DLMGCP, LOGL_ERROR, "MDCX: selected endpoint not available!\n"); return create_err_response(rq->trunk, NULL, 501, "MDCX", pdata->trans); } - if (mgcp_endp_num_conns(endp) <= 0) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, - "MDCX: endpoint is not holding a connection.\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_NO_CONN)); - return create_err_response(endp, endp, 400, "MDCX", pdata->trans); - } rc = mgcp_parse_hdr_pars(pdata); if (rc < 0) return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans); - /* If a CallID is provided during MDCX, validate (unless endp was explicitly configured to ignore it - * through "X-Osmo-IGN: C") that it matches the one previously set. */ - if (hpars->callid && - !(endp->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID) && - mgcp_verify_call_id(endp, hpars->callid) < 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CALLID)); - return create_err_response(endp, endp, 516, "MDCX", pdata->trans); - } - - if (!hpars->connid) { - LOGPENDP(endp, DLMGCP, LOGL_ERROR, - "MDCX: insufficient parameters, missing ci (connectionIdentifier)\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_NO_CONNID)); - return create_err_response(endp, endp, 515, "MDCX", pdata->trans); - } else if ((error_code = mgcp_verify_ci(endp, hpars->connid)) != 0) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CONNID)); - return create_err_response(endp, endp, error_code, "MDCX", pdata->trans); - } - /* Parse SDP if found: */ if (hpars->have_sdp) { rc = mgcp_parse_sdp_data(pdata); @@ -926,6 +943,10 @@ } } + rc = validate_parsed_mdcx(rq); + if (rc < 0) + return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans); + conn = mgcp_endp_get_conn(endp, hpars->connid); if (!conn) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_CONN_NOT_FOUND)); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39738?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I469413dbe89cc4795deb50d76b434655cba9b776 Gerrit-Change-Number: 39738 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[XS] Change in osmo-mgw[master]: mgw: mdcx: Fix inc of non-existing ctr in mdcx counter group

by pespin

pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39758?usp=email ) Change subject: mgw: mdcx: Fix inc of non-existing ctr in mdcx counter group ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39758?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: If849e3922c7ae864771c41ff7c0fac37ef0280de Gerrit-Change-Number: 39758 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Mar 2025 11:08:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

4 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgw: Rearrange last steps of CRCX and MDCX

by pespin

pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39756?usp=email ) Change subject: mgw: Rearrange last steps of CRCX and MDCX ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39756?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I42acda16cb2d59a9b7aae06b7584d4dfc1e91f9e Gerrit-Change-Number: 39756 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 11 Mar 2025 11:08:35 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

4 months

1
0
0 0

2025

2024

2023

2022

gerrit-log March 2025