pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/29784 )
Change subject: mgcp-client: Use random free local port by default ......................................................................
mgcp-client: Use random free local port by default
Deprecate mgcp_client_connect2() and all the related messy system where configured port is incremented until a free port is found. Let's simply use local_port=0 by default and let the kernel take care of doing all that. The mgcp_client_connect2() is actually not being used anywhere outside of libosmo-mgcp-client, so it's not a real problem deprecating it. We could poitentially remove it too.
This helps in fixing a regression in recent refactoring commit f48cd95bbc1383f33ad9cd9cf0b0a02f9371b21a, which unified calls to mgcp_client_connect() and in the process dropped the code path which was setting the retry_n_ports=99. As a result, since all libosmo-mgcp-client instances set local_port=2727 and addr=NULL by default, using MGW pooling or having several osmo-bsc or osmo-msc processes would make creating the socket fail.
Change-Id: I5683bcf3b2c4e8058338433f2cd1b143753f6512 --- M include/osmocom/mgcp_client/mgcp_client.h M src/libosmo-mgcp-client/mgcp_client.c M src/libosmo-mgcp-client/mgcp_client_pool.c 3 files changed, 11 insertions(+), 53 deletions(-)
Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved
diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h index d421273..b40923d 100644 --- a/include/osmocom/mgcp_client/mgcp_client.h +++ b/include/osmocom/mgcp_client/mgcp_client.h @@ -7,7 +7,7 @@
/* See also: RFC 3435, chapter 3.5 Transmission over UDP */ #define MGCP_CLIENT_LOCAL_ADDR_DEFAULT NULL /* INADDR(6)_ANY */ -#define MGCP_CLIENT_LOCAL_PORT_DEFAULT 2727 +#define MGCP_CLIENT_LOCAL_PORT_DEFAULT 0 #define MGCP_CLIENT_REMOTE_ADDR_DEFAULT "127.0.0.1" #define MGCP_CLIENT_REMOTE_PORT_DEFAULT 2427
@@ -140,7 +140,7 @@ struct mgcp_client *mgcp_client_init(void *ctx, struct mgcp_client_conf *conf); int mgcp_client_connect(struct mgcp_client *mgcp); -int mgcp_client_connect2(struct mgcp_client *mgcp, unsigned int retry_n_ports); +int mgcp_client_connect2(struct mgcp_client *mgcp, unsigned int retry_n_ports) OSMO_DEPRECATED("Use mgcp_client_connect() instead"); void mgcp_client_disconnect(struct mgcp_client *mgcp);
const char *mgcp_client_remote_addr_str(struct mgcp_client *mgcp); diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index 2ca3b66..6f1ab6b 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -769,7 +769,7 @@
mgcp->actual.local_addr = conf->local_addr ? conf->local_addr : MGCP_CLIENT_LOCAL_ADDR_DEFAULT; - mgcp->actual.local_port = conf->local_port >= 0 ? (uint16_t)conf->local_port : + mgcp->actual.local_port = conf->local_port > 0 ? (uint16_t)conf->local_port : MGCP_CLIENT_LOCAL_PORT_DEFAULT;
mgcp->actual.remote_addr = conf->remote_addr ? conf->remote_addr : @@ -799,49 +799,6 @@ return mgcp; }
-static int init_socket(struct mgcp_client *mgcp, unsigned int retry_n_ports) -{ - int rc; - struct osmo_wqueue *wq; - unsigned int i; - - wq = &mgcp->wq; - - for (i = 0; i < retry_n_ports + 1; i++) { - - /* Initialize socket with the currently configured port number */ - rc = osmo_sock_init2_ofd(&wq->bfd, AF_UNSPEC, SOCK_DGRAM, IPPROTO_UDP, mgcp->actual.local_addr, - mgcp->actual.local_port, mgcp->actual.remote_addr, mgcp->actual.remote_port, - OSMO_SOCK_F_BIND | OSMO_SOCK_F_CONNECT); - if (rc > 0) - return rc; - - /* If there is a different port than the default port configured then we assume that the user has - * chosen that port conciously and we will not try to resolve this by silently choosing a different - * port. */ - if (mgcp->actual.local_port != MGCP_CLIENT_LOCAL_PORT_DEFAULT && i == 0) - return -EINVAL; - - if (i == retry_n_ports) { - /* Last try failed */ - LOGPMGW(mgcp, LOGL_NOTICE, "Failed to bind to %s:%d -- check configuration!\n", - mgcp->actual.local_addr ? mgcp->actual.local_addr : "(any)", mgcp->actual.local_port); - if (retry_n_ports == 0) - return -EINVAL; - } else { - /* Choose a new port number to try next */ - LOGPMGW(mgcp, LOGL_NOTICE, - "Failed to bind to %s:%d, retrying with port %d -- check configuration!\n", - mgcp->actual.local_addr ? mgcp->actual.local_addr : "(any)", mgcp->actual.local_port, - mgcp->actual.local_port + 1); - mgcp->actual.local_port++; - } - } - - LOGPMGW(mgcp, LOGL_FATAL, "Failed to find a port to bind on %u times -- check configuration!\n", i); - return -EINVAL; -} - /* Safely ignore the MGCP response to the DLCX sent via _mgcp_client_send_dlcx() */ static void _ignore_mgcp_response(struct mgcp_response *response, void *priv) { }
@@ -878,9 +835,8 @@
/*! Initialize client connection (opens socket) * \param[in,out] mgcp MGCP client descriptor. - * \param[in] retry_n_ports number of consecutive local ports that should be used to retry on failure. * \returns 0 on success, -EINVAL on error. */ -int mgcp_client_connect2(struct mgcp_client *mgcp, unsigned int retry_n_ports) +int mgcp_client_connect(struct mgcp_client *mgcp) { struct osmo_wqueue *wq; int rc; @@ -899,7 +855,9 @@
osmo_fd_setup(&wq->bfd, -1, OSMO_FD_READ, osmo_wqueue_bfd_cb, mgcp, 0);
- rc = init_socket(mgcp, retry_n_ports); + rc = osmo_sock_init2_ofd(&wq->bfd, AF_UNSPEC, SOCK_DGRAM, IPPROTO_UDP, mgcp->actual.local_addr, + mgcp->actual.local_port, mgcp->actual.remote_addr, mgcp->actual.remote_port, + OSMO_SOCK_F_BIND | OSMO_SOCK_F_CONNECT); if (rc < 0) { LOGPMGW(mgcp, LOGL_FATAL, "Failed to initialize socket %s:%u -> %s:%u for MGW: %s\n", @@ -925,12 +883,12 @@ return rc; }
-/*! Initialize client connection (opens socket) +/*! DEPRECATED: Initialize client connection (opens socket) * \param[in,out] mgcp MGCP client descriptor. * \returns 0 on success, -EINVAL on error. */ -int mgcp_client_connect(struct mgcp_client *mgcp) +int mgcp_client_connect2(struct mgcp_client *mgcp, unsigned int retry_n_ports) { - return mgcp_client_connect2(mgcp, 99); + return mgcp_client_connect(mgcp); }
/*! Terminate client connection diff --git a/src/libosmo-mgcp-client/mgcp_client_pool.c b/src/libosmo-mgcp-client/mgcp_client_pool.c index caec535..fe3cccd 100644 --- a/src/libosmo-mgcp-client/mgcp_client_pool.c +++ b/src/libosmo-mgcp-client/mgcp_client_pool.c @@ -236,7 +236,7 @@ pool_member->client->pool_member = pool_member;
/* Connect client */ - if (mgcp_client_connect2(pool_member->client, 0)) { + if (mgcp_client_connect(pool_member->client)) { LOGPPMGW(pool_member, LOGL_ERROR, "MGCP client connect failed at (%s:%u)\n", pool_member->conf.remote_addr, pool_member->conf.remote_port); talloc_free(pool_member->client);
-
pespin