pespin has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-mgw/+/39723?usp=email )
Change subject: mgw: Split MGCP LocalConnectionOptions parsing from updating
endpoint
......................................................................
mgw: Split MGCP LocalConnectionOptions parsing from updating endpoint
Split current code into 2 steps, so first read-only parsing can be done,
then when all parsing and checks have been done, finally can be applied
o the object.
Change-Id: If75731fb92329dca6d092ffc7ff17cb354d28c0d
---
M include/osmocom/mgcp/mgcp_endp.h
M include/osmocom/mgcp/mgcp_protocol.h
M src/libosmo-mgcp/mgcp_endp.c
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
5 files changed, 259 insertions(+), 244 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/23/39723/1
diff --git a/include/osmocom/mgcp/mgcp_endp.h b/include/osmocom/mgcp/mgcp_endp.h
index fbe0850..8a2c8a2 100644
--- a/include/osmocom/mgcp/mgcp_endp.h
+++ b/include/osmocom/mgcp/mgcp_endp.h
@@ -149,6 +149,7 @@
const struct mgcp_trunk *trunk);
struct mgcp_endpoint *mgcp_endp_find_specific(const char *epname,
const struct mgcp_trunk *trunk);
+void mgcp_endp_update_lco(struct mgcp_endpoint *endp, const struct mgcp_lco
*lco);
void mgcp_endp_release(struct mgcp_endpoint *endp);
struct mgcp_conn *mgcp_endp_get_conn(struct mgcp_endpoint *endp, const char
*id);
diff --git a/include/osmocom/mgcp/mgcp_protocol.h
b/include/osmocom/mgcp/mgcp_protocol.h
index 2ef4c3e..9f79334 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -42,12 +42,26 @@
mgcp_codecset_reset(&sdp->cset);
}
+/* Local connection options */
+struct mgcp_lco {
+ bool present;
+ char *codec; /* talloc-allocated to some parent */
+ int pkt_period_min; /* time in ms */
+ int pkt_period_max; /* time in ms */
+};
+static inline void mgcp_lco_init(struct mgcp_lco *lco)
+{
+ *lco = (struct mgcp_lco){};
+}
+char *get_lco_identifier(const char *options);
+int check_local_cx_options(void *ctx, const char *options);
#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 *lco_string;
+ struct mgcp_lco lco;
const char *callid;
const char *connid;
enum mgcp_connection_mode mode;
@@ -59,15 +73,14 @@
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,
- .connid = NULL,
- .mode = MGCP_CONN_NONE,
- .remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET,
- .have_sdp = false,
- .x_osmo_ign = 0,
- };
+ hpars->lco_string = NULL;
+ mgcp_lco_init(&hpars->lco);
+ hpars->callid = NULL;
+ hpars->connid = NULL;
+ hpars->mode = MGCP_CONN_NONE;
+ hpars->remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET;
+ hpars->have_sdp = false;
+ hpars->x_osmo_ign = 0;
}
/* Internal structure while parsing a request */
@@ -111,18 +124,8 @@
int mgcp_cause;
};
-/* Local connection options */
-struct mgcp_lco {
- char *string;
- char *codec;
- int pkt_period_min; /* time in ms */
- int pkt_period_max; /* time in ms */
-};
-
char *mgcp_debug_get_last_endpoint_name(void);
-char *get_lco_identifier(const char *options);
-int check_local_cx_options(void *ctx, const char *options);
struct mgcp_rtp_end;
struct mgcp_endpoint;
diff --git a/src/libosmo-mgcp/mgcp_endp.c b/src/libosmo-mgcp/mgcp_endp.c
index 1d2287c..a4ab860 100644
--- a/src/libosmo-mgcp/mgcp_endp.c
+++ b/src/libosmo-mgcp/mgcp_endp.c
@@ -764,6 +764,22 @@
return NULL;
}
+/* Helps assigning a new lco structure, since "codec" is talloc allocated. */
+void mgcp_endp_update_lco(struct mgcp_endpoint *endp, const struct mgcp_lco
*lco)
+{
+ /* First free old talloc allocated codec string: */
+ talloc_free(endp->local_options.codec);
+ endp->local_options.codec = NULL;
+
+ if (lco) {
+ endp->local_options = *lco;
+ if (lco->codec)
+ endp->local_options.codec = talloc_strdup(endp,
lco->codec);
+ } else {
+ endp->local_options = (struct mgcp_lco){0};
+ }
+}
+
/*! release endpoint, all open connections are closed.
* \param[in] endp endpoint to release */
void mgcp_endp_release(struct mgcp_endpoint *endp)
@@ -785,10 +801,7 @@
/* Reset endpoint parameters and states */
talloc_free(endp->callid);
endp->callid = NULL;
- talloc_free(endp->local_options.string);
- endp->local_options.string = NULL;
- talloc_free(endp->local_options.codec);
- endp->local_options.codec = NULL;
+ mgcp_endp_update_lco(endp, NULL);
if (endp->trunk->trunk_type == MGCP_TRUNK_E1) {
uint8_t ts = e1_ts_nr_from_epname(endp->name);
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index af4f9d0..afeadd5 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -101,6 +101,197 @@
return MGCP_CONN_NONE;
}
+/*! Helper function for check_local_cx_options() to get a pointer of the next
+ * lco option identifier
+ * \param[in] lco string
+ * \returns pointer to the beginning of the LCO identifier, NULL on failure */
+char *get_lco_identifier(const char *options)
+{
+ char *ptr;
+ unsigned int count = 0;
+
+ /* Jump to the end of the lco identifier */
+ ptr = strstr(options, ":");
+ if (!ptr)
+ return NULL;
+
+ /* Walk backwards until the pointer points to the beginning of the
+ * lco identifier. We know that we stand at the beginning when we
+ * are either at the beginning of the memory or see a space or
+ * comma. (this is tolerant, it will accept a:10, b:11 as well as
+ * a:10,b:11) */
+ while (1) {
+ /* Endless loop protection */
+ if (count > 10000)
+ return NULL;
+ else if (ptr < options || *ptr == ' ' || *ptr == ',') {
+ ptr++;
+ break;
+ }
+ ptr--;
+ count++;
+ }
+
+ /* Check if we got any result */
+ if (*ptr == ':')
+ return NULL;
+
+ return ptr;
+}
+
+/*! Check the LCO option. This function checks for multiple appearance of LCO
+* options, which is illegal
+* \param[in] ctx talloc context
+* \param[in] lco string
+* \returns 0 on success, -1 on failure */
+int check_local_cx_options(void *ctx, const char *options)
+{
+ int i;
+ char *options_copy;
+ char *lco_identifier;
+ char *lco_identifier_end;
+ char *next_lco_identifier;
+
+ char **lco_seen;
+ unsigned int lco_seen_n = 0;
+
+ if (!options)
+ return -1;
+
+ lco_seen =
+ (char **)talloc_zero_size(ctx, strlen(options) * sizeof(char *));
+ options_copy = talloc_strdup(ctx, options);
+ lco_identifier = options_copy;
+
+ do {
+ /* Move the lco_identifier pointer to the beginning of the
+ * current lco option identifier */
+ lco_identifier = get_lco_identifier(lco_identifier);
+ if (!lco_identifier)
+ goto error;
+
+ /* Look ahead to the next LCO option early, since we
+ * will parse destructively */
+ next_lco_identifier = strstr(lco_identifier + 1, ",");
+
+ /* Pinch off the end of the lco field identifier name
+ * and see if we still got something, also check if
+ * there is some value after the colon. */
+ lco_identifier_end = strstr(lco_identifier, ":");
+ if (!lco_identifier_end)
+ goto error;
+ if (*(lco_identifier_end + 1) == ' '
+ || *(lco_identifier_end + 1) == ','
+ || *(lco_identifier_end + 1) == '\0')
+ goto error;
+ *lco_identifier_end = '\0';
+ if (strlen(lco_identifier) == 0)
+ goto error;
+
+ /* Check if we have already seen the current field identifier
+ * before. If yes, we must bail, an LCO must only appear once
+ * in the LCO string */
+ for (i = 0; i < lco_seen_n; i++) {
+ if (strcasecmp(lco_seen[i], lco_identifier) == 0)
+ goto error;
+ }
+ lco_seen[lco_seen_n] = lco_identifier;
+ lco_seen_n++;
+
+ /* The first identifier must always be found at the beginnning
+ * of the LCO string */
+ if (lco_seen[0] != options_copy)
+ goto error;
+
+ /* Go to the next lco option */
+ lco_identifier = next_lco_identifier;
+ } while (lco_identifier);
+
+ talloc_free(lco_seen);
+ talloc_free(options_copy);
+ return 0;
+error:
+ talloc_free(lco_seen);
+ talloc_free(options_copy);
+ return -1;
+}
+
+/* Set the LCO from a string (see RFC 3435).
+* The string is stored in the 'string' field. A NULL string is handled exactly
+* like an empty string, the 'string' field is never NULL after this function
+* has been called. */
+static int mgcp_parse_lco(void *ctx, struct mgcp_lco *lco, const char *options)
+{
+ const char *lco_id;
+ char codec[17];
+ char nt[17];
+ int len;
+
+ if (!options)
+ return 0;
+ if (strlen(options) == 0)
+ return 0;
+
+ lco->present = true;
+
+ /* Make sure the encoding of the LCO is consistent before we proceed */
+ if (check_local_cx_options(ctx, options) != 0) {
+ LOGP(DLMGCP, LOGL_ERROR,
+ "local CX options: Internal inconsistency in Local
Connection Options!\n");
+ return -524;
+ }
+
+ lco_id = options;
+ while ((lco_id = get_lco_identifier(lco_id))) {
+ switch (tolower(lco_id[0])) {
+ case 'p':
+ if (sscanf(lco_id + 1, ":%d-%d",
+ &lco->pkt_period_min, &lco->pkt_period_max) ==
1)
+ lco->pkt_period_max = lco->pkt_period_min;
+ break;
+ case 'a':
+ /* FIXME: LCO also supports the negotiation of more
than one codec.
+ * (e.g. a:PCMU;G726-32) But this implementation only
supports a single
+ * codec only. Ignoring all but the first codec. */
+ if (sscanf(lco_id + 1, ":%16[^,;]", codec) == 1) {
+ talloc_free(lco->codec);
+ /* MGCP header is case insensive, and we'll need
+ codec in uppercase when using it later: */
+ len = strlen(codec);
+ lco->codec = talloc_size(ctx, len + 1);
+ osmo_str_toupper_buf(lco->codec, len + 1,
codec);
+ }
+ break;
+ case 'n':
+ if (lco_id[1] == 't' && sscanf(lco_id + 2, ":%16[^,]",
nt) == 1)
+ break;
+ /* else: fall through to print notice log */
+ default:
+ LOGP(DLMGCP, LOGL_NOTICE,
+ "LCO: unhandled option: '%c'/%d in \"%s\"\n",
+ *lco_id, *lco_id, options);
+ break;
+ }
+
+ lco_id = strchr(lco_id, ',');
+ if (!lco_id)
+ break;
+ }
+
+ LOGP(DLMGCP, LOGL_DEBUG,
+ "local CX options: lco->pkt_period_max: %d, lco->codec: %s\n",
+ lco->pkt_period_max, lco->codec);
+
+ /* Check if the packetization fits the 20ms raster */
+ if (lco->pkt_period_min % 20 && lco->pkt_period_max % 20) {
+ LOGP(DLMGCP, LOGL_ERROR,
+ "local CX options: packetization interval is not a
multiple of 20ms!\n");
+ return -535;
+ }
+
+ return 0;
+}
+
/*! 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.
@@ -177,6 +368,7 @@
{
struct mgcp_parse_hdr_pars *hp = &pdata->hpars;
char *line;
+ int rc;
mgcp_parse_hdr_pars_init(hp);
@@ -188,7 +380,24 @@
switch (toupper(line[0])) {
case 'L':
- hp->local_options = (const char *)line + 3;
+ hp->lco_string = (const char *)line + 3;
+ rc = mgcp_parse_lco(pdata, &hp->lco, hp->lco_string);
+ if (rc < 0) {
+ LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "Invalid
LocalConnectionOptions line: '%s'", line);
+ switch (pdata->rq->verb) {
+ case MGCP_VERB_CRCX:
+
rate_ctr_inc(rate_ctr_group_get_ctr(pdata->rq->trunk->ratectr.mgcp_crcx_ctr_group,
+
MGCP_CRCX_FAIL_INVALID_CONN_OPTIONS));
+ break;
+ case MGCP_VERB_MDCX:
+
rate_ctr_inc(rate_ctr_group_get_ctr(pdata->rq->trunk->ratectr.mgcp_mdcx_ctr_group,
+
MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS));
+ break;
+ default:
+ break;
+ }
+ return rc;
+ }
break;
case 'C':
hp->callid = (const char *)line + 3;
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 06da91a..49a4e83 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -455,199 +455,6 @@
return create_ok_response(rq->trunk, rq->endp, 200, "AUEP",
rq->pdata->trans);
}
-/*! Helper function for check_local_cx_options() to get a pointer of the next
- * lco option identifier
- * \param[in] lco string
- * \returns pointer to the beginning of the LCO identifier, NULL on failure */
-char *get_lco_identifier(const char *options)
-{
- char *ptr;
- unsigned int count = 0;
-
- /* Jump to the end of the lco identifier */
- ptr = strstr(options, ":");
- if (!ptr)
- return NULL;
-
- /* Walk backwards until the pointer points to the beginning of the
- * lco identifier. We know that we stand at the beginning when we
- * are either at the beginning of the memory or see a space or
- * comma. (this is tolerant, it will accept a:10, b:11 as well as
- * a:10,b:11) */
- while (1) {
- /* Endless loop protection */
- if (count > 10000)
- return NULL;
- else if (ptr < options || *ptr == ' ' || *ptr == ',') {
- ptr++;
- break;
- }
- ptr--;
- count++;
- }
-
- /* Check if we got any result */
- if (*ptr == ':')
- return NULL;
-
- return ptr;
-}
-
-/*! Check the LCO option. This function checks for multiple appearence of LCO
- * options, which is illegal
- * \param[in] ctx talloc context
- * \param[in] lco string
- * \returns 0 on success, -1 on failure */
-int check_local_cx_options(void *ctx, const char *options)
-{
- int i;
- char *options_copy;
- char *lco_identifier;
- char *lco_identifier_end;
- char *next_lco_identifier;
-
- char **lco_seen;
- unsigned int lco_seen_n = 0;
-
- if (!options)
- return -1;
-
- lco_seen =
- (char **)talloc_zero_size(ctx, strlen(options) * sizeof(char *));
- options_copy = talloc_strdup(ctx, options);
- lco_identifier = options_copy;
-
- do {
- /* Move the lco_identifier pointer to the beginning of the
- * current lco option identifier */
- lco_identifier = get_lco_identifier(lco_identifier);
- if (!lco_identifier)
- goto error;
-
- /* Look ahead to the next LCO option early, since we
- * will parse destructively */
- next_lco_identifier = strstr(lco_identifier + 1, ",");
-
- /* Pinch off the end of the lco field identifier name
- * and see if we still got something, also check if
- * there is some value after the colon. */
- lco_identifier_end = strstr(lco_identifier, ":");
- if (!lco_identifier_end)
- goto error;
- if (*(lco_identifier_end + 1) == ' '
- || *(lco_identifier_end + 1) == ','
- || *(lco_identifier_end + 1) == '\0')
- goto error;
- *lco_identifier_end = '\0';
- if (strlen(lco_identifier) == 0)
- goto error;
-
- /* Check if we have already seen the current field identifier
- * before. If yes, we must bail, an LCO must only appear once
- * in the LCO string */
- for (i = 0; i < lco_seen_n; i++) {
- if (strcasecmp(lco_seen[i], lco_identifier) == 0)
- goto error;
- }
- lco_seen[lco_seen_n] = lco_identifier;
- lco_seen_n++;
-
- /* The first identifier must always be found at the beginnning
- * of the LCO string */
- if (lco_seen[0] != options_copy)
- goto error;
-
- /* Go to the next lco option */
- lco_identifier = next_lco_identifier;
- } while (lco_identifier);
-
- talloc_free(lco_seen);
- talloc_free(options_copy);
- return 0;
-error:
- talloc_free(lco_seen);
- talloc_free(options_copy);
- return -1;
-}
-
-/* Set the LCO from a string (see RFC 3435).
- * The string is stored in the 'string' field. A NULL string is handled exactly
- * like an empty string, the 'string' field is never NULL after this function
- * has been called. */
-static int set_local_cx_options(void *ctx, struct mgcp_lco *lco,
- const char *options)
-{
- char *lco_id;
- char codec[17];
- char nt[17];
- int len;
-
- if (!options)
- return 0;
- if (strlen(options) == 0)
- return 0;
-
- /* Make sure the encoding of the LCO is consistant before we proceed */
- if (check_local_cx_options(ctx, options) != 0) {
- LOGP(DLMGCP, LOGL_ERROR,
- "local CX options: Internal inconsistency in Local
Connection Options!\n");
- return 524;
- }
-
- talloc_free(lco->string);
- lco->string = talloc_strdup(ctx, options);
-
- lco_id = lco->string;
- while ((lco_id = get_lco_identifier(lco_id))) {
- switch (tolower(lco_id[0])) {
- case 'p':
- if (sscanf(lco_id + 1, ":%d-%d",
- &lco->pkt_period_min, &lco->pkt_period_max)
== 1)
- lco->pkt_period_max = lco->pkt_period_min;
- break;
- case 'a':
- /* FIXME: LCO also supports the negotiation of more
than one codec.
- * (e.g. a:PCMU;G726-32) But this implementation only
supports a single
- * codec only. Ignoring all but the first codec. */
- if (sscanf(lco_id + 1, ":%16[^,;]", codec) == 1) {
- talloc_free(lco->codec);
- /* MGCP header is case insensive, and we'll need
- codec in uppercase when using it later: */
- len = strlen(codec);
- lco->codec = talloc_size(ctx, len + 1);
- osmo_str_toupper_buf(lco->codec, len + 1,
codec);
- }
- break;
- case 'n':
- if (lco_id[1] == 't' && sscanf(lco_id + 2, ":%16[^,]",
nt) == 1)
- break;
- /* else: fall throught to print notice log */
- default:
- LOGP(DLMGCP, LOGL_NOTICE,
- "LCO: unhandled option: '%c'/%d in \"%s\"\n",
- *lco_id, *lco_id, lco->string);
- break;
- }
-
- lco_id = strchr(lco_id, ',');
- if (!lco_id)
- break;
- }
-
- LOGP(DLMGCP, LOGL_DEBUG,
- "local CX options: lco->pkt_period_max: %i, lco->codec: %s\n",
- lco->pkt_period_max, lco->codec);
-
- /* Check if the packetization fits the 20ms raster */
- if (lco->pkt_period_min % 20 && lco->pkt_period_max % 20) {
- LOGP(DLMGCP, LOGL_ERROR,
- "local CX options: packetization interval is not a
multiple of 20ms!\n");
- return 535;
- }
-
- return 0;
-}
-
uint32_t mgcp_rtp_packet_duration(const struct mgcp_endpoint *endp,
const struct mgcp_rtp_end *rtp)
{
@@ -907,6 +714,10 @@
/* Update endp->x_osmo_ign: */
endp->x_osmo_ign |= hpars->x_osmo_ign;
+ /* Set local connection options, if present */
+ if (hpars->lco.present)
+ mgcp_endp_update_lco(endp, &hpars->lco);
+
if (!endp->callid) {
/* Claim endpoint resources. This will also set the callid,
* creating additional connections will only be possible if
@@ -953,19 +764,6 @@
} /* else: -1 (wildcard) */
}
- /* Set local connection options, if present */
- if (hpars->local_options) {
- rc = set_local_cx_options(trunk->endpoints,
- &endp->local_options,
hpars->local_options);
- if (rc != 0) {
- LOGPCONN(conn, DLMGCP, LOGL_ERROR,
- "CRCX: invalid local connection options!\n");
- error_code = rc;
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_CRCX_FAIL_INVALID_CONN_OPTIONS));
- goto error2;
- }
- }
-
/* Handle codec information and decide for a suitable codec */
rc = handle_codec_info(conn_rtp, rq);
mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn));
@@ -1120,6 +918,10 @@
return create_err_response(endp, endp, 400, "MDCX",
pdata->trans);
}
+ /* Set local connection options, if present */
+ if (hpars->lco.present)
+ mgcp_endp_update_lco(endp, &hpars->lco);
+
mgcp_conn_watchdog_kick(conn);
if (hpars->mode == MGCP_CONN_NONE) {
@@ -1130,19 +932,6 @@
return create_err_response(endp, endp, 517, "MDCX",
pdata->trans);
}
- /* Set local connection options, if present */
- if (hpars->local_options) {
- rc = set_local_cx_options(trunk->endpoints,
- &endp->local_options,
hpars->local_options);
- if (rc != 0) {
- LOGPCONN(conn, DLMGCP, LOGL_ERROR,
- "MDCX: invalid local connection options!\n");
- error_code = rc;
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs,
MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS));
- goto error3;
- }
- }
-
conn_rtp = mgcp_conn_get_conn_rtp(conn);
OSMO_ASSERT(conn_rtp);
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39723?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: If75731fb92329dca6d092ffc7ff17cb354d28c0d
Gerrit-Change-Number: 39723
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pes...@sysmocom.de>
