pespin has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-mgw/+/29589 )
Change subject: WIP: Get rid of separate rtp_port field
......................................................................
WIP: 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
---
M include/osmocom/mgcp/mgcp.h
M include/osmocom/mgcp/mgcp_network.h
M src/libosmo-mgcp/mgcp_conn.c
M src/libosmo-mgcp/mgcp_iuup.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_osmux.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
M tests/mgcp/mgcp_test.c
9 files changed, 50 insertions(+), 70 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/89/29589/1
diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h
index a4d4e6f..4c541cb 100644
--- a/include/osmocom/mgcp/mgcp.h
+++ b/include/osmocom/mgcp/mgcp.h
@@ -208,4 +208,4 @@
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);
diff --git a/include/osmocom/mgcp/mgcp_network.h
b/include/osmocom/mgcp/mgcp_network.h
index 854c92d..349ff94 100644
--- a/include/osmocom/mgcp/mgcp_network.h
+++ b/include/osmocom/mgcp/mgcp_network.h
@@ -93,7 +93,7 @@
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;
diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index cc309a3..08c9301 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -105,7 +105,8 @@
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 @@
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;
diff --git a/src/libosmo-mgcp/mgcp_iuup.c b/src/libosmo-mgcp/mgcp_iuup.c
index 48e4ce7..90021f3 100644
--- a/src/libosmo-mgcp/mgcp_iuup.c
+++ b/src/libosmo-mgcp/mgcp_iuup.c
@@ -489,7 +489,7 @@
"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 @@
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 @@
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 @@
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;
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index 86a6330..401bb09 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -74,7 +74,8 @@
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 @@
* 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 @@
{
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 @@
/*! 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 rc;
int was_rtcp = 0;
+ struct osmo_sockaddr rtcp_addr;
OSMO_ASSERT(endp);
OSMO_ASSERT(conn);
@@ -1083,7 +1082,7 @@
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 @@
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 @@
"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 @@
"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 @@
} 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 @@
* 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 @@
* 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);
}
diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c
index 23e4dad..b5a2c57 100644
--- a/src/libosmo-mgcp/mgcp_osmux.c
+++ b/src/libosmo-mgcp/mgcp_osmux.c
@@ -338,7 +338,6 @@
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 @@
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. */
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 5493917..a257ffe 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -708,7 +708,7 @@
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 @@
/* 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;
diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c
index 98b3099..1414cf9 100644
--- a/src/libosmo-mgcp/mgcp_sdp.c
+++ b/src/libosmo-mgcp/mgcp_sdp.c
@@ -384,7 +384,7 @@
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 @@
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");
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index e28a574..d843d10 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1492,7 +1492,7 @@
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);
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/29589
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I294eb5d85fae79bf62d36eb9e818426e187d442c
Gerrit-Change-Number: 29589
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
Gerrit-MessageType: newchange
