Change in osmo-mgw[master]: mgcp_protocol: refactor MGCP request handling

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

dexter gerrit-no-reply at lists.osmocom.org
Wed Jul 14 15:49:27 UTC 2021


dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/24941 )


Change subject: mgcp_protocol: refactor MGCP request handling
......................................................................

mgcp_protocol: refactor MGCP request handling

At the moment the MGCP request handling and message parsing is not
clearly separated. The function mgcp_parse_header() in mgcp_msg.c is
also responsible for resolving an endpoint. This leads to unclear layer
separation. We eventually end up in a situation where we can not execute
any request handler without beeing able to resolve an endpoint, however
this is necessary if we want to implement wildcarded DLCX resquests.

In the current situation a wildcarded DLCX is not possible to implement
as we always have to resolve a an to get to the trunk which we need to
iterate. However, we just can't resolve a free endpoint in a situation
where all endpoints on te trunk are in use.

We have to refactor the request handler so that the parsing in mgcp_msg
only extracts us the endpoint name. The resolving is then done in
mgcp_handle_message() in mgcp_protocol.c. Then we are able to decide
what to do if we are unable to resolve an endpoint but still be able to
resolve the trunk.

This patch does not change the behaviour of osmo-mgw yet, but it lays
the foundation for request handler implementations that can still
perform useful actions if no endpoint but a trunk has been resolved. A
wilcarded DLCX is such a case. It does not need an endpoint, just the
trunk.

Change-Id: I9f519d8a0ee8a513fa1e74acf3ee7dbc0991cdde
Related: SYS#5535
---
M include/osmocom/mgcp/mgcp_protocol.h
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
4 files changed, 190 insertions(+), 126 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/41/24941/1

diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h
index cdce02d..7ab283d 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -3,7 +3,7 @@
 /* Internal structure while parsing a request */
 struct mgcp_parse_data {
 	struct mgcp_config *cfg;
-	struct mgcp_endpoint *endp;
+	char *epname;
 	char *trans;
 	char *save;
 };
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index 8783e20..fdd2e0d 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -132,10 +132,10 @@
 }
 
 /*! Analyze and parse the the hader of an MGCP messeage string.
- *  \param[out] pdata caller provided memory to store the parsing results
- *  \param[in] data mgcp message string
- *  \returns when the status line was complete and transaction_id and
- *  endp out parameters are set, -1 on error */
+ *  \param[out] pdata caller provided memory to store the parsing results.
+ *  \param[in] data mgcp message string.
+ *  \returns 0 when the status line was complete and parseable, negative (MGCP
+ *  cause code) on error. */
 int mgcp_parse_header(struct mgcp_parse_data *pdata, char *data)
 {
 	int i = 0;
@@ -143,10 +143,7 @@
 	int cause;
 
 	/*! This function will parse the header part of the received
-	 *  MGCP message. The parsing results are stored in pdata.
-	 *  The function will also automatically search the pool with
-	 *  available endpoints in order to find an endpoint that matches
-	 *  the endpoint string in in the header */
+	 *  MGCP message. The parsing results are stored in pdata. */
 
 	OSMO_ASSERT(data);
 	pdata->trans = "000000";
@@ -158,26 +155,20 @@
 			pdata->trans = elem;
 			break;
 		case 1:
-			pdata->endp = mgcp_endp_by_name(&cause, elem, pdata->cfg);
-			if (!pdata->endp) {
-				LOGP(DLMGCP, LOGL_ERROR,
-				     "Unable to find Endpoint `%s'\n", elem);
-				OSMO_ASSERT(cause < 0);
-				return cause;
-			}
+			pdata->epname = elem;
 			break;
 		case 2:
 			if (strcasecmp("MGCP", elem)) {
 				LOGP(DLMGCP, LOGL_ERROR,
 				     "MGCP header parsing error\n");
-				return -510;
+				cause = -510;
+				goto error;
 			}
 			break;
 		case 3:
 			if (strcmp("1.0", elem)) {
-				LOGP(DLMGCP, LOGL_ERROR, "MGCP version `%s' "
-				     "not supported\n", elem);
-				return -528;
+				cause = -528;
+				goto error;
 			}
 			break;
 		}
@@ -186,12 +177,15 @@
 
 	if (i != 4) {
 		LOGP(DLMGCP, LOGL_ERROR, "MGCP status line too short.\n");
-		pdata->trans = "000000";
-		pdata->endp = NULL;
-		return -510;
+		cause = -510;
+		goto error;
 	}
 
 	return 0;
+error:
+	pdata->trans = "000000";
+	pdata->epname = NULL;
+	return cause;
 }
 
 /*! Extract OSMUX CID from an MGCP parameter line (string).
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index dbbf308..7f6fa69 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -46,6 +46,24 @@
 #include <osmocom/mgcp/mgcp_codec.h>
 #include <osmocom/mgcp/mgcp_conn.h>
 
+/* Request data passed to the request handler */
+struct mgcp_request_data {
+	/* request name (e.g. "MDCX") */
+	const char *name;
+
+	/* parsing results from the MGCP header (trans id, endpoint name ...) */
+	struct mgcp_parse_data *pdata;
+
+	/* pointer to endpoint resource (may be NULL, handler must check!) */
+	struct mgcp_endpoint *endp;
+
+	/* pointer to trunk resource (may be NULL, handler must check!) */
+	struct mgcp_trunk *trunk;
+
+	/* contains cause code in case of problems during endp/trunk resolution */
+	int mgcp_cause;
+};
+
 /* Request handler specification, here we specify an array with function
  * pointers to the various MGCP requests implemented below */
 struct mgcp_request {
@@ -53,7 +71,11 @@
 	char *name;
 
 	/* function pointer to the request handler */
-	struct msgb *(*handle_request) (struct mgcp_parse_data * data);
+	struct msgb *(*handle_request) (struct mgcp_request_data * data);
+
+	/* true if the request requires an endpoint, false if only a trunk
+	 * is sufficient. (corner cases, e.g. wildcarded DLCX) */
+	bool require_endp;
 
 	/* a humen readable name that describes the request */
 	char *debug_name;
@@ -68,24 +90,30 @@
 static const struct mgcp_request mgcp_requests[] = {
 	{ .name = "AUEP",
 	  .handle_request = handle_audit_endpoint,
-	  .debug_name = "AuditEndpoint" },
+	  .debug_name = "AuditEndpoint",
+	  .require_endp = true },
 	{ .name = "CRCX",
 	  .handle_request = handle_create_con,
-	  .debug_name = "CreateConnection" },
+	  .debug_name = "CreateConnection",
+	  .require_endp = true },
 	{ .name = "DLCX",
 	  .handle_request = handle_delete_con,
-	  .debug_name = "DeleteConnection" },
+	  .debug_name = "DeleteConnection",
+	  .require_endp = true },
 	{ .name = "MDCX",
 	  .handle_request = handle_modify_con,
-	  .debug_name = "ModifiyConnection" },
+	  .debug_name = "ModifiyConnection",
+	  .require_endp = true },
 	{ .name = "RQNT",
 	  .handle_request = handle_noti_req,
-	  .debug_name = "NotificationRequest" },
+	  .debug_name = "NotificationRequest",
+	  .require_endp = true },
 
 	/* SPEC extension */
 	{ .name = "RSIP",
 	  .handle_request = handle_rsip,
-	  .debug_name = "ReSetInProgress" },
+	  .debug_name = "ReSetInProgress",
+	  .require_endp = true },
 };
 
 /* Initalize transcoder */
