[PATCH] osmo-mgw[master]: mgcp: permit wildcarded endpoint assignment (CRCX)

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
Fri Jan 19 17:59:14 UTC 2018


Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/5879

to look at the new patch set (#3).

mgcp: permit wildcarded endpoint assignment (CRCX)

The mgcp protocol in general allows wildcarded endpoints on CRCX.
osmo-mgw does not support this feature yet.

- when the endpoint name contains a wildcard character, search
  a free endpoint automatically

- return the resulting endpoint name in the parameter section of
  the mgcp response

- add parsing support for the returned endpoint names

- Be more concious about the parameters that are returned with
  each response. Do not unnecessarily attach known parameters.
  Return the connection ID only on CRCX commands. Only return
  the endpoint ID on CRCX commands that are wildcarded.

Change-Id: Iebc95043569191b6f5fbc8fe266b13fcfcab2e48
related: OS#2631
---
M include/osmocom/mgcp/mgcp_internal.h
M include/osmocom/mgcp_client/mgcp_client.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
M tests/mgcp/mgcp_test.c
6 files changed, 140 insertions(+), 40 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/79/5879/3

diff --git a/include/osmocom/mgcp/mgcp_internal.h b/include/osmocom/mgcp/mgcp_internal.h
index 33a754c..c0ee556 100644
--- a/include/osmocom/mgcp/mgcp_internal.h
+++ b/include/osmocom/mgcp/mgcp_internal.h
@@ -260,6 +260,10 @@
 	/* fields for re-transmission */
 	char *last_trans;
 	char *last_response;
+
+	/* Memorize if this endpoint was choosen by the MGW (wildcarded, true)
+	 * or if the user has choosen the particular endpoint explicitly */
+	bool wildcarded_crcx;
 };
 
 
diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index 882c908..676850f 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -31,6 +31,7 @@
 	mgcp_trans_id_t trans_id;
 	const char *comment;
 	char conn_id[MGCP_CONN_ID_LENGTH];
+	char endpoint[MGCP_ENDPOINT_MAXLEN];
 };
 
 struct mgcp_response {
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index 9fc414d..1d9a858 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -146,8 +146,8 @@
 	/* Mark the end of the comment */
 	*end = '\0';
 	r->body = end + 1;
-	if (r->body[0] == '\n')
-		r->body ++;
+//	if (r->body[0] == '\n')
+//		r->body ++;
 	return 0;
 
 response_parse_failure:
@@ -226,6 +226,10 @@
 	OSMO_ASSERT(r->body);
 	char *data = strstr(r->body, "\n\n");
 
+	/* Warning: This function performs a destructive parsing on r->body.
+	 * Since this function is called at the very end of the persing
+	 * process, destructive parsing is acceptable. */
+
 	if (!data) {
 		LOGP(DLMGCP, LOGL_ERROR,
 		     "MGCP response: cannot find start of parameters\n");
@@ -261,21 +265,29 @@
 	return 0;
 }
 
-/* Parse a line like "I: 0cedfd5a19542d197af9afe5231f1d61" */
-static int mgcp_parse_conn_id(struct mgcp_response *r, const char *line)
+/* Parse a line like "X: something" */
+static int mgcp_parse_head_param(char *result, unsigned int result_len,
+				 char label, const char *line)
 {
+	char label_string[4];
+
+	/* Detect empty parameters */
 	if (strlen(line) < 4)
 		goto response_parse_failure;
 
-	if (memcmp("I: ", line, 3) != 0)
+	/* Check if the label matches */
+	snprintf(label_string, sizeof(label_string), "%c: ", label);
+	if (memcmp(label_string, line, 3) != 0)
 		goto response_parse_failure;
 
-	osmo_strlcpy(r->head.conn_id, line + 3, sizeof(r->head.conn_id));
+	/* Copy payload part of the string to destinations (the label string
+	 * is always 3 chars long) */
+	osmo_strlcpy(result, line + 3, result_len);
 	return 0;
 
 response_parse_failure:
 	LOGP(DLMGCP, LOGL_ERROR,
-	     "Failed to parse MGCP response (connectionIdentifier)\n");
+	     "Failed to parse MGCP response (parameter label: %c)\n", label);
 	return -EINVAL;
 }
 
@@ -285,18 +297,37 @@
 	char *line;
 	int rc = 0;
 	OSMO_ASSERT(r->body);
-	char *data = r->body;
-	char *data_end = strstr(r->body, "\n\n");
+	char *data;
+	char *data_ptr;
+	char *data_end;
 
-	/* Protect SDP body, for_each_non_empty_line() will
-	 * only parse until it hits \0 mark. */
+	/* Since this functions performs a destructive parsing, we create a
+	 * local copy of the body data */
+	data = talloc_zero_size(NULL, strlen(r->body)+1);
+	OSMO_ASSERT(data);
+	data_ptr = data;
+	osmo_strlcpy(data, r->body, strlen(r->body));
+
+	/* If there is an SDP body attached, prevent for_each_non_empty_line()
+	 * into running in there, we are not yet interested in the parameters
+	 * stored there. */
+	data_end = strstr(data, "\n\n");
 	if (data_end)
 		*data_end = '\0';
 
-	for_each_non_empty_line(line, data) {
+	for_each_non_empty_line(line, data_ptr) {
 		switch (line[0]) {
+		case 'Z':
+			rc = mgcp_parse_head_param(r->head.endpoint,
+						   sizeof(r->head.endpoint),
+						   'Z', line);
+			if (rc)
+				goto exit;
+			break;
 		case 'I':
-			rc = mgcp_parse_conn_id(r, line);
+			rc = mgcp_parse_head_param(r->head.conn_id,
+						   sizeof(r->head.conn_id),
+						   'I', line);
 			if (rc)
 				goto exit;
 			break;
@@ -306,10 +337,7 @@
 		}
 	}
 exit:
-	/* Restore original state */
-	if (data_end)
-		*data_end = '\n';
-
+	talloc_free(data);
 	return rc;
 }
 
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index 74acffa..9bb2805 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -181,6 +181,30 @@
 	return &tcfg->endpoints[endp];
 }
 
+/* Find an endpoint that is not in use. Do this by going through the endpoint
+ * array, check the callid. A callid nullpointer indicates that the endpoint
+ * is free */
+static struct mgcp_endpoint *find_free_endpoint(struct mgcp_endpoint *endpoints,
+						unsigned int number_endpoints)
+{
+	struct mgcp_endpoint *endp;
+	unsigned int i;
+
+	for (i = 0; i < number_endpoints; i++) {
+		if (endpoints[i].callid == NULL) {
+			endp = &endpoints[i];
+			LOGP(DLMGCP, LOGL_DEBUG,
+			     "endpoint:0x%x found free endpoint\n",
+			     ENDPOINT_NUMBER(endp));
+			endp->wildcarded_crcx = true;
+			return endp;
+		}
+	}
+
+	LOGP(DLMGCP, LOGL_ERROR, "Not able to find a free endpoint");
+	return NULL;
+}
+
 /* Check if the domain name, which is supplied with the endpoint name
  * matches the configuration. */
 static int check_domain_name(struct mgcp_config *cfg, const char *mgcp)
@@ -213,6 +237,11 @@
 	if (strncmp(mgcp, "ds/e1", 5) == 0)
 		return find_e1_endpoint(cfg, mgcp);
 
+	if (strncmp(mgcp, "*", 1) == 0) {
+		return find_free_endpoint(cfg->trunk.endpoints,
+					  cfg->trunk.number_endpoints);
+	}
+
 	gw = strtoul(mgcp, &endptr, 16);
 	if (gw < cfg->trunk.number_endpoints && endptr[0] == '@')
 		return &cfg->trunk.endpoints[gw];
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 4c04712..16e9cb8 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -192,11 +192,32 @@
 	return create_resp(endp, code, " FAIL", msg, trans, NULL, NULL);
 }
 
+/* Add MGCP parameters to a message buffer */
+static int add_params(struct msgb *msg, const struct mgcp_endpoint *endp,
+		      const struct mgcp_conn_rtp *conn)
+{
+	int rc;
+
+	if (endp->wildcarded_crcx) {
+		rc = msgb_printf(msg, "Z: %u@%s\n", ENDPOINT_NUMBER(endp),
+				 endp->cfg->domain);
+		if (rc < 0)
+			return -EINVAL;
+	}
+
+	rc = msgb_printf(msg, "I: %s\n", conn->conn->id);
+	if (rc < 0)
+		return -EINVAL;
+
+	return 0;
+}
+
 /* Format MGCP response string (with SDP attached) */
 static struct msgb *create_response_with_sdp(struct mgcp_endpoint *endp,
 					     struct mgcp_conn_rtp *conn,
 					     const char *msg,
-					     const char *trans_id)
+					     const char *trans_id,
+					     bool add_conn_params)
 {
 	const char *addr = endp->cfg->local_ip;
 	struct msgb *sdp;
@@ -221,7 +242,14 @@
 		osmux_extension[0] = '\0';
 	}
 
-	rc = msgb_printf(sdp, "I: %s%s\n\n", conn->conn->id, osmux_extension);
+	/* Attach optional connection parameters */
+	if (add_conn_params) {
+		rc = add_params(sdp, endp, conn);
+		if (rc < 0)
+			goto error;
+	}
+
+	rc = msgb_printf(sdp, "%s\n", osmux_extension);
 	if (rc < 0)
 		goto error;
 
