[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
Thu Jan 18 17:32:27 UTC 2018


Review at  https://gerrit.osmocom.org/5879

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

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

- add parsing support for the returned endpoint names

Change-Id: Iebc95043569191b6f5fbc8fe266b13fcfcab2e48
---
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
5 files changed, 126 insertions(+), 21 deletions(-)


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

diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index 882c908..3393bc4 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 ep_id[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..e4ca96d 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -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.ep_id,
+						   sizeof(r->head.ep_id),
+						   '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..994b00a 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -181,6 +181,29 @@
 	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));
+			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 +236,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 9c21d38..24d5bf1 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -221,6 +221,24 @@
 		osmux_extension[0] = '\0';
 	}
 
+	/* FIXME: create_response_with_sdp() is called in the most cases, it
+	 * is certainly called on CRCX, which gives us Z and I on CRCX where
+	 * we depend on it. In all other steps, we do not necessarly depend
+	 * on Z and I. Also, the parameters Z and I are not SDP parameters,
+	 * they are just normal MGCP parameters and should not be generated
+	 * here since they are not in the scope of SDP. There could be a
+	 * separate generator function that generates a string that then
+	 * submits the parameter data to create_resp(). Unfortunately all
+	 * this gets a mess. We sould get rid of all those
+	 * create_response_with_something() functions and generalize this
+	 * in some way. This is the reason why this is not fixed now and
+	 * there is this FIXME note instead. Also the I parameter, which is
+	 * misplaced as well has been introduced with an earlier patch. */
+
+	rc = msgb_printf(sdp, "Z: %u@%s\n", ENDPOINT_NUMBER(endp), endp->cfg->domain);
+	if (rc < 0)
+		goto error;
+
 	rc = msgb_printf(sdp, "I: %s%s\n\n", conn->conn->id, osmux_extension);
 	if (rc < 0)
 		goto error;
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 467cb6c..58296f4 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -86,6 +86,7 @@
 
 #define MDCX3_RET \
 	"200 18983215 OK\r\n" \
+	"Z: %s\n" \
 	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
@@ -99,6 +100,7 @@
 
 #define MDCX3A_RET \
 	"200 18983215 OK\r\n" \
+	"Z: %s\n" \
 	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
@@ -112,6 +114,7 @@
 
 #define MDCX3_FMTP_RET \
 	"200 18983215 OK\r\n" \
+	"Z: %s\n" \
 	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
@@ -141,6 +144,7 @@
 
 #define MDCX4_RET(Ident) \
 	"200 " Ident " OK\r\n" \
+	"Z: %s\n" \
 	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
@@ -154,6 +158,7 @@
 
 #define MDCX4_RO_RET(Ident) \
 	"200 " Ident " OK\r\n" \
+	"Z: %s\n" \
 	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
@@ -252,6 +257,7 @@
 
 #define CRCX_RET \
 	"200 2 OK\r\n" \
+	"Z: %s\n" \
 	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
@@ -265,6 +271,7 @@
 
 #define CRCX_RET_NO_RTPMAP \
 	"200 2 OK\r\n" \
+	"Z: %s\n" \
 	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
@@ -277,6 +284,7 @@
 
 #define CRCX_FMTP_RET \
 	"200 2 OK\r\n" \
+	"Z: %s\n" \
 	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
@@ -301,6 +309,7 @@
 
 #define CRCX_ZYN_RET \
 	"200 2 OK\r\n" \
+	"Z: %s\n" \
 	"I: %s\n" \
 	"\n" \
 	"v=0\r\n" \
@@ -578,7 +587,29 @@
 	 * not exceed a length of 32 digits */
 	OSMO_ASSERT(strlen(conn_id) <= 32);
 	OSMO_ASSERT(strlen(conn_id) > 0);
+	return 0;
+}
 
+/* Extract a endpoint ID from a response (CRCX) */
+static int get_endpoint_id_from_response(uint8_t *resp, char *endpoint_id,
+					 unsigned int endpoint_id_len)
+{
+	char *endpoint_id_ptr;
+	int i;
+
+	endpoint_id_ptr = strstr((char *)resp, "Z: ");
+	if (!endpoint_id_ptr)
+		return -EINVAL;
+
+	memset(endpoint_id, 0, endpoint_id_len);
+	memcpy(endpoint_id, endpoint_id_ptr + 3, 5);
+
+	for (i = 0; i < endpoint_id_len; i++) {
+		if (endpoint_id[i] == '\n' || endpoint_id[i] == '\r')
+			endpoint_id[i] = '\0';
+	}
+
+	OSMO_ASSERT(strlen(endpoint_id) > 0);
 	return 0;
 }
 
@@ -588,6 +619,7 @@
 	char exp_resp_patched[4096];
 	const char *exp_resp_ptr;
 	char conn_id[256];
+	char endpoint_id[1024];
 
 	printf("checking response:\n");
 
@@ -598,15 +630,13 @@
 	 * is generated by the mgcp code on CRCX and we can
 	 * not know it in advance */
 	if (strstr(exp_resp, "%s")) {
-		if (get_conn_id_from_response(resp, conn_id, sizeof(conn_id)) ==
-		    0) {
-			sprintf(exp_resp_patched, exp_resp, conn_id, conn_id);
+		if (get_conn_id_from_response(resp, conn_id, sizeof(conn_id)) == 0 && get_endpoint_id_from_response(resp, endpoint_id, sizeof(endpoint_id)) == 0) {
+			sprintf(exp_resp_patched, exp_resp, endpoint_id, conn_id, conn_id);
 			exp_resp_ptr = exp_resp_patched;
-			printf
-			    ("using message with patched conn_id for comparison\n");
+			printf("using message with patched conn_id for comparison\n");
 		} else {
 			printf
-			    ("patching conn_id failed, using message as statically defined for comparison\n");
+			    ("patching conn/endpoint_id failed, using message as statically defined for comparison\n");
 			exp_resp_ptr = exp_resp;
 		}
 	} else {

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iebc95043569191b6f5fbc8fe266b13fcfcab2e48
Gerrit-PatchSet: 1
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>



More information about the gerrit-log mailing list