@@ -295,6 +323,7 @@
 {
 	struct rate_ctr_group *rate_ctrs = cfg->ratectr.mgcp_general_ctr_group;
 	struct mgcp_parse_data pdata;
+	struct mgcp_request_data rq;
 	int rc, i, code, handled = 0;
 	struct msgb *resp = NULL;
 	char *data;
@@ -324,53 +353,91 @@
 
 	msg->l3h = &msg->l2h[4];
 
-	/*
-	 * Check for a duplicate message and respond.
-	 */
+	/* Parse message, extract endpoint name and transaction identifier */
 	memset(&pdata, 0, sizeof(pdata));
 	pdata.cfg = cfg;
 	data = mgcp_strline((char *)msg->l3h, &pdata.save);
+	rq.name = (const char *)&msg->l2h[0];
 	rc = mgcp_parse_header(&pdata, data);
-	if (pdata.endp && pdata.trans
-	    && pdata.endp->last_trans
-	    && strcmp(pdata.endp->last_trans, pdata.trans) == 0) {
-		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_RETRANSMITTED));
-		return do_retransmission(pdata.endp);
-	}
-
-	/* check for general parser failure */
 	if (rc < 0) {
-		LOGP(DLMGCP, LOGL_NOTICE, "%s: failed to find the endpoint\n", msg->l2h);
-		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_FAIL_NO_ENDPOINT));
-		return create_err_response(NULL, -rc, (const char *) msg->l2h, pdata.trans);
+		LOGP(DLMGCP, LOGL_ERROR, "%s: failed to parse MCGP message\n", rq.name);
+		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_FAIL_MSG_PARSE));
+		return create_err_response(NULL, -rc, rq.name, pdata.trans);
 	}
 
-	for (i = 0; i < ARRAY_SIZE(mgcp_requests); ++i) {
-		if (strncmp
-		    (mgcp_requests[i].name, (const char *)&msg->l2h[0],
-		     4) == 0) {
+	/* Locate endpoint and trunk, if no endpoint can be located try at least to identify the trunk. */
+	rq.pdata = &pdata;
+	rq.endp = mgcp_endp_by_name(&rc, pdata.epname, pdata.cfg);
+	if (!rq.endp) {
+		LOGP(DLMGCP, LOGL_ERROR,
+		     "%s: failed to find endpoint \"%s\", cause=%d -- trying to identify trunk...\n", rq.name,
+		     pdata.epname, -rc);
+		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_FAIL_NO_ENDPOINT));
+
+		/* If we are unable to find the endpoint we still may be able to identify the trunk. Some request
+		 * handler will still be able to perform a useful action if the request refers to the whole trunk
+		 * (wildcarded) */
+		rq.trunk = mgcp_trunk_by_name(pdata.cfg, pdata.epname);
+		if (!rq.trunk) {
+			LOGP(DLMGCP, LOGL_ERROR, "%s: failed to identify trunk for endpoint \"%s\" -- abort\n",
+			     rq.name, pdata.epname);
+			return create_err_response(NULL, -rc, rq.name, pdata.trans);
+		}
+		rq.mgcp_cause = rc;
+	} else {
+		rq.trunk = rq.endp->trunk;
+		rq.mgcp_cause = 0;
+
+		/* Check if we have to retransmit a response from a previous transaction */
+		if (pdata.trans && rq.endp->last_trans && strcmp(rq.endp->last_trans, pdata.trans) == 0) {
+			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_RETRANSMITTED));
+			return do_retransmission(rq.endp);
+		}
+	}
+
+	/* Find an approriate handler for the current request and execute it */
+	for (i = 0; i < ARRAY_SIZE(mgcp_requests); i++) {
+		if (strncmp(mgcp_requests[i].name, rq.name, 4) == 0) {
+			/* Check if the request requires and endpoint, if yes, check if we have it, otherwise don't
+			 * execute the request handler. */
+			if (mgcp_requests[i].require_endp && !rq.endp) {
+				LOGP(DLMGCP, LOGL_ERROR,
+				     "%s: the request handler \"%s\" requires an endpoint resource for \"%s\", which is not available -- abort\n",
+				     rq.name, mgcp_requests[i].debug_name, pdata.epname);
+				return create_err_response(NULL, -rq.mgcp_cause, rq.name, pdata.trans);
+			}
+
+			/* Execute request handler */
+			if (rq.endp)
+				LOGP(DLMGCP, LOGL_NOTICE,
+				     "%s: executing request handler \"%s\" for endpoint resource \"%s\"\n", rq.name,
+				     mgcp_requests[i].debug_name, rq.endp->name);
+			else
+				LOGP(DLMGCP, LOGL_NOTICE,
+				     "%s: executing request handler \"%s\" for trunk resource of endpoint \"%s\"\n",
+				     rq.name, mgcp_requests[i].debug_name, pdata.epname);
+			resp = mgcp_requests[i].handle_request(&rq);
 			handled = 1;
-			resp = mgcp_requests[i].handle_request(&pdata);
 			break;
 		}
 	}
 
