From 5ffd12738467c0d8a9d0b7061e4d4b4ac925bfcb Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Tue, 4 Oct 2022 13:45:48 +0200 Subject: [PATCH] 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 --- include/osmocom/mgcp/mgcp.h | 2 +- include/osmocom/mgcp/mgcp_network.h | 2 +- src/libosmo-mgcp/mgcp_conn.c | 5 +- src/libosmo-mgcp/mgcp_iuup.c | 12 ++--- src/libosmo-mgcp/mgcp_network.c | 83 ++++++++++++----------------- src/libosmo-mgcp/mgcp_osmux.c | 6 +-- src/libosmo-mgcp/mgcp_protocol.c | 4 +- src/libosmo-mgcp/mgcp_sdp.c | 4 +- tests/mgcp/mgcp_test.c | 2 +- 9 files changed, 50 insertions(+), 70 deletions(-) diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h index a4d4e6f2c..4c541cb3a 100644 --- a/include/osmocom/mgcp/mgcp.h +++ b/include/osmocom/mgcp/mgcp.h @@ -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); diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h index 854c92d4a..349ff9480 100644 --- a/include/osmocom/mgcp/mgcp_network.h +++ b/include/osmocom/mgcp/mgcp_network.h @@ -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; diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c index cc309a3fe..08c930196 100644 --- a/src/libosmo-mgcp/mgcp_conn.c +++ b/src/libosmo-mgcp/mgcp_conn.c @@ -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; diff --git a/src/libosmo-mgcp/mgcp_iuup.c b/src/libosmo-mgcp/mgcp_iuup.c index 48e4ce7bd..90021f39d 100644 --- a/src/libosmo-mgcp/mgcp_iuup.c +++ b/src/libosmo-mgcp/mgcp_iuup.c @@ -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; diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 86a63300f..401bb090c 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -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); } diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c index 23e4dad8f..242f9074e 100644 --- a/src/libosmo-mgcp/mgcp_osmux.c +++ b/src/libosmo-mgcp/mgcp_osmux.c @@ -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. */ diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 54939176e..a257ffef3 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -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; diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c index 98b309929..1414cf9e4 100644 --- a/src/libosmo-mgcp/mgcp_sdp.c +++ b/src/libosmo-mgcp/mgcp_sdp.c @@ -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"); diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index e28a5748d..d843d1033 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -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);