mgw: MDCX: Simplify early return code paths

Change-Id: Icb03a95e34e0c7d9396fefc2e37ee33ab09daa89

include/osmocom/mgcp/mgcp_msg.h

@@ -39,10 +39,11 @@ void mgcp_disp_msg(unsigned char *message, unsigned int len, char *preamble);
 enum mgcp_connection_mode mgcp_parse_conn_mode(const char *msg);
 
 int mgcp_parse_header(struct mgcp_parse_data *pdata, char *data);
+int mgcp_parse_hdr_pars(struct mgcp_parse_data *pdata);
 
 int mgcp_parse_osmux_cid(const char *line);
 
-bool mgcp_check_param(const struct mgcp_endpoint *endp, struct mgcp_trunk *trunk, const char *line);
+bool mgcp_check_param(const char *line);
 
 int mgcp_verify_call_id(struct mgcp_endpoint *endp, const char *callid);
 

include/osmocom/mgcp/mgcp_protocol.h

@@ -1,14 +1,73 @@
 #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)
+
+struct mgcp_parse_hdr_pars {
+	const char *local_options;
+	const char *callid;
+	const char *connid;
+	enum mgcp_connection_mode mode;
+	int remote_osmux_cid;
+	bool have_sdp;
+	/*! MGCP_X_OSMO_IGN_* flags from 'X-Osmo-IGN:' header */
+	uint32_t x_osmo_ign;
+};
+
+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,
+	};
+}
 
 /* Internal structure while parsing a request */
 struct mgcp_parse_data {
 	struct mgcp_config *cfg;
+	char *save;
+	/* MGCP Header: */
 	char *epname;
 	char *trans;
-	char *save;
+	struct mgcp_parse_hdr_pars hpars;
+	/* MGCP Body: */
+	struct mgcp_parse_sdp sdp;
 };
 
 /* Local connection options */

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,

src/libosmo-mgcp/mgcp_msg.c

@@ -23,6 +23,7 @@
  */
 
 #include <limits.h>
+#include <ctype.h>
 
 #include <osmocom/mgcp/mgcp.h>
 #include <osmocom/mgcp/osmux.h>
@@ -33,6 +34,10 @@
 #include <osmocom/mgcp/mgcp_endp.h>
 #include <osmocom/mgcp/mgcp_trunk.h>
 
