From fbbe351f7d8edb13a5c0fe8681e10c117681796b Mon Sep 17 00:00:00 2001 From: Autechre Date: Sat, 4 Dec 2021 21:16:34 +0100 Subject: [PATCH] UPnP cleanups and refactorings (#13325) --- libretro-common/include/net/net_natt.h | 20 +-- libretro-common/net/net_natt.c | 188 +++++++++++++------------ network/netplay/netplay_frontend.c | 25 +--- network/netplay/netplay_private.h | 2 +- tasks/task_netplay_nat_traversal.c | 104 ++++++++------ 5 files changed, 163 insertions(+), 176 deletions(-) diff --git a/libretro-common/include/net/net_natt.h b/libretro-common/include/net/net_natt.h index 724febee56..0b66ecdc5d 100644 --- a/libretro-common/include/net/net_natt.h +++ b/libretro-common/include/net/net_natt.h @@ -32,31 +32,17 @@ RETRO_BEGIN_DECLS struct natt_status { - /** nfds for select when checking for input */ - int nfds; - - /** The fdset to be selected upon to check for responses */ - fd_set fds; - - /** True if there might be a request outstanding */ - bool request_outstanding; - - /** True if we've resolved an external IPv4 address */ + /* True if we've resolved an external IPv4 address */ bool have_inet4; + /* True if we've resolved an external IPv6 address */ + bool have_inet6; /** External IPv4 address */ struct sockaddr_in ext_inet4_addr; - - /** True if we've resolved an external IPv6 address */ - bool have_inet6; - #if defined(AF_INET6) && !defined(HAVE_SOCKET_LEGACY) && !defined(_3DS) /** External IPv6 address */ struct sockaddr_in6 ext_inet6_addr; #endif - - /** Internal status (currently unused) */ - void *internal; }; /** diff --git a/libretro-common/net/net_natt.c b/libretro-common/net/net_natt.c index e2254027ac..6000d15593 100644 --- a/libretro-common/net/net_natt.c +++ b/libretro-common/net/net_natt.c @@ -43,8 +43,8 @@ #endif #if HAVE_MINIUPNPC -static struct UPNPUrls urls; -static struct IGDdatas data; +static struct IGDdatas data = {0}; +static struct UPNPUrls urls = {0}; #endif /* @@ -55,43 +55,39 @@ static struct IGDdatas data; void natt_init(struct natt_status *status, uint16_t port, enum socket_protocol proto) { -#ifndef HAVE_SOCKET_LEGACY -#if HAVE_MINIUPNPC - struct UPNPDev * devlist; - struct UPNPDev * dev; - char * descXML; - int descXMLsize = 0; - int upnperror = 0; - devlist = upnpDiscover(2000, NULL, NULL, 0, 0, 2, &upnperror); - if (devlist) +#if !defined(HAVE_SOCKET_LEGACY) && HAVE_MINIUPNPC + struct UPNPDev *devlist = + upnpDiscover(2000, NULL, NULL, 0, 0, 2, NULL); + struct UPNPDev *dev = devlist; + + while (dev) { - dev = devlist; - while (dev) + if (strstr(dev->st, "InternetGatewayDevice")) { - if (strstr (dev->st, "InternetGatewayDevice")) + int len; + char *desc = (char *) miniwget(dev->descURL, + &len, 0, NULL); + + if (desc) { - memset(&urls, 0, sizeof(struct UPNPUrls)); - memset(&data, 0, sizeof(struct IGDdatas)); - descXML = (char *) miniwget(dev->descURL, &descXMLsize, 0, NULL); - if (descXML) - { - parserootdesc(descXML, descXMLsize, &data); - free (descXML); - descXML = 0; + memset(&data, 0, sizeof(data)); + FreeUPNPUrls(&urls); + + parserootdesc(desc, len, &data); + free(desc); + + GetUPNPUrls(&urls, &data, dev->descURL, 0); - GetUPNPUrls (&urls, &data, dev->descURL, 0); - } if(natt_open_port_any(status, port, proto)) - goto end; - + break; } - dev = dev->pNext; } + + dev = dev->pNext; } -end: + freeUPNPDevlist(devlist); #endif -#endif } void natt_deinit(struct natt_status *status, @@ -101,14 +97,14 @@ void natt_deinit(struct natt_status *status, natt_close_port(status, proto); natt_free(status); - memset(&urls, 0, sizeof(urls)); memset(&data, 0, sizeof(data)); + FreeUPNPUrls(&urls); #endif } bool natt_new(struct natt_status *status) { - memset(status, 0, sizeof(struct natt_status)); + memset(status, 0, sizeof(*status)); return true; } @@ -121,127 +117,134 @@ void natt_free(struct natt_status *status) static bool natt_open_port(struct natt_status *status, struct sockaddr *addr, socklen_t addrlen, enum socket_protocol proto) { -#ifndef HAVE_SOCKET_LEGACY -#if HAVE_MINIUPNPC +#if !defined(HAVE_SOCKET_LEGACY) && HAVE_MINIUPNPC int r; - char host[PATH_MAX_LENGTH], ext_host[PATH_MAX_LENGTH], + char host[256], ext_host[256], port_str[6], ext_port_str[6]; + const char *proto_str = (proto == SOCKET_PROTOCOL_UDP) ? + "UDP" : "TCP"; + struct natt_status tmp = {0}; struct addrinfo hints = {0}; - const char *proto_str = NULL; struct addrinfo *ext_addrinfo = NULL; /* if NAT traversal is uninitialized or unavailable, oh well */ - if (!urls.controlURL || !urls.controlURL[0]) + if (!status) + return false; + + if (string_is_empty(urls.controlURL)) return false; /* figure out the internal info */ - if (getnameinfo(addr, addrlen, host, PATH_MAX_LENGTH, - port_str, 6, NI_NUMERICHOST|NI_NUMERICSERV) != 0) - return false; - - proto_str = (proto == SOCKET_PROTOCOL_UDP) ? "UDP" : "TCP"; - - /* add the port mapping */ - r = UPNP_AddAnyPortMapping(urls.controlURL, - data.first.servicetype, port_str, - port_str, host, "retroarch", - proto_str, NULL, "0", ext_port_str); - - if (r != 0) - { - /* try the older AddPortMapping */ - memcpy(ext_port_str, port_str, 6); - r = UPNP_AddPortMapping(urls.controlURL, - data.first.servicetype, port_str, - port_str, host, "retroarch", - proto_str, NULL, "0"); - } - if (r != 0) + if (getnameinfo(addr, addrlen, host, sizeof(host), + port_str, sizeof(port_str), NI_NUMERICHOST | NI_NUMERICSERV)) return false; /* get the external IP */ - r = UPNP_GetExternalIPAddress(urls.controlURL, - data.first.servicetype, ext_host); - if (r != 0) + r = UPNP_GetExternalIPAddress(urls.controlURL, data.first.servicetype, + ext_host); + if (r) return false; + /* add the port mapping */ + r = UPNP_AddAnyPortMapping(urls.controlURL, data.first.servicetype, + port_str, port_str, host, "retroarch", + proto_str, NULL, "0", ext_port_str); + if (r) + { + /* try the older AddPortMapping */ + r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype, + port_str, port_str, host, "retroarch", + proto_str, NULL, "0"); + if (r) + return false; + + memcpy(ext_port_str, port_str, sizeof(ext_port_str)); + } + /* update the status */ - if (getaddrinfo_retro(ext_host, - ext_port_str, &hints, &ext_addrinfo) != 0) - return false; + if (getaddrinfo_retro(ext_host, ext_port_str, + &hints, &ext_addrinfo) || !ext_addrinfo) + goto failure; if (ext_addrinfo->ai_family == AF_INET && - ext_addrinfo->ai_addrlen >= sizeof(struct sockaddr_in)) + ext_addrinfo->ai_addrlen >= sizeof(status->ext_inet4_addr)) { - status->have_inet4 = true; - status->ext_inet4_addr = *((struct sockaddr_in *) - ext_addrinfo->ai_addr); + status->have_inet4 = true; + memcpy(&status->ext_inet4_addr, ext_addrinfo->ai_addr, + sizeof(status->ext_inet4_addr)); } #if defined(AF_INET6) && !defined(HAVE_SOCKET_LEGACY) else if (ext_addrinfo->ai_family == AF_INET6 && - ext_addrinfo->ai_addrlen >= sizeof(struct sockaddr_in6)) + ext_addrinfo->ai_addrlen >= sizeof(status->ext_inet6_addr)) { - status->have_inet6 = true; - status->ext_inet6_addr = *((struct sockaddr_in6 *) - ext_addrinfo->ai_addr); + status->have_inet6 = true; + memcpy(&status->ext_inet6_addr, ext_addrinfo->ai_addr, + sizeof(status->ext_inet6_addr)); } #endif else { freeaddrinfo_retro(ext_addrinfo); - return false; + goto failure; } freeaddrinfo_retro(ext_addrinfo); + return true; -#else - return false; +failure: + tmp.have_inet4 = true; + tmp.ext_inet4_addr.sin_family = AF_INET; + sscanf(ext_port_str, "%hu", &tmp.ext_inet4_addr.sin_port); + + natt_close_port(&tmp, proto); #endif -#else + return false; -#endif } bool natt_open_port_any(struct natt_status *status, uint16_t port, enum socket_protocol proto) { -#if !defined(HAVE_SOCKET_LEGACY) && (!defined(SWITCH) || defined(SWITCH) && defined(HAVE_LIBNX)) +#if !defined(HAVE_SOCKET_LEGACY) && (!defined(SWITCH) || defined(HAVE_LIBNX)) size_t i; - char port_str[6]; struct net_ifinfo list; - struct addrinfo hints = {0}, *addr; + struct addrinfo *addr; + char port_str[6]; + struct addrinfo hints = {0}; bool ret = false; - snprintf(port_str, sizeof(port_str), "%hu", port); - /* get our interfaces */ if (!net_ifinfo_new(&list)) return false; /* loop through them */ + snprintf(port_str, sizeof(port_str), "%hu", port); for (i = 0; i < list.size; i++) { - struct net_ifinfo_entry *entry = list.entries + i; + struct net_ifinfo_entry *entry = &list.entries[i]; /* ignore localhost */ - if ( string_is_equal(entry->host, "127.0.0.1") || + if (string_is_equal(entry->host, "127.0.0.1") || string_is_equal(entry->host, "::1")) continue; + addr = NULL; + if (getaddrinfo_retro(entry->host, port_str, &hints, &addr) || + !addr) + continue; + /* make a request for this host */ - if (getaddrinfo_retro(entry->host, port_str, &hints, &addr) == 0) - { - ret = natt_open_port(status, addr->ai_addr, - addr->ai_addrlen, proto) || ret; - freeaddrinfo_retro(addr); - } + if (natt_open_port(status, addr->ai_addr, addr->ai_addrlen, + proto)) + ret = true; + + freeaddrinfo_retro(addr); } net_ifinfo_free(&list); return ret; - #else return false; #endif @@ -276,6 +279,7 @@ bool natt_close_port(struct natt_status *status, } else return false; + if (getnameinfo(addr, addrlen, NULL, 0, port_str, sizeof(port_str), NI_NUMERICSERV)) return false; @@ -283,6 +287,8 @@ bool natt_close_port(struct natt_status *status, /* Request the device to remove our port forwarding. */ return !UPNP_DeletePortMapping(urls.controlURL, data.first.servicetype, port_str, proto_str, NULL); +#else + return false; #endif } diff --git a/network/netplay/netplay_frontend.c b/network/netplay/netplay_frontend.c index 7e88fb9788..2959de41a0 100644 --- a/network/netplay/netplay_frontend.c +++ b/network/netplay/netplay_frontend.c @@ -5530,7 +5530,7 @@ void netplay_handle_slaves(netplay_t *netplay) */ void netplay_announce_nat_traversal(netplay_t *netplay) { -#ifndef HAVE_SOCKET_LEGACY +#if !defined(HAVE_SOCKET_LEGACY) && HAVE_MINIUPNPC char msg[512], host[256], port[6]; const char *dmsg = NULL; @@ -5586,8 +5586,6 @@ void netplay_announce_nat_traversal(netplay_t *netplay) */ void netplay_init_nat_traversal(netplay_t *netplay) { - memset(&netplay->nat_traversal_state, 0, sizeof(netplay->nat_traversal_state)); - netplay->nat_traversal_task_oustanding = true; task_push_netplay_nat_traversal(&netplay->nat_traversal_state, netplay->tcp_port); } @@ -7298,24 +7296,6 @@ static bool netplay_pre_frame( /* Advertise our server */ netplay_lan_ad_server(netplay); #endif - - /* NAT traversal if applicable */ - if (netplay->nat_traversal && - !netplay->nat_traversal_task_oustanding && - netplay->nat_traversal_state.request_outstanding && - !netplay->nat_traversal_state.have_inet4) - { - struct timeval tmptv = {0}; - fd_set fds = netplay->nat_traversal_state.fds; - if (socket_select(netplay->nat_traversal_state.nfds, &fds, NULL, NULL, &tmptv) > 0) - natt_read(&netplay->nat_traversal_state); - -#ifndef HAVE_SOCKET_LEGACY - if (!netplay->nat_traversal_state.request_outstanding || - netplay->nat_traversal_state.have_inet4) - netplay_announce_nat_traversal(netplay); -#endif - } } sync_stalled = !netplay_sync_pre_frame(netplay); @@ -7563,10 +7543,7 @@ bool netplay_driver_ctl(enum rarch_netplay_ctl_state state, void *data) netplay_disconnect(netplay); goto done; case RARCH_NETPLAY_CTL_FINISHED_NAT_TRAVERSAL: - netplay->nat_traversal_task_oustanding = false; -#ifndef HAVE_SOCKET_LEGACY netplay_announce_nat_traversal(netplay); -#endif goto done; case RARCH_NETPLAY_CTL_DESYNC_PUSH: netplay->desync++; diff --git a/network/netplay/netplay_private.h b/network/netplay/netplay_private.h index a34ee2f654..ac8e85e336 100644 --- a/network/netplay/netplay_private.h +++ b/network/netplay/netplay_private.h @@ -541,7 +541,7 @@ struct netplay /* Our nickname */ char nick[NETPLAY_NICK_LEN]; - bool nat_traversal, nat_traversal_task_oustanding; + bool nat_traversal; /* Set to true if we have a device that most cores * translate to "up/down" actions, typically a keyboard. diff --git a/tasks/task_netplay_nat_traversal.c b/tasks/task_netplay_nat_traversal.c index 62591e559b..fc57706e62 100644 --- a/tasks/task_netplay_nat_traversal.c +++ b/tasks/task_netplay_nat_traversal.c @@ -31,18 +31,6 @@ struct nat_traversal_state_data uint16_t port; }; -static void netplay_nat_traversal_callback(retro_task_t *task, - void *task_data, - void *user_data, const char *error) -{ - struct nat_traversal_state_data *ntsd = - (struct nat_traversal_state_data *) task_data; - - free(ntsd); - - netplay_driver_ctl(RARCH_NETPLAY_CTL_FINISHED_NAT_TRAVERSAL, NULL); -} - static void task_netplay_nat_traversal_handler(retro_task_t *task) { struct nat_traversal_state_data *ntsd = @@ -55,36 +43,6 @@ static void task_netplay_nat_traversal_handler(retro_task_t *task) task_set_finished(task, true); } -bool task_push_netplay_nat_traversal(void *nat_traversal_state, uint16_t port) -{ - struct nat_traversal_state_data *ntsd; - retro_task_t *task = task_init(); - - if (!task) - return false; - - ntsd = (struct nat_traversal_state_data *) - calloc(1, sizeof(*ntsd)); - - if (!ntsd) - { - free(task); - return false; - } - - ntsd->nat_traversal_state = (struct natt_status *)nat_traversal_state; - ntsd->port = port; - - task->type = TASK_TYPE_BLOCKING; - task->handler = task_netplay_nat_traversal_handler; - task->callback = netplay_nat_traversal_callback; - task->task_data = ntsd; - - task_queue_push(task); - - return true; -} - static void task_netplay_nat_close_handler(retro_task_t *task) { natt_deinit((struct natt_status *) task->task_data, @@ -94,10 +52,70 @@ static void task_netplay_nat_close_handler(retro_task_t *task) task_set_finished(task, true); } +static void netplay_nat_traversal_callback(retro_task_t *task, + void *task_data, + void *user_data, const char *error) +{ + free(task_data); + + netplay_driver_ctl(RARCH_NETPLAY_CTL_FINISHED_NAT_TRAVERSAL, NULL); +} + +static bool nat_task_finder(retro_task_t *task, void *userdata) +{ + if (!task) + return false; + + return task->handler == task_netplay_nat_traversal_handler || + task->handler == task_netplay_nat_close_handler; +} + +static bool nat_task_queued(void *data) +{ + task_finder_data_t find_data = {nat_task_finder, NULL}; + + return task_queue_find(&find_data); +} + +bool task_push_netplay_nat_traversal(void *nat_traversal_state, uint16_t port) +{ + retro_task_t *task; + struct nat_traversal_state_data *ntsd; + + /* Do not run more than one NAT task at a time. */ + task_queue_wait(nat_task_queued, NULL); + + task = task_init(); + if (!task) + return false; + + ntsd = (struct nat_traversal_state_data *) malloc(sizeof(*ntsd)); + if (!ntsd) + { + free(task); + return false; + } + + ntsd->nat_traversal_state = (struct natt_status *) nat_traversal_state; + ntsd->port = port; + + task->handler = task_netplay_nat_traversal_handler; + task->callback = netplay_nat_traversal_callback; + task->task_data = ntsd; + + task_queue_push(task); + + return true; +} + bool task_push_netplay_nat_close(void *nat_traversal_state) { - retro_task_t *task = task_init(); + retro_task_t *task; + /* Do not run more than one NAT task at a time. */ + task_queue_wait(nat_task_queued, NULL); + + task = task_init(); if (!task) return false;