From 23ddea7485ea5d28b8ad1a0d35e0d8c4de7ad54b Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Tue, 2 Jan 2018 21:43:52 +0100 Subject: connect: Alternate address families for addresses As a first step to implementing Happy Eyeballs version 2, we need to order the list of hosts getaddrinfo() gave us so it alternates between preferred and other address families. RFC: https://tools.ietf.org/html/rfc8305 Gbp-Dch: ignore --- methods/connect.cc | 72 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/methods/connect.cc b/methods/connect.cc index 1354fe97b..22dd41bee 100644 --- a/methods/connect.cc +++ b/methods/connect.cc @@ -198,6 +198,50 @@ static ResultState DoConnect(struct addrinfo *Addr, std::string const &Host, return ResultState::SUCCESSFUL; } /*}}}*/ +// Order the given host names returned by getaddrinfo() /*{{{*/ +static std::vector OrderAddresses(struct addrinfo *CurHost) +{ + std::vector preferredAddrs; + std::vector otherAddrs; + std::vector allAddrs; + + // Partition addresses into preferred and other address families + while (CurHost != 0) + { + if (preferredAddrs.empty() || CurHost->ai_family == preferredAddrs[0]->ai_family) + preferredAddrs.push_back(CurHost); + else + otherAddrs.push_back(CurHost); + + // Ignore UNIX domain sockets + do + { + CurHost = CurHost->ai_next; + } while (CurHost != 0 && CurHost->ai_family == AF_UNIX); + + /* If we reached the end of the search list then wrap around to the + start */ + if (CurHost == 0 && LastUsed != 0) + CurHost = LastHostAddr; + + // Reached the end of the search cycle + if (CurHost == LastUsed) + break; + } + + // Build a new address vector alternating between preferred and other + for (auto prefIter = preferredAddrs.cbegin(), otherIter = otherAddrs.cbegin(); + prefIter != preferredAddrs.end() || otherIter != otherAddrs.end();) + { + if (prefIter != preferredAddrs.end()) + allAddrs.push_back(*prefIter++); + if (otherIter != otherAddrs.end()) + allAddrs.push_back(*otherIter++); + } + + return std::move(allAddrs); +} + /*}}}*/ // Connect to a given Hostname /*{{{*/ static ResultState ConnectToHostname(std::string const &Host, int const Port, const char *const Service, int DefPort, std::unique_ptr &Fd, @@ -301,12 +345,11 @@ static ResultState ConnectToHostname(std::string const &Host, int const Port, } // When we have an IP rotation stay with the last IP. - struct addrinfo *CurHost = LastHostAddr; - if (LastUsed != 0) - CurHost = LastUsed; - - while (CurHost != 0) + auto Addresses = OrderAddresses(LastUsed != nullptr ? LastUsed : LastHostAddr); + + for (auto CurHost : Addresses) { + _error->Discard(); auto const result = DoConnect(CurHost, Host, TimeOut, Fd, Owner); if (result == ResultState::SUCCESSFUL) { @@ -314,25 +357,6 @@ static ResultState ConnectToHostname(std::string const &Host, int const Port, return result; } Fd->Close(); - - // Ignore UNIX domain sockets - do - { - CurHost = CurHost->ai_next; - } - while (CurHost != 0 && CurHost->ai_family == AF_UNIX); - - /* If we reached the end of the search list then wrap around to the - start */ - if (CurHost == 0 && LastUsed != 0) - CurHost = LastHostAddr; - - // Reached the end of the search cycle - if (CurHost == LastUsed) - break; - - if (CurHost != 0) - _error->Discard(); } if (_error->PendingError() == true) -- cgit v1.2.3 From 2369d1249ce7119abb30b616182454a56f124f8d Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Tue, 2 Jan 2018 21:53:46 +0100 Subject: connect: Extract a Connection struct This struct holds information about a connection attempt, like the addrinfo, the resolved address, the fd for the connection, and so on. Gbp-Dch: ignore --- methods/connect.cc | 44 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/methods/connect.cc b/methods/connect.cc index 22dd41bee..c1a9d347f 100644 --- a/methods/connect.cc +++ b/methods/connect.cc @@ -112,16 +112,43 @@ std::unique_ptr MethodFd::FromFd(int iFd) // DoConnect - Attempt a connect operation /*{{{*/ // --------------------------------------------------------------------- /* This helper function attempts a connection to a single address. */ -static ResultState DoConnect(struct addrinfo *Addr, std::string const &Host, - unsigned long TimeOut, std::unique_ptr &Fd, aptMethod *Owner) +struct Connection { - // Show a status indicator + struct addrinfo *Addr; + std::string Host; + aptMethod *Owner; + std::unique_ptr Fd; char Name[NI_MAXHOST]; char Service[NI_MAXSERV]; - Fd.reset(new FdFd()); - Name[0] = 0; - Service[0] = 0; + Connection(struct addrinfo *Addr, std::string const &Host, aptMethod *Owner) : Addr(Addr), Host(Host), Owner(Owner), Fd(new FdFd()), Name{0}, Service{0} + { + } + + // Allow moving values, but not connections. + Connection(Connection &&Conn) = default; + Connection(const Connection &Conn) = delete; + Connection &operator=(const Connection &) = delete; + Connection &operator=(Connection &&Conn) = default; + + ~Connection() + { + if (Fd != nullptr) + { + Fd->Close(); + } + } + + std::unique_ptr Take() + { + return std::move(Fd); + } + + ResultState DoConnect(unsigned long TimeOut); +}; + +ResultState Connection::DoConnect(unsigned long TimeOut) +{ getnameinfo(Addr->ai_addr,Addr->ai_addrlen, Name,sizeof(Name),Service,sizeof(Service), NI_NUMERICHOST|NI_NUMERICSERV); @@ -350,13 +377,14 @@ static ResultState ConnectToHostname(std::string const &Host, int const Port, for (auto CurHost : Addresses) { _error->Discard(); - auto const result = DoConnect(CurHost, Host, TimeOut, Fd, Owner); + Connection Conn(CurHost, Host, Owner); + auto const result = Conn.DoConnect(TimeOut); if (result == ResultState::SUCCESSFUL) { + Fd = Conn.Take(); LastUsed = CurHost; return result; } - Fd->Close(); } if (_error->PendingError() == true) -- cgit v1.2.3 From c9a5a6f2140758c0ed08764a07dd454a8f3ff986 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Tue, 2 Jan 2018 21:56:40 +0100 Subject: connect: Store the IP used when picking a connection There's no real point in storing the IP address while resolving it - failure messages include the IP address in any case. Do this when picking the connection for actual use instead. --- methods/connect.cc | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/methods/connect.cc b/methods/connect.cc index c1a9d347f..07427c651 100644 --- a/methods/connect.cc +++ b/methods/connect.cc @@ -141,6 +141,11 @@ struct Connection std::unique_ptr Take() { + /* Store the IP we are using.. If something goes + wrong this will get tacked onto the end of the error message */ + std::stringstream ss; + ioprintf(ss, _("[IP: %s %s]"), Name, Service); + Owner->SetIP(ss.str()); return std::move(Fd); } @@ -157,15 +162,6 @@ ResultState Connection::DoConnect(unsigned long TimeOut) // if that addr did timeout before, we do not try it again if(bad_addr.find(std::string(Name)) != bad_addr.end()) return ResultState::TRANSIENT_ERROR; - - /* If this is an IP rotation store the IP we are using.. If something goes - wrong this will get tacked onto the end of the error message */ - if (LastHostAddr->ai_next != 0) - { - std::stringstream ss; - ioprintf(ss, _("[IP: %s %s]"),Name,Service); - Owner->SetIP(ss.str()); - } // Get a socket if ((static_cast(Fd.get())->fd = socket(Addr->ai_family, Addr->ai_socktype, -- cgit v1.2.3 From 53bdec3ebea66153b320ee497871355eb526e0f2 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Tue, 2 Jan 2018 21:59:02 +0100 Subject: connect: Extract Connection::CheckError() method Extracting the error checking method allows us to reuse it in different places, so we can move the waiting and checking out of DoConnect() eventually. Gbp-Dch: ignore --- methods/connect.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/methods/connect.cc b/methods/connect.cc index 07427c651..160491d96 100644 --- a/methods/connect.cc +++ b/methods/connect.cc @@ -150,6 +150,7 @@ struct Connection } ResultState DoConnect(unsigned long TimeOut); + ResultState CheckError(); }; ResultState Connection::DoConnect(unsigned long TimeOut) @@ -194,6 +195,11 @@ ResultState Connection::DoConnect(unsigned long TimeOut) return ResultState::TRANSIENT_ERROR; } + return CheckError(); +} + +ResultState Connection::CheckError() +{ // Check the socket for an error condition unsigned int Err; unsigned int Len = sizeof(Err); -- cgit v1.2.3 From 3bbd328396745d0dd6c5585935040082a2c41e3e Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Tue, 2 Jan 2018 22:15:50 +0100 Subject: Add rapid "happy eyeballs" connection fallback (RFC 8305) Try establishing connections in alternating address families in rapid intervals of 250 ms, adding more connections to the wait list until one succeeds (RFC 8305, happy eyeballs 2). It is important that WaitAndCheckErrors() waits until it has a successful connection, a time out, or all connections failed - otherwise the timing between tries might be wrong, and the final long wait might exit early because one connection failed without trying the others. Timing wise, this only works correctly on Linux, as select() counts down there. But we rely on that in some other places too, so this is not the time to fix that. Timeouts are only reported in the final long wait - the short inner waits are expected to time out more often, and multiple times, we do not want to report them. Closes: #668948 LP: #1308200 Gbp-Dch: paragraph --- doc/examples/configure-index | 3 ++ methods/connect.cc | 126 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 103 insertions(+), 26 deletions(-) diff --git a/doc/examples/configure-index b/doc/examples/configure-index index a765fbe42..153637ebc 100644 --- a/doc/examples/configure-index +++ b/doc/examples/configure-index @@ -257,6 +257,7 @@ Acquire Proxy "http://127.0.0.1:3128"; Proxy::http.us.debian.org "DIRECT"; // Specific per-host setting Timeout "120"; + ConnectionAttemptDelayMsec "250"; Pipeline-Depth "5"; AllowRedirect "true"; @@ -285,6 +286,7 @@ Acquire AllowRedirect "true"; Timeout "120"; + ConnectionAttemptDelayMsec "250"; AllowRedirect "true"; // Cache Control. Note these do not work with Squid 2.0.2 @@ -312,6 +314,7 @@ Acquire }; Timeout "120"; + ConnectionAttemptDelayMsec "250"; /* Passive mode control, proxy, non-proxy and per-host. Pasv mode is preferred if possible */ diff --git a/methods/connect.cc b/methods/connect.cc index 160491d96..334a1d3f3 100644 --- a/methods/connect.cc +++ b/methods/connect.cc @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -35,6 +36,7 @@ #include #include #include +#include #include #include "aptmethod.h" @@ -146,14 +148,19 @@ struct Connection std::stringstream ss; ioprintf(ss, _("[IP: %s %s]"), Name, Service); Owner->SetIP(ss.str()); + Owner->Status(_("Connected to %s (%s)"), Host.c_str(), Name); + _error->Discard(); + Owner->SetFailReason(""); + LastUsed = Addr; return std::move(Fd); } - ResultState DoConnect(unsigned long TimeOut); + ResultState DoConnect(); + ResultState CheckError(); }; -ResultState Connection::DoConnect(unsigned long TimeOut) +ResultState Connection::DoConnect() { getnameinfo(Addr->ai_addr,Addr->ai_addrlen, Name,sizeof(Name),Service,sizeof(Service), @@ -183,19 +190,7 @@ ResultState Connection::DoConnect(unsigned long TimeOut) return ResultState::TRANSIENT_ERROR; } - /* This implements a timeout for connect by opening the connection - nonblocking */ - if (WaitFd(Fd->Fd(), true, TimeOut) == false) - { - bad_addr.insert(bad_addr.begin(), std::string(Name)); - Owner->SetFailReason("Timeout"); - _error->Error(_("Could not connect to %s:%s (%s), " - "connection timed out"), - Host.c_str(), Service, Name); - return ResultState::TRANSIENT_ERROR; - } - - return CheckError(); + return ResultState::SUCCESSFUL; } ResultState Connection::CheckError() @@ -271,6 +266,82 @@ static std::vector OrderAddresses(struct addrinfo *CurHost) return std::move(allAddrs); } /*}}}*/ +// Check for errors and report them /*{{{*/ +static ResultState WaitAndCheckErrors(std::list &Conns, std::unique_ptr &Fd, long TimeoutMsec, bool ReportTimeout) +{ + // The last error detected + ResultState Result = ResultState::TRANSIENT_ERROR; + + struct timeval tv = { + // Split our millisecond timeout into seconds and microseconds + .tv_sec = TimeoutMsec / 1000, + .tv_usec = (TimeoutMsec % 1000) * 1000, + }; + + // We will return once we have no more connections, a time out, or + // a success. + while (!Conns.empty()) + { + fd_set Set; + int nfds = -1; + + FD_ZERO(&Set); + + for (auto &Conn : Conns) + { + int fd = Conn.Fd->Fd(); + FD_SET(fd, &Set); + nfds = std::max(nfds, fd); + } + + { + int Res; + do + { + Res = select(nfds + 1, 0, &Set, 0, (TimeoutMsec != 0 ? &tv : 0)); + } while (Res < 0 && errno == EINTR); + + if (Res == 0) + { + if (ReportTimeout) + { + for (auto &Conn : Conns) + { + Conn.Owner->SetFailReason("Timeout"); + _error->Error(_("Could not connect to %s:%s (%s), " + "connection timed out"), + Conn.Host.c_str(), Conn.Service, Conn.Name); + } + } + return ResultState::TRANSIENT_ERROR; + } + } + + // iterate over connections, remove failed ones, and return if + // there was a successful one. + for (auto ConnI = Conns.begin(); ConnI != Conns.end();) + { + if (!FD_ISSET(ConnI->Fd->Fd(), &Set)) + { + ConnI++; + continue; + } + + Result = ConnI->CheckError(); + if (Result == ResultState::SUCCESSFUL) + { + Fd = ConnI->Take(); + return Result; + } + + // Connection failed. Erase it and continue to next position + ConnI = Conns.erase(ConnI); + } + } + + return Result; +} + /*}}}*/ // Connect to a given Hostname /*{{{*/ static ResultState ConnectToHostname(std::string const &Host, int const Port, const char *const Service, int DefPort, std::unique_ptr &Fd, @@ -375,19 +446,22 @@ static ResultState ConnectToHostname(std::string const &Host, int const Port, // When we have an IP rotation stay with the last IP. auto Addresses = OrderAddresses(LastUsed != nullptr ? LastUsed : LastHostAddr); + std::list Conns; - for (auto CurHost : Addresses) + for (auto Addr : Addresses) { - _error->Discard(); - Connection Conn(CurHost, Host, Owner); - auto const result = Conn.DoConnect(TimeOut); - if (result == ResultState::SUCCESSFUL) - { - Fd = Conn.Take(); - LastUsed = CurHost; - return result; - } - } + Connection Conn(Addr, Host, Owner); + if (Conn.DoConnect() != ResultState::SUCCESSFUL) + continue; + + Conns.push_back(std::move(Conn)); + + if (WaitAndCheckErrors(Conns, Fd, Owner->ConfigFindI("ConnectionAttemptDelayMsec", 250), false) == ResultState::SUCCESSFUL) + return ResultState::SUCCESSFUL; + } + + if (WaitAndCheckErrors(Conns, Fd, TimeOut * 1000, true) == ResultState::SUCCESSFUL) + return ResultState::SUCCESSFUL; if (_error->PendingError() == true) return ResultState::FATAL_ERROR; -- cgit v1.2.3