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 --- methods/connect.cc | 13 +++++++++++++ methods/makefile | 8 ++++---- 2 files changed, 17 insertions(+), 4 deletions(-) (limited to 'methods') 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(-) (limited to 'methods') 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 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(-) (limited to 'methods') 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 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 (limited to 'methods') 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