pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39222?usp=email )
Change subject: mgw: Clean up code allocating conn_rtp rtp/rtcp sockets ......................................................................
mgw: Clean up code allocating conn_rtp rtp/rtcp sockets
Clean up pointers, join 2 functions only making stuff more confusing. Rename function to make it clear it operates on a conn_rtp object.
Change-Id: I50a251b66e85ceeeccb30dcd1813863d7c754961 --- M include/osmocom/mgcp/mgcp_network.h M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c 3 files changed, 56 insertions(+), 75 deletions(-)
Approvals: daniel: Looks good to me, approved Jenkins Builder: Verified
diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h index fe35e78..8a1fc47 100644 --- a/include/osmocom/mgcp/mgcp_network.h +++ b/include/osmocom/mgcp/mgcp_network.h @@ -91,8 +91,7 @@ void mgcp_cleanup_rtp_bridge_cb(struct mgcp_endpoint *endp, struct mgcp_conn *conn); int mgcp_dispatch_e1_bridge_cb(struct msgb *msg); void mgcp_cleanup_e1_bridge_cb(struct mgcp_endpoint *endp, struct mgcp_conn *conn); -int mgcp_bind_net_rtp_port(struct mgcp_endpoint *endp, int rtp_port, - struct mgcp_conn_rtp *conn); +int mgcp_conn_rtp_bind_rtp_ports(struct mgcp_conn_rtp *conn, int rtp_port); void mgcp_patch_and_count(const struct mgcp_endpoint *endp, struct mgcp_rtp_state *state, struct mgcp_rtp_end *rtp_end, diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 894ca2a..de837c1 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -1625,78 +1625,23 @@ return rc; }
-/* Bind RTP and RTCP port (helper function for mgcp_bind_net_rtp_port()) */ -static int bind_rtp(struct mgcp_config *cfg, const char *source_addr, - struct mgcp_rtp_end *rtp_end, struct mgcp_endpoint *endp) -{ - int rc, rtp_fd, rtcp_fd; - - /* NOTE: The port that is used for RTCP is the RTP port incremented by one - * (e.g. RTP-Port = 16000 ==> RTCP-Port = 16001) */ - - rc = mgcp_create_bind(source_addr, rtp_end->local_port, cfg->endp_dscp, cfg->endp_priority); - if (rc < 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, - "failed to create RTP port: %s:%d\n", - source_addr, rtp_end->local_port); - goto cleanup0; - } - rtp_fd = rc; - - rc = mgcp_create_bind(source_addr, rtp_end->local_port + 1, cfg->endp_dscp, cfg->endp_priority); - if (rc < 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, - "failed to create RTCP port: %s:%d\n", - source_addr, rtp_end->local_port + 1); - goto cleanup1; - } - rtcp_fd = rc; - - if (osmo_iofd_register(rtp_end->rtp, rtp_fd) < 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, - "failed to register RTP port %d\n", - rtp_end->local_port); - goto cleanup2; - } - - if (osmo_iofd_register(rtp_end->rtcp, rtcp_fd) != 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, - "failed to register RTCP port %d\n", - rtp_end->local_port + 1); - goto cleanup3; - } - - return 0; - -cleanup3: - osmo_iofd_unregister(rtp_end->rtp); -cleanup2: - close(rtcp_fd); -cleanup1: - close(rtp_fd); -cleanup0: - return -1; -} - /*! bind RTP port to endpoint/connection. - * \param[in] endp endpoint that holds the RTP connection. - * \param[in] rtp_port port number to bind on. * \param[in] conn associated RTP connection. + * \param[in] rtp_port port number to bind on. * \returns 0 on success, -1 on ERROR. */ -int mgcp_bind_net_rtp_port(struct mgcp_endpoint *endp, int rtp_port, - struct mgcp_conn_rtp *conn) +int mgcp_conn_rtp_bind_rtp_ports(struct mgcp_conn_rtp *conn_rtp, int rtp_port) { char name[512]; - struct mgcp_rtp_end *end; + struct mgcp_conn *conn = conn_rtp->conn; + struct mgcp_config *cfg = conn->endp->trunk->cfg; + struct mgcp_rtp_end *end = &conn_rtp->end; + int rc, rtp_fd, rtcp_fd;
- snprintf(name, sizeof(name), "%s-%s", conn->conn->name, conn->conn->id); - end = &conn->end; + snprintf(name, sizeof(name), "%s-%s", conn->name, conn->id);
if ((end->rtp && osmo_iofd_get_fd(end->rtp) != -1) || (end->rtcp && osmo_iofd_get_fd(end->rtcp) != -1)) { - LOGPENDP(endp, DRTP, LOGL_ERROR, "%u was already bound on conn:%s\n", - rtp_port, mgcp_conn_dump(conn->conn)); - + LOG_CONN_RTP(conn_rtp, LOGL_ERROR, "%u was already bound\n", rtp_port); /* Double bindings should never occour! Since we always allocate * connections dynamically and free them when they are not * needed anymore, there must be no previous binding leftover. @@ -1706,18 +1651,55 @@ }
end->local_port = rtp_port; - end->rtp = osmo_iofd_setup(conn->conn, -1, name, OSMO_IO_FD_MODE_RECVFROM_SENDTO, &rtp_ioops, conn); + end->rtp = osmo_iofd_setup(conn, -1, name, OSMO_IO_FD_MODE_RECVFROM_SENDTO, &rtp_ioops, conn_rtp); if (!end->rtp) - return -EIO; + goto free_iofd_ret; osmo_iofd_set_alloc_info(end->rtp, RTP_BUF_SIZE, 0); - end->rtcp = osmo_iofd_setup(conn->conn, -1, name, OSMO_IO_FD_MODE_RECVFROM_SENDTO, &rtp_ioops, conn); - if (!end->rtcp) { - osmo_iofd_free(end->rtp); - end->rtp = NULL; - return -EIO; - } + end->rtcp = osmo_iofd_setup(conn, -1, name, OSMO_IO_FD_MODE_RECVFROM_SENDTO, &rtp_ioops, conn_rtp); + if (!end->rtcp) + goto free_iofd_ret; osmo_iofd_set_alloc_info(end->rtcp, RTP_BUF_SIZE, 0); osmo_iofd_set_priv_nr(end->rtcp, 1); /* we use priv_nr as identifier for RTCP */
- return bind_rtp(endp->trunk->cfg, conn->end.local_addr, end, endp); + /* NOTE: The port that is used for RTCP is the RTP port incremented by one + * (e.g. RTP-Port = 16000 ==> RTCP-Port = 16001) */ + + rc = mgcp_create_bind(end->local_addr, end->local_port, cfg->endp_dscp, cfg->endp_priority); + if (rc < 0) { + LOG_CONN_RTP(conn_rtp, LOGL_ERROR, "failed to create RTP port: %s:%d\n", end->local_addr, end->local_port); + goto free_iofd_ret; + } + rtp_fd = rc; + + rc = mgcp_create_bind(end->local_addr, end->local_port + 1, cfg->endp_dscp, cfg->endp_priority); + if (rc < 0) { + LOG_CONN_RTP(conn_rtp, LOGL_ERROR, "failed to create RTCP port: %s:%d\n", end->local_addr, end->local_port + 1); + goto cleanup1; + } + rtcp_fd = rc; + + if (osmo_iofd_register(end->rtp, rtp_fd) < 0) { + LOG_CONN_RTP(conn_rtp, LOGL_ERROR, "failed to register RTP port %d\n", end->local_port); + goto cleanup2; + } + + if (osmo_iofd_register(end->rtcp, rtcp_fd) != 0) { + LOG_CONN_RTP(conn_rtp, LOGL_ERROR, "failed to register RTCP port %d\n", end->local_port + 1); + goto cleanup3; + } + + return 0; + +cleanup3: + osmo_iofd_unregister(end->rtp); +cleanup2: + close(rtcp_fd); +cleanup1: + close(rtp_fd); +free_iofd_ret: + osmo_iofd_free(end->rtcp); + end->rtcp = NULL; + osmo_iofd_free(end->rtp); + end->rtp = NULL; + return -EIO; } diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index bbd38da..3efa0e0 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -513,7 +513,7 @@ if (range->last_port >= range->range_end) range->last_port = range->range_start;
- rc = mgcp_bind_net_rtp_port(endp, range->last_port, conn); + rc = mgcp_conn_rtp_bind_rtp_ports(conn, range->last_port);
range->last_port += 2; if (rc == 0) {