+	/* Check if the MGCP request was handled and increment rate counters accordingly. */
 	if (handled) {
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_HANDLED));
 	} else {
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_UNHANDLED));
-		LOGP(DLMGCP, LOGL_NOTICE, "MSG with type: '%.4s' not handled\n",
-		     &msg->l2h[0]);
+		LOGP(DLMGCP, LOGL_ERROR, "MSG with type: '%.4s' not handled\n", &msg->l2h[0]);
 	}
 
 	return resp;
 }
 
 /* AUEP command handler, processes the received command */
-static struct msgb *handle_audit_endpoint(struct mgcp_parse_data *p)
+static struct msgb *handle_audit_endpoint(struct mgcp_request_data *rq)
 {
-	LOGPENDP(p->endp, DLMGCP, LOGL_NOTICE, "AUEP: auditing endpoint ...\n");
-	return create_ok_response(p->endp, 200, "AUEP", p->trans);
+	LOGPENDP(rq->endp, DLMGCP, LOGL_NOTICE, "AUEP: auditing endpoint ...\n");
+	return create_ok_response(rq->endp, 200, "AUEP", rq->pdata->trans);
 }
 
 /* Try to find a free port by attempting to bind on it. Also handle the
@@ -661,9 +728,9 @@
 
 /* Process codec information contained in CRCX/MDCX */
 static int handle_codec_info(struct mgcp_conn_rtp *conn,
-			     struct mgcp_parse_data *p, int have_sdp, bool crcx)
+			     struct mgcp_request_data *rq, int have_sdp, bool crcx)
 {
-	struct mgcp_endpoint *endp = p->endp;
+	struct mgcp_endpoint *endp = rq->endp;
 	int rc;
 	char *cmd;
 
@@ -677,7 +744,7 @@
 		/* If we have SDP, we ignore the local connection options and
 		 * use only the SDP information. */
 		mgcp_codec_reset_all(conn);
-		rc = mgcp_parse_sdp_data(endp, conn, p);
+		rc = mgcp_parse_sdp_data(endp, conn, rq->pdata);
 		if (rc != 0) {
 			LOGPCONN(conn->conn, DLMGCP,  LOGL_ERROR,
 				 "%s: sdp not parseable\n", cmd);
@@ -743,10 +810,11 @@
 }
 
 /* CRCX command handler, processes the received command */
-static struct msgb *handle_create_con(struct mgcp_parse_data *p)
+static struct msgb *handle_create_con(struct mgcp_request_data *rq)
 {
-	struct mgcp_trunk *trunk = p->endp->trunk;
-	struct mgcp_endpoint *endp = p->endp;
+	struct mgcp_parse_data *pdata = rq->pdata;
+	struct mgcp_trunk *trunk = rq->trunk;
+	struct mgcp_endpoint *endp = rq->endp;
 	struct rate_ctr_group *rate_ctrs = trunk->ratectr.mgcp_crcx_ctr_group;
 	int error_code = 400;
 	const char *local_options = NULL;
@@ -765,11 +833,11 @@
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_AVAIL));
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "CRCX: selected endpoint not available!\n");
-		return create_err_response(NULL, 501, "CRCX", p->trans);
+		return create_err_response(NULL, 501, "CRCX", pdata->trans);
 	}
 
 	/* parse CallID C: and LocalParameters L: */
-	for_each_line(line, p->save) {
+	for_each_line(line, pdata->save) {
 		if (!mgcp_check_param(endp, line))
 			continue;
 
@@ -785,7 +853,7 @@
 			 * 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(NULL, 523, "CRCX", p->trans);
+			return create_err_response(NULL, 523, "CRCX", pdata->trans);
 			break;
 		case 'M':
 			mode = (const char *)line + 3;
@@ -793,7 +861,7 @@
 		case 'X':
 			if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) {
 				/* If osmux is disabled, just skip setting it up */
-				if (!p->endp->cfg->osmux)
+				if (!rq->endp->cfg->osmux)
 					break;
 				osmux_cid = mgcp_osmux_setup(endp, line);
 				break;
@@ -811,7 +879,7 @@
 			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(NULL, 539, "CRCX", p->trans);
+			return create_err_response(NULL, 539, "CRCX", pdata->trans);
 			break;
 		}
 	}
@@ -822,14 +890,14 @@
 		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, 516, "CRCX", p->trans);
+		return create_err_response(endp, 516, "CRCX", pdata->trans);
 	}
 
 	if (!mode) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "CRCX: insufficient parameters, missing mode\n");
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE));
-		return create_err_response(endp, 517, "CRCX", p->trans);
+		return create_err_response(endp, 517, "CRCX", pdata->trans);
 	}
 
 	/* Check if we are able to accept the creation of another connection */
@@ -846,7 +914,7 @@
 			/* 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, 540, "CRCX", p->trans);
+			return create_err_response(endp, 540, "CRCX", pdata->trans);
 		}
 	}
 
@@ -864,7 +932,7 @@
 			/* 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, 400, "CRCX", p->trans);
+			return create_err_response(endp, 400, "CRCX", pdata->trans);
 		}
 	}
 
@@ -875,7 +943,7 @@
 		rc = mgcp_endp_claim(endp, callid);
 		if (rc != 0) {
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_CLAIM));
-			return create_err_response(endp, 502, "CRCX", p->trans);
+			return create_err_response(endp, 502, "CRCX", pdata->trans);
 		}
 	}
 
@@ -928,7 +996,7 @@
 	}
 
 	/* Handle codec information and decide for a suitable codec */
-	rc = handle_codec_info(conn, p, have_sdp, true);
+	rc = handle_codec_info(conn, rq, have_sdp, true);
 	mgcp_codec_summary(conn);
 	if (rc) {
 		error_code = rc;
@@ -939,8 +1007,8 @@
 	conn->end.fmtp_extra = talloc_strdup(trunk->endpoints,
 					     trunk->audio_fmtp_extra);
 
-	if (p->cfg->force_ptime) {
-		conn->end.packet_duration_ms = p->cfg->force_ptime;
+	if (pdata->cfg->force_ptime) {
+		conn->end.packet_duration_ms = pdata->cfg->force_ptime;
 		conn->end.force_output_ptime = 1;
 	}
 
@@ -973,16 +1041,16 @@
 	}
 
 	/* policy CB */
-	if (p->cfg->policy_cb) {
+	if (pdata->cfg->policy_cb) {
 		int rc;
-		rc = p->cfg->policy_cb(endp, MGCP_ENDP_CRCX, p->trans);
+		rc = pdata->cfg->policy_cb(endp, MGCP_ENDP_CRCX, pdata->trans);
 		switch (rc) {
 		case MGCP_POLICY_REJECT:
 			LOGPCONN(_conn, DLMGCP, LOGL_NOTICE,
 				 "CRCX: CRCX rejected by policy\n");
 			mgcp_endp_release(endp);
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_REJECTED_BY_POLICY));
-			return create_err_response(endp, 400, "CRCX", p->trans);
+			return create_err_response(endp, 400, "CRCX", pdata->trans);
 			break;
 		case MGCP_POLICY_DEFER:
 			/* stop processing */
@@ -996,8 +1064,8 @@
 
 	LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG,
 		 "CRCX: Creating connection: port: %u\n", conn->end.local_port);
-	if (p->cfg->change_cb)
-		p->cfg->change_cb(endp, MGCP_ENDP_CRCX);
+	if (pdata->cfg->change_cb)
+		pdata->cfg->change_cb(endp, MGCP_ENDP_CRCX);
 
 	/* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */
 	OSMO_ASSERT(trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE);
@@ -1010,19 +1078,20 @@
 		 "CRCX: connection successfully created\n");
 	rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_SUCCESS));
 	mgcp_endp_update(endp);
-	return create_response_with_sdp(endp, conn, "CRCX", p->trans, true);
+	return create_response_with_sdp(endp, conn, "CRCX", pdata->trans, true);
 error2:
 	mgcp_endp_release(endp);
 	LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
 		 "CRCX: unable to create connection\n");
-	return create_err_response(endp, error_code, "CRCX", p->trans);
+	return create_err_response(endp, error_code, "CRCX", pdata->trans);
 }
 
 /* MDCX command handler, processes the received command */
