pespin has uploaded this change for review.
mgw: Decouple SDP parsing step from conn obj update
This allows delaying modification of the conn before full parsing
succeeds.
Change-Id: I4a2aecb542886672c1f586c6607714e0356abe35
---
M include/osmocom/mgcp/mgcp_protocol.h
M include/osmocom/mgcp/mgcp_sdp.h
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
4 files changed, 118 insertions(+), 69 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/48/39248/1
diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h
index ffe7ae0..a6a7cec 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -1,7 +1,35 @@
#pragma once
+#include <stdint.h>
+#include <sys/socket.h>
+
#include <osmocom/core/utils.h>
+#include <osmocom/core/socket.h>
#include <osmocom/mgcp/mgcp_common.h>
+#include <osmocom/mgcp/mgcp_codec.h>
+
+
+#define MGCP_PARSE_SDP_PTIME_UNSET (-1)
+#define MGCP_PARSE_SDP_MAXPTIME_UNSET (-1)
+#define MGCP_PARSE_SDP_RTP_PORT_UNSET (0)
+
+struct mgcp_parse_sdp {
+ int ptime;
+ int maxptime;
+ int rtp_port;
+ struct osmo_sockaddr rem_addr; /* Only IP address, port is in rtp_port above */
+ struct mgcp_rtp_codecset cset;
+};
+
+static inline void mgcp_parse_sdp_init(struct mgcp_parse_sdp *sdp)
+{
+ sdp->ptime = MGCP_PARSE_SDP_PTIME_UNSET;
+ sdp->maxptime = MGCP_PARSE_SDP_MAXPTIME_UNSET;
+ sdp->rtp_port = MGCP_PARSE_SDP_RTP_PORT_UNSET;
+ sdp->rem_addr = (struct osmo_sockaddr){ .u.sa.sa_family = AF_UNSPEC };
+ mgcp_codecset_reset(&sdp->cset);
+}
+
#define MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET (-2)
#define MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD (-1)
@@ -39,6 +67,7 @@
char *trans;
/* MGCP Body: */
struct mgcp_parse_hdr_pars hpars;
+ struct mgcp_parse_sdp sdp;
};
/* Local connection options */
diff --git a/include/osmocom/mgcp/mgcp_sdp.h b/include/osmocom/mgcp/mgcp_sdp.h
index 950092e..4a1313f 100644
--- a/include/osmocom/mgcp/mgcp_sdp.h
+++ b/include/osmocom/mgcp/mgcp_sdp.h
@@ -22,9 +22,9 @@
#pragma once
-int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
- struct mgcp_conn_rtp *conn,
- struct mgcp_parse_data *p);
+struct mgcp_parse_data;
+
+int mgcp_parse_sdp_data(struct mgcp_parse_data *p);
int mgcp_write_response_sdp(const struct mgcp_endpoint *endp,
const struct mgcp_conn_rtp *conn, struct msgb *sdp,
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index d066f29..dfd3e0c 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -716,36 +716,59 @@
return 0;
}
+/* Apply parsed SDP information stored in struct mgcp_parse_sdp to conn_rtp: */
+static int handle_sdp(struct mgcp_conn_rtp *conn, struct mgcp_request_data *rq)
+{
+ OSMO_ASSERT(conn);
+ OSMO_ASSERT(rq);
+ struct mgcp_parse_data *p = rq->pdata;
+ OSMO_ASSERT(p);
+ OSMO_ASSERT(p->hpars.have_sdp);
+ struct mgcp_parse_sdp *sdp = &p->sdp;
+ struct mgcp_rtp_end *rtp;
+
+ rtp = &conn->end;
+
+ if (sdp->ptime != MGCP_PARSE_SDP_PTIME_UNSET)
+ mgcp_rtp_end_set_packet_duration_ms(rtp, sdp->ptime);
+
+ if (sdp->maxptime != MGCP_PARSE_SDP_MAXPTIME_UNSET)
+ rtp->maximum_packet_time = sdp->maxptime;
+
+ if (sdp->rem_addr.u.sa.sa_family != AF_UNSPEC) {
+ /* Keep port, only apply ip address: */
+ uint16_t port = osmo_sockaddr_port(&rtp->addr.u.sa);
+ memcpy(&rtp->addr, &sdp->rem_addr, sizeof(rtp->addr));
+ osmo_sockaddr_set_port(&rtp->addr.u.sa, port);
+ }
+
+ if (sdp->rtp_port != MGCP_PARSE_SDP_RTP_PORT_UNSET) {
+ osmo_sockaddr_set_port(&rtp->addr.u.sa, sdp->rtp_port);
+ rtp->rtcp_port = htons(sdp->rtp_port + 1);
+ }
+
+ /* Copy parsed codec set to conn: */
+ rtp->cset = sdp->cset;
+
+ return 0;
+}
+
/* Process codec information contained in CRCX/MDCX */
-static int handle_codec_info(struct mgcp_conn_rtp *conn,
- struct mgcp_request_data *rq, int have_sdp, bool crcx)
+static int handle_codec_info(struct mgcp_conn_rtp *conn, struct mgcp_request_data *rq)
{
struct mgcp_endpoint *endp = rq->endp;
struct mgcp_conn *conn_dst;
struct mgcp_conn_rtp *conn_dst_rtp;
struct mgcp_rtp_codecset *cset = &conn->end.cset;
-
int rc;
- char *cmd;
-
- if (crcx)
- cmd = "CRCX";
- else
- cmd = "MDCX";
/* Collect codec information */
- if (have_sdp) {
+ if (rq->pdata->hpars.have_sdp) {
/* If we have SDP, we ignore the local connection options and
* use only the SDP information. */
- mgcp_codecset_reset(cset);
- rc = mgcp_parse_sdp_data(endp, conn, rq->pdata);
- if (rc != 0) {
- LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR,
- "%s: sdp not parseable\n", cmd);
-
- /* See also RFC 3661: Protocol error */
- return 510;
- }
+ rc = handle_sdp(conn, rq);
+ if (rc != 0)
+ goto error;
} else if (endp->local_options.codec) {
/* When no SDP is available, we use the codec information from
* the local connection options (if present) */
@@ -780,9 +803,7 @@
return 0;
error:
- LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR,
- "%s: codec negotiation failure\n", cmd);
-
+ LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR, "%s: codec negotiation failure\n", rq->name);
/* See also RFC 3661: Codec negotiation failure */
return 534;
}
@@ -861,6 +882,15 @@
return create_err_response(endp, endp, 523, "CRCX", pdata->trans);
}
+ /* Parse SDP if found: */
+ if (hpars->have_sdp) {
+ rc = mgcp_parse_sdp_data(pdata);
+ if (rc < 0) { /* See also RFC 3661: Protocol error */
+ LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: sdp not parseable\n");
+ return create_err_response(endp, endp, 510, "CRCX", pdata->trans);
+ }
+ }
+
/* If osmux is disabled, just skip setting it up */
if (trunk->cfg->osmux.usage == OSMUX_USAGE_OFF)
hpars->remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET;
@@ -969,7 +999,7 @@
}
/* Handle codec information and decide for a suitable codec */
- rc = handle_codec_info(conn_rtp, rq, hpars->have_sdp, true);
+ rc = handle_codec_info(conn_rtp, rq);
mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn));
if (rc) {
error_code = rc;
@@ -1103,6 +1133,15 @@
return create_err_response(endp, endp, error_code, "MDCX", pdata->trans);
}
+ /* Parse SDP if found: */
+ if (hpars->have_sdp) {
+ rc = mgcp_parse_sdp_data(pdata);
+ if (rc < 0) { /* See also RFC 3661: Protocol error */
+ LOGPENDP(endp, DLMGCP, LOGL_ERROR, "MDCX: sdp not parseable\n");
+ return create_err_response(endp, endp, 510, "MDCX", pdata->trans);
+ }
+ }
+
conn = mgcp_endp_get_conn(endp, hpars->connid);
if (!conn) {
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_CONN_NOT_FOUND));
@@ -1136,7 +1175,7 @@
OSMO_ASSERT(conn_rtp);
/* Handle codec information and decide for a suitable codec */
- rc = handle_codec_info(conn_rtp, rq, hpars->have_sdp, false);
+ rc = handle_codec_info(conn_rtp, rq);
mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn));
if (rc) {
error_code = rc;
diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c
index ffd0f18..1d76ddb 100644
--- a/src/libosmo-mgcp/mgcp_sdp.c
+++ b/src/libosmo-mgcp/mgcp_sdp.c
@@ -312,16 +312,13 @@
}
/*! Analyze SDP input string.
- * \param[in] endp trunk endpoint.
- * \param[out] conn associated rtp connection.
- * \param[out] caller provided memory to store the parsing results.
+ * \param[inout] p provided memory to store the parsing results.
*
- * Note: In conn (conn->end) the function returns the packet duration,
- * rtp port, rtcp port and the codec information.
* \returns 0 on success, -1 on failure. */
-int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
- struct mgcp_conn_rtp *conn, struct mgcp_parse_data *p)
+int mgcp_parse_sdp_data(struct mgcp_parse_data *p)
{
+ OSMO_ASSERT(p);
+ struct mgcp_parse_sdp *sdp = &p->sdp;
struct sdp_rtp_map codecs[MGCP_MAX_CODECS];
unsigned int codecs_used = 0;
struct sdp_fmtp_param fmtp_params[MGCP_MAX_CODECS];
@@ -331,19 +328,14 @@
char *line;
unsigned int i;
void *tmp_ctx = talloc_new(NULL);
- struct mgcp_rtp_end *rtp;
int payload_type;
int ptime, ptime2 = 0;
char audio_name[64];
int port, rc;
- OSMO_ASSERT(endp);
- OSMO_ASSERT(conn);
- OSMO_ASSERT(p);
-
- rtp = &conn->end;
memset(&codecs, 0, sizeof(codecs));
+ mgcp_parse_sdp_init(sdp);
for_each_line(line, p->save) {
switch (line[0]) {
@@ -361,19 +353,19 @@
if (sscanf(line, "a=ptime:%d-%d", &ptime, &ptime2) >= 1) {
if (ptime2 > 0 && ptime2 != ptime)
- mgcp_rtp_end_set_packet_duration_ms(rtp, 0);
+ sdp->ptime = 0;
else
- mgcp_rtp_end_set_packet_duration_ms(rtp, ptime);
+ sdp->ptime = ptime;
break;
}
if (sscanf(line, "a=maxptime:%d", &ptime2) == 1) {
- rtp->maximum_packet_time = ptime2;
+ sdp->maxptime = ptime2;
break;
}
if (strncmp("a=fmtp:", line, 6) == 0) {
- rc = fmtp_from_sdp(conn->conn, &fmtp_params[fmtp_used], line);
+ rc = fmtp_from_sdp(tmp_ctx, &fmtp_params[fmtp_used], line);
if (rc >= 0)
fmtp_used++;
break;
@@ -382,33 +374,23 @@
break;
case 'm':
rc = sscanf(line, "m=audio %d RTP/AVP", &port);
- if (rc == 1) {
- osmo_sockaddr_set_port(&rtp->addr.u.sa, port);
- rtp->rtcp_port = htons(port + 1);
- }
+ if (rc == 1)
+ sdp->rtp_port = port;
- rc = pt_from_sdp(conn->conn, codecs,
- ARRAY_SIZE(codecs), line);
+ rc = pt_from_sdp(tmp_ctx, codecs, ARRAY_SIZE(codecs), line);
if (rc > 0)
codecs_used = rc;
break;
case 'c':
- if (audio_ip_from_sdp(&rtp->addr, line) < 0) {
+ if (audio_ip_from_sdp(&sdp->rem_addr, line) < 0) {
talloc_free(tmp_ctx);
return -1;
}
break;
default:
- if (endp)
- /* TODO: Check spec: We used the bare endpoint number before,
- * now we use the endpoint name as a whole? Is this allowed? */
- LOGP(DLMGCP, LOGL_NOTICE,
- "Unhandled SDP option: '%c'/%d on %s\n",
- line[0], line[0], endp->name);
- else
- LOGP(DLMGCP, LOGL_NOTICE,
- "Unhandled SDP option: '%c'/%d\n",
- line[0], line[0]);
+ LOGP(DLMGCP, LOGL_NOTICE,
+ "Unhandled SDP option: '%c'/%d on %s\n",
+ line[0], line[0], p->epname);
break;
}
}
@@ -422,23 +404,22 @@
/* Store parsed codec information */
for (i = 0; i < codecs_used; i++) {
codec_param = param_by_pt(codecs[i].payload_type, fmtp_params, fmtp_used);
- rc = mgcp_codecset_add_codec(&conn->end.cset, codecs[i].payload_type, codecs[i].map_line, codec_param);
+ rc = mgcp_codecset_add_codec(&sdp->cset, codecs[i].payload_type, codecs[i].map_line, codec_param);
if (rc < 0)
- LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "failed to add codec\n");
+ LOGP(DLMGCP, LOGL_NOTICE, "%s: failed to add codec\n", p->epname);
}
talloc_free(tmp_ctx);
- LOGPCONN(conn->conn, DLMGCP, LOGL_NOTICE,
- "Got media info via SDP: port:%d, addr:%s, duration:%d, payload-types:",
- osmo_sockaddr_port(&rtp->addr.u.sa), osmo_sockaddr_ntop(&rtp->addr.u.sa, ipbuf),
- rtp->packet_duration_ms);
+ LOGP(DLMGCP, LOGL_NOTICE,
+ "%s: Got media info via SDP: port:%d, addr:%s, duration:%d, payload-types:",
+ p->epname, sdp->rtp_port, osmo_sockaddr_ntop(&sdp->rem_addr.u.sa, ipbuf), sdp->ptime);
if (codecs_used == 0)
LOGPC(DLMGCP, LOGL_NOTICE, "none");
for (i = 0; i < codecs_used; i++) {
LOGPC(DLMGCP, LOGL_NOTICE, "%d=%s",
- rtp->cset.codecs[i].payload_type,
- strlen(rtp->cset.codecs[i].subtype_name) ? rtp->cset.codecs[i].subtype_name : "unknown");
+ sdp->cset.codecs[i].payload_type,
+ strlen(sdp->cset.codecs[i].subtype_name) ? sdp->cset.codecs[i].subtype_name : "unknown");
LOGPC(DLMGCP, LOGL_NOTICE, " ");
}
LOGPC(DLMGCP, LOGL_NOTICE, "\n");
To view, visit change 39248. To unsubscribe, or for help writing mail filters, visit settings.