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.orgReview 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>