-static struct msgb *handle_modify_con(struct mgcp_parse_data *p)
+static struct msgb *handle_modify_con(struct mgcp_request_data *rq)
 {
-	struct mgcp_endpoint *endp = p->endp;
-	struct rate_ctr_group *rate_ctrs = endp->trunk->ratectr.mgcp_mdcx_ctr_group;
+	struct mgcp_parse_data *pdata = rq->pdata;
+	struct mgcp_endpoint *endp = rq->endp;
+	struct rate_ctr_group *rate_ctrs = endp->trunk->ratectr.mgcp_mdcx_ctr_group;;
 	char new_local_addr[INET6_ADDRSTRLEN];
 	int error_code = 500;
 	int silent = 0;
@@ -1041,7 +1110,7 @@
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_AVAIL));
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "MDCX: selected endpoint not available!\n");
-		return create_err_response(NULL, 501, "MDCX", p->trans);
+		return create_err_response(NULL, 501, "MDCX", pdata->trans);
 	}
 
 	/* Prohibit wildcarded requests */
@@ -1049,17 +1118,17 @@
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "MDCX: wildcarded endpoint names not supported.\n");
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_WILDCARD));
-		return create_err_response(endp, 507, "MDCX", p->trans);
+		return create_err_response(endp, 507, "MDCX", pdata->trans);
 	}
 
 	if (llist_count(&endp->conns) <= 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, 400, "MDCX", p->trans);
+		return create_err_response(endp, 400, "MDCX", pdata->trans);
 	}
 
-	for_each_line(line, p->save) {
+	for_each_line(line, pdata->save) {
 		if (!mgcp_check_param(endp, line))
 			continue;
 
@@ -1090,7 +1159,7 @@
 		case 'X':
 			if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) {
 				/* If osmux is disabled, just skip setting it up */
-				if (!p->endp->cfg->osmux)
+				if (!endp->cfg->osmux)
 					break;
 				osmux_cid = mgcp_osmux_setup(endp, line);
 				break;
@@ -1106,7 +1175,7 @@
 				 "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(NULL, 539, "MDCX", p->trans);
+			return create_err_response(NULL, 539, "MDCX", pdata->trans);
 			break;
 		}
 	}
@@ -1116,13 +1185,13 @@
 		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, 515, "MDCX", p->trans);
+		return create_err_response(endp, 515, "MDCX", pdata->trans);
 	}
 
 	conn = mgcp_conn_get_rtp(endp, conn_id);
 	if (!conn) {
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_CONN_NOT_FOUND));
-		return create_err_response(endp, 400, "MDCX", p->trans);
+		return create_err_response(endp, 400, "MDCX", pdata->trans);
 	}
 
 	mgcp_conn_watchdog_kick(conn->conn);
@@ -1150,7 +1219,7 @@
 	}
 
 	/* Handle codec information and decide for a suitable codec */
