pespin has uploaded this change for review.

View Change

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, 168 insertions(+), 107 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/24/39224/1
diff --git a/include/osmocom/mgcp/mgcp_msg.h b/include/osmocom/mgcp/mgcp_msg.h
index d11fb85..d56a36c 100644
--- a/include/osmocom/mgcp/mgcp_msg.h
+++ b/include/osmocom/mgcp/mgcp_msg.h
@@ -26,6 +26,7 @@

#include <stdint.h>
#include <stdbool.h>
+#include <ctype.h>

#include <osmocom/mgcp/mgcp_common.h>

@@ -39,10 +40,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..dd6e17c 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -3,12 +3,43 @@
#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,
+ };
+}
+
+//struct mgcp_parsed_body {
+//};
+
/* Internal structure while parsing a request */
struct mgcp_parse_data {
struct mgcp_config *cfg;
+ char *save;
+ /* MGCP Header: */
char *epname;
char *trans;
- char *save;
+ /* MGCP Body: */
+ 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..5d6a110 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -143,6 +143,80 @@
return 0;
}

+static bool parse_x_osmo_ign(struct mgcp_parse_hdr_pars *hpars, 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"))
+ hpars->x_osmo_ign |= MGCP_X_OSMO_IGN_CALLID;
+ else
+ LOGP(DLMGCP, LOGL_ERROR, "received unknown X-Osmo-IGN item '%s'\n", token);
+ }
+
+ return true;
+}
+
+/*! Analyze and parse the the hader of an MGCP messeage 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);
+ /* parse CallID C: and LocalParameters L: */
+ for_each_line(line, pdata->save) {
+ if (!mgcp_check_param(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(hp, line))
+ break;
+ /* Ignore unknown X-headers */
+ break;
+ case '\0':
+ hp->have_sdp = true;
+ goto mgcp_header_done;
+ default:
+ LOGP(DLMGCP, 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 +227,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,18 +247,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);
+ LOGP(DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line);
return false;
}

diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 32fbd43..d592cc9 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,15 @@
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;
struct mgcp_conn *conn = NULL;
struct mgcp_conn_rtp *conn_rtp = NULL;
char conn_name[512];
@@ -857,69 +839,43 @@
}

/* 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, 539, "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);
}

+ /* 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;
+
+ /* Make sure osmux is setup: */
+ if (hpars->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 +895,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 +920,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 +936,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;
@@ -987,14 +946,14 @@
OSMO_ASSERT(conn_rtp);

/* If X-Osmux (remote CID) was received (-1 is wilcard), alloc next avail CID as local CID */
- if (remote_osmux_cid >= -1) {
+ if (hpars->remote_osmux_cid >= -1) {
if (osmux_init_conn(conn_rtp) < 0) {
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_NO_OSMUX));
goto error2;
}
- if (remote_osmux_cid >= 0) {
+ if (hpars->remote_osmux_cid >= 0) {
conn_rtp->osmux.remote_cid_present = true;
- conn_rtp->osmux.remote_cid = remote_osmux_cid;
+ conn_rtp->osmux.remote_cid = hpars->remote_osmux_cid;
}
} else if (endp->trunk->cfg->osmux.usage == OSMUX_USAGE_ONLY) {
LOGPCONN(conn, DLMGCP, LOGL_ERROR,
@@ -1004,9 +963,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 +976,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 +1090,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 +1316,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 change 39224. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: newchange
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I3ee5158c254213203830fe9c38de11c15b4b19c1
Gerrit-Change-Number: 39224
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin@sysmocom.de>