@@ -648,7 +676,7 @@
 	LOGP(DLMGCP, LOGL_NOTICE,
 	     "CRCX: endpoint:0x%x connection successfully created\n",
 	     ENDPOINT_NUMBER(endp));
-	return create_response_with_sdp(endp, conn, "CRCX", p->trans);
+	return create_response_with_sdp(endp, conn, "CRCX", p->trans, true);
 error2:
 	mgcp_release_endp(endp);
 	LOGP(DLMGCP, LOGL_NOTICE,
@@ -801,7 +829,7 @@
 	LOGP(DLMGCP, LOGL_NOTICE,
 	     "MDCX: endpoint:0x%x connection successfully modified\n",
 	     ENDPOINT_NUMBER(endp));
-	return create_response_with_sdp(endp, conn, "MDCX", p->trans);
+	return create_response_with_sdp(endp, conn, "MDCX", p->trans, false);
 error3:
 	return create_err_response(endp, error_code, "MDCX", p->trans);
 
@@ -1196,6 +1224,7 @@
 	endp->local_options.string = NULL;
 	talloc_free(endp->local_options.codec);
 	endp->local_options.codec = NULL;
+	endp->wildcarded_crcx = false;
 }
 
 static int send_agent(struct mgcp_config *cfg, const char *buf, int len)
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 467cb6c..ddee8c5 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -86,7 +86,6 @@
 
 #define MDCX3_RET \
 	"200 18983215 OK\r\n" \
-	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
 	"o=- %s 23 IN IP4 0.0.0.0\r\n" \
@@ -99,7 +98,6 @@
 
 #define MDCX3A_RET \
 	"200 18983215 OK\r\n" \
-	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
 	"o=- %s 23 IN IP4 0.0.0.0\r\n" \
@@ -112,7 +110,6 @@
 
 #define MDCX3_FMTP_RET \
 	"200 18983215 OK\r\n" \
-	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
 	"o=- %s 23 IN IP4 0.0.0.0\r\n" \
@@ -141,7 +138,6 @@
 
 #define MDCX4_RET(Ident) \
 	"200 " Ident " OK\r\n" \
-	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
 	"o=- %s 23 IN IP4 0.0.0.0\r\n" \
@@ -154,7 +150,6 @@
 
 #define MDCX4_RO_RET(Ident) \
 	"200 " Ident " OK\r\n" \
-	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
 	"o=- %s 23 IN IP4 0.0.0.0\r\n" \
@@ -561,25 +556,39 @@
 {
 	char *conn_id_ptr;
 	int i;
+	bool got_conn_id = false;
 
+	/* First try to get the conn_id from the I: parameter */
 	conn_id_ptr = strstr((char *)resp, "I: ");
-	if (!conn_id_ptr)
-		return -EINVAL;
-
-	memset(conn_id, 0, conn_id_len);
-	memcpy(conn_id, conn_id_ptr + 3, 32);
-
-	for (i = 0; i < conn_id_len; i++) {
-		if (conn_id[i] == '\n' || conn_id[i] == '\r')
-			conn_id[i] = '\0';
+	if (conn_id_ptr) {
+		memset(conn_id, 0, conn_id_len);
+		memcpy(conn_id, conn_id_ptr + 3, 32);
+		got_conn_id = true;
+	} else {
+		/* Alternatively try to extract the conn_id from the o=- SDP
+		 * parameter */
+		conn_id_ptr = strstr((char *)resp, "o=- ");
+		if(conn_id_ptr) {
+			memset(conn_id, 0, conn_id_len);
+			memcpy(conn_id, conn_id_ptr + 4, 32);
+			got_conn_id = true;
+		}
 	}
 
-	/* A valid conn_id must at least contain one digit, and must
-	 * not exceed a length of 32 digits */
-	OSMO_ASSERT(strlen(conn_id) <= 32);
-	OSMO_ASSERT(strlen(conn_id) > 0);
+	if (got_conn_id) {
+		for (i = 0; i < conn_id_len; i++) {
+			if (conn_id[i] == '\n' || conn_id[i] == '\r')
+				conn_id[i] = '\0';
+		}
 
-	return 0;
+		/* A valid conn_id must at least contain one digit, and must
+		 * not exceed a length of 32 digits */
+		OSMO_ASSERT(strlen(conn_id) <= 32);
+		OSMO_ASSERT(strlen(conn_id) > 0);
+
+		return 0;
+	}
+	return -EINVAL;
 }
 
 /* Check response, automatically patch connection ID if needed */

-- 
To view, visit https://gerrit.osmocom.org/5879
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebc95043569191b6f5fbc8fe266b13fcfcab2e48
Gerrit-PatchSet: 3
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list