-	rc = handle_codec_info(conn, p, have_sdp, false);
+	rc = handle_codec_info(conn, rq, have_sdp, false);
 	mgcp_codec_summary(conn);
 	if (rc) {
 		error_code = rc;
@@ -1209,9 +1278,9 @@
 
 
 	/* policy CB */
-	if (p->cfg->policy_cb) {
+	if (pdata->cfg->policy_cb) {
 		int rc;
-		rc = p->cfg->policy_cb(endp, MGCP_ENDP_MDCX, p->trans);
+		rc = pdata->cfg->policy_cb(endp, MGCP_ENDP_MDCX, pdata->trans);
 		switch (rc) {
 		case MGCP_POLICY_REJECT:
 			LOGPCONN(conn->conn, DLMGCP, LOGL_NOTICE,
@@ -1219,7 +1288,7 @@
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_REJECTED_BY_POLICY));
 			if (silent)
 				goto out_silent;
-			return create_err_response(endp, 400, "MDCX", p->trans);
+			return create_err_response(endp, 400, "MDCX", pdata->trans);
 			break;
 		case MGCP_POLICY_DEFER:
 			/* stop processing */
@@ -1239,8 +1308,8 @@
 	/* modify */
 	LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG,
 		 "MDCX: modified conn:%s\n", mgcp_conn_dump(conn->conn));
-	if (p->cfg->change_cb)
-		p->cfg->change_cb(endp, MGCP_ENDP_MDCX);
+	if (pdata->cfg->change_cb)
+		pdata->cfg->change_cb(endp, MGCP_ENDP_MDCX);
 
 	/* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */
 	OSMO_ASSERT(endp->trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE);
@@ -1256,9 +1325,9 @@
 	LOGPCONN(conn->conn, DLMGCP, LOGL_NOTICE,
 		 "MDCX: connection successfully modified\n");
 	mgcp_endp_update(endp);
-	return create_response_with_sdp(endp, conn, "MDCX", p->trans, false);
+	return create_response_with_sdp(endp, conn, "MDCX", pdata->trans, false);
 error3:
-	return create_err_response(endp, error_code, "MDCX", p->trans);
+	return create_err_response(endp, error_code, "MDCX", pdata->trans);
 
 out_silent:
 	LOGPENDP(endp, DLMGCP, LOGL_DEBUG, "MDCX: silent exit\n");
@@ -1266,10 +1335,11 @@
 }
 
 /* DLCX command handler, processes the received command */
-static struct msgb *handle_delete_con(struct mgcp_parse_data *p)
+static struct msgb *handle_delete_con(struct mgcp_request_data *rq)
 {
-	struct mgcp_endpoint *endp = p->endp;
-	struct rate_ctr_group *rate_ctrs = endp->trunk->ratectr.mgcp_dlcx_ctr_group;
+	struct mgcp_parse_data *pdata = rq->pdata;
+	struct mgcp_endpoint *endp = rq->endp;;
+	struct rate_ctr_group *rate_ctrs = endp->trunk->ratectr.mgcp_dlcx_ctr_group;;
 	int error_code = 400;
 	int silent = 0;
 	char *line;
@@ -1284,7 +1354,7 @@
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_AVAIL));
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "DLCX: selected endpoint not available!\n");
-		return create_err_response(NULL, 501, "DLCX", p->trans);
+		return create_err_response(NULL, 501, "DLCX", pdata->trans);
 	}
 
 	/* Prohibit wildcarded requests */
@@ -1292,17 +1362,17 @@
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "DLCX: wildcarded endpoint names not supported.\n");
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_WILDCARD));
-		return create_err_response(endp, 507, "DLCX", p->trans);
+		return create_err_response(endp, 507, "DLCX", pdata->trans);
 	}
 
 	if (llist_count(&endp->conns) <= 0) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "DLCX: endpoint is not holding a connection.\n");
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_NO_CONN));
-		return create_err_response(endp, 515, "DLCX", p->trans);
+		return create_err_response(endp, 515, "DLCX", pdata->trans);
 	}
 
-	for_each_line(line, p->save) {
+	for_each_line(line, pdata->save) {
 		if (!mgcp_check_param(endp, line))
 			continue;
 
@@ -1329,22 +1399,22 @@
 				 "DLCX: Unhandled MGCP option: '%c'/%d\n",
 				 line[0], line[0]);
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
-			return create_err_response(NULL, 539, "DLCX", p->trans);
+			return create_err_response(NULL, 539, "DLCX", pdata->trans);
 			break;
 		}
 	}
 
 	/* policy CB */
-	if (p->cfg->policy_cb) {
+	if (pdata->cfg->policy_cb) {
 		int rc;
-		rc = p->cfg->policy_cb(endp, MGCP_ENDP_DLCX, p->trans);
+		rc = pdata->cfg->policy_cb(endp, MGCP_ENDP_DLCX, pdata->trans);
 		switch (rc) {
 		case MGCP_POLICY_REJECT:
 			LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "DLCX: rejected by policy\n");
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_REJECTED_BY_POLICY));
 			if (silent)
 				goto out_silent;
-			return create_err_response(endp, 400, "DLCX", p->trans);
+			return create_err_response(endp, 400, "DLCX", pdata->trans);
 			break;
 		case MGCP_POLICY_DEFER:
 			/* stop processing */
@@ -1374,7 +1444,7 @@
 		/* Note: In this case we do not return any statistics,
 		 * as we assume that the client is not interested in
 		 * this case. */
-		return create_ok_response(endp, 200, "DLCX", p->trans);
+		return create_ok_response(endp, 200, "DLCX", pdata->trans);
 	}
 
 	/* Find the connection */
@@ -1400,16 +1470,16 @@
 		LOGPENDP(endp, DLMGCP, LOGL_DEBUG, "DLCX: endpoint released\n");
 	}
 
-	if (p->cfg->change_cb)
-		p->cfg->change_cb(endp, MGCP_ENDP_DLCX);
+	if (pdata->cfg->change_cb)
+		pdata->cfg->change_cb(endp, MGCP_ENDP_DLCX);
 
 	rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_SUCCESS));
 	if (silent)
 		goto out_silent;
-	return create_ok_resp_with_param(endp, 250, "DLCX", p->trans, stats);
+	return create_ok_resp_with_param(endp, 250, "DLCX", pdata->trans, stats);
 
 error3:
-	return create_err_response(endp, error_code, "DLCX", p->trans);
+	return create_err_response(endp, error_code, "DLCX", pdata->trans);
 
 out_silent:
 	LOGPENDP(endp, DLMGCP, LOGL_DEBUG, "DLCX: silent exit\n");
@@ -1417,7 +1487,7 @@
 }
 
 /* RSIP command handler, processes the received command */
-static struct msgb *handle_rsip(struct mgcp_parse_data *p)
+static struct msgb *handle_rsip(struct mgcp_request_data *rq)
 {
 	/* TODO: Also implement the resetting of a specific endpoint
 	 * to make mgcp_send_reset_ep() work. Currently this will call
@@ -1429,8 +1499,8 @@
 
 	LOGP(DLMGCP, LOGL_NOTICE, "RSIP: resetting all endpoints ...\n");
 
-	if (p->cfg->reset_cb)
-		p->cfg->reset_cb(p->endp->trunk);
+	if (rq->pdata->cfg->reset_cb)
+		rq->pdata->cfg->reset_cb(rq->endp->trunk);
 	return NULL;
 }
 
@@ -1446,7 +1516,7 @@
 /* This can request like DTMF detection and forward, fax detection... it
  * can also request when the notification should be send and such. We don't
  * do this right now. */
-static struct msgb *handle_noti_req(struct mgcp_parse_data *p)
+static struct msgb *handle_noti_req(struct mgcp_request_data *rq)
 {
 	int res = 0;
 	char *line;
@@ -1454,7 +1524,7 @@
 
 	LOGP(DLMGCP, LOGL_NOTICE, "RQNT: processing request for notification ...\n");
 
-	for_each_line(line, p->save) {
+	for_each_line(line, rq->pdata->save) {
 		switch (toupper(line[0])) {
 		case 'S':
 			tone = extract_tone(line);
@@ -1464,14 +1534,14 @@
 
 	/* we didn't see a signal request with a tone */
 	if (tone == CHAR_MAX)
-		return create_ok_response(p->endp, 200, "RQNT", p->trans);
+		return create_ok_response(rq->endp, 200, "RQNT", rq->pdata->trans);
 
-	if (p->cfg->rqnt_cb)
-		res = p->cfg->rqnt_cb(p->endp, tone);
+	if (rq->pdata->cfg->rqnt_cb)
+		res = rq->pdata->cfg->rqnt_cb(rq->endp, tone);
 
 	return res == 0 ?
-	    create_ok_response(p->endp, 200, "RQNT", p->trans) :
-	    create_err_response(p->endp, res, "RQNT", p->trans);
+	    create_ok_response(rq->endp, 200, "RQNT", rq->pdata->trans) :
+	    create_err_response(rq->endp, res, "RQNT", rq->pdata->trans);
 }
 
 /* Connection keepalive timer, will take care that dummy packets are send
diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c
index e98ca94..0759c96 100644
--- a/src/libosmo-mgcp/mgcp_sdp.c
+++ b/src/libosmo-mgcp/mgcp_sdp.c
@@ -398,7 +398,7 @@
 				return -1;
 			break;
 		default:
-			if (p->endp)
+			if (endp)
 				/* TODO: Check spec: We used the bare endpoint number before,
 				 * now we use the endpoint name as a whole? Is this allowed? */
 				LOGP(DLMGCP, LOGL_NOTICE,

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/24941
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I9f519d8a0ee8a513fa1e74acf3ee7dbc0991cdde
Gerrit-Change-Number: 24941
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210714/ee87be4e/attachment.htm>


More information about the gerrit-log mailing list