Eric Wong <normalper...@yhbt.net> writes: > getaddrinfo() may return multiple addresses, not all of which > are equally performant. In some cases, a user behind a non-IPv6 > capable network may get an IPv6 address which stalls connect(). > Instead of waiting synchronously for a connect() to timeout, use > non-blocking connect() in parallel and take the first successful > connection. > > This may increase network traffic and server load slightly, but > makes the worst-case user experience more bearable when one > lacks permissions to edit /etc/gai.conf to favor IPv4 addresses.
Umm. I am not sure what to think about this change--I generally do not like a selfish "I'll try to use whatever resource given to me to make my process go faster, screw the rest of the world" approach and I cannot decide if this falls into that category. I'll wait for opinions from others. Thanks. > Signed-off-by: Eric Wong <normalper...@yhbt.net> > --- > connect.c | 118 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 104 insertions(+), 14 deletions(-) > > diff --git a/connect.c b/connect.c > index fd7ffe1..74d2bb5 100644 > --- a/connect.c > +++ b/connect.c > @@ -14,6 +14,42 @@ > static char *server_capabilities; > static const char *parse_feature_value(const char *, const char *, int *); > > +#ifdef SOCK_NONBLOCK /* Linux-only flag */ > +# define GIT_SOCK_NONBLOCK SOCK_NONBLOCK > +#else > +# define GIT_SOCK_NONBLOCK 0 > +#endif > + > +static int socket_nb(int domain, int type, int protocol) > +{ > + static int flags = GIT_SOCK_NONBLOCK; > + int fd = socket(domain, type | flags, protocol); > + > + /* new headers, old kernel? */ > + if (fd < 0 && errno == EINVAL && flags != 0) { > + flags = 0; > + fd = socket(domain, type, protocol); > + } > + > + /* couldn't use SOCK_NONBLOCK, set non-blocking the old way */ > + if (flags == 0 && fd >= 0) { > + int fl = fcntl(fd, F_GETFL); > + > + if (fcntl(fd, F_SETFL, fl | O_NONBLOCK) < 0) > + die_errno("failed to set nonblocking flag\n"); > + } > + > + return fd; > +} > + > +static void set_blocking(int fd) > +{ > + int fl = fcntl(fd, F_GETFL); > + > + if (fcntl(fd, F_SETFL, fl & ~O_NONBLOCK) < 0) > + die_errno("failed to clear nonblocking flag\n"); > +} > + > static int check_ref(const char *name, unsigned int flags) > { > if (!flags) > @@ -351,6 +387,9 @@ static int git_tcp_connect_sock(char *host, int flags) > struct addrinfo hints, *ai0, *ai; > int gai; > int cnt = 0; > + nfds_t n = 0, nfds = 0; > + struct pollfd *fds = NULL; > + struct addrinfo **inprogress = NULL; > > get_host_and_port(&host, &port); > if (!*port) > @@ -371,20 +410,76 @@ static int git_tcp_connect_sock(char *host, int flags) > fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, > port); > > for (ai0 = ai; ai; ai = ai->ai_next, cnt++) { > - sockfd = socket(ai->ai_family, > - ai->ai_socktype, ai->ai_protocol); > - if ((sockfd < 0) || > - (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) { > + size_t cur; > + int fd = socket_nb(ai->ai_family, ai->ai_socktype, > + ai->ai_protocol); > + if (fd < 0) { > strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n", > host, cnt, ai_name(ai), strerror(errno)); > - if (0 <= sockfd) > - close(sockfd); > - sockfd = -1; > continue; > } > + > + if (connect(fd, ai->ai_addr, ai->ai_addrlen) < 0 && > + errno != EINPROGRESS) { > + strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n", > + host, cnt, ai_name(ai), strerror(errno)); > + close(fd); > + continue; > + } > + > if (flags & CONNECT_VERBOSE) > - fprintf(stderr, "%s ", ai_name(ai)); > - break; > + fprintf(stderr, "%s (started)\n", ai_name(ai)); > + > + nfds = n + 1; > + cur = n; > + ALLOC_GROW(fds, nfds, cur); > + cur = n; > + ALLOC_GROW(inprogress, nfds, cur); > + inprogress[n] = ai; > + fds[n].fd = fd; > + fds[n].events = POLLIN|POLLOUT; > + fds[n].revents = 0; > + n = nfds; > + } > + > + /* > + * nfds is tiny, no need to limit loop based on poll() retval, > + * just do not let poll sleep forever if nfds is zero > + */ > + if (nfds > 0) > + poll(fds, nfds, -1); > + > + for (n = 0; n < nfds && sockfd < 0; n++) { > + if (fds[n].revents & (POLLERR|POLLHUP)) > + continue; > + if (fds[n].revents & POLLOUT) { > + int err; > + socklen_t len = (socklen_t)sizeof(err); > + int rc = getsockopt(fds[n].fd, SOL_SOCKET, SO_ERROR, > + &err, &len); > + if (rc != 0) > + die_errno("getsockopt errno=%s\n", > + strerror(errno)); > + if (err == 0) { /* success! */ > + sockfd = fds[n].fd; > + ai = inprogress[n]; > + } > + } > + } > + > + /* cleanup */ > + for (n = 0; n < nfds; n++) { > + if (fds[n].fd != sockfd) > + close(fds[n].fd); > + } > + free(inprogress); > + free(fds); > + > + if (sockfd >= 0) { > + enable_keepalive(sockfd); > + set_blocking(sockfd); /* the rest of git expects blocking */ > + if (flags & CONNECT_VERBOSE) > + fprintf(stderr, "%s done.\n", ai_name(ai)); > } > > freeaddrinfo(ai0); > @@ -392,11 +487,6 @@ static int git_tcp_connect_sock(char *host, int flags) > if (sockfd < 0) > die("unable to connect to %s:\n%s", host, error_message.buf); > > - enable_keepalive(sockfd); > - > - if (flags & CONNECT_VERBOSE) > - fprintf(stderr, "done.\n"); > - > strbuf_release(&error_message); > > return sockfd; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html