From f106fecc1d7d0250d1d76c2ba837dc0c870c5fff Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 20 May 2014 13:15:51 +0200 Subject: add GetSrvRecord helper to apt-pkg/contrib/srvrec.{cc,h} --- apt-pkg/contrib/srvrec.cc | 93 +++++++++++++++++++++++++++++++++++++++++++++++ apt-pkg/contrib/srvrec.h | 30 +++++++++++++++ cmdline/apt-helper.cc | 25 +++++++++++++ cmdline/makefile | 2 +- 4 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 apt-pkg/contrib/srvrec.cc create mode 100644 apt-pkg/contrib/srvrec.h diff --git a/apt-pkg/contrib/srvrec.cc b/apt-pkg/contrib/srvrec.cc new file mode 100644 index 000000000..d5d89fc67 --- /dev/null +++ b/apt-pkg/contrib/srvrec.cc @@ -0,0 +1,93 @@ +// -*- mode: cpp; mode: fold -*- +// Description /*{{{*/ +/* ###################################################################### + + SRV record support + + ##################################################################### */ + /*}}}*/ +#include + +#include +#include +#include + +#include +#include "srvrec.h" + +bool GetSrvRecords(std::string name, std::vector &Result) +{ + unsigned char answer[PACKETSZ]; + int answer_len, compressed_name_len; + int answer_count; + + if (res_init() != 0) + return _error->Errno("res_init", "Failed to init resolver"); + + answer_len = res_query(name.c_str(), C_IN, T_SRV, answer, sizeof(answer)); + if (answer_len < (int)sizeof(HEADER)) + return _error->Warning("Not enough data from res_query (%i)", answer_len); + + // check the header + HEADER *header = (HEADER*)answer; + if (header->rcode != NOERROR) + return _error->Warning("res_query returned rcode %i", header->rcode); + answer_count = ntohs(header->ancount); + if (answer_count <= 0) + return _error->Warning("res_query returned no answers (%i) ", answer_count); + + // skip the header + compressed_name_len = dn_skipname(answer+sizeof(HEADER), answer+answer_len); + if(compressed_name_len < 0) + return _error->Warning("dn_skipname failed %i", compressed_name_len); + + // pt points to the first answer record, go over all of them now + unsigned char *pt = answer+sizeof(HEADER)+compressed_name_len+QFIXEDSZ; + while ((int)Result.size() < answer_count && pt < answer+answer_len) + { + SrvRec rec; + u_int16_t type, klass, priority, weight, port, dlen; + char buf[MAXDNAME]; + + compressed_name_len = dn_skipname(pt, answer+answer_len); + if (compressed_name_len < 0) + return _error->Warning("dn_skipname failed (2): %i", + compressed_name_len); + pt += compressed_name_len; + if (((answer+answer_len) - pt) < 16) + return _error->Warning("packet too short"); + + // extract the data out of the result buffer + #define extract_u16(target, p) target = *p++ << 8; target |= *p++; + + extract_u16(type, pt); + if(type != T_SRV) + return _error->Warning("Unexpected type excepted %x != %x", + T_SRV, type); + extract_u16(klass, pt); + if(klass != C_IN) + return _error->Warning("Unexpected class excepted %x != %x", + C_IN, klass); + pt += 4; // ttl + extract_u16(dlen, pt); + extract_u16(priority, pt); + extract_u16(weight, pt); + extract_u16(port, pt); + + #undef extract_u16 + + compressed_name_len = dn_expand(answer, answer+answer_len, pt, buf, sizeof(buf)); + if(compressed_name_len < 0) + return _error->Warning("dn_expand failed %i", compressed_name_len); + pt += compressed_name_len; + + // add it to our class + rec.priority = priority; + rec.weight = weight; + rec.port = port; + rec.target = buf; + Result.push_back(rec); + } + + return true; +} diff --git a/apt-pkg/contrib/srvrec.h b/apt-pkg/contrib/srvrec.h new file mode 100644 index 000000000..583907706 --- /dev/null +++ b/apt-pkg/contrib/srvrec.h @@ -0,0 +1,30 @@ +// -*- mode: cpp; mode: fold -*- +// Description /*{{{*/ +/* ###################################################################### + + SRV record support + + ##################################################################### */ + /*}}}*/ +#ifndef SRVREC_H +#define SRVREC_H + +#include +#include +#include + +class SrvRec +{ + public: + std::string target; + u_int16_t priority; + u_int16_t weight; + u_int16_t port; + + // see rfc-2782 + //int random; +}; + +bool GetSrvRecords(std::string name, std::vector &Result); + +#endif diff --git a/cmdline/apt-helper.cc b/cmdline/apt-helper.cc index 2c1107d90..aeeccf06d 100644 --- a/cmdline/apt-helper.cc +++ b/cmdline/apt-helper.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -53,6 +54,29 @@ static bool DoDownloadFile(CommandLine &CmdL) return true; } +static bool DoSrvLookup(CommandLine &CmdL) +{ + if (CmdL.FileSize() < 1) + return _error->Error(_("Must specifc at least one srv record")); + + std::vector srv_records; + for(int i=1; CmdL.FileList[i] != NULL; i++) + { + if(GetSrvRecords(CmdL.FileList[i], srv_records) == false) + _error->Warning(_("GetSrvRec failed for %s"), CmdL.FileList[i]); + for (std::vector::const_iterator I = srv_records.begin(); + I != srv_records.end(); ++I) + { + c1out << (*I).target.c_str() << " " + << (*I).priority << " " + << (*I).weight << " " + << (*I).port << " " + << std::endl; + } + } + return true; +} + static bool ShowHelp(CommandLine &) { ioprintf(std::cout,_("%s %s for %s compiled on %s %s\n"),PACKAGE,PACKAGE_VERSION, @@ -79,6 +103,7 @@ int main(int argc,const char *argv[]) /*{{{*/ { CommandLine::Dispatch Cmds[] = {{"help",&ShowHelp}, {"download-file", &DoDownloadFile}, + {"srv-lookup", &DoSrvLookup}, {0,0}}; std::vector Args = getCommandArgs( diff --git a/cmdline/makefile b/cmdline/makefile index c4a249cd6..a24738e63 100644 --- a/cmdline/makefile +++ b/cmdline/makefile @@ -49,7 +49,7 @@ include $(PROGRAM_H) # The apt-helper PROGRAM=apt-helper -SLIBS = -lapt-pkg -lapt-private $(INTLLIBS) +SLIBS = -lapt-pkg -lapt-private $(INTLLIBS) -lresolv LIB_MAKES = apt-pkg/makefile SOURCE = apt-helper.cc include $(PROGRAM_H) -- cgit v1.2.3-70-g09d2 From 8194f97685b4af2bbf0cdbdc88fe5332ad147353 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 20 May 2014 15:08:21 +0200 Subject: add sorting by priority --- apt-pkg/contrib/srvrec.cc | 7 +++++++ apt-pkg/contrib/srvrec.h | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/apt-pkg/contrib/srvrec.cc b/apt-pkg/contrib/srvrec.cc index d5d89fc67..352a56970 100644 --- a/apt-pkg/contrib/srvrec.cc +++ b/apt-pkg/contrib/srvrec.cc @@ -12,6 +12,8 @@ #include #include +#include + #include #include "srvrec.h" @@ -89,5 +91,10 @@ bool GetSrvRecords(std::string name, std::vector &Result) Result.push_back(rec); } + // sort them + std::stable_sort(Result.begin(), Result.end()); + + // FIXME: implement weight selection as specified in RFC-2782 + return true; } diff --git a/apt-pkg/contrib/srvrec.h b/apt-pkg/contrib/srvrec.h index 583907706..78d238c46 100644 --- a/apt-pkg/contrib/srvrec.h +++ b/apt-pkg/contrib/srvrec.h @@ -21,8 +21,9 @@ class SrvRec u_int16_t weight; u_int16_t port; - // see rfc-2782 - //int random; + bool operator<(SrvRec const &other) const { + return this->priority < other.priority; + } }; bool GetSrvRecords(std::string name, std::vector &Result); -- cgit v1.2.3-70-g09d2 From a01695e82cfbfa6e296c66879c1e41802d3ea413 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 22 May 2014 08:55:14 +0200 Subject: WIP make connect use GetSrvRecords --- apt-pkg/contrib/srvrec.cc | 14 ++++++++++++++ apt-pkg/contrib/srvrec.h | 6 ++++++ methods/connect.cc | 13 +++++++++++++ methods/makefile | 8 ++++---- 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/apt-pkg/contrib/srvrec.cc b/apt-pkg/contrib/srvrec.cc index 352a56970..c473192fc 100644 --- a/apt-pkg/contrib/srvrec.cc +++ b/apt-pkg/contrib/srvrec.cc @@ -8,15 +8,29 @@ /*}}}*/ #include +#include + #include #include #include #include +#include #include #include "srvrec.h" +bool GetSrvRecords(std::string host, int port, std::vector &Result) +{ + std::string target; + struct servent *s_ent = getservbyport(htons(port), "tcp"); + if (s_ent == NULL) + return false; + + strprintf(target, "_%s._tcp.%s", s_ent->s_name, host.c_str()); + return GetSrvRecords(target, Result); +} + bool GetSrvRecords(std::string name, std::vector &Result) { unsigned char answer[PACKETSZ]; diff --git a/apt-pkg/contrib/srvrec.h b/apt-pkg/contrib/srvrec.h index 78d238c46..fd71e697f 100644 --- a/apt-pkg/contrib/srvrec.h +++ b/apt-pkg/contrib/srvrec.h @@ -26,6 +26,12 @@ class SrvRec } }; +/** \brief Get SRV records from host/port (builds the query string internally) + */ bool GetSrvRecords(std::string name, std::vector &Result); +/** \brief Get SRV records for query string like: _http._tcp.example.com + */ +bool GetSrvRecords(std::string host, int port, std::vector &Result); + #endif diff --git a/methods/connect.cc b/methods/connect.cc index e2cbf4f5c..a90bc8084 100644 --- a/methods/connect.cc +++ b/methods/connect.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -43,6 +44,9 @@ static int LastPort = 0; static struct addrinfo *LastHostAddr = 0; static struct addrinfo *LastUsed = 0; +static std::vector SrvRecords; +static int LastSrvRecord = 0; + // Set of IP/hostnames that we timed out before or couldn't resolve static std::set bad_addr; @@ -151,6 +155,15 @@ bool Connect(std::string Host,int Port,const char *Service,int DefPort,int &Fd, sensible */ if (LastHost != Host || LastPort != Port) { + // FIXME: NOT READY FOR MERGING IN THIS FORM + // we need to first check SRV, then round-robin DNS + // this code will only ever use the first srv record + + // FIXME: ensure we cycle over the SrvRecords first before + // we do round-robin IP + if(GetSrvRecords(Host, Port, SrvRecords) && SrvRecords.size() > 0) + Host = SrvRecords[0].target; + Owner->Status(_("Connecting to %s"),Host.c_str()); // Free the old address structure diff --git a/methods/makefile b/methods/makefile index 6b7781294..868c52a40 100644 --- a/methods/makefile +++ b/methods/makefile @@ -46,21 +46,21 @@ include $(PROGRAM_H) # The http method PROGRAM=http -SLIBS = -lapt-pkg $(SOCKETLIBS) $(INTLLIBS) +SLIBS = -lapt-pkg $(SOCKETLIBS) $(INTLLIBS) -lresolv LIB_MAKES = apt-pkg/makefile SOURCE = http.cc http_main.cc rfc2553emu.cc connect.cc server.cc include $(PROGRAM_H) # The https method PROGRAM=https -SLIBS = -lapt-pkg -lcurl $(INTLLIBS) +SLIBS = -lapt-pkg -lcurl $(INTLLIBS) -lresolv LIB_MAKES = apt-pkg/makefile SOURCE = https.cc server.cc include $(PROGRAM_H) # The ftp method PROGRAM=ftp -SLIBS = -lapt-pkg $(SOCKETLIBS) $(INTLLIBS) +SLIBS = -lapt-pkg $(SOCKETLIBS) $(INTLLIBS) -lresolv LIB_MAKES = apt-pkg/makefile SOURCE = ftp.cc rfc2553emu.cc connect.cc include $(PROGRAM_H) @@ -81,7 +81,7 @@ include $(PROGRAM_H) # The mirror method PROGRAM=mirror -SLIBS = -lapt-pkg $(SOCKETLIBS) +SLIBS = -lapt-pkg $(SOCKETLIBS) -lresolv LIB_MAKES = apt-pkg/makefile SOURCE = mirror.cc http.cc rfc2553emu.cc connect.cc server.cc include $(PROGRAM_H) -- cgit v1.2.3-70-g09d2 From cc4800145b408de0b4afef88f4489a541024e75a Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 23 May 2014 16:46:32 +0200 Subject: when using srv records, use the next server if one fails to connect --- apt-pkg/makefile | 2 +- methods/connect.cc | 57 ++++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/apt-pkg/makefile b/apt-pkg/makefile index 5603b51ed..45dcddfe3 100644 --- a/apt-pkg/makefile +++ b/apt-pkg/makefile @@ -15,7 +15,7 @@ include ../buildlib/libversion.mak LIBRARY=apt-pkg MAJOR=$(LIBAPTPKG_MAJOR) MINOR=$(LIBAPTPKG_RELEASE) -SLIBS=$(PTHREADLIB) $(INTLLIBS) -lutil -ldl +SLIBS=$(PTHREADLIB) $(INTLLIBS) -lutil -ldl -lresolv ifeq ($(HAVE_ZLIB),yes) SLIBS+= -lz endif diff --git a/methods/connect.cc b/methods/connect.cc index a90bc8084..8de4ad747 100644 --- a/methods/connect.cc +++ b/methods/connect.cc @@ -134,15 +134,12 @@ static bool DoConnect(struct addrinfo *Addr,std::string Host, return true; } /*}}}*/ -// Connect - Connect to a server /*{{{*/ -// --------------------------------------------------------------------- -/* Performs a connection to the server */ -bool Connect(std::string Host,int Port,const char *Service,int DefPort,int &Fd, - unsigned long TimeOut,pkgAcqMethod *Owner) -{ - if (_error->PendingError() == true) - return false; +// Connect to a given Hostname +bool ConnectAfterSrvRecords(std::string Host,int Port,const char *Service, + int DefPort,int &Fd, + unsigned long TimeOut,pkgAcqMethod *Owner) +{ // Convert the port name/number char ServStr[300]; if (Port != 0) @@ -155,15 +152,6 @@ bool Connect(std::string Host,int Port,const char *Service,int DefPort,int &Fd, sensible */ if (LastHost != Host || LastPort != Port) { - // FIXME: NOT READY FOR MERGING IN THIS FORM - // we need to first check SRV, then round-robin DNS - // this code will only ever use the first srv record - - // FIXME: ensure we cycle over the SrvRecords first before - // we do round-robin IP - if(GetSrvRecords(Host, Port, SrvRecords) && SrvRecords.size() > 0) - Host = SrvRecords[0].target; - Owner->Status(_("Connecting to %s"),Host.c_str()); // Free the old address structure @@ -271,3 +259,38 @@ bool Connect(std::string Host,int Port,const char *Service,int DefPort,int &Fd, return _error->Error(_("Unable to connect to %s:%s:"),Host.c_str(),ServStr); } /*}}}*/ +// Connect - Connect to a server /*{{{*/ +// --------------------------------------------------------------------- +/* Performs a connection to the server */ +bool Connect(std::string Host,int Port,const char *Service, + int DefPort,int &Fd, + unsigned long TimeOut,pkgAcqMethod *Owner) +{ +#if 0 + if (_error->PendingError() == true) + return false; +#endif + + if(LastHost != Host || LastPort != Port) + { + SrvRecords.clear(); + bool res = GetSrvRecords(Host, DefPort, SrvRecords); + } + if(SrvRecords.size() == 0) + return ConnectAfterSrvRecords(Host, Port, Service, DefPort, Fd, + TimeOut, Owner); + + bool connected = false; + while(SrvRecords.size() > 0) + { + Host = SrvRecords[0].target; + connected = ConnectAfterSrvRecords(Host, Port, Service, DefPort, Fd, + TimeOut, Owner); + if(connected == true) + return true; + + // we couldn't connect to this one, use the next + SrvRecords.erase(SrvRecords.begin()); + } + return false; +} -- cgit v1.2.3-70-g09d2 From e5f34ad3b043abf033c1626eb8449b75955d6760 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 23 May 2014 17:24:17 +0200 Subject: WIP start randomizing --- apt-pkg/contrib/srvrec.cc | 31 +++++++++++++++++++++++++++++-- apt-pkg/contrib/srvrec.h | 5 +++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/apt-pkg/contrib/srvrec.cc b/apt-pkg/contrib/srvrec.cc index c473192fc..70247c2ae 100644 --- a/apt-pkg/contrib/srvrec.cc +++ b/apt-pkg/contrib/srvrec.cc @@ -105,10 +105,37 @@ bool GetSrvRecords(std::string name, std::vector &Result) Result.push_back(rec); } - // sort them + // implement load balancing as specified in RFC-2782 + + // sort them by priority std::stable_sort(Result.begin(), Result.end()); - // FIXME: implement weight selection as specified in RFC-2782 + // assign random number ranges + int prev_weight = 0; + int prev_priority = 0; + for(std::vector::iterator I = Result.begin(); + I != Result.end(); ++I) + { + if(prev_priority != I->priority) + prev_weight = 0; + I->random_number_range_start = prev_weight; + I->random_number_range_end = prev_weight + I->weight; + prev_weight = I->random_number_range_end; + prev_priority = I->priority; + } + + // go over the code in reverse order and note the max random range + int max = 0; + prev_priority = 0; + for(std::vector::iterator I = Result.end(); + I != Result.begin(); --I) + { + if(prev_priority != I->priority) + max = I->random_number_range_end; + I->random_number_range_max = max; + } + + // now shuffle return true; } diff --git a/apt-pkg/contrib/srvrec.h b/apt-pkg/contrib/srvrec.h index fd71e697f..79b318b48 100644 --- a/apt-pkg/contrib/srvrec.h +++ b/apt-pkg/contrib/srvrec.h @@ -21,6 +21,11 @@ class SrvRec u_int16_t weight; u_int16_t port; + // each server is assigned a interval [start, end] in the space of [0, max] + int random_number_range_start; + int random_number_range_end; + int random_number_range_max; + bool operator<(SrvRec const &other) const { return this->priority < other.priority; } -- cgit v1.2.3-70-g09d2 From cdeb54d4626ddc841d8898a8283084a8de3b25ee Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 18 Aug 2015 15:41:02 +0200 Subject: cleanup --- apt-pkg/contrib/srvrec.cc | 10 +++++----- apt-pkg/contrib/srvrec.h | 2 +- cmdline/apt-helper.cc | 9 ++++++--- methods/connect.cc | 18 ++++++++---------- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/apt-pkg/contrib/srvrec.cc b/apt-pkg/contrib/srvrec.cc index 70247c2ae..1dcda8b54 100644 --- a/apt-pkg/contrib/srvrec.cc +++ b/apt-pkg/contrib/srvrec.cc @@ -64,7 +64,7 @@ bool GetSrvRecords(std::string name, std::vector &Result) SrvRec rec; u_int16_t type, klass, priority, weight, port, dlen; char buf[MAXDNAME]; - + compressed_name_len = dn_skipname(pt, answer+answer_len); if (compressed_name_len < 0) return _error->Warning("dn_skipname failed (2): %i", @@ -74,15 +74,15 @@ bool GetSrvRecords(std::string name, std::vector &Result) return _error->Warning("packet too short"); // extract the data out of the result buffer - #define extract_u16(target, p) target = *p++ << 8; target |= *p++; + #define extract_u16(target, p) target = *p++ << 8; target |= *p++; extract_u16(type, pt); if(type != T_SRV) - return _error->Warning("Unexpected type excepted %x != %x", + return _error->Warning("Unexpected type excepted %x != %x", T_SRV, type); extract_u16(klass, pt); if(klass != C_IN) - return _error->Warning("Unexpected class excepted %x != %x", + return _error->Warning("Unexpected class excepted %x != %x", C_IN, klass); pt += 4; // ttl extract_u16(dlen, pt); @@ -135,7 +135,7 @@ bool GetSrvRecords(std::string name, std::vector &Result) I->random_number_range_max = max; } - // now shuffle + // FIXME: now shuffle return true; } diff --git a/apt-pkg/contrib/srvrec.h b/apt-pkg/contrib/srvrec.h index 79b318b48..9323623a5 100644 --- a/apt-pkg/contrib/srvrec.h +++ b/apt-pkg/contrib/srvrec.h @@ -3,7 +3,7 @@ /* ###################################################################### SRV record support - + ##################################################################### */ /*}}}*/ #ifndef SRVREC_H diff --git a/cmdline/apt-helper.cc b/cmdline/apt-helper.cc index 482e64dd1..d235ac315 100644 --- a/cmdline/apt-helper.cc +++ b/cmdline/apt-helper.cc @@ -86,8 +86,9 @@ static bool DoSrvLookup(CommandLine &CmdL) { if (CmdL.FileSize() < 1) return _error->Error(_("Must specifc at least one srv record")); - + std::vector srv_records; + c1out << "# target priority weight port" << std::endl; for(int i=1; CmdL.FileList[i] != NULL; i++) { if(GetSrvRecords(CmdL.FileList[i], srv_records) == false) @@ -95,13 +96,14 @@ static bool DoSrvLookup(CommandLine &CmdL) for (std::vector::const_iterator I = srv_records.begin(); I != srv_records.end(); ++I) { - c1out << (*I).target.c_str() << " " - << (*I).priority << " " + c1out << (*I).target.c_str() << " " + << (*I).priority << " " << (*I).weight << " " << (*I).port << " " << std::endl; } } + return true; } @@ -120,6 +122,7 @@ static bool ShowHelp(CommandLine &) "\n" "Commands:\n" " download-file - download the given uri to the target-path\n" + " srv-lookup - lookup a SRV record (e.g. _http._tcp.ftp.debian.org)\n" " auto-detect-proxy - detect proxy using apt.conf\n" "\n" " This APT helper has Super Meep Powers.\n"); diff --git a/methods/connect.cc b/methods/connect.cc index 8de4ad747..9e0f01ee3 100644 --- a/methods/connect.cc +++ b/methods/connect.cc @@ -136,7 +136,7 @@ static bool DoConnect(struct addrinfo *Addr,std::string Host, /*}}}*/ // Connect to a given Hostname -bool ConnectAfterSrvRecords(std::string Host,int Port,const char *Service, +bool ConnectToHostname(std::string Host,int Port,const char *Service, int DefPort,int &Fd, unsigned long TimeOut,pkgAcqMethod *Owner) { @@ -261,36 +261,34 @@ bool ConnectAfterSrvRecords(std::string Host,int Port,const char *Service, /*}}}*/ // Connect - Connect to a server /*{{{*/ // --------------------------------------------------------------------- -/* Performs a connection to the server */ +/* Performs a connection to the server (including SRV record lookup) */ bool Connect(std::string Host,int Port,const char *Service, int DefPort,int &Fd, unsigned long TimeOut,pkgAcqMethod *Owner) { -#if 0 if (_error->PendingError() == true) return false; -#endif if(LastHost != Host || LastPort != Port) { SrvRecords.clear(); bool res = GetSrvRecords(Host, DefPort, SrvRecords); } + // we have no SrvRecords for this host, connect right away if(SrvRecords.size() == 0) - return ConnectAfterSrvRecords(Host, Port, Service, DefPort, Fd, + return ConnectToHostname(Host, Port, Service, DefPort, Fd, TimeOut, Owner); - bool connected = false; + // try to connect in the priority order of the srv records while(SrvRecords.size() > 0) { Host = SrvRecords[0].target; - connected = ConnectAfterSrvRecords(Host, Port, Service, DefPort, Fd, - TimeOut, Owner); - if(connected == true) + if(ConnectToHostname(Host, Port, Service, DefPort, Fd, TimeOut, Owner)) return true; // we couldn't connect to this one, use the next SrvRecords.erase(SrvRecords.begin()); - } + } + return false; } -- cgit v1.2.3-70-g09d2 From 9b70edba6796ebff3935af1cfb5c9bbc98d020b4 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 18 Aug 2015 16:46:34 +0200 Subject: apt-pkg/contrib/srvrec.cc: res_query() should not generate a _error->Warning() --- apt-pkg/contrib/srvrec.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apt-pkg/contrib/srvrec.cc b/apt-pkg/contrib/srvrec.cc index 1dcda8b54..85241c1d7 100644 --- a/apt-pkg/contrib/srvrec.cc +++ b/apt-pkg/contrib/srvrec.cc @@ -41,6 +41,8 @@ bool GetSrvRecords(std::string name, std::vector &Result) return _error->Errno("res_init", "Failed to init resolver"); answer_len = res_query(name.c_str(), C_IN, T_SRV, answer, sizeof(answer)); + if (answer_len == -1) + return false; if (answer_len < (int)sizeof(HEADER)) return _error->Warning("Not enough data from res_query (%i)", answer_len); -- cgit v1.2.3-70-g09d2 From c29dbdffcb6f67812f823f1f844b87320cf6b437 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 20 Aug 2015 10:40:45 +0200 Subject: Add basic (non weight adjusted) shuffling for SrvRecords selection Also add "Debug::Acquire::SrvRecs" debug option and the option "Acquire::EnableSrvRecods" to allow disabling this lookup. --- apt-pkg/contrib/srvrec.cc | 59 ++++++++++++++++++++++++++++++++++++++++++--- apt-pkg/contrib/srvrec.h | 5 ++++ methods/connect.cc | 5 ++-- test/libapt/srvrecs_test.cc | 33 +++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 test/libapt/srvrecs_test.cc diff --git a/apt-pkg/contrib/srvrec.cc b/apt-pkg/contrib/srvrec.cc index 85241c1d7..b4a3d97d2 100644 --- a/apt-pkg/contrib/srvrec.cc +++ b/apt-pkg/contrib/srvrec.cc @@ -13,13 +13,18 @@ #include #include #include +#include #include -#include +#include #include +#include + + #include "srvrec.h" + bool GetSrvRecords(std::string host, int port, std::vector &Result) { std::string target; @@ -112,6 +117,29 @@ bool GetSrvRecords(std::string name, std::vector &Result) // sort them by priority std::stable_sort(Result.begin(), Result.end()); + for(std::vector::iterator I = Result.begin(); + I != Result.end(); ++I) + { + if (_config->FindB("Debug::Acquire::SrvRecs", false) == true) + { + std::cerr << "SrvRecs: got " << I->target + << " prio: " << I->priority + << " weight: " << I->weight + << std::endl; + } + } + + return true; +} + +SrvRec PopFromSrvRecs(std::vector &Recs) +{ + // FIXME: instead of the simplistic shuffle below use the algorithm + // described in rfc2782 (with weights) + // and figure out how the weights need to be adjusted if + // a host refuses connections + +#if 0 // all code below is only needed for the weight adjusted selection // assign random number ranges int prev_weight = 0; int prev_priority = 0; @@ -124,6 +152,12 @@ bool GetSrvRecords(std::string name, std::vector &Result) I->random_number_range_end = prev_weight + I->weight; prev_weight = I->random_number_range_end; prev_priority = I->priority; + + if (_config->FindB("Debug::Acquire::SrvRecs", false) == true) + std::cerr << "SrvRecs: got " << I->target + << " prio: " << I->priority + << " weight: " << I->weight + << std::endl; } // go over the code in reverse order and note the max random range @@ -136,8 +170,27 @@ bool GetSrvRecords(std::string name, std::vector &Result) max = I->random_number_range_end; I->random_number_range_max = max; } +#endif - // FIXME: now shuffle + // shuffle in a very simplistic way for now (equal weights) + std::vector::iterator I, J; + I = J = Recs.begin(); + for(;I != Recs.end(); ++I) + { + if(I->priority != J->priority) + break; + } - return true; + // FIXME: meeeeh, where to init this properly + unsigned seed = std::chrono::system_clock::now().time_since_epoch().count(); + std::shuffle(J, I, std::default_random_engine(seed)); + + // meh, no pop_front() in std::vector? + SrvRec selected = *Recs.begin(); + Recs.erase(Recs.begin()); + + if (_config->FindB("Debug::Acquire::SrvRecs", false) == true) + std::cerr << "PopFromSrvRecs: selecting " << selected.target << std::endl; + + return selected; } diff --git a/apt-pkg/contrib/srvrec.h b/apt-pkg/contrib/srvrec.h index 9323623a5..e07edc683 100644 --- a/apt-pkg/contrib/srvrec.h +++ b/apt-pkg/contrib/srvrec.h @@ -39,4 +39,9 @@ bool GetSrvRecords(std::string name, std::vector &Result); */ bool GetSrvRecords(std::string host, int port, std::vector &Result); +/** \brief Pop a single SRV record from the vector of SrvRec taking + * priority and weight into account + */ +SrvRec PopFromSrvRecs(std::vector &Recs); + #endif diff --git a/methods/connect.cc b/methods/connect.cc index 9e0f01ee3..5612af6ec 100644 --- a/methods/connect.cc +++ b/methods/connect.cc @@ -272,7 +272,8 @@ bool Connect(std::string Host,int Port,const char *Service, if(LastHost != Host || LastPort != Port) { SrvRecords.clear(); - bool res = GetSrvRecords(Host, DefPort, SrvRecords); + if (_config->FindB("Acquire::EnableSrvRecods", true) == true) + GetSrvRecords(Host, DefPort, SrvRecords); } // we have no SrvRecords for this host, connect right away if(SrvRecords.size() == 0) @@ -282,7 +283,7 @@ bool Connect(std::string Host,int Port,const char *Service, // try to connect in the priority order of the srv records while(SrvRecords.size() > 0) { - Host = SrvRecords[0].target; + Host = PopFromSrvRecs(SrvRecords).target; if(ConnectToHostname(Host, Port, Service, DefPort, Fd, TimeOut, Owner)) return true; diff --git a/test/libapt/srvrecs_test.cc b/test/libapt/srvrecs_test.cc new file mode 100644 index 000000000..7e43cc757 --- /dev/null +++ b/test/libapt/srvrecs_test.cc @@ -0,0 +1,33 @@ +#include + +#include + +#include +#include + +#include + +TEST(SrvRecTest, PopFromSrvRecs) +{ + // the PopFromSrvRecs() is using a random number so we must + // run it a bunch of times to ensure we are not fooled by randomness + std::set selected; + for(int i=0;i<100;i++) + { + std::vector Meep; + SrvRec foo = {target:"foo", priority: 20, weight: 0, port: 80}; + Meep.push_back(foo); + + SrvRec bar = {target:"bar", priority: 20, weight: 0, port: 80}; + Meep.push_back(bar); + + EXPECT_EQ(Meep.size(), 2); + SrvRec result = PopFromSrvRecs(Meep); + selected.insert(result.target); + // ensure that pop removed one element + EXPECT_EQ(Meep.size(), 1); + } + + // ensure that after enough runs we end up with both selected + EXPECT_EQ(selected.size(), 2); +} -- cgit v1.2.3-70-g09d2