[L] Change in osmo-mgw[master]: mgw: CRCX: Split mgcp header pars parsing into a previous step - gerrit-log

24 Dec 2024

pespin has uploaded this change for review. ( 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, 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 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: 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



    