+/* (same fmt as LOGPENDP()) */
+#define LOG_MGCP_PDATA(PDATA, LEVEL, FMT, ARGS...) \
+	LOGP(DLMGCP, LEVEL, "endpoint:%s " FMT, (PDATA) ? ((PDATA)->epname ? : "null-epname") : "null-pdata", ##ARGS)
+
 /*! Display an mgcp message on the log output.
  *  \param[in] message mgcp message string
  *  \param[in] len message mgcp message string length
@@ -122,8 +127,7 @@ int mgcp_parse_header(struct mgcp_parse_data *pdata, char *data)
 			break;
 		case 2:
 			if (strcasecmp("MGCP", elem)) {
-				LOGP(DLMGCP, LOGL_ERROR,
-				     "MGCP header parsing error\n");
+				LOG_MGCP_PDATA(pdata, LOGL_ERROR, "MGCP header parsing error\n");
 				return -510;
 			}
 			break;
@@ -136,13 +140,87 @@ int mgcp_parse_header(struct mgcp_parse_data *pdata, char *data)
 	}
 
 	if (i != 4) {
-		LOGP(DLMGCP, LOGL_ERROR, "MGCP status line too short.\n");
+		LOG_MGCP_PDATA(pdata, LOGL_ERROR, "MGCP status line too short.\n");
 		return -510;
 	}
 
 	return 0;
 }
 
+static bool parse_x_osmo_ign(struct mgcp_parse_data *pdata, char *line)
+{
+	char *saveptr = NULL;
+
+	if (strncasecmp(line, MGCP_X_OSMO_IGN_HEADER, strlen(MGCP_X_OSMO_IGN_HEADER)))
+		return false;
+	line += strlen(MGCP_X_OSMO_IGN_HEADER);
+
+	while (1) {
+		char *token = strtok_r(line, " ", &saveptr);
+		line = NULL;
+		if (!token)
+			break;
+
+		if (!strcasecmp(token, "C"))
+			pdata->hpars.x_osmo_ign |= MGCP_X_OSMO_IGN_CALLID;
+		else
+			LOG_MGCP_PDATA(pdata, LOGL_ERROR,"received unknown X-Osmo-IGN item '%s'\n", token);
+	}
+
+	return true;
+}
+
+/*! Analyze and parse the the header of an MGCP message string.
+ *  \param[inout] pdata caller provided memory to store the parsing results.
+ *  \returns 0 when parsing was successful, negative (MGCP cause code) on error. */
+int mgcp_parse_hdr_pars(struct mgcp_parse_data *pdata)
+{
+	struct mgcp_parse_hdr_pars *hp = &pdata->hpars;
+	char *line;
+
+	mgcp_parse_hdr_pars_init(hp);
+
+	for_each_line(line, pdata->save) {
+		if (!mgcp_check_param(line)) {
+			LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line);
+			continue;
+		}
+
+		switch (toupper(line[0])) {
+		case 'L':
+			hp->local_options = (const char *)line + 3;
+			break;
+		case 'C':
+			hp->callid = (const char *)line + 3;
+			break;
+		case 'I':
+			hp->connid = (const char *)line + 3;
+			break;
+		case 'M':
+			hp->mode = mgcp_parse_conn_mode((const char *)line + 3);
+			break;
+		case 'X':
+			if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) {
+				hp->remote_osmux_cid = mgcp_parse_osmux_cid(line);
+				break;
+			}
+			if (parse_x_osmo_ign(pdata, line))
+				break;
+			/* Ignore unknown X-headers */
+			break;
+		case '\0':
+			hp->have_sdp = true;
+			goto mgcp_header_done;
+		default:
+			LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "CRCX: unhandled option: '%c'/%d\n", *line, *line);
+			return -539;
+		}
+	}
+
+mgcp_header_done:
+	return 0;
+}
+
 /*! Extract OSMUX CID from an MGCP parameter line (string).
  *  \param[in] line single parameter line from the MGCP message
  *  \returns OSMUX CID, -1 wildcard, -2 on error */
@@ -153,19 +231,19 @@ int mgcp_parse_osmux_cid(const char *line)
 
 	if (strcasecmp(line + 2, "Osmux: *") == 0) {
 		LOGP(DLMGCP, LOGL_DEBUG, "Parsed wilcard Osmux CID\n");
-		return -1;
+		return MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD;
 	}
 
 	if (sscanf(line + 2 + 7, "%u", &osmux_cid) != 1) {
 		LOGP(DLMGCP, LOGL_ERROR, "Failed parsing Osmux in MGCP msg line: %s\n",
 		     line);
-		return -2;
+		return MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET;
 	}
 
 	if (osmux_cid > OSMUX_CID_MAX) {
 		LOGP(DLMGCP, LOGL_ERROR, "Osmux ID too large: %u > %u\n",
 		     osmux_cid, OSMUX_CID_MAX);
-		return -2;
+		return MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET;
 	}
 	LOGP(DLMGCP, LOGL_DEBUG, "MGCP client offered Osmux CID %u\n", osmux_cid);
 
@@ -173,20 +251,13 @@ int mgcp_parse_osmux_cid(const char *line)
 }
 
 /*! Check MGCP parameter line (string) for plausibility.
- *  \param[in] endp pointer to endpoint (only used for log output, may be NULL)
- *  \param[in] trunk pointer to trunk (only used for log output, may be NULL if endp is not NULL)
  *  \param[in] line single parameter line from the MGCP message
  *  \returns true when line seems plausible, false on error */
-bool mgcp_check_param(const struct mgcp_endpoint *endp, struct mgcp_trunk *trunk, const char *line)
+bool mgcp_check_param(const char *line)
 {
 	const size_t line_len = strlen(line);
-	if (line[0] != '\0' && line_len < 2) {
-		if (endp)
-			LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line);
-		else
-			LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line);
+	if (line[0] != '\0' && line_len < 2)
 		return false;
-	}
 
 	/* FIXME: A couple more checks wouldn't hurt... */
 

src/libosmo-mgcp/mgcp_protocol.c

@@ -702,54 +702,73 @@ uint32_t mgcp_rtp_packet_duration(const struct mgcp_endpoint *endp,
 	    codec->frame_duration_den;
 }
 
-/*! Initializes osmux socket if not yet initialized. Parses Osmux CID from MGCP line.
- *  \param[in] endp Endpoint willing to initialize osmux
- *  \param[in] line Line X-Osmux from MGCP header msg to parse
- *  \returns OSMUX CID, -1 for wildcard, -2 on parse error, -3 on osmux initalize error
- */
-static int mgcp_osmux_setup(struct mgcp_endpoint *endp, const char *line)
+static int mgcp_trunk_osmux_init_if_needed(struct mgcp_trunk *trunk)
 {
-	if (!endp->trunk->cfg->osmux.initialized) {
-		if (osmux_init(endp->trunk) < 0) {
-			LOGPENDP(endp, DOSMUX, LOGL_ERROR, "Cannot init OSMUX\n");
-			return -3;
-		}
-		LOGPENDP(endp, DOSMUX, LOGL_NOTICE, "OSMUX socket has been set up\n");
+	if (trunk->cfg->osmux.initialized)
+		return 0;
+
+	if (osmux_init(trunk) < 0) {
+		LOGPTRUNK(trunk, DOSMUX, LOGL_ERROR, "Cannot init OSMUX\n");
+		return -3;
+	}
+	LOGPTRUNK(trunk, DOSMUX, LOGL_NOTICE, "OSMUX socket has been set up\n");
+
+	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);
 	}
 
-	return mgcp_parse_osmux_cid(line);
+	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) */
@@ -784,49 +803,21 @@ static int handle_codec_info(struct mgcp_conn_rtp *conn,
 	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;
 }
 
-static bool parse_x_osmo_ign(struct mgcp_endpoint *endp, char *line)
-{
-	char *saveptr = NULL;
-
-	if (strncasecmp(line, MGCP_X_OSMO_IGN_HEADER, strlen(MGCP_X_OSMO_IGN_HEADER)))
-		return false;
-	line += strlen(MGCP_X_OSMO_IGN_HEADER);
-
-	while (1) {
-		char *token = strtok_r(line, " ", &saveptr);
-		line = NULL;
-		if (!token)
-			break;
-
-		if (!strcasecmp(token, "C"))
-			endp->x_osmo_ign |= MGCP_X_OSMO_IGN_CALLID;
-		else
-			LOGPENDP(endp, DLMGCP, LOGL_ERROR, "received unknown X-Osmo-IGN item '%s'\n", token);
-	}
-
-	return true;
-}
-
 /* CRCX command handler, processes the received command */
 static struct msgb *handle_create_con(struct mgcp_request_data *rq)
 {
 	struct mgcp_parse_data *pdata = rq->pdata;
 	struct mgcp_trunk *trunk = rq->trunk;
 	struct mgcp_endpoint *endp = rq->endp;
+	struct mgcp_parse_hdr_pars *hpars = &pdata->hpars;
 	struct rate_ctr_group *rate_ctrs;
 	int error_code = 400;
-	const char *local_options = NULL;
-	const char *callid = NULL;
-	enum mgcp_connection_mode mode = MGCP_CONN_NONE;
-	char *line;
-	int have_sdp = 0, remote_osmux_cid = -2;
+	int remote_osmux_cid;
 	struct mgcp_conn *conn = NULL;
 	struct mgcp_conn_rtp *conn_rtp = NULL;
 	char conn_name[512];
@@ -857,69 +848,59 @@ static struct msgb *handle_create_con(struct mgcp_request_data *rq)
 	}
 
 	/* parse CallID C: and LocalParameters L: */
