Commit 7ec7f66d authored by Fabrice Bellet's avatar Fabrice Bellet Committed by Olivier Crête
Browse files

agent: report duplicated port in udp bsd sockets too

This patch fixes cases, where the range is full, some ports fail with
HOST_CANDIDATE_CANT_CREATE_SOCKET, other fail with
HOST_CANDIDATE_DUPLICATE_PORT, the value of res we keep when leaving the
loop is randomly the one of the last iteration of the loop.

CANT_CREATE_SOCKET still happens when trying to create an udp bsd socket
with the same address and port than one of another component, so it is
also a case of duplicate port in fact.

To be homogeneous, we add a gerror for nice_udp_bsd_socket_new(), like
we did in nice_tcp_passive_socket_new(), and we can catch the same
G_IO_ERROR_ADDRESS_IN_USE there too, when failing to get free available
udp ports.

This patch is a complement to merge request !158
parent cadf5118
Pipeline #240350 passed with stages
in 19 minutes and 27 seconds
......@@ -2699,7 +2699,7 @@ priv_add_new_candidate_discovery_turn (NiceAgent *agent,
NiceSocket *new_socket;
nice_address_set_port (&addr, 0);
new_socket = nice_udp_bsd_socket_new (&addr);
new_socket = nice_udp_bsd_socket_new (&addr, NULL);
if (new_socket) {
_priv_set_socket_tos (agent, new_socket, stream->tos);
nice_component_attach_socket (component, new_socket);
......@@ -3229,6 +3229,25 @@ agent_remove_local_candidate (NiceAgent *agent, NiceStream *stream,
#endif
static const gchar *
priv_host_candidate_result_to_string (HostCandidateResult result)
{
switch (result) {
case HOST_CANDIDATE_SUCCESS:
return "success";
case HOST_CANDIDATE_FAILED:
return "failed";
case HOST_CANDIDATE_CANT_CREATE_SOCKET:
return "can't create socket";
case HOST_CANDIDATE_REDUNDANT:
return "redundant";
case HOST_CANDIDATE_DUPLICATE_PORT:
return "duplicate port";
default:
g_assert_not_reached ();
}
}
NICEAPI_EXPORT gboolean
nice_agent_gather_candidates (
NiceAgent *agent,
......@@ -3349,11 +3368,20 @@ nice_agent_gather_candidates (
host_candidate = NULL;
while (res == HOST_CANDIDATE_CANT_CREATE_SOCKET ||
res == HOST_CANDIDATE_DUPLICATE_PORT) {
nice_debug ("Agent %p: Trying to create %s host candidate on port %d", agent,
nice_candidate_transport_to_string (transport), current_port);
nice_address_set_port (addr, current_port);
res = discovery_add_local_host_candidate (agent, stream->id, cid,
addr, transport, accept_duplicate, &host_candidate);
if (nice_debug_is_enabled ()) {
gchar ip[NICE_ADDRESS_STRING_LEN];
nice_address_to_string (addr, ip);
nice_debug ("Agent %p: s%d/c%d: creation of host candidate "
"%s:[%s]:%u: %s%s", agent, stream->id, cid,
nice_candidate_transport_to_string (transport), ip,
transport == NICE_CANDIDATE_TRANSPORT_TCP_ACTIVE ?
0 : current_port,
priv_host_candidate_result_to_string (res),
accept_duplicate ? " (accept duplicate)" : "");
}
if (current_port > 0)
current_port++;
if (current_port > component->max_port)
......@@ -3367,38 +3395,13 @@ nice_agent_gather_candidates (
break;
}
if (res == HOST_CANDIDATE_REDUNDANT) {
nice_debug ("Agent %p: Ignoring local candidate, it's redundant",
agent);
continue;
} else if (res == HOST_CANDIDATE_FAILED) {
nice_debug ("Agent %p: Could not retrieve component %d/%d", agent,
stream->id, cid);
continue;
} else if (res == HOST_CANDIDATE_CANT_CREATE_SOCKET) {
if (nice_debug_is_enabled ()) {
gchar ip[NICE_ADDRESS_STRING_LEN];
nice_address_to_string (addr, ip);
nice_debug ("Agent %p: Unable to add local host %s candidate %s"
" for s%d:%d. Invalid interface?", agent,
nice_candidate_transport_to_string (transport), ip,
stream->id, component->id);
}
if (res == HOST_CANDIDATE_REDUNDANT ||
res == HOST_CANDIDATE_FAILED ||
res == HOST_CANDIDATE_CANT_CREATE_SOCKET)
continue;
} else if (res == HOST_CANDIDATE_DUPLICATE_PORT) {
if (nice_debug_is_enabled ()) {
gchar ip[NICE_ADDRESS_STRING_LEN];
nice_address_to_string (addr, ip);
nice_debug ("Agent %p: Unable to add local host %s candidate %s"
" for"
" s%d:%d. Every port is duplicated", agent, ip,
nice_candidate_transport_to_string (transport), stream->id,
component->id);
}
ret = FALSE;
goto error;
else if (res == HOST_CANDIDATE_DUPLICATE_PORT) {
ret = FALSE;
goto error;
}
found_local_address = TRUE;
......
......@@ -646,13 +646,30 @@ priv_local_host_candidate_duplicate_port (NiceAgent *agent,
nice_address_to_string (&candidate->addr, ip);
nice_address_to_string (&c->addr, ip2);
nice_debug ("Agent %p: Local host %s candidate %s"
" for s%d:%d will use the same port %d as %s .", agent, ip,
nice_debug ("Agent %p: s%d/c%d: host candidate %s:[%s]:%u "
" will use the same port as %s:[%s]:%u", agent,
stream->id, component->id,
nice_candidate_transport_to_string (candidate->transport),
ip, nice_address_get_port (&candidate->addr),
nice_candidate_transport_to_string (c->transport),
stream->id, component->id, nice_address_get_port (&c->addr),
ip2);
ip2, nice_address_get_port (&c->addr));
return FALSE;
}
{
gchar ip[NICE_ADDRESS_STRING_LEN];
gchar ip2[NICE_ADDRESS_STRING_LEN];
nice_address_to_string (&candidate->addr, ip);
nice_address_to_string (&c->addr, ip2);
nice_debug ("Agent %p: s%d/c%d: host candidate %s:[%s]:%u "
" has the same port as %s:[%s]:%u from s%d/c%d", agent,
candidate->stream_id, candidate->component_id,
nice_candidate_transport_to_string (candidate->transport),
ip, nice_address_get_port (&candidate->addr),
nice_candidate_transport_to_string (c->transport),
ip2, nice_address_get_port (&c->addr),
stream->id, component->id);
}
return TRUE;
}
......@@ -714,7 +731,7 @@ HostCandidateResult discovery_add_local_host_candidate (
/* note: candidate username and password are left NULL as stream
level ufrag/password are used */
if (transport == NICE_CANDIDATE_TRANSPORT_UDP) {
nicesock = nice_udp_bsd_socket_new (address);
nicesock = nice_udp_bsd_socket_new (address, &error);
} else if (transport == NICE_CANDIDATE_TRANSPORT_TCP_ACTIVE) {
nicesock = nice_tcp_active_socket_new (agent->main_context, address);
} else if (transport == NICE_CANDIDATE_TRANSPORT_TCP_PASSIVE) {
......
......@@ -79,7 +79,7 @@ struct UdpBsdSocketPrivate
};
NiceSocket *
nice_udp_bsd_socket_new (NiceAddress *addr)
nice_udp_bsd_socket_new (NiceAddress *addr, GError **error)
{
union {
struct sockaddr_storage storage;
......@@ -100,7 +100,7 @@ nice_udp_bsd_socket_new (NiceAddress *addr)
if (name.storage.ss_family == AF_UNSPEC || name.storage.ss_family == AF_INET) {
gsock = g_socket_new (G_SOCKET_FAMILY_IPV4, G_SOCKET_TYPE_DATAGRAM,
G_SOCKET_PROTOCOL_UDP, NULL);
G_SOCKET_PROTOCOL_UDP, error);
name.storage.ss_family = AF_INET;
#ifdef HAVE_SA_LEN
name.storage.ss_len = sizeof (struct sockaddr_in);
......@@ -123,7 +123,7 @@ nice_udp_bsd_socket_new (NiceAddress *addr)
g_socket_set_blocking (gsock, false);
gaddr = g_socket_address_new_from_native (&name.addr, sizeof (name));
if (gaddr != NULL) {
gret = g_socket_bind (gsock, gaddr, FALSE, NULL);
gret = g_socket_bind (gsock, gaddr, FALSE, error);
g_object_unref (gaddr);
}
......
......@@ -44,7 +44,7 @@
G_BEGIN_DECLS
NiceSocket *
nice_udp_bsd_socket_new (NiceAddress *addr);
nice_udp_bsd_socket_new (NiceAddress *addr, GError **error);
G_END_DECLS
......
......@@ -61,8 +61,10 @@ static void
test_socket_initial_properties (void)
{
NiceSocket *sock;
GError *error = NULL;
sock = nice_udp_bsd_socket_new (NULL);
sock = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
g_assert (sock != NULL);
// not bound to a particular interface
......@@ -78,8 +80,10 @@ test_socket_address_properties (void)
{
NiceSocket *sock;
NiceAddress tmp;
GError *error = NULL;
sock = nice_udp_bsd_socket_new (NULL);
sock = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
g_assert (sock != NULL);
g_assert (nice_address_set_from_string (&tmp, "127.0.0.1"));
......@@ -97,11 +101,14 @@ test_simple_send_recv (void)
NiceSocket *client;
NiceAddress tmp;
gchar buf[5];
GError *error = NULL;
server = nice_udp_bsd_socket_new (NULL);
server = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
g_assert (server != NULL);
client = nice_udp_bsd_socket_new (NULL);
client = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
g_assert (client != NULL);
g_assert (nice_address_set_from_string (&tmp, "127.0.0.1"));
......@@ -132,8 +139,10 @@ test_zero_send_recv (void)
gchar buf[5];
NiceOutputMessage local_out_message;
NiceInputMessage local_in_message;
GError *error = NULL;
sock = nice_udp_bsd_socket_new (NULL);
sock = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
g_assert (sock != NULL);
g_assert (nice_address_set_from_string (&tmp, "127.0.0.1"));
......@@ -168,11 +177,14 @@ test_multi_buffer_recv (void)
NiceAddress tmp;
guint8 buf[20];
guint8 dummy_buf[9];
GError *error = NULL;
server = nice_udp_bsd_socket_new (NULL);
server = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
g_assert (server != NULL);
client = nice_udp_bsd_socket_new (NULL);
client = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
g_assert (client != NULL);
g_assert (nice_address_set_from_string (&tmp, "127.0.0.1"));
......@@ -239,11 +251,14 @@ test_multi_message_recv (guint n_sends, guint n_receives,
NiceSocket *server;
NiceSocket *client;
NiceAddress tmp;
GError *error = NULL;
server = nice_udp_bsd_socket_new (NULL);
server = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
g_assert (server != NULL);
client = nice_udp_bsd_socket_new (NULL);
client = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
g_assert (client != NULL);
g_assert (nice_address_set_from_string (&tmp, "127.0.0.1"));
......
......@@ -419,13 +419,15 @@ static int run_full_test (NiceAgent *lagent, NiceAgent *ragent, NiceAddress *bas
NiceCandidate *local_cand = NULL;
NiceCandidate *remote_cand = NULL;
NiceSocket *tmpsock;
GError *error = NULL;
g_assert (nice_agent_get_selected_pair (lagent, ls_id, 1, &local_cand,
&remote_cand));
g_assert (local_cand);
g_assert (remote_cand);
tmpsock = nice_udp_bsd_socket_new (NULL);
tmpsock = nice_udp_bsd_socket_new (NULL, &error);
g_assert_no_error (error);
nice_socket_send (tmpsock, &remote_cand->addr, 4, "ABCD");
nice_socket_send (tmpsock, &local_cand->addr, 5, "ABCDE");
nice_socket_free (tmpsock);
......
......@@ -84,6 +84,7 @@ main (int argc, char *argv[])
GMainLoop *mainloop = NULL;
NiceAddress addr;
GError *error = NULL;
g_networking_init ();
......@@ -95,7 +96,8 @@ main (int argc, char *argv[])
nice_address_set_from_string (&addr, "127.0.0.1");
/* Standalone socket */
udp_bsd = nice_udp_bsd_socket_new (&addr);
udp_bsd = nice_udp_bsd_socket_new (&addr, &error);
g_assert_no_error (error);
/* tcp_passive -> pseudossl -> udp_turn_over_tcp */
tcp_active = nice_tcp_active_socket_new (g_main_loop_get_context (mainloop),
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment