From dc2968253216141c9027b7b88631a7de0a885d7e Mon Sep 17 00:00:00 2001 From: Chris Tallon Date: Mon, 13 Sep 2021 15:15:13 +0100 Subject: [PATCH] Rewrite VConnect server selection logic. More things to config --- config.cc | 8 ++ config.h | 1 + config.json.sample | 10 +- control.cc | 2 +- inputman.h | 2 +- main.cc | 9 +- udp6.cc | 3 + udp6.h | 2 + vconnect.cc | 237 ++++++++++++++++++++++++++++----------------- vconnect.h | 7 +- vdpc.cc | 63 ++++++------ vdpc.h | 26 +++-- vdr.cc | 22 ++--- vdr.h | 4 +- 14 files changed, 243 insertions(+), 153 deletions(-) diff --git a/config.cc b/config.cc index d58601a..f187910 100644 --- a/config.cc +++ b/config.cc @@ -32,6 +32,7 @@ void Config::applyDefaults() // Insert the value only if it doesn't already exist auto insertBool = [&] (const char* s, const char* k, bool v) { if (jconfig[s][k].isNull()) jconfig[s][k] = v; }; auto insertString = [&] (const char* s, const char* k, const char* v) { if (jconfig[s][k].isNull()) jconfig[s][k] = v; }; + auto insertInt = [&] (const char* s, const char* k, int v) { if (jconfig[s][k].isNull()) jconfig[s][k] = v; }; insertBool("main", "daemonize", true); @@ -47,6 +48,8 @@ void Config::applyDefaults() insertBool("input", "mod_udp_enabled", true); insertBool("input", "mod_lirc_enabled", false); + + insertInt("server-discovery", "prefer-ipv", 6); } bool Config::loadFile() @@ -154,3 +157,8 @@ void Config::set(const std::string& section, const std::string& key, const char* { jconfig[section][key] = value; } + +void Config::set(const std::string& section, const std::string& key, int value) +{ + jconfig[section][key] = value; +} diff --git a/config.h b/config.h index 0124b5b..18a519f 100644 --- a/config.h +++ b/config.h @@ -43,6 +43,7 @@ class Config void set(const std::string& section, const std::string& key, const std::string& value); void set(const std::string& section, const std::string& key, bool value); void set(const std::string& section, const std::string& key, const char* value); + void set(const std::string& section, const std::string& key, int value); private: static Config* instance; diff --git a/config.json.sample b/config.json.sample index 6148411..3b97e7f 100644 --- a/config.json.sample +++ b/config.json.sample @@ -8,6 +8,7 @@ settings you want to change. Place the file in the current working directory for vompclient. + Command line options override config.json. */ { @@ -31,7 +32,14 @@ "address": "vdrserver.example.com", "port": 3024 - } + }, + + "server-discovery": + { + // If exactly two servers are found by auto server location and they have the same + // name and port but different IP versions, prefer which? + "prefer-ipv": 6 + }, "input": { diff --git a/control.cc b/control.cc index 434e1f5..2f747a5 100644 --- a/control.cc +++ b/control.cc @@ -726,7 +726,7 @@ void Control::doFromTheTop(bool which) // at this point, everything should be reset to first-go - VConnect* vconnect = new VConnect(); // FIXME never deleted? + VConnect* vconnect = new VConnect(); // deleted eventually in boxstack boxstack->add(vconnect); vconnect->run(); } diff --git a/inputman.h b/inputman.h index cc08427..923a241 100644 --- a/inputman.h +++ b/inputman.h @@ -58,7 +58,7 @@ only uses what is present at vomp-startup). The CEC code can be removed from InputLinux. Under the new design they are nothing to do with each other. -Direct Lirc input can be brought back - probably very easily. TODO +Direct Lirc input is now brought back, via TCP or unix domain socket. Input objects can be easily and individually enabled / disabled. diff --git a/main.cc b/main.cc index af2dae8..9a0f1a8 100644 --- a/main.cc +++ b/main.cc @@ -22,6 +22,8 @@ #include #include +#include + // For open #include #include @@ -86,7 +88,7 @@ int main(int argc, char** argv) } int c; - while ((c = getopt(argc, argv, "clf:ns:")) != -1) + while ((c = getopt(argc, argv, "hclf:ns:p:")) != -1) { switch (c) { @@ -105,13 +107,18 @@ int main(int argc, char** argv) case 's': config->set("server", "address", optarg); break; + case 'p': + config->set("server", "port", std::atoi(optarg)); + break; case '?': + case 'h': printf("Vompclient\n\n"); printf("Usage:\n"); printf(" -l Enable logging\n"); printf(" -f F Log to file F instead of stdout\n"); printf(" -n Disable daemonize\n"); printf(" -s S Connect to server S\n"); + printf(" -p P .. at port P\n"); return 1; default: printf("Option error\n"); diff --git a/udp6.cc b/udp6.cc index b2f9fdc..1edc895 100644 --- a/udp6.cc +++ b/udp6.cc @@ -22,6 +22,7 @@ #include #include #include +#include #define SOCKET_ERROR 0 #include #else @@ -149,6 +150,7 @@ unsigned char UDP6::waitforMessage(unsigned char how, int quitPipe) memset(&buf[mlength], 0, MAXBUFLEN - mlength); inet_ntop(AF_INET6, &theirAddr.sin6_addr, fromIPA, 40); fromPort = ntohs(theirAddr.sin6_port); + fromIPisLL = IN6_IS_ADDR_LINKLOCAL(&theirAddr.sin6_addr); return 2; } @@ -166,6 +168,7 @@ UINT UDP6::getDataLength(void) const const void* UDP6::getData() const { return buf; } const char* UDP6::getFromIPA() const { return fromIPA; } short UDP6::getFromPort() const { return fromPort; } +bool UDP6::getFromIPisLL() const { return fromIPisLL; } bool UDP6::send(const char *ipa, USHORT port, char *message, int length, bool mcast) { diff --git a/udp6.h b/udp6.h index 7e111cf..9caad45 100644 --- a/udp6.h +++ b/udp6.h @@ -44,6 +44,7 @@ class UDP6 const void* getData() const; // returns a pointer to the data const char* getFromIPA() const; // returns a pointer to from IP address short getFromPort() const; + bool getFromIPisLL() const; bool send(const char *, USHORT, char *, int, bool mcast = false); // send wants: text IP, port, data, length of data, is_mcast private: @@ -58,6 +59,7 @@ class UDP6 char buf[MAXBUFLEN]; // main data buffer char fromIPA[40]; // from string (ip address) short fromPort; // which port user sent on + bool fromIPisLL; // packet came from a IPv6 link-local address int mlength; // length of message struct timeval tv; fd_set readfds; diff --git a/vconnect.cc b/vconnect.cc index 1528931..593a0c1 100644 --- a/vconnect.cc +++ b/vconnect.cc @@ -97,133 +97,197 @@ void VConnect::stop() connectThread.join(); } +void VConnect::processMessage(Message* m) +{ + if (m->message == Message::SERVER_SELECTED) + { + serverSelectResponse = m->parameter; + threadCond.notify_one(); + } +} + void VConnect::threadMethod() { logger->debug(TAG, "start threadMethod"); + Config* config = Config::getInstance(); - ULONG delay = 0; - int success; + // Try to get server and port from config. If successful then + // either a server &? port was supplied in config.json or + // overridden by command line args - std::unique_lock ul(threadMutex, std::defer_lock); - - VDRServer configServerStr; - const VDRServer* selectedServerStr = NULL; - - bool serverFromConfig{}; // FIXME Redesign all this. - - if (Config::getInstance()->getString("server", "address", configServerStr.ip)) + VDRServer serverFromConfig; + if (config->getString("server", "address", serverFromConfig.ip)) { - configServerStr.port = 3024; - int newPort; - if (Config::getInstance()->getInt("server", "port", newPort)) + logger->debug(TAG, "Server read from config: {}", serverFromConfig.ip); + serverFromConfig.port = 3024; + int newPort; // copy around because of type mismatch + if (config->getInt("server", "port", newPort)) { - configServerStr.port = newPort; + serverFromConfig.port = newPort; + logger->debug(TAG, "Port read fron config: {}", serverFromConfig.port); } - selectedServerStr = &configServerStr; - serverFromConfig = true; + while(1) // Only attempt connection to this server from config + { + if (threadReqQuit) return; + if (attemptConnect(&serverFromConfig)) return; + } } - else // No server specified in config. Start VDPC + + // No config server. Start VDPC + + setOneLiner(tr("Locating server")); + draw(); + boxstack->update(this); + + if (threadReqQuit) return; + + if (!vdpc.init()) + { + logger->crit(TAG, "Failed to init VDPC"); + return; + } + + while(1) { - if (!vdpc.init()) + if (threadReqQuit) return; + int numServers = vdpc.go(); + if (threadReqQuit) return; + + if (numServers == 0) { - logger->crit(TAG, "Failed to init VDPC"); + logger->crit(TAG, "VDPC returned 0 servers"); return; } - } + // Now we have > 0 servers found - do - { - if (!selectedServerStr) + for (ULONG i = 0; i < numServers; i++) + logger->info(TAG, "Found server: {} {} {} {}", vdpc[i].ipVersion, vdpc[i].ip.c_str(), vdpc[i].name.c_str(), vdpc[i].port, vdpc[i].version); + + if (numServers == 1) { - setOneLiner(tr("Locating server")); - draw(); - boxstack->update(this); - vdpc.go(); if (threadReqQuit) return; + if (attemptConnect(&vdpc[0])) return; // successful connect + continue; // restart discovery + } + + // Multiple servers found + + // Special case: If there are two servers returned and they only differ by IP version and IP, select one and connect automatically + + if ( (numServers == 2) + && !vdpc[0].name.compare(vdpc[1].name) // if the names are the same... + && (vdpc[0].port == vdpc[1].port) // and the port number is the same... + && (vdpc[0].ipVersion != vdpc[1].ipVersion) // and the IP versions DO NOT match... + /* and the IPs don't match - they can't if ipVersions are different */ + ) + { + int which = 6; + config->getInt("server-discovery", "prefer-ipv", which); + + logger->debug(TAG, "Auto select IPv{}", which); + + const VDRServer* selected; + if (vdpc[0].ipVersion == which) selected = &vdpc[0]; + else selected = &vdpc[1]; + + if (attemptConnect(selected)) return; // successful connect - for (ULONG i = 0; i < vdpc.numServers(); i++) - logger->info(TAG, "Found server: {} {} {} {}", vdpc[i].ipVersion, vdpc[i].ip.c_str(), vdpc[i].name.c_str(), vdpc[i].port, vdpc[i].version); + // Try the other IPv ?! + logger->warn(TAG, "Switching IPv"); - if (vdpc.numServers() == 1) - { - selectedServer = 0; - selectedServerStr = &vdpc[0]; - } - else - { - selectedServer = -1; - VServerSelect* vs = new VServerSelect(vdpc, this); // FIXME is this deleted? - vs->draw(); - boxstack->add(vs); - boxstack->update(vs); + if (vdpc[0].ipVersion == which) selected = &vdpc[1]; + else selected = &vdpc[0]; - ul.lock(); - if (threadReqQuit) { ul.unlock(); return; } - threadCond.wait(ul); + if (attemptConnect(selected)) return; // successful connect - // This thread has been notified by VServerSelect. selectedServer is now an index into vdpc of the chosen server + continue; // restart discovery + } - selectedServerStr = &vdpc[selectedServer]; + // Now numServers > 1 - ul.unlock(); - } + VServerSelect* vs = new VServerSelect(vdpc, this); // deleted by handleCommand returning 4 + vs->draw(); + boxstack->add(vs); + boxstack->update(vs); + { + std::unique_lock ul(threadMutex); if (threadReqQuit) return; + threadCond.wait(ul); + // This thread has been notified by VServerSelect. serverSelectResponse is now an index into vdpc of the chosen server } + if (threadReqQuit) return; + + if (attemptConnect(&vdpc[serverSelectResponse])) return; // successful connect + continue; // restart discovery + } +} + +bool VConnect::attemptConnect(const VDRServer* vdrServer) +{ + logger->info(TAG, "Attempting connect to {} {}", vdrServer->ip, vdrServer->port); - logger->info(TAG, "Connecting to server at {} {}", selectedServerStr->ip, selectedServerStr->port); - Wol::getInstance()->setWakeUpIP(selectedServerStr->ip.c_str()); - vdr->setServerIP(selectedServerStr->ip.c_str()); - vdr->setServerPort(selectedServerStr->port); + vdr->setServerIP(vdrServer->ip.c_str()); + vdr->setServerPort(vdrServer->port); - // In case of go around, clear selectedServerStr here if it was discovered - if (!serverFromConfig) selectedServerStr = NULL; + if (threadReqQuit) return false; + setOneLiner(tr("Connecting to VDR")); + draw(); + boxstack->update(this); - setOneLiner(tr("Connecting to VDR")); + bool connectSuccess = vdr->connect(); + if (threadReqQuit) + { + vdr->disconnect(); + return false; + } + + if (!connectSuccess) + { + setOneLiner(tr("Connection failed")); draw(); boxstack->update(this); + if (threadReqQuit) return false; + MILLISLEEP(3000); // FIXME wait on cond? + return false; + } - if (threadReqQuit) return; - success = vdr->connect(); - if (threadReqQuit) return; + logger->debug(TAG, "Connected ok, doing login"); + + // FIXME - what is all this? Can it be moved to NCONFIG? + unsigned int version_server_min, version_server_max, version_client; + int subtitles; + bool loginSuccess = vdr->doLogin(&version_server_min, &version_server_max, &version_client, Control::getInstance()->getASLList(), subtitles); - if (success) + if (!loginSuccess) + { + vdr->disconnect(); + if ((version_server_min > version_client) || (version_client > version_server_max)) { - logger->debug(TAG, "Connected ok, doing login"); - unsigned int version_server_min, version_server_max, version_client; - int subtitles; - success = vdr->doLogin(&version_server_min, &version_server_max, &version_client, Control::getInstance()->getASLList(), subtitles); - Control::getInstance()->setSubDefault(subtitles); - - if (!success) - { - vdr->disconnect(); - if (version_server_min >version_client || version_client > version_server_max) { - char buffer[1024]; - sprintf(buffer,"Version error: s min: %x s max: %x c: %x",version_server_min,version_server_max,version_client); - setOneLiner(buffer); - } else { - setOneLiner(tr("Login failed")); - } - delay = 3000; - } + char buffer[1024]; + sprintf(buffer, "Version error: s min: %x s max: %x c: %x", version_server_min, version_server_max, version_client); + setOneLiner(buffer); } else { - setOneLiner(tr("Connection failed")); - delay = 3000; + setOneLiner(tr("Login failed")); } draw(); boxstack->update(this); + if (threadReqQuit) return false; + MILLISLEEP(3000); // FIXME wait on cond? + return false; + } - MILLISLEEP(delay); // FIXME wait on cond? - if (threadReqQuit) return; - } while(!success); + logger->debug(TAG, "VDR login ok"); + + Control::getInstance()->setSubDefault(subtitles); + Wol::getInstance()->setWakeUpIP(vdrServer->ip.c_str()); logger->info(TAG, "Send VDR connected message"); Message* m = new Message(); // Must be done after this thread ends @@ -231,13 +295,6 @@ void VConnect::threadMethod() m->p_to = Message::CONTROL; m->message = Message::VDR_CONNECTED; MessageQueue::getInstance()->postMessage(m); -} -void VConnect::processMessage(Message* m) -{ - if (m->message == Message::SERVER_SELECTED) - { - selectedServer = m->parameter; - threadCond.notify_one(); - } + return true; } diff --git a/vconnect.h b/vconnect.h index 32970b7..dfa6b8c 100644 --- a/vconnect.h +++ b/vconnect.h @@ -30,6 +30,7 @@ class LogNT; class BoxStack; class Message; +class VDRServer; class VConnect : public VInfo { @@ -49,14 +50,16 @@ class VConnect : public VInfo VDPC vdpc; VDR* vdr; - int selectedServer; + int serverSelectResponse; std::thread connectThread; std::mutex threadMutex; std::condition_variable threadCond; - bool threadReqQuit; + bool threadReqQuit{}; void threadMethod(); void stop(); + + bool attemptConnect(const VDRServer*); }; #endif diff --git a/vdpc.cc b/vdpc.cc index 573b5ac..079fa67 100644 --- a/vdpc.cc +++ b/vdpc.cc @@ -39,35 +39,12 @@ bool VDPC::init() { std::lock_guard lg(serversLock); - if (dedupServers) + for (auto& s : servers) { - UINT i = 0; - for (; i < servers.size(); i++) - { - if (servers[i].name == name) break; - } - - if (i == servers.size()) - { - servers.emplace_back(ipVersion, ip, name, port, version); - } - else - { - if (ipVersion == preferIPV) // delete the other - { - servers[i].ipVersion = ipVersion; - servers[i].ip = ip; - servers[i].name = name; - servers[i].port = port; - servers[i].version = version; - } - } - - } - else - { - servers.emplace_back(ipVersion, ip, name, port, version); + if (!s.ip.compare(ip) && (port == s.port)) return; } + + servers.emplace_back(ipVersion, ip, name, port, version); }); #ifdef IPV4 @@ -119,7 +96,7 @@ int VDPC::go() vdpc6.stop(); #endif - std::sort(servers.begin(), servers.end(), ServerSorter(preferIPV)); + std::sort(servers.begin(), servers.end(), ServerSorter()); return servers.size(); } @@ -331,20 +308,34 @@ void VDPC::VDPC6::threadMethod() const char* vdpreply = static_cast(udp6.getData()); if ((udp6.getDataLength() >= 24) && !strncmp(vdpreply, "VDP-0002", 8)) { - // FIXME upgrade this to look for a return IP in the reply packet + if (udp6.getFromIPisLL()) + { + // Reject responses from link local addresses + // To use them the incoming interface name would need to be kept for use in the tcp connect + // This would be possible but there doesn't seem to be any point, there + // are a few spare real or ULA IPv6 addresses left + // Though it would be cool for it to just-work on an unconfigured IPv6 system... + // FIXME one day + + logger->debug("VDPC6", "Rejecting response from a link-local address"); + } + else + { + // FIXME upgrade this to look for a return IP in the reply packet - USHORT newServerPort; - memcpy(&newServerPort, &vdpreply[26], 2); + USHORT newServerPort; + memcpy(&newServerPort, &vdpreply[26], 2); - ULONG newServerVersion; - memcpy(&newServerVersion, &vdpreply[28], 4); + ULONG newServerVersion; + memcpy(&newServerVersion, &vdpreply[28], 4); - // FIXME - packet length > 24 not checked, end NULL not checked!! - addFunc(6, udp6.getFromIPA(), &vdpreply[32], ntohs(newServerPort), ntohl(newServerVersion)); + // FIXME - packet length > 24 not checked, end NULL not checked!! + addFunc(6, udp6.getFromIPA(), &vdpreply[32], ntohs(newServerPort), ntohl(newServerVersion)); + } } } - logger->debug("VDPC6", "loop stopnow = %i", stopNow); + logger->debug("VDPC6", "loop stopnow = {}", stopNow); } } diff --git a/vdpc.h b/vdpc.h index 119ece9..aad164c 100644 --- a/vdpc.h +++ b/vdpc.h @@ -59,16 +59,30 @@ struct VDRServer class ServerSorter { public: - ServerSorter(int tipPreferred) : ipPreferred(tipPreferred) {} + ServerSorter() {} bool operator() (const VDRServer& a, const VDRServer& b) { + // First sort by name if (a.name == b.name) - return a.ipVersion == ipPreferred; + { + // Then by IP version + if (a.ipVersion == b.ipVersion) + { + // Then by IP + if (a.ip == b.ip) + { + // Then by port + return a.port < b.port; + } + else + return a.ip < b.ip; // yeah, weird + } + else + return a.ipVersion < b.ipVersion; + } else return a.name < b.name; } - private: - int ipPreferred; }; class VDPC @@ -140,10 +154,6 @@ class VDPC std::mutex waitMutex; bool stopNow{}; - // FIXME retrieve from new config system NCONFIG - bool dedupServers{true}; - int preferIPV{4}; - #ifdef IPV4 VDPC4 vdpc4; #endif diff --git a/vdr.cc b/vdr.cc index 89bbc9a..d8af43b 100644 --- a/vdr.cc +++ b/vdr.cc @@ -164,13 +164,13 @@ void VDR::setServerPort(USHORT newPort) } -int VDR::connect() +bool VDR::connect() { maxChannelNumber = 0; channelNumberWidth = 1; connectStateMutex.lock(); - if (connecting || connected) return 0; + if (connecting || connected) return true; connecting = true; connectStateMutex.unlock(); // now I have connecting @@ -184,7 +184,7 @@ int VDR::connect() if (connectResult) tcp.shutdown(); babortConnect = false; connectStateMutex.unlock(); - return 0; + return false; } if (connectResult) @@ -201,12 +201,12 @@ int VDR::connect() threadStartProtect.unlock(); connectStateMutex.unlock(); - return 1; + return true; } else { connectStateMutex.unlock(); - return 0; + return false; } } @@ -631,17 +631,17 @@ void VDR_PacketReceiver::call(void* userTag, bool& r_deregisterEDR, bool& r_wake ///////////////////////////////////////////////////////////////////////////// -int VDR::doLogin(unsigned int* v_server_min, unsigned int* v_server_max, unsigned int* v_client, +bool VDR::doLogin(unsigned int* v_server_min, unsigned int* v_server_max, unsigned int* v_client, ASLPrefList& list, int &subtitles) { VDR_RequestPacket vrp; - if (!vrp.init(VDR_LOGIN, true, 6)) return 0; + if (!vrp.init(VDR_LOGIN, true, 6)) return false; MACAddress myMAC = tcp.getMAC(); - if (!vrp.copyin(reinterpret_cast(&myMAC), 6)) return 0; + if (!vrp.copyin(reinterpret_cast(&myMAC), 6)) return false; VDR_ResponsePacket* vresp = RequestResponse(&vrp); - if (vresp->noResponse()) { delete vresp; return 0; } + if (vresp->noResponse()) { delete vresp; return false; } ULONG vdrTime = vresp->extractULONG(); logger->debug(TAG, "vdrtime = {}", vdrTime); @@ -675,7 +675,7 @@ int VDR::doLogin(unsigned int* v_server_min, unsigned int* v_server_max, unsigne if ((version_min > VOMP_PROTOCOL_VERSION) || (version_max < VOMP_PROTOCOL_VERSION) ) { - return 0; + return false; } @@ -717,7 +717,7 @@ int VDR::doLogin(unsigned int* v_server_min, unsigned int* v_server_max, unsigne setCharset(Osd::getInstance()->charSet()); - return 1; + return true; } bool VDR::LogExtern(const char* logString) diff --git a/vdr.h b/vdr.h index 363edc1..d4befe1 100644 --- a/vdr.h +++ b/vdr.h @@ -127,7 +127,7 @@ public ExternLogger void setServerIP(const char*); void setServerPort(USHORT); void setReceiveWindow(size_t size); - int connect(); + bool connect(); void disconnect(); void abortConnect(); // If there is one, force a running connect call to abort bool isConnected() { return connected; } @@ -154,7 +154,7 @@ public ExternLogger // configSave // setEventTimer - int doLogin(unsigned int* v_server_min, unsigned int* v_server_max, unsigned int* v_client, ASLPrefList& list, int &subtitles); + bool doLogin(unsigned int* v_server_min, unsigned int* v_server_max, unsigned int* v_client, ASLPrefList& list, int &subtitles); bool getRecordingsList(RecMan* recman); RecInfo* getRecInfo(char* fileName); int deleteRecording(char* fileName); -- 2.39.5