Get rid of separate rtp_port field

Let's use the port part of the struct mgcp_rtp_end->addr field instead
of keeping it separate. This makes it easier passing around and
using/checking the RTP remote address + port, and avoids confusion
having to check stuff outside of the address, with its port part
potentially unset.

Change-Id: I294eb5d85fae79bf62d36eb9e818426e187d442c
This commit is contained in:
Pau Espin 2022-10-04 13:45:48 +02:00
parent 051eb4d563
commit 5ffd127384
9 changed files with 50 additions and 70 deletions

View File

@ -208,4 +208,4 @@ int mgcp_send_reset_all(struct mgcp_config *cfg);
int mgcp_create_bind(const char *source_addr, struct osmo_fd *fd, int port, uint8_t dscp,
uint8_t prio);
int mgcp_udp_send(int fd, struct osmo_sockaddr *addr, int port, const char *buf, int len);
int mgcp_udp_send(int fd, const struct osmo_sockaddr *addr, const char *buf, int len);

View File

@ -93,7 +93,7 @@ struct mgcp_rtp_end {
struct osmo_sockaddr addr;
/* in network byte order */
int rtp_port, rtcp_port;
int rtcp_port;
/* currently selected audio codec */
struct mgcp_rtp_codec *codec;

View File

@ -105,7 +105,8 @@ static int mgcp_rtp_conn_init(struct mgcp_conn_rtp *conn_rtp, struct mgcp_conn *
end->rtp.fd = -1;
end->rtcp.fd = -1;
end->rtp_port = end->rtcp_port = 0;
memset(&end->addr, 0, sizeof(end->addr));
end->rtcp_port = 0;
talloc_free(end->fmtp_extra);
end->fmtp_extra = NULL;
@ -368,7 +369,7 @@ char *mgcp_conn_dump(struct mgcp_conn *conn)
conn->name,
conn->id,
osmo_sockaddr_ntop(&conn->u.rtp.end.addr.u.sa, ipbuf),
ntohs(conn->u.rtp.end.rtp_port),
osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa),
ntohs(conn->u.rtp.end.rtcp_port));
break;

View File

@ -489,7 +489,7 @@ static int mgcp_send_iuup(struct mgcp_endpoint *endp, struct msgb *msg,
"rtp_port:%u rtcp_port:%u\n",
dest_name,
osmo_sockaddr_ntop(&rtp_end->addr.u.sa, ipbuf),
ntohs(rtp_end->rtp_port), ntohs(rtp_end->rtcp_port)
osmo_sockaddr_port(&rtp_end->addr.u.sa), ntohs(rtp_end->rtcp_port)
);
return 0;
}
@ -505,14 +505,13 @@ static int mgcp_send_iuup(struct mgcp_endpoint *endp, struct msgb *msg,
LOGPENDP(endp, DRTP, LOGL_DEBUG,
"process/send IuUP to %s %s rtp_port:%u rtcp_port:%u\n",
dest_name, osmo_sockaddr_ntop(&rtp_end->addr.u.sa, ipbuf),
ntohs(rtp_end->rtp_port), ntohs(rtp_end->rtcp_port));
osmo_sockaddr_port(&rtp_end->addr.u.sa), ntohs(rtp_end->rtcp_port));
/* Forward a copy of the RTP data to a debug ip/port */
forward_data_tap(rtp_end->rtp.fd, &conn_src->tap_out,
msg);
len = mgcp_udp_send(rtp_end->rtp.fd, &rtp_end->addr, rtp_end->rtp_port,
(char *)hdr, buflen);
len = mgcp_udp_send(rtp_end->rtp.fd, &rtp_end->addr, (char *)hdr, buflen);
if (len <= 0)
return len;
@ -617,9 +616,8 @@ int mgcp_conn_iuup_dispatch_rtp(struct msgb *msg)
force_output_enabled = true;
/* Fill in the peer address so that we can send Init-ACK back: */
prev_rem_addr = conn_rtp_src->end.addr;
prev_rem_rtp_port = conn_rtp_src->end.rtp_port;
prev_rem_rtp_port = osmo_sockaddr_port(&conn_rtp_src->end.addr.u.sa);
conn_rtp_src->end.addr = *mc->from_addr;
conn_rtp_src->end.rtp_port = htons(osmo_sockaddr_port(&mc->from_addr->u.sa));
}
rc = _conn_iuup_rtp_pl_up(conn_rtp_src, msg);
@ -627,7 +625,7 @@ int mgcp_conn_iuup_dispatch_rtp(struct msgb *msg)
if (force_output_enabled) {
conn_rtp_src->end.output_enabled = prev_output_enabled;
conn_rtp_src->end.addr = prev_rem_addr;
conn_rtp_src->end.rtp_port = prev_rem_rtp_port;
osmo_sockaddr_set_port(&conn_rtp_src->end.addr.u.sa, prev_rem_rtp_port);
}
return rc;

View File

@ -74,7 +74,8 @@ static int rx_rtp(struct msgb *msg);
bool mgcp_rtp_end_remote_addr_available(const struct mgcp_rtp_end *rtp_end)
{
return rtp_end->rtp_port && (osmo_sockaddr_is_any(&rtp_end->addr) == 0);
return (osmo_sockaddr_port(&rtp_end->addr.u.sa) != 0) &&
(osmo_sockaddr_is_any(&rtp_end->addr) == 0);
}
/*! Determine the local rtp bind IP-address.
@ -870,14 +871,14 @@ static int check_rtp_origin(struct mgcp_conn_rtp *conn, struct osmo_sockaddr *ad
* the same as the remote port where we transmit outgoing RTP traffic
* to (set by MDCX). We use this to check the origin of the data for
* plausibility. */
if (ntohs(conn->end.rtp_port) != osmo_sockaddr_port(&addr->u.sa) &&
if (osmo_sockaddr_port(&conn->end.addr.u.sa) != osmo_sockaddr_port(&addr->u.sa) &&
ntohs(conn->end.rtcp_port) != osmo_sockaddr_port(&addr->u.sa)) {
LOGPCONN(conn->conn, DRTP, LOGL_ERROR,
"data from wrong source port: %d, ",
osmo_sockaddr_port(&addr->u.sa));
LOGPC(DRTP, LOGL_ERROR,
"expected: %d for RTP or %d for RTCP\n",
ntohs(conn->end.rtp_port), ntohs(conn->end.rtcp_port));
osmo_sockaddr_port(&conn->end.addr.u.sa), ntohs(conn->end.rtcp_port));
LOGPCONN(conn->conn, DRTP, LOGL_ERROR, "packet tossed\n");
return -1;
}
@ -891,27 +892,28 @@ static int check_rtp_destin(struct mgcp_conn_rtp *conn)
{
char ipbuf[INET6_ADDRSTRLEN];
bool ip_is_any = osmo_sockaddr_is_any(&conn->end.addr) != 0;
uint16_t port = osmo_sockaddr_port(&conn->end.addr.u.sa);
/* Note: it is legal to create a connection but never setting a port
* and IP-address for outgoing data. */
if (ip_is_any && conn->end.rtp_port == 0) {
if (ip_is_any && port == 0) {
LOGPCONN(conn->conn, DRTP, LOGL_DEBUG,
"destination IP-address and rtp port is not (yet) known (%s:%u)\n",
osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf), conn->end.rtp_port);
osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf), port);
return -1;
}
if (ip_is_any) {
LOGPCONN(conn->conn, DRTP, LOGL_ERROR,
"destination IP-address is invalid (%s:%u)\n",
osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf), conn->end.rtp_port);
osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf), port);
return -1;
}
if (conn->end.rtp_port == 0) {
if (port == 0) {
LOGPCONN(conn->conn, DRTP, LOGL_ERROR,
"destination rtp port is invalid (%s:%u)\n",
osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf), conn->end.rtp_port);
osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf), port);
return -1;
}
@ -1033,26 +1035,22 @@ static int mgcp_send_rtp(struct mgcp_conn_rtp *conn_dst, struct msgb *msg)
/*! send udp packet.
* \param[in] fd associated file descriptor.
* \param[in] addr destination ip-address.
* \param[in] port destination UDP port (network byte order).
* \param[in] buf buffer that holds the data to be send.
* \param[in] len length of the data to be sent.
* \returns bytes sent, -1 on error. */
int mgcp_udp_send(int fd, struct osmo_sockaddr *addr, int port, const char *buf, int len)
int mgcp_udp_send(int fd, const struct osmo_sockaddr *addr, const char *buf, int len)
{
char ipbuf[INET6_ADDRSTRLEN];
size_t addr_len;
bool is_ipv6 = addr->u.sa.sa_family == AF_INET6;
LOGP(DRTP, LOGL_DEBUG,
"sending %i bytes length packet to %s:%u ...\n", len,
osmo_sockaddr_ntop(&addr->u.sa, ipbuf),
ntohs(port));
osmo_sockaddr_port(&addr->u.sa));
if (is_ipv6) {
addr->u.sin6.sin6_port = port;
if (addr->u.sa.sa_family == AF_INET6) {
addr_len = sizeof(addr->u.sin6);
} else {
addr->u.sin.sin_port = port;
addr_len = sizeof(addr->u.sin);
}
@ -1067,6 +1065,7 @@ int mgcp_send_dummy(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn)
{
int rc;
int was_rtcp = 0;
struct osmo_sockaddr rtcp_addr;
OSMO_ASSERT(endp);
OSMO_ASSERT(conn);
@ -1083,7 +1082,7 @@ int mgcp_send_dummy(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn)
if (mgcp_conn_rtp_is_iuup(conn))
rc = mgcp_conn_iuup_send_dummy(conn);
else
rc = mgcp_udp_send(conn->end.rtp.fd, &conn->end.addr, conn->end.rtp_port,
rc = mgcp_udp_send(conn->end.rtp.fd, &conn->end.addr,
rtp_dummy_payload, sizeof(rtp_dummy_payload));
if (rc == -1)
@ -1093,8 +1092,10 @@ int mgcp_send_dummy(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn)
return rc;
was_rtcp = 1;
rc = mgcp_udp_send(conn->end.rtcp.fd, &conn->end.addr,
conn->end.rtcp_port, rtp_dummy_payload, sizeof(rtp_dummy_payload));
rtcp_addr = conn->end.addr;
osmo_sockaddr_set_port(&rtcp_addr.u.sa, ntohs(conn->end.rtcp_port));
rc = mgcp_udp_send(conn->end.rtcp.fd, &rtcp_addr,
rtp_dummy_payload, sizeof(rtp_dummy_payload));
if (rc >= 0)
return rc;
@ -1174,7 +1175,7 @@ int mgcp_send(struct mgcp_endpoint *endp, int is_rtp, struct osmo_sockaddr *addr
"rtp_port:%u rtcp_port:%u\n",
dest_name,
osmo_sockaddr_ntop(&rtp_end->addr.u.sa, ipbuf),
ntohs(rtp_end->rtp_port), ntohs(rtp_end->rtcp_port)
osmo_sockaddr_port(&rtp_end->addr.u.sa), ntohs(rtp_end->rtcp_port)
);
} else if (is_rtp) {
int cont;
@ -1219,14 +1220,14 @@ int mgcp_send(struct mgcp_endpoint *endp, int is_rtp, struct osmo_sockaddr *addr
"rtp_port:%u rtcp_port:%u\n",
dest_name,
osmo_sockaddr_ntop(&rtp_end->addr.u.sa, ipbuf),
ntohs(rtp_end->rtp_port), ntohs(rtp_end->rtcp_port)
osmo_sockaddr_port(&rtp_end->addr.u.sa), ntohs(rtp_end->rtcp_port)
);
/* Forward a copy of the RTP data to a debug ip/port */
forward_data_tap(rtp_end->rtp.fd, &conn_src->tap_out,
msg);
len = mgcp_udp_send(rtp_end->rtp.fd, &rtp_end->addr, rtp_end->rtp_port,
len = mgcp_udp_send(rtp_end->rtp.fd, &rtp_end->addr,
(char *)msgb_data(msg), msgb_length(msg));
if (len <= 0)
@ -1241,15 +1242,17 @@ int mgcp_send(struct mgcp_endpoint *endp, int is_rtp, struct osmo_sockaddr *addr
} while (buflen > 0);
return nbytes;
} else if (!trunk->omit_rtcp) {
struct osmo_sockaddr rtcp_addr = rtp_end->addr;
osmo_sockaddr_set_port(&rtcp_addr.u.sa, rtp_end->rtcp_port);
LOGPENDP(endp, DRTP, LOGL_DEBUG,
"send to %s %s rtp_port:%u rtcp_port:%u\n",
dest_name, osmo_sockaddr_ntop(&rtp_end->addr.u.sa, ipbuf),
ntohs(rtp_end->rtp_port), ntohs(rtp_end->rtcp_port)
dest_name, osmo_sockaddr_ntop(&rtcp_addr.u.sa, ipbuf),
osmo_sockaddr_port(&rtp_end->addr.u.sa),
osmo_sockaddr_port(&rtcp_addr.u.sa)
);
len = mgcp_udp_send(rtp_end->rtcp.fd,
&rtp_end->addr,
rtp_end->rtcp_port, (char *)msgb_data(msg), msgb_length(msg));
len = mgcp_udp_send(rtp_end->rtcp.fd, &rtcp_addr,
(char *)msgb_data(msg), msgb_length(msg));
rtpconn_rate_ctr_inc(conn_dst, endp, RTP_PACKETS_TX_CTR);
rtpconn_rate_ctr_add(conn_dst, endp, RTP_OCTETS_TX_CTR, len);
@ -1295,23 +1298,13 @@ int mgcp_dispatch_rtp_bridge_cb(struct msgb *msg)
* packets back to their origin. We will use the originating
* address data from the UDP packet header to patch the
* outgoing address in connection on the fly */
if (conn->u.rtp.end.rtp_port == 0) {
if (osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa) == 0) {
memcpy(&conn->u.rtp.end.addr, from_addr,
sizeof(conn->u.rtp.end.addr));
switch (from_addr->u.sa.sa_family) {
case AF_INET:
conn->u.rtp.end.rtp_port = from_addr->u.sin.sin_port;
break;
case AF_INET6:
conn->u.rtp.end.rtp_port = from_addr->u.sin6.sin6_port;
break;
default:
OSMO_ASSERT(false);
}
LOG_CONN_RTP(conn_src, LOGL_NOTICE,
"loopback mode: implicitly using source address (%s:%u) as destination address\n",
osmo_sockaddr_ntop(&from_addr->u.sa, ipbuf),
conn->u.rtp.end.rtp_port);
osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa));
}
return mgcp_send_rtp(conn_src, msg);
}
@ -1370,23 +1363,13 @@ int mgcp_dispatch_e1_bridge_cb(struct msgb *msg)
* packets back to their origin. We will use the originating
* address data from the UDP packet header to patch the
* outgoing address in connection on the fly */
if (conn->u.rtp.end.rtp_port == 0) {
if (osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa) == 0) {
memcpy(&conn->u.rtp.end.addr, from_addr,
sizeof(conn->u.rtp.end.addr));
switch (from_addr->u.sa.sa_family) {
case AF_INET:
conn->u.rtp.end.rtp_port = from_addr->u.sin.sin_port;
break;
case AF_INET6:
conn->u.rtp.end.rtp_port = from_addr->u.sin6.sin6_port;
break;
default:
OSMO_ASSERT(false);
}
LOG_CONN_RTP(conn_src, LOGL_NOTICE,
"loopback mode: implicitly using source address (%s:%u) as destination address\n",
osmo_sockaddr_ntop(&from_addr->u.sa, ipbuf),
conn->u.rtp.end.rtp_port);
osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa));
}
return mgcp_send_rtp(conn_src, msg);
}

View File

@ -338,7 +338,6 @@ static int endp_osmux_state_check(struct mgcp_endpoint *endp, struct mgcp_conn_r
switch(conn->osmux.state) {
case OSMUX_STATE_ACTIVATING:
rem_addr = conn->end.addr;
osmo_sockaddr_set_port(&rem_addr.u.sa, ntohs(conn->end.rtp_port));
if (osmux_enable_conn(endp, conn, &rem_addr) < 0) {
LOGPCONN(conn->conn, DOSMUX, LOGL_ERROR,
"Could not enable osmux for conn on %s: %s\n",
@ -651,10 +650,9 @@ int osmux_send_dummy(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn)
LOGPCONN(conn->conn, DOSMUX, LOGL_DEBUG,
"sending OSMUX dummy load to %s:%u CID %u\n",
osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf),
ntohs(conn->end.rtp_port), conn->osmux.remote_cid);
osmo_sockaddr_port(&conn->end.addr.u.sa), conn->osmux.remote_cid);
return mgcp_udp_send(osmux_fd.fd, &conn->end.addr,
conn->end.rtp_port, (char*)osmuxh, buf_len);
return mgcp_udp_send(osmux_fd.fd, &conn->end.addr, (char *)osmuxh, buf_len);
}
/* bsc-nat allocates/releases the Osmux circuit ID. +7 to round up to 8 bit boundary. */

View File

@ -708,7 +708,7 @@ void mgcp_rtp_end_config(struct mgcp_endpoint *endp, int expect_ssrc_change,
LOGPENDP(endp, DLMGCP, LOGL_DEBUG,
"Configuring RTP endpoint: local port %d%s%s\n",
ntohs(rtp->rtp_port),
osmo_sockaddr_port(&rtp->addr.u.sa),
rtp->force_aligned_timing ? ", force constant timing" : "",
rtp->force_constant_ssrc ? ", force constant ssrc" : "");
}
@ -1057,7 +1057,7 @@ mgcp_header_done:
/* check connection mode setting */
if (conn->conn->mode != MGCP_CONN_LOOPBACK
&& conn->conn->mode != MGCP_CONN_RECV_ONLY
&& conn->end.rtp_port == 0) {
&& osmo_sockaddr_port(&conn->end.addr.u.sa) == 0) {
LOGPCONN(_conn, DLMGCP, LOGL_ERROR,
"CRCX: selected connection mode type requires an opposite end!\n");
error_code = 527;

View File

@ -384,7 +384,7 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
case 'm':
rc = sscanf(line, "m=audio %d RTP/AVP", &port);
if (rc == 1) {
rtp->rtp_port = htons(port);
osmo_sockaddr_set_port(&rtp->addr.u.sa, port);
rtp->rtcp_port = htons(port + 1);
}
@ -432,7 +432,7 @@ int mgcp_parse_sdp_data(const struct mgcp_endpoint *endp,
LOGPCONN(conn->conn, DLMGCP, LOGL_NOTICE,
"Got media info via SDP: port:%d, addr:%s, duration:%d, payload-types:",
ntohs(rtp->rtp_port), osmo_sockaddr_ntop(&rtp->addr.u.sa, ipbuf),
osmo_sockaddr_port(&rtp->addr.u.sa), osmo_sockaddr_ntop(&rtp->addr.u.sa, ipbuf),
rtp->packet_duration_ms);
if (codecs_used == 0)
LOGPC(DLMGCP, LOGL_NOTICE, "none");

View File

@ -1492,7 +1492,7 @@ static void test_multilple_codec(void)
conn = mgcp_conn_get_rtp(endp, conn_id);
OSMO_ASSERT(conn);
OSMO_ASSERT(conn->end.codec->payload_type == 3);
OSMO_ASSERT(conn->end.rtp_port == htons(16434));
OSMO_ASSERT(osmo_sockaddr_port(&conn->end.addr.u.sa) == 16434);
memset(&addr, 0, sizeof(addr));
inet_aton("8.8.8.8", &addr);
OSMO_ASSERT(conn->end.addr.u.sa.sa_family == AF_INET);