pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39396?usp=email )
(
10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: ASP loadsharing: Pass ownership of Tx xua_msg down the stack [1/2]
......................................................................
ASP loadsharing: Pass ownership of Tx xua_msg down the stack [1/2]
This is a first step towards getting MTP OPC & SLS fields down the stack
so that they can be used to implement loadsharing at ASP level.
So far, we always encode the xua_msg (which holds the OPC and SLS info
into xua_msg->mtp) into a msgb before passing it to as->fi through event
XUA_AS_E_TRANSFER_REQ, which in turn calls xua_as_transmit_msg(as, msg).
This patch is part 1 of 2 patches, which only modifies mostly the
Tx path, but still requires modifications on Rx path (done in patch 2/2)
to properly handle the forwarding path from Rx to Tx without double
freeing. It is submitted in 2 parts to ease code reviewing.
Future patches will modify that code to pass a xua_msg and only
encode it into a msgb deeper into the stack after an ASP has been
selected and msg needs to be transmitted.
All previous calls to sua_tx_xua_asp() actually had a memory leak before
this change, hence why no xua_msg_free() is removed from callers in this
patch. Since now ownership is taken by sua_tx_xua_asp() which always
frees the xua msg, the memleak is fixed.
Related: SYS#7112
Change-Id: I49e718e624da08510a9c3d28bdb360c59f9a65d3
---
M src/ipa.c
M src/m3ua.c
M src/osmo_ss7_hmrt.c
M src/sua.c
4 files changed, 48 insertions(+), 19 deletions(-)
Approvals:
laforge: Looks good to me, but someone else must approve
Jenkins Builder: Verified
osmith: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/src/ipa.c b/src/ipa.c
index 4e0180c..47ddc0d 100644
--- a/src/ipa.c
+++ b/src/ipa.c
@@ -94,13 +94,17 @@
/*! \brief Send a given xUA message via a given IPA "Application Server"
* \param[in] as Application Server through which to send \a xua
* \param[in] xua xUA message to be sent
- * \return 0 on success; negative on error */
+ * \return 0 on success; negative on error
+ *
+ * This function takes ownership of xua msg passed to it.
+ */
int ipa_tx_xua_as(struct osmo_ss7_as *as, struct xua_msg *xua)
{
struct msgb *msg;
OSMO_ASSERT(as->cfg.proto == OSMO_SS7_ASP_PROT_IPA);
msg = ipa_to_msg(xua);
+ xua_msg_free(xua);
if (!msg) {
LOGPAS(as, DLSS7, LOGL_ERROR, "Error encoding IPA Msg\n");
return -1;
diff --git a/src/m3ua.c b/src/m3ua.c
index 2f180c3..cdbfc48 100644
--- a/src/m3ua.c
+++ b/src/m3ua.c
@@ -506,23 +506,31 @@
return msg;
}
-/* transmit given xua_msg via given ASP */
+/* transmit given xua_msg via given ASP
+ * This function takes ownership of xua msg passed to it.
+ */
static int m3ua_tx_xua_asp(struct osmo_ss7_asp *asp, struct xua_msg *xua)
{
- struct msgb *msg = m3ua_to_msg(xua);
+ struct msgb *msg;
OSMO_ASSERT(asp->cfg.proto == OSMO_SS7_ASP_PROT_M3UA);
+ msg = m3ua_to_msg(xua);
+ xua_msg_free(xua);
if (!msg)
return -1;
+ /* msg becomes owned by osmo_ss7_asp_send here: */
return osmo_ss7_asp_send(asp, msg);
}
/*! \brief Send a given xUA message via a given M3UA Application Server
* \param[in] as Application Server through which to send \ref xua
* \param[in] xua xUA message to be sent
- * \return 0 on success; negative on error */
+ * \return 0 on success; negative on error
+ *
+ * This function takes ownership of xua msg passed to it.
+ */
int m3ua_tx_xua_as(struct osmo_ss7_as *as, struct xua_msg *xua)
{
struct msgb *msg;
@@ -535,6 +543,7 @@
xua_msg_add_u32(xua, M3UA_IEI_ROUTE_CTX, as->cfg.routing_key.context);
msg = m3ua_to_msg(xua);
+ xua_msg_free(xua);
if (!msg) {
LOGPAS(as, DLM3UA, LOGL_ERROR, "Error encoding M3UA Msg\n");
return -1;
@@ -794,10 +803,8 @@
err = m3ua_gen_error_msg(rc, msg);
out:
- if (err) {
+ if (err)
m3ua_tx_xua_asp(asp, err);
- xua_msg_free(err);
- }
xua_msg_free(xua);
@@ -900,7 +907,6 @@
xua = m3ua_encode_duna(rctx, num_rctx, aff_pc, num_aff_pc, info_string);
m3ua_tx_xua_asp(asp, xua);
- xua_msg_free(xua);
}
/*! Transmit SSNM SCON message indicating congestion
@@ -934,7 +940,6 @@
xua_msg_add_data(xua, M3UA_IEI_INFO_STRING, strlen(info_string)+1, (const uint8_t *) info_string);
m3ua_tx_xua_asp(asp, xua);
- xua_msg_free(xua);
}
/*! Transmit SSNM DUPU message indicating user unavailability.
@@ -950,7 +955,6 @@
{
struct xua_msg *xua = m3ua_encode_dupu(rctx, num_rctx, dpc, user, cause, info_str);
m3ua_tx_xua_asp(asp, xua);
- xua_msg_free(xua);
}
/* received SNM message on ASP side */
diff --git a/src/osmo_ss7_hmrt.c b/src/osmo_ss7_hmrt.c
index 675b231..654692a 100644
--- a/src/osmo_ss7_hmrt.c
+++ b/src/osmo_ss7_hmrt.c
@@ -86,24 +86,35 @@
return m3ua_xfer_from_data(&data_hdr, msgb_l2(msg), msgb_l2len(msg));
}
-/* delivery given XUA message to given SS7 user */
+/* delivery given XUA message to given SS7 user
+ * Ownership of xua_msg passed is transferred to this function.
+ */
static int deliver_to_mtp_user(const struct osmo_ss7_user *osu,
struct xua_msg *xua)
{
struct osmo_mtp_prim *prim;
+ int rc;
/* Create MTP-TRANSFER.ind and feed to user */
prim = m3ua_to_xfer_ind(xua);
- if (!prim)
- return -1;
+ if (!prim) {
+ rc = -1;
+ goto ret_free;
+ }
prim->u.transfer = xua->mtp;
- return osu->prim_cb(&prim->oph, (void *) osu->priv);
+ rc = osu->prim_cb(&prim->oph, (void *) osu->priv);
+
+ret_free:
+ xua_msg_free(xua);
+ return rc;
}
/* HMDC -> HMDT: Message for distribution; Figure 25/Q.704 */
/* This means it is a message we received from remote/L2, and it is to
- * be routed to a local user part */
+ * be routed to a local user part.
+ * Ownership of xua_msg passed is transferred to this function.
+ */
static int hmdt_message_for_distribution(struct osmo_ss7_instance *inst, struct xua_msg *xua)
{
struct m3ua_data_hdr *mdh;
@@ -120,6 +131,7 @@
default:
LOGP(DLSS7, LOGL_ERROR, "Unknown M3UA XFER Message "
"Type %u\n", xua->hdr.msg_type);
+ xua_msg_free(xua);
return -1;
}
break;
@@ -131,6 +143,7 @@
/* Discard Message */
LOGP(DLSS7, LOGL_ERROR, "Unknown M3UA Message Class %u\n",
xua->hdr.msg_class);
+ xua_msg_free(xua);
return -1;
}
@@ -142,13 +155,16 @@
LOGP(DLSS7, LOGL_NOTICE, "No MTP-User for SI %u\n", service_ind);
/* Discard Message */
/* FIXME: User Part Unavailable HMDT -> HMRT */
+ xua_msg_free(xua);
return -1;
}
}
/* HMDC->HMRT Msg For Routing; Figure 26/Q.704 */
/* local message was receive d from L4, SRM, SLM, STM or SLTC, or
- * remote message received from L2 and HMDC determined msg for routing */
+ * remote message received from L2 and HMDC determined msg for routing
+ * Ownership of xua_msg passed is transferred to this function.
+ */
static int hmrt_message_for_routing(struct osmo_ss7_instance *inst,
struct xua_msg *xua)
{
@@ -215,12 +231,14 @@
/* Message Received for inaccesible SP HMRT ->RTPC */
/* Discard Message */
}
+ xua_msg_free(xua);
return -1;
}
/* HMDC: Received Message L2 -> L3; Figure 24/Q.704 */
/* This means a message was received from L2 and we have to decide if it
- * is for the local stack (HMDT) or for routng (HMRT) */
+ * is for the local stack (HMDT) or for routng (HMRT)
+ * Ownership of xua_msg passed is transferred to this function. */
int m3ua_hmdc_rx_from_l2(struct osmo_ss7_instance *inst, struct xua_msg *xua)
{
uint32_t dpc = xua->mtp.dpc;
@@ -257,7 +275,6 @@
* IPv4). So we call m3ua_hmdc_rx_from_l2() just like
* the MTP-TRANSFER had been received from L2. */
rc = m3ua_hmdc_rx_from_l2(inst, xua);
- xua_msg_free(xua);
break;
default:
LOGP(DLSS7, LOGL_ERROR, "Ignoring unknown primitive %u:%u\n",
diff --git a/src/sua.c b/src/sua.c
index 6d023cb..4d90f9b 100644
--- a/src/sua.c
+++ b/src/sua.c
@@ -291,13 +291,16 @@
static int sua_tx_xua_asp(struct osmo_ss7_asp *asp, struct xua_msg *xua)
{
- struct msgb *msg = sua_to_msg(xua);
+ struct msgb *msg;
OSMO_ASSERT(asp->cfg.proto == OSMO_SS7_ASP_PROT_SUA);
+ msg = sua_to_msg(xua);
+ xua_msg_free(xua);
if (!msg)
return -1;
+ /* msg becomes owned by osmo_ss7_asp_send here: */
return osmo_ss7_asp_send(asp, msg);
}
@@ -317,6 +320,7 @@
xua_msg_add_u32(xua, SUA_IEI_ROUTE_CTX, as->cfg.routing_key.context);
msg = sua_to_msg(xua);
+ xua_msg_free(xua);
if (!msg) {
LOGPAS(as, DLSUA, LOGL_ERROR, "Error encoding SUA Msg\n");
return -1;
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39396?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I49e718e624da08510a9c3d28bdb360c59f9a65d3
Gerrit-Change-Number: 39396
Gerrit-PatchSet: 11
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Attention is currently required from: laforge.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39377?usp=email )
Change subject: AS loadsharing: Initial routing implementation based on extended-SLS
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39377?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I0fb4ca4959096f748a23082efa0481300de56436
Gerrit-Change-Number: 39377
Gerrit-PatchSet: 12
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Thu, 06 Mar 2025 20:11:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
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 <pespin(a)sysmocom.de>
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39718?usp=email )
Change subject: mgw: Move struct mgcp_request_data definition to header file
......................................................................
mgw: Move struct mgcp_request_data definition to header file
All the related handling structs are placed in the header file, let's
move them there. This will also allow accessing the rq object
information in other files.
Change-Id: I1b2831874c1aaad054ad19bf87646062ba9d4da4
---
M include/osmocom/mgcp/mgcp_protocol.h
M src/libosmo-mgcp/mgcp_protocol.c
2 files changed, 25 insertions(+), 25 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/18/39718/1
diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h
index 4fed248..dda0fb3 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -82,6 +82,31 @@
struct mgcp_parse_sdp sdp;
};
+/* Request data passed to the request handler */
+struct mgcp_request_data {
+ enum mgcp_verb verb;
+ /* Verb string (e.g. "MDCX") */
+ char name[4+1];
+
+ /* parsing results from the MGCP header (trans id, endpoint name ...) */
+ struct mgcp_parse_data *pdata;
+
+ /* pointer to endpoint resource (may be NULL for wildcarded requests) */
+ struct mgcp_endpoint *endp;
+
+ /* pointer to trunk resource */
+ struct mgcp_trunk *trunk;
+
+ /* set to true when the request has been classified as wildcarded */
+ bool wildcarded;
+
+ /* Set to true when the request is targeted at the "null" endpoint */
+ bool null_endp;
+
+ /* contains cause code in case of problems during endp/trunk resolution */
+ int mgcp_cause;
+};
+
/* Local connection options */
struct mgcp_lco {
char *string;
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 36d9ebf..74ed14e 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -89,31 +89,6 @@
LOGPTRUNK(trunk, cat, level, fmt, ## args); \
} while (0)
-/* Request data passed to the request handler */
-struct mgcp_request_data {
- enum mgcp_verb verb;
- /* Verb string (e.g. "MDCX") */
- char name[4+1];
-
- /* parsing results from the MGCP header (trans id, endpoint name ...) */
- struct mgcp_parse_data *pdata;
-
- /* pointer to endpoint resource (may be NULL for wildcarded requests) */
- struct mgcp_endpoint *endp;
-
- /* pointer to trunk resource */
- struct mgcp_trunk *trunk;
-
- /* set to true when the request has been classified as wildcarded */
- bool wildcarded;
-
- /* Set to true when the request is targeted at the "null" endpoint */
- bool null_endp;
-
- /* contains cause code in case of problems during endp/trunk resolution */
- int mgcp_cause;
-};
-
static struct msgb *handle_audit_endpoint(struct mgcp_request_data *data);
static struct msgb *handle_create_con(struct mgcp_request_data *data);
static struct msgb *handle_delete_con(struct mgcp_request_data *data);
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39718?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: I1b2831874c1aaad054ad19bf87646062ba9d4da4
Gerrit-Change-Number: 39718
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>