pespin has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-mgw/+/39239?usp=email )
Change subject: mgw: MDCX: Split mgcp header pars parsing into a previous step
......................................................................
mgw: MDCX: Split mgcp header pars parsing into a previous step
Do most of the initial parsing and verification in a prior step, filling
in a "parsed" struct which simplifies logic coming after for different
message types.
This commit only modifies stuff enough to work for MDCX.
Further work (commits) will follow for DLCX.
Change-Id: I6ecb41fc2cc737c3a161b6bc98bd08ae01909655
---
M include/osmocom/mgcp/mgcp_protocol.h
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
3 files changed, 45 insertions(+), 90 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/39/39239/1
diff --git a/include/osmocom/mgcp/mgcp_protocol.h
b/include/osmocom/mgcp/mgcp_protocol.h
index 633f3e9..ffe7ae0 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -9,6 +9,7 @@
struct mgcp_parse_hdr_pars {
const char *local_options;
const char *callid;
+ const char *connid;
enum mgcp_connection_mode mode;
int remote_osmux_cid;
bool have_sdp;
@@ -21,6 +22,7 @@
*hpars = (struct mgcp_parse_hdr_pars){
.local_options = NULL,
.callid = NULL,
+ .connid = NULL,
.mode = MGCP_CONN_NONE,
.remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET,
.have_sdp = false,
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index 493c473..577914f 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -189,10 +189,8 @@
hp->callid = (const char *)line + 3;
break;
case 'I':
- /* It is illegal to send a connection identifier
- * together with a CRCX, the MGW will assign the
- * connection identifier by itself on CRCX */
- return 523;
+ hp->connid = (const char *)line + 3;
+ break;
case 'M':
hp->mode = mgcp_parse_conn_mode((const char *)line + 3);
break;
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index d592cc9..3751bc1 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -716,19 +716,6 @@
return 0;
}
-/*! Initializes osmux socket if not yet initialized. Parses Osmux CID from
MGCP line.
- * \param[in] endp Endpoint willing to initialize osmux
- * \param[in] line Line X-Osmux from MGCP header msg to parse
- * \returns OSMUX CID, -1 for wildcard, -2 on parse error, -3 on osmux
initalize error
- */
-static int mgcp_osmux_setup(struct mgcp_endpoint *endp, const char *line)
-{
- if (mgcp_trunk_osmux_init_if_needed(endp->trunk) < 0)
- return -3;
-
- return mgcp_parse_osmux_cid(line);
-}
-
/* Process codec information contained in CRCX/MDCX */
static int handle_codec_info(struct mgcp_conn_rtp *conn,
struct mgcp_request_data *rq, int have_sdp, bool
crcx)
@@ -843,9 +830,6 @@
switch (rc) {
case 0:
break; /* all good, continue below */
- case 523:
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_CRCX_FAIL_BAD_ACTION));
- return create_err_response(rq->trunk, NULL, rc, "CRCX",
pdata->trans);
case 539:
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_CRCX_FAIL_UNHANDLED_PARAM));
return create_err_response(rq->trunk, NULL, 539, "CRCX",
pdata->trans);
@@ -868,6 +852,15 @@
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);
+ }
+
/* If osmux is disabled, just skip setting it up */
if (trunk->cfg->osmux.usage == OSMUX_USAGE_OFF)
hpars->remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET;
@@ -1046,17 +1039,12 @@
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;
char new_local_addr[INET6_ADDRSTRLEN];
int error_code = 500;
- int have_sdp = 0;
- char *line;
- const char *local_options = NULL;
- enum mgcp_connection_mode mode = MGCP_CONN_NONE;
struct mgcp_conn *conn = NULL;
struct mgcp_conn_rtp *conn_rtp = NULL;
- const char *conn_id = NULL;
- int remote_osmux_cid = -2;
int rc;
LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "MDCX: modifying existing
connection ...\n");
@@ -1089,64 +1077,33 @@
return create_err_response(endp, endp, 400, "MDCX",
pdata->trans);
}
- for_each_line(line, pdata->save) {
- if (!mgcp_check_param(line))
- continue;
-
- switch (toupper(line[0])) {
- case 'C':
- if (mgcp_verify_call_id(endp, line + 3) != 0) {
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_MDCX_FAIL_INVALID_CALLID));
- error_code = 516;
- goto error3;
- }
- break;
- case 'I':
- conn_id = (const char *)line + 3;
- if ((error_code = mgcp_verify_ci(endp, conn_id))) {
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_MDCX_FAIL_INVALID_CONNID));
- goto error3;
- }
- break;
- case 'L':
- local_options = (const char *)line + 3;
- break;
- case 'M':
- mode = mgcp_parse_conn_mode((const char *)line + 3);
- break;
- case 'X':
- if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: "))
== 0) {
- /* If osmux is disabled, just skip setting it
up */
- if (endp->trunk->cfg->osmux.usage ==
OSMUX_USAGE_OFF)
- break;
- remote_osmux_cid = mgcp_osmux_setup(endp, line);
- break;
- }
- /* Ignore unknown X-headers */
- break;
- case '\0':
- have_sdp = 1;
- goto mgcp_header_done;
- break;
- default:
- LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
- "MDCX: Unhandled MGCP option: '%c'/%d\n",
- line[0], line[0]);
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_MDCX_FAIL_UNHANDLED_PARAM));
- return create_err_response(rq->trunk, NULL, 539,
"MDCX", pdata->trans);
- break;
- }
+ 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));
+ return create_err_response(rq->trunk, NULL, 539, "MDCX",
pdata->trans);
+ default:
+ return create_err_response(rq->trunk, NULL, rc, "MDCX",
pdata->trans);
}
-mgcp_header_done:
- if (!conn_id) {
+ if (hpars->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);
}
- conn = mgcp_endp_get_conn(endp, conn_id);
+ 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));
return create_err_response(endp, endp, 400, "MDCX",
pdata->trans);
@@ -1154,20 +1111,18 @@
mgcp_conn_watchdog_kick(conn);
- if (mode != MGCP_CONN_NONE) {
- if (mgcp_conn_set_mode(conn, mode) < 0) {
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_MDCX_FAIL_INVALID_MODE));
- error_code = 517;
- goto error3;
- }
- } else {
+ if (hpars->mode == MGCP_CONN_NONE) {
+ /* Reset conn mode in case it was tweaked through VTY: */
conn->mode = conn->mode_orig;
+ } else if (mgcp_conn_set_mode(conn, hpars->mode) < 0) {
+ rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_MDCX_FAIL_INVALID_MODE));
+ return create_err_response(endp, endp, 517, "MDCX",
pdata->trans);
}
/* Set local connection options, if present */
- if (local_options) {
+ if (hpars->local_options) {
rc = set_local_cx_options(trunk->endpoints,
- &endp->local_options, local_options);
+ &endp->local_options,
hpars->local_options);
if (rc != 0) {
LOGPCONN(conn, DLMGCP, LOGL_ERROR,
"MDCX: invalid local connection options!\n");
@@ -1181,7 +1136,7 @@
OSMO_ASSERT(conn_rtp);
/* Handle codec information and decide for a suitable codec */
- rc = handle_codec_info(conn_rtp, rq, have_sdp, false);
+ rc = handle_codec_info(conn_rtp, rq, hpars->have_sdp, false);
mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn));
if (rc) {
error_code = rc;
@@ -1195,22 +1150,22 @@
if (mgcp_conn_rtp_is_osmux(conn_rtp)) {
OSMO_ASSERT(conn_rtp->osmux.local_cid_allocated);
- if (remote_osmux_cid < -1) {
+ if (hpars->remote_osmux_cid < -1) {
LOGPCONN(conn, DLMGCP, LOGL_ERROR,
"MDCX: Failed to parse Osmux CID!\n");
goto error3;
- } else if (remote_osmux_cid == -1) {
+ } else if (hpars->remote_osmux_cid == -1) {
LOGPCONN(conn, DLMGCP, LOGL_ERROR,
"MDCX: wilcard in MDCX is not supported!\n");
goto error3;
} else if (conn_rtp->osmux.remote_cid_present &&
- remote_osmux_cid != conn_rtp->osmux.remote_cid) {
+ hpars->remote_osmux_cid !=
conn_rtp->osmux.remote_cid) {
LOGPCONN(conn, DLMGCP, LOGL_ERROR,
"MDCX: changing already allocated CID is not
supported!\n");
goto error3;
} else {
conn_rtp->osmux.remote_cid_present = true;
- conn_rtp->osmux.remote_cid = remote_osmux_cid;
+ 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");
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39239?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: I6ecb41fc2cc737c3a161b6bc98bd08ae01909655
Gerrit-Change-Number: 39239
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