-	for_each_line(line, pdata->save) {
-		if (!mgcp_check_param(endp, trunk, line))
-			continue;
-
-		switch (toupper(line[0])) {
-		case 'L':
-			local_options = (const char *)line + 3;
-			break;
-		case 'C':
-			callid = (const char *)line + 3;
-			break;
-		case 'I':
-			/* It is illegal to send a connection identifier
-			 * together with a CRCX, the MGW will assign the
-			 * connection identifier by itself on CRCX */
-			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BAD_ACTION));
-			return create_err_response(rq->trunk, NULL, 523, "CRCX", pdata->trans);
-			break;
-		case 'M':
-			mode = mgcp_parse_conn_mode((const char *)line + 3);
-			break;
-		case 'X':
-			if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) {
-				/* If osmux is disabled, just skip setting it up */
-				if (rq->endp->trunk->cfg->osmux.usage == OSMUX_USAGE_OFF)
-					break;
-				remote_osmux_cid = mgcp_osmux_setup(endp, line);
-				break;
-			}
-
-			if (parse_x_osmo_ign(endp, line))
-				break;
-
-			/* Ignore unknown X-headers */
-			break;
-		case '\0':
-			have_sdp = 1;
-			goto mgcp_header_done;
-		default:
-			LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
-				 "CRCX: unhandled option: '%c'/%d\n", *line, *line);
-			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNHANDLED_PARAM));
-			return create_err_response(rq->trunk, NULL, 539, "CRCX", pdata->trans);
-			break;
-		}
+	rc = mgcp_parse_hdr_pars(pdata);
+	switch (rc) {
+	case 0:
+		break; /* all good, continue below */
+	case -539:
+		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNHANDLED_PARAM));
+		return create_err_response(rq->trunk, NULL, -rc, "CRCX", pdata->trans);
+	default:
+		return create_err_response(rq->trunk, NULL, -rc, "CRCX", pdata->trans);
 	}
 
-mgcp_header_done:
 	/* Check parameters */
-	if (!callid) {
+	if (!hpars->callid) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "CRCX: insufficient parameters, missing callid\n");
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_MISSING_CALLID));
 		return create_err_response(endp, endp, 516, "CRCX", pdata->trans);
 	}
 
-	if (mode == MGCP_CONN_NONE) {
+	if (hpars->mode == MGCP_CONN_NONE) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "CRCX: insufficient parameters, invalid mode\n");
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE));
 		return create_err_response(endp, endp, 517, "CRCX", pdata->trans);
 	}
 
+	/* It is illegal to send a connection identifier
+	 * together with a CRCX, the MGW will assign the
+	 * connection identifier by itself on CRCX */
+	if (hpars->connid) {
+		LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: 'I: %s' not expected!\n", hpars->connid);
+		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BAD_ACTION));
+		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);
+		}
+	}
+
+	remote_osmux_cid = hpars->remote_osmux_cid;
+	/* If osmux is disabled, just skip setting it up */
+	if (trunk->cfg->osmux.usage == OSMUX_USAGE_OFF)
+		remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET;
+
+	/* Make sure osmux is setup: */
+	if (remote_osmux_cid != MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET)
+		mgcp_trunk_osmux_init_if_needed(trunk);
+
 	/* Check if we are able to accept the creation of another connection */
 	if (mgcp_endp_is_full(endp)) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
@@ -939,9 +920,12 @@ mgcp_header_done:
 		}
 	}
 
+	/* Update endp->x_osmo_ign: */
+	endp->x_osmo_ign |= hpars->x_osmo_ign;
+
 	/* Check if this endpoint already serves a call, if so, check if the
 	 * callids match up so that we are sure that this is our call */
