pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39248?usp=email ) Change subject: mgw: Decouple SDP parsing step from conn obj update ...................................................................... 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 https://gerrit.osmocom.org/c/osmo-mgw/+/39248?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: I4a2aecb542886672c1f586c6607714e0356abe35 Gerrit-Change-Number: 39248 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt;