pespin has submitted this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/39224?usp=email )
Change subject: mgw: CRCX: Split mgcp header pars parsing into a previous step
......................................................................
mgw: CRCX: 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 CRCX.
Further work (commits) will follow for MDCX and DLCX.
Change-Id: I3ee5158c254213203830fe9c38de11c15b4b19c1
---
M include/osmocom/mgcp/mgcp_msg.h
M include/osmocom/mgcp/mgcp_protocol.h
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
4 files changed, 171 insertions(+), 109 deletions(-)
Approvals:
Jenkins Builder: Verified
neels: Looks good to me, approved
pespin: Looks good to me, approved
diff --git a/include/osmocom/mgcp/mgcp_msg.h b/include/osmocom/mgcp/mgcp_msg.h
index d11fb85..631a4ae 100644
--- a/include/osmocom/mgcp/mgcp_msg.h
+++ b/include/osmocom/mgcp/mgcp_msg.h
@@ -39,10 +39,11 @@
enum mgcp_connection_mode mgcp_parse_conn_mode(const char *msg);
int mgcp_parse_header(struct mgcp_parse_data *pdata, char *data);
+int mgcp_parse_hdr_pars(struct mgcp_parse_data *pdata);
int mgcp_parse_osmux_cid(const char *line);
-bool mgcp_check_param(const struct mgcp_endpoint *endp, struct mgcp_trunk
*trunk, const char *line);
+bool mgcp_check_param(const char *line);
int mgcp_verify_call_id(struct mgcp_endpoint *endp, const char *callid);
diff --git a/include/osmocom/mgcp/mgcp_protocol.h
b/include/osmocom/mgcp/mgcp_protocol.h
index 77f17d5..9cda8d6 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -3,12 +3,39 @@
#include <osmocom/core/utils.h>
#include <osmocom/mgcp/mgcp_common.h>
+#define MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET (-2)
+#define MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD (-1)
+
+struct mgcp_parse_hdr_pars {
+ const char *local_options;
+ const char *callid;
+ enum mgcp_connection_mode mode;
+ int remote_osmux_cid;
+ bool have_sdp;
+ /*! MGCP_X_OSMO_IGN_* flags from 'X-Osmo-IGN:' header */
+ uint32_t x_osmo_ign;
+};
+
+static inline void mgcp_parse_hdr_pars_init(struct mgcp_parse_hdr_pars *hpars)
+{
+ *hpars = (struct mgcp_parse_hdr_pars){
+ .local_options = NULL,
+ .callid = NULL,
+ .mode = MGCP_CONN_NONE,
+ .remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET,
+ .have_sdp = false,
+ .x_osmo_ign = 0,
+ };
+}
+
/* Internal structure while parsing a request */
struct mgcp_parse_data {
struct mgcp_config *cfg;
+ char *save;
+ /* MGCP Header: */
char *epname;
char *trans;
- char *save;
+ struct mgcp_parse_hdr_pars hpars;
};
/* Local connection options */
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index f8ef50b..bcbe258 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -23,6 +23,7 @@
*/
#include <limits.h>
+#include <ctype.h>
#include <osmocom/mgcp/mgcp.h>
#include <osmocom/mgcp/osmux.h>
@@ -33,6 +34,10 @@
#include <osmocom/mgcp/mgcp_endp.h>
#include <osmocom/mgcp/mgcp_trunk.h>
+/* (same fmt as LOGPENDP()) */
+#define LOG_MGCP_PDATA(PDATA, LEVEL, FMT, ARGS...) \
+ LOGP(DLMGCP, LEVEL, "endpoint:%s " FMT, (PDATA) ? ((PDATA)->epname ? :
"null-epname") : "null-pdata", ##ARGS)
+
/*! Display an mgcp message on the log output.
* \param[in] message mgcp message string
* \param[in] len message mgcp message string length
@@ -122,8 +127,7 @@
break;
case 2:
if (strcasecmp("MGCP", elem)) {
- LOGP(DLMGCP, LOGL_ERROR,
- "MGCP header parsing error\n");
+ LOG_MGCP_PDATA(pdata, LOGL_ERROR, "MGCP header
parsing error\n");
return -510;
}
break;
@@ -136,13 +140,89 @@
}
if (i != 4) {
- LOGP(DLMGCP, LOGL_ERROR, "MGCP status line too short.\n");
+ LOG_MGCP_PDATA(pdata, LOGL_ERROR, "MGCP status line too
short.\n");
return -510;
}
return 0;
}
+static bool parse_x_osmo_ign(struct mgcp_parse_data *pdata, char *line)
+{
+ char *saveptr = NULL;
+
+ if (strncasecmp(line, MGCP_X_OSMO_IGN_HEADER,
strlen(MGCP_X_OSMO_IGN_HEADER)))
+ return false;
+ line += strlen(MGCP_X_OSMO_IGN_HEADER);
+
+ while (1) {
+ char *token = strtok_r(line, " ", &saveptr);
+ line = NULL;
+ if (!token)
+ break;
+
+ if (!strcasecmp(token, "C"))
+ pdata->hpars.x_osmo_ign |= MGCP_X_OSMO_IGN_CALLID;
+ else
+ LOG_MGCP_PDATA(pdata, LOGL_ERROR, "received unknown
X-Osmo-IGN item '%s'\n", token);
+ }
+
+ return true;
+}
+
+/*! Analyze and parse the the header of an MGCP message string.
+ * \param[inout] pdata caller provided memory to store the parsing results.
+ * \returns 0 when parsing was successful, negative (MGCP cause code) on
error. */
+int mgcp_parse_hdr_pars(struct mgcp_parse_data *pdata)
+{
+ struct mgcp_parse_hdr_pars *hp = &pdata->hpars;
+ char *line;
+
+ mgcp_parse_hdr_pars_init(hp);
+
+ for_each_line(line, pdata->save) {
+ if (!mgcp_check_param(line)) {
+ LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "wrong MGCP option
format: '%s'\n", line);
+ continue;
+ }
+
+ switch (toupper(line[0])) {
+ case 'L':
+ hp->local_options = (const char *)line + 3;
+ break;
+ case 'C':
+ 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;
+ case 'M':
+ hp->mode = mgcp_parse_conn_mode((const char *)line + 3);
+ break;
+ case 'X':
+ if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: "))
== 0) {
+ hp->remote_osmux_cid =
mgcp_parse_osmux_cid(line);
+ break;
+ }
+ if (parse_x_osmo_ign(pdata, line))
+ break;
+ /* Ignore unknown X-headers */
+ break;
+ case '\0':
+ hp->have_sdp = true;
+ goto mgcp_header_done;
+ default:
+ LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "CRCX: unhandled
option: '%c'/%d\n", *line, *line);
+ return -539;
+ }
+ }
+
+mgcp_header_done:
+ return 0;
+}
+
/*! Extract OSMUX CID from an MGCP parameter line (string).
* \param[in] line single parameter line from the MGCP message
* \returns OSMUX CID, -1 wildcard, -2 on error */
@@ -153,19 +233,19 @@
if (strcasecmp(line + 2, "Osmux: *") == 0) {
LOGP(DLMGCP, LOGL_DEBUG, "Parsed wilcard Osmux CID\n");
- return -1;
+ return MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD;
}
if (sscanf(line + 2 + 7, "%u", &osmux_cid) != 1) {
LOGP(DLMGCP, LOGL_ERROR, "Failed parsing Osmux in MGCP msg
line: %s\n",
line);
- return -2;
+ return MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET;
}
if (osmux_cid > OSMUX_CID_MAX) {
LOGP(DLMGCP, LOGL_ERROR, "Osmux ID too large: %u > %u\n",
osmux_cid, OSMUX_CID_MAX);
- return -2;
+ return MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET;
}
LOGP(DLMGCP, LOGL_DEBUG, "MGCP client offered Osmux CID %u\n",
osmux_cid);
@@ -173,20 +253,13 @@
}
/*! Check MGCP parameter line (string) for plausibility.
- * \param[in] endp pointer to endpoint (only used for log output, may be NULL)
- * \param[in] trunk pointer to trunk (only used for log output, may be NULL
if endp is not NULL)
* \param[in] line single parameter line from the MGCP message
* \returns true when line seems plausible, false on error */
-bool mgcp_check_param(const struct mgcp_endpoint *endp, struct mgcp_trunk
*trunk, const char *line)
+bool mgcp_check_param(const char *line)
{
const size_t line_len = strlen(line);
- if (line[0] != '\0' && line_len < 2) {
- if (endp)
- LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "wrong MGCP option
format: '%s'\n", line);
- else
- LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, "wrong MGCP
option format: '%s'\n", line);
+ if (line[0] != '\0' && line_len < 2)
return false;
- }
/* FIXME: A couple more checks wouldn't hurt... */
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 32fbd43..b185435 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -702,6 +702,20 @@
codec->frame_duration_den;
}
+static int mgcp_trunk_osmux_init_if_needed(struct mgcp_trunk *trunk)
+{
+ if (trunk->cfg->osmux.initialized)
+ return 0;
+
+ if (osmux_init(trunk) < 0) {
+ LOGPTRUNK(trunk, DOSMUX, LOGL_ERROR, "Cannot init OSMUX\n");
+ return -3;
+ }
+ LOGPTRUNK(trunk, DOSMUX, LOGL_NOTICE, "OSMUX socket has been set up\n");
+
+ 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
@@ -709,13 +723,8 @@
*/
static int mgcp_osmux_setup(struct mgcp_endpoint *endp, const char *line)
{
- if (!endp->trunk->cfg->osmux.initialized) {
- if (osmux_init(endp->trunk) < 0) {
- LOGPENDP(endp, DOSMUX, LOGL_ERROR, "Cannot init
OSMUX\n");
- return -3;
- }
- LOGPENDP(endp, DOSMUX, LOGL_NOTICE, "OSMUX socket has been set
up\n");
- }
+ if (mgcp_trunk_osmux_init_if_needed(endp->trunk) < 0)
+ return -3;
return mgcp_parse_osmux_cid(line);
}
@@ -791,42 +800,16 @@
return 534;
}
-static bool parse_x_osmo_ign(struct mgcp_endpoint *endp, char *line)
-{
- char *saveptr = NULL;
-
- if (strncasecmp(line, MGCP_X_OSMO_IGN_HEADER,
strlen(MGCP_X_OSMO_IGN_HEADER)))
- return false;
- line += strlen(MGCP_X_OSMO_IGN_HEADER);
-
- while (1) {
- char *token = strtok_r(line, " ", &saveptr);
- line = NULL;
- if (!token)
- break;
-
- if (!strcasecmp(token, "C"))
- endp->x_osmo_ign |= MGCP_X_OSMO_IGN_CALLID;
- else
- LOGPENDP(endp, DLMGCP, LOGL_ERROR, "received unknown
X-Osmo-IGN item '%s'\n", token);
- }
-
- return true;
-}
-
/* CRCX command handler, processes the received command */
static struct msgb *handle_create_con(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;
int error_code = 400;
- const char *local_options = NULL;
- const char *callid = NULL;
- enum mgcp_connection_mode mode = MGCP_CONN_NONE;
- char *line;
- int have_sdp = 0, remote_osmux_cid = -2;
+ int remote_osmux_cid;
struct mgcp_conn *conn = NULL;
struct mgcp_conn_rtp *conn_rtp = NULL;
char conn_name[512];
@@ -857,69 +840,44 @@
}
/* parse CallID C: and LocalParameters L: */
- for_each_line(line, pdata->save) {
- if (!mgcp_check_param(endp, trunk, line))
- continue;
-
- switch (toupper(line[0])) {
- case 'L':
- local_options = (const char *)line + 3;
- break;
- case 'C':
- 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 */
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_CRCX_FAIL_BAD_ACTION));
- return create_err_response(rq->trunk, NULL, 523,
"CRCX", pdata->trans);
- 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 (rq->endp->trunk->cfg->osmux.usage ==
OSMUX_USAGE_OFF)
- break;
- remote_osmux_cid = mgcp_osmux_setup(endp, line);
- break;
- }
-
- if (parse_x_osmo_ign(endp, line))
- break;
-
- /* Ignore unknown X-headers */
- break;
- case '\0':
- have_sdp = 1;
- goto mgcp_header_done;
- default:
- LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
- "CRCX: unhandled option: '%c'/%d\n", *line,
*line);
- 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);
- break;
- }
+ rc = mgcp_parse_hdr_pars(pdata);
+ 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, -rc, "CRCX",
pdata->trans);
+ default:
+ return create_err_response(rq->trunk, NULL, -rc, "CRCX",
pdata->trans);
}
-mgcp_header_done:
/* Check parameters */
- if (!callid) {
+ 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 (mode == MGCP_CONN_NONE) {
+ 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);
}
+ remote_osmux_cid = hpars->remote_osmux_cid;
+ /* If osmux is disabled, just skip setting it up */
+ if (trunk->cfg->osmux.usage == OSMUX_USAGE_OFF)
+ remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET;
+
+ /* Make sure osmux is setup: */
+ if (remote_osmux_cid != MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET)
+ mgcp_trunk_osmux_init_if_needed(trunk);
+
/* Check if we are able to accept the creation of another connection */
if (mgcp_endp_is_full(endp)) {
LOGPENDP(endp, DLMGCP, LOGL_ERROR,
@@ -939,9 +897,12 @@
}
}
+ /* Update endp->x_osmo_ign: */
+ endp->x_osmo_ign |= hpars->x_osmo_ign;
+
/* 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 */
- if (endp->callid && mgcp_verify_call_id(endp, callid)) {
+ if (endp->callid && mgcp_verify_call_id(endp, hpars->callid)) {
LOGPENDP(endp, DLMGCP, LOGL_ERROR,
"CRCX: already seized by other call (%s)\n",
endp->callid);
@@ -961,14 +922,14 @@
/* Claim endpoint resources. This will also set the callid,
* creating additional connections will only be possible if
* the callid matches up (see above). */
- rc = mgcp_endp_claim(endp, callid);
+ rc = mgcp_endp_claim(endp, hpars->callid);
if (rc != 0) {
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_CRCX_FAIL_CLAIM));
return create_err_response(endp, endp, 502, "CRCX",
pdata->trans);
}
}
- snprintf(conn_name, sizeof(conn_name), "%s", callid);
+ snprintf(conn_name, sizeof(conn_name), "%s", hpars->callid);
conn = mgcp_conn_alloc(trunk->endpoints, endp, MGCP_CONN_TYPE_RTP,
conn_name);
if (!conn) {
LOGPENDP(endp, DLMGCP, LOGL_ERROR,
@@ -977,7 +938,7 @@
goto error2;
}
- if (mgcp_conn_set_mode(conn, mode) < 0) {
+ if (mgcp_conn_set_mode(conn, hpars->mode) < 0) {
error_code = 517;
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_CRCX_FAIL_INVALID_MODE));
goto error2;
@@ -1004,9 +965,9 @@
}
/* 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,
"CRCX: invalid local connection options!\n");
@@ -1017,7 +978,7 @@
}
/* Handle codec information and decide for a suitable codec */
- rc = handle_codec_info(conn_rtp, rq, have_sdp, true);
+ rc = handle_codec_info(conn_rtp, rq, hpars->have_sdp, true);
mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn));
if (rc) {
error_code = rc;
@@ -1131,7 +1092,7 @@
}
for_each_line(line, pdata->save) {
- if (!mgcp_check_param(endp, trunk, line))
+ if (!mgcp_check_param(line))
continue;
switch (toupper(line[0])) {
@@ -1357,7 +1318,7 @@
}
for_each_line(line, pdata->save) {
- if (!mgcp_check_param(endp, trunk, line))
+ if (!mgcp_check_param(line))
continue;
switch (toupper(line[0])) {
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39224?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: I3ee5158c254213203830fe9c38de11c15b4b19c1
Gerrit-Change-Number: 39224
Gerrit-PatchSet: 9
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