-	if (endp->callid && mgcp_verify_call_id(endp, callid)) {
+	if (endp->callid && mgcp_verify_call_id(endp, hpars->callid)) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "CRCX: already seized by other call (%s)\n",
 			 endp->callid);
@@ -961,14 +945,14 @@ mgcp_header_done:
 		/* Claim endpoint resources. This will also set the callid,
 		 * creating additional connections will only be possible if
 		 * the callid matches up (see above). */
-		rc = mgcp_endp_claim(endp, callid);
+		rc = mgcp_endp_claim(endp, hpars->callid);
 		if (rc != 0) {
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_CLAIM));
 			return create_err_response(endp, endp, 502, "CRCX", pdata->trans);
 		}
 	}
 
-	snprintf(conn_name, sizeof(conn_name), "%s", callid);
+	snprintf(conn_name, sizeof(conn_name), "%s", hpars->callid);
 	conn = mgcp_conn_alloc(trunk->endpoints, endp, MGCP_CONN_TYPE_RTP, conn_name);
 	if (!conn) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
@@ -977,7 +961,7 @@ mgcp_header_done:
 		goto error2;
 	}
 
-	if (mgcp_conn_set_mode(conn, mode) < 0) {
+	if (mgcp_conn_set_mode(conn, hpars->mode) < 0) {
 		error_code = 517;
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE));
 		goto error2;
@@ -1004,9 +988,9 @@ mgcp_header_done:
 	}
 
 	/* Set local connection options, if present */
-	if (local_options) {
+	if (hpars->local_options) {
 		rc = set_local_cx_options(trunk->endpoints,
-					  &endp->local_options, local_options);
+					  &endp->local_options, hpars->local_options);
 		if (rc != 0) {
 			LOGPCONN(conn, DLMGCP, LOGL_ERROR,
 				 "CRCX: invalid local connection options!\n");
@@ -1017,7 +1001,7 @@ mgcp_header_done:
 	}
 
 	/* Handle codec information and decide for a suitable codec */
-	rc = handle_codec_info(conn_rtp, rq, 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;
@@ -1087,17 +1071,12 @@ static struct msgb *handle_modify_con(struct mgcp_request_data *rq)
 	struct mgcp_parse_data *pdata = rq->pdata;
 	struct mgcp_trunk *trunk = rq->trunk;
 	struct mgcp_endpoint *endp = rq->endp;
+	struct mgcp_parse_hdr_pars *hpars = &pdata->hpars;
 	struct rate_ctr_group *rate_ctrs;
 	char new_local_addr[INET6_ADDRSTRLEN];
 	int error_code = 500;
-	int have_sdp = 0;
-	char *line;
-	const char *local_options = NULL;
-	enum mgcp_connection_mode mode = MGCP_CONN_NONE;
 	struct mgcp_conn *conn = NULL;
 	struct mgcp_conn_rtp *conn_rtp = NULL;
-	const char *conn_id = NULL;
-	int remote_osmux_cid = -2;
 	int rc;
 
 	LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "MDCX: modifying existing connection ...\n");
@@ -1130,64 +1109,43 @@ static struct msgb *handle_modify_con(struct mgcp_request_data *rq)
 		return create_err_response(endp, endp, 400, "MDCX", pdata->trans);
 	}
 
-	for_each_line(line, pdata->save) {
-		if (!mgcp_check_param(endp, trunk, line))
-			continue;
-
-		switch (toupper(line[0])) {
-		case 'C':
-			if (mgcp_verify_call_id(endp, line + 3) != 0) {
-				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CALLID));
-				error_code = 516;
-				goto error3;
-			}
-			break;
-		case 'I':
-			conn_id = (const char *)line + 3;
-			if ((error_code = mgcp_verify_ci(endp, conn_id))) {
-				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CONNID));
-				goto error3;
-			}
-			break;
-		case 'L':
-			local_options = (const char *)line + 3;
-			break;
-		case 'M':
-			mode = mgcp_parse_conn_mode((const char *)line + 3);
-			break;
-		case 'X':
-			if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) {
-				/* If osmux is disabled, just skip setting it up */
-				if (endp->trunk->cfg->osmux.usage == OSMUX_USAGE_OFF)
-					break;
-				remote_osmux_cid = mgcp_osmux_setup(endp, line);
-				break;
-			}
-			/* Ignore unknown X-headers */
-			break;
-		case '\0':
-			have_sdp = 1;
-			goto mgcp_header_done;
-			break;
-		default:
-			LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
-				 "MDCX: Unhandled MGCP option: '%c'/%d\n",
-				 line[0], line[0]);
-			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_UNHANDLED_PARAM));
-			return create_err_response(rq->trunk, NULL, 539, "MDCX", pdata->trans);
-			break;
-		}
+	rc = mgcp_parse_hdr_pars(pdata);
+	switch (rc) {
+	case 0:
+		break; /* all good, continue below */
+	case -539:
+		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_UNHANDLED_PARAM));
+		return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans);
+	default:
+		return create_err_response(rq->trunk, NULL, -rc, "MDCX", pdata->trans);
 	}
 
-mgcp_header_done:
-	if (!conn_id) {
+	if (hpars->callid && mgcp_verify_call_id(endp, hpars->callid) < 0) {
+		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CALLID));
+		return create_err_response(endp, endp, 516, "MDCX", pdata->trans);
+	}
+
+	if (!hpars->connid) {
 		LOGPENDP(endp, DLMGCP, LOGL_ERROR,
 			 "MDCX: insufficient parameters, missing ci (connectionIdentifier)\n");
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_NO_CONNID));
 		return create_err_response(endp, endp, 515, "MDCX", pdata->trans);
+	} else if ((error_code = mgcp_verify_ci(endp, hpars->connid)) != 0) {
+		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CONNID));
+		return create_err_response(endp, endp, error_code, "MDCX", pdata->trans);
 	}
 
-	conn = mgcp_endp_get_conn(endp, conn_id);
+	/* 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));
 		return create_err_response(endp, endp, 400, "MDCX", pdata->trans);
@@ -1195,20 +1153,18 @@ mgcp_header_done:
 
 	mgcp_conn_watchdog_kick(conn);
 
-	if (mode != MGCP_CONN_NONE) {
-		if (mgcp_conn_set_mode(conn, mode) < 0) {
-			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_MODE));
-			error_code = 517;
-			goto error3;
-		}
-	} else {
+	if (hpars->mode == MGCP_CONN_NONE) {
+		/* Reset conn mode in case it was tweaked through VTY: */
 		conn->mode = conn->mode_orig;
+	} else if (mgcp_conn_set_mode(conn, hpars->mode) < 0) {
+		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_MODE));
+		return create_err_response(endp, endp, 517, "MDCX", pdata->trans);
 	}
 
 	/* Set local connection options, if present */
-	if (local_options) {
+	if (hpars->local_options) {
 		rc = set_local_cx_options(trunk->endpoints,
-					  &endp->local_options, local_options);
+					  &endp->local_options, hpars->local_options);
 		if (rc != 0) {
 			LOGPCONN(conn, DLMGCP, LOGL_ERROR,
 				 "MDCX: invalid local connection options!\n");
@@ -1222,41 +1178,37 @@ mgcp_header_done:
 	OSMO_ASSERT(conn_rtp);
 
 	/* Handle codec information and decide for a suitable codec */
-	rc = handle_codec_info(conn_rtp, rq, 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;
 		goto error3;
 	}
 	/* Upgrade the conn type RTP_DEFAULT->RTP_IUUP if needed based on requested codec: */
-	/* TODO: "codec" probably needs to be moved from endp to conn */
 	if (conn_rtp->type == MGCP_RTP_DEFAULT &&
 	    strcmp(conn_rtp->end.cset.codec->subtype_name, "VND.3GPP.IUFP") == 0)
 		rc = mgcp_conn_iuup_init(conn_rtp);
 
 	if (mgcp_conn_rtp_is_osmux(conn_rtp)) {
 		OSMO_ASSERT(conn_rtp->osmux.local_cid_allocated);
-		if (remote_osmux_cid < -1) {
-			LOGPCONN(conn, DLMGCP, LOGL_ERROR,
-				 "MDCX: Failed to parse Osmux CID!\n");
+		if (hpars->remote_osmux_cid < -1) {
+			LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: Failed to parse Osmux CID!\n");
 			goto error3;
-		} else if (remote_osmux_cid == -1) {
-			LOGPCONN(conn, DLMGCP, LOGL_ERROR,
-				 "MDCX: wilcard in MDCX is not supported!\n");
+		}
+		if (hpars->remote_osmux_cid == -1) {
+			LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: wilcard in MDCX is not supported!\n");
 			goto error3;
-		} else if (conn_rtp->osmux.remote_cid_present &&
-			   remote_osmux_cid != conn_rtp->osmux.remote_cid) {
-			LOGPCONN(conn, DLMGCP, LOGL_ERROR,
-				"MDCX: changing already allocated CID is not supported!\n");
+		}
+		if (conn_rtp->osmux.remote_cid_present &&
+		    hpars->remote_osmux_cid != conn_rtp->osmux.remote_cid) {
+			LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: changing already allocated CID is not supported!\n");
+			goto error3;
+		}
+		conn_rtp->osmux.remote_cid_present = true;
+		conn_rtp->osmux.remote_cid = hpars->remote_osmux_cid;
+		if (conn_osmux_event_rx_crcx_mdcx(conn_rtp) < 0) {
+			LOGPCONN(conn, DLMGCP, LOGL_ERROR, "MDCX: Osmux handling failed!\n");
 			goto error3;
-		} else {
-			conn_rtp->osmux.remote_cid_present = true;
-			conn_rtp->osmux.remote_cid = remote_osmux_cid;
-			if (conn_osmux_event_rx_crcx_mdcx(conn_rtp) < 0) {
-				LOGPCONN(conn, DLMGCP, LOGL_ERROR,
-					"MDCX: Osmux handling failed!\n");
-				goto error3;
-			}
 		}
 	}
 
@@ -1308,13 +1260,12 @@ static struct msgb *handle_delete_con(struct mgcp_request_data *rq)
 	struct mgcp_parse_data *pdata = rq->pdata;
 	struct mgcp_trunk *trunk = rq->trunk;
 	struct mgcp_endpoint *endp = rq->endp;
+	struct mgcp_parse_hdr_pars *hpars = &pdata->hpars;
 	struct rate_ctr_group *rate_ctrs;
-	int error_code = 400;
-	char *line;
 	char stats[1048];
-	const char *conn_id = NULL;
 	struct mgcp_conn *conn = NULL;
 	unsigned int i;
+	int rc;
 
 	/* NOTE: In this handler we can not take it for granted that the endp
 	 * pointer will be populated, however a trunk is always guaranteed (except for 'null' endp).
@@ -1356,49 +1307,42 @@ static struct msgb *handle_delete_con(struct mgcp_request_data *rq)
 		return create_ok_response(trunk, NULL, 200, "DLCX", pdata->trans);
 	}
 
-	for_each_line(line, pdata->save) {
-		if (!mgcp_check_param(endp, trunk, line))
-			continue;
+	rc = mgcp_parse_hdr_pars(pdata);
+	switch (rc) {
+	case 0:
+		break; /* all good, continue below */
+	case -539:
+		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
+		return create_err_response(rq->trunk, NULL, -rc, "DLCX", pdata->trans);
+	default:
+		return create_err_response(rq->trunk, NULL, -rc, "DLCX", pdata->trans);
+	}
 
-		switch (toupper(line[0])) {
-		case 'C':
-			/* If we have no endpoint, but a call id in the request,
-			   then this request cannot be handled */
-			if (!endp) {
-				LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE,
-					  "cannot handle requests with call-id (C) without endpoint -- abort!");
-				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
-				return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans);
-			}
-
-			if (mgcp_verify_call_id(endp, line + 3) != 0) {
-				error_code = 516;
-				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CALLID));
-				goto error3;
-			}
-			break;
-		case 'I':
-			/* If we have no endpoint, but a connection id in the request,
-			   then this request cannot be handled */
-			if (!endp) {
-				LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE,
-					  "cannot handle requests with conn-id (I) without endpoint -- abort!");
-				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
-				return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans);
-			}
-
-			conn_id = (const char *)line + 3;
-			if ((error_code = mgcp_verify_ci(endp, conn_id))) {
-				rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID));
-				goto error3;
-			}
-			break;
-		default:
-			LOGPEPTR(endp, trunk, DLMGCP, LOGL_NOTICE, "DLCX: Unhandled MGCP option: '%c'/%d\n",
-				 line[0], line[0]);
+	if (hpars->callid) {
+		/* If we have no endpoint, but a call id in the request, then this request cannot be handled */
+		if (!endp) {
+			LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE,
+				"cannot handle requests with call-id (C) without endpoint -- abort!");
 			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
 			return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans);
-			break;
+		}
+		if (mgcp_verify_call_id(endp, hpars->callid) != 0) {
+			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CALLID));
+			return create_err_response(endp, endp, 516, "DLCX", pdata->trans);
+		}
+	}
+
+	if (hpars->connid) {
+		/* If we have no endpoint, but a connection id in the request, then this request cannot be handled */
+		if (!endp) {
+			LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE,
+				  "cannot handle requests with conn-id (I) without endpoint -- abort!");
+			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM));
+			return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans);
+		}
+		if ((rc = mgcp_verify_ci(endp, hpars->connid)) != 0) {
+			rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID));
+			return create_err_response(endp, endp, rc, "DLCX", pdata->trans);
 		}
 	}
 
@@ -1410,7 +1354,7 @@ static struct msgb *handle_delete_con(struct mgcp_request_data *rq)
 	 * wildcarded DLCX that refers to the selected endpoint. This means
 	 * that we drop all connections on that specific endpoint at once.
 	 * (See also RFC3435 Section F.7) */
-	if (!conn_id) {
+	if (!hpars->connid) {
 		int num_conns = mgcp_endp_num_conns(endp);
 		LOGPENDP(endp, DLMGCP, LOGL_NOTICE,
 			 "DLCX: missing ci (connectionIdentifier), will remove all connections (%d total) at once\n",
@@ -1428,10 +1372,10 @@ static struct msgb *handle_delete_con(struct mgcp_request_data *rq)
 	}
 
 	/* Find the connection */
-	conn = mgcp_endp_get_conn(endp, conn_id);
+	conn = mgcp_endp_get_conn(endp, hpars->connid);
 	if (!conn) {
 		rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID));
-		goto error3;
+		return create_err_response(endp, endp, 400, "DLCX", pdata->trans);
 	}
 	/* save the statistics of the current connection */
 	mgcp_format_stats(stats, sizeof(stats), conn);
@@ -1452,9 +1396,6 @@ static struct msgb *handle_delete_con(struct mgcp_request_data *rq)
 
 	rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_SUCCESS));
 	return create_ok_resp_with_param(endp, endp, 250, "DLCX", pdata->trans, stats);
-
-error3:
-	return create_err_response(endp, endp, error_code, "DLCX", pdata->trans);
 }
 
 /* RSIP command handler, processes the received command */

src/libosmo-mgcp/mgcp_sdp.c

@@ -312,16 +312,13 @@ static struct mgcp_codec_param *param_by_pt(int pt, struct sdp_fmtp_param *fmtp_
 }
 
 /*! 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 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
 	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 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
 
 			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 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
 			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 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
 	/* 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");