From 7831871b54c752142143c35e6ac95aa0d956782e Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Fri, 22 Mar 2024 10:46:22 +0100 Subject: [PATCH 01/51] Refs #20629: Spelling Signed-off-by: cferreiragonz --- .../attributes/RTPSParticipantAttributes.hpp | 2 +- .../rtps/attributes/ServerAttributes.hpp | 6 +- .../discovery/database/DiscoveryDataBase.cpp | 38 ++++---- .../discovery/database/DiscoveryDataBase.hpp | 14 +-- .../discovery/participant/PDPClient.cpp | 91 +++++++++---------- .../discovery/participant/PDPServer.cpp | 66 +++++++------- .../participant/PDPServerListener.cpp | 2 +- src/cpp/rtps/writer/StatefulWriter.cpp | 6 +- 8 files changed, 112 insertions(+), 113 deletions(-) diff --git a/include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp b/include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp index 665297f6ed7..e534f505c7d 100644 --- a/include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp +++ b/include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp @@ -276,7 +276,7 @@ class DiscoverySettings */ Duration_t discoveryServer_client_syncperiod = { 0, 450 * 1000000}; // 450 milliseconds - //! Discovery Server settings, only needed if use_CLIENT_DiscoveryProtocol=true + //! Discovery Server initial connections, needed if `discoveryProtocol` = CLIENT | SUPER_CLIENT | SERVER | BACKUP eprosima::fastdds::rtps::RemoteServerList_t m_DiscoveryServers; //! Filtering participants out depending on location diff --git a/include/fastdds/rtps/attributes/ServerAttributes.hpp b/include/fastdds/rtps/attributes/ServerAttributes.hpp index fe1b55e321b..7f0bdec3998 100644 --- a/include/fastdds/rtps/attributes/ServerAttributes.hpp +++ b/include/fastdds/rtps/attributes/ServerAttributes.hpp @@ -151,9 +151,9 @@ constexpr uint16_t DEFAULT_TCP_SERVER_PORT = 42100; * 1. The server's GUID prefix must be compliant with the schema * "44.53..5f.45.50.52.4f.53.49.4d.41", which is the schema followed by the prefixes generated * when creating server using fastdds cli, being DEFAULT_ROS2_SERVER_GUIDPREFIX the prefix for ID=0. - * 1. The discovery protocol must be either SIMPLE or SERVER. - * 1. In the case of SIMPLE, the participant is created as a CLIENT instead. - * 1. In the case of SERVER, the participant is created as a SERVER, using the DEFAULT_ROS2_MASTER_URI list to + * 2. The discovery protocol must be either SIMPLE or SERVER. + * a. In the case of SIMPLE, the participant is created as a CLIENT instead. + * b. In the case of SERVER, the participant is created as a SERVER, using the DEFAULT_ROS2_MASTER_URI list to * expand the list of remote servers. */ const char* const DEFAULT_ROS2_MASTER_URI = "ROS_DISCOVERY_SERVER"; diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index 84c29edf267..ab08c3ea0e0 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -180,7 +180,7 @@ bool DiscoveryDataBase::pdp_is_relevant( // Lock(shared mode) mutex locally std::lock_guard guard(mutex_); - EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "PDP is " << change.instanceHandle << " relevant to " << reader_guid); + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "PDP " << change.instanceHandle << " is relevant to " << reader_guid); auto it = participants_.find(change_guid_prefix); if (it != participants_.end()) @@ -596,9 +596,9 @@ void DiscoveryDataBase::create_participant_from_change_( void DiscoveryDataBase::match_new_server_( eprosima::fastdds::rtps::GuidPrefix_t& participant_prefix) { - // Send Our DATA(p) to the new participant - // If this is not done, our data could be skip afterwards because a gap sent in newer DATA(p)s - // so the new participant could never receive out data + // Send Our DATA(p) to the new participant. + // If this is not done, our data could be skipped afterwards because of a gap sent in newer DATA(p)s, + // so the new participant could never receive our data auto our_data_it = participants_.find(server_guid_prefix_); assert(our_data_it != participants_.end()); add_pdp_to_send_(our_data_it->second.change()); @@ -699,7 +699,7 @@ void DiscoveryDataBase::create_new_participant_from_change_( // If the DATA(p) it's from this server, it is already in history and we do nothing here if (change_guid.guidPrefix != server_guid_prefix_) { - // If the participant is a new participant, mark that not everyone has ACKed this server's DATA(p) + // If the participant is a new participant, mark that not everyone has ACKed this server's DATA(p). // TODO if the new participant is a server it may be that our DATA(p) is already acked because he is // our server and we have pinged it. But also if we are its server it could be the case that // our DATA(p) is not acked even when it is our server. Solution: see in PDPServer how the change has @@ -821,7 +821,7 @@ void DiscoveryDataBase::create_writers_from_change_( // The change could be newer and at the same time not being an update. // This happens with DATAs coming from servers, since they take their own DATAs in and out frequently, // so the sequence number in `write_params` changes. - // To account for that, we discard the DATA if the payload is exactly the same as what wee have. + // To account for that, we discard the DATA if the payload is exactly the same as what we have. if (!(ch->serializedPayload == writer_it->second.change()->serializedPayload)) { // Update the change related to the writer and return the old change to the pool @@ -838,11 +838,11 @@ void DiscoveryDataBase::create_writers_from_change_( } } } - // if the cache is not new we have to release it, because it is repeated or outdated + // If the cache is not new we have to release it, because it is repeated or outdated else { - // if the change is the same that we already have, we update the ack list. This is because we have - // received the data from two servers, so we have to set that both of them already know this data + // If the change is the same that we already have, we update the ack list. This is because we have + // received the data from two servers, so we have to set that both of them already know this data if (ch->write_params.sample_identity().sequence_number() == writer_it->second.change()->write_params.sample_identity().sequence_number()) { @@ -956,7 +956,7 @@ void DiscoveryDataBase::create_readers_from_change_( } } } - // if the cache is not new we have to release it, because it is repeated or outdated + // If the cache is not new we have to release it, because it is repeated or outdated else { // if the change is the same that we already have, we update the ack list. This is because we have @@ -1014,7 +1014,7 @@ void DiscoveryDataBase::create_readers_from_change_( // we avoid backprogation of the data. reader_it->second.add_or_update_ack_participant(ch->writerGUID.guidPrefix, true); - // if topic is virtual, it must iterate over all readers + // If topic is virtual, it must iterate over all readers if (topic_name == virtual_topic_) { for (auto writer_it : writers_) @@ -1091,7 +1091,7 @@ void DiscoveryDataBase::match_writer_reader_( // TODO reduce number of cases. This is more visual, but can be reduce joining them if (writer_info.is_virtual()) { - // writer virtual + // Writer virtual // If reader is virtual do not exchange info // If not, writer needs all the info from this endpoint @@ -1426,7 +1426,7 @@ bool DiscoveryDataBase::process_dirty_topics() // If the status is 0, add DATA(r) to a `edp_publications_to_send_` (if it's not there). if (add_edp_subscriptions_to_send_(readers_it->second.change())) { - EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Addind DATA(r) to send: " + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Adding DATA(r) to send: " << readers_it->second.change()->instanceHandle); } } @@ -1436,7 +1436,7 @@ bool DiscoveryDataBase::process_dirty_topics() // Add DATA(p) of the client with the writer to `pdp_to_send_` (if it's not there). if (add_pdp_to_send_(parts_reader_it->second.change())) { - EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Addind readers' DATA(p) to send: " + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Adding readers' DATA(p) to send: " << parts_reader_it->second.change()->instanceHandle); } // Set topic as not-clearable. @@ -1458,7 +1458,7 @@ bool DiscoveryDataBase::process_dirty_topics() // If the status is 0, add DATA(w) to a `edp_subscriptions_to_send_` (if it's not there). if (add_edp_publications_to_send_(writers_it->second.change())) { - EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Addind DATA(w) to send: " + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Adding DATA(w) to send: " << writers_it->second.change()->instanceHandle); } } @@ -1468,7 +1468,7 @@ bool DiscoveryDataBase::process_dirty_topics() // Add DATA(p) of the client with the reader to `pdp_to_send_` (if it's not there). if (add_pdp_to_send_(parts_writer_it->second.change())) { - EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Addind writers' DATA(p) to send: " + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Adding writers' DATA(p) to send: " << parts_writer_it->second.change()->instanceHandle); } // Set topic as not-clearable. @@ -2323,7 +2323,7 @@ bool DiscoveryDataBase::add_pdp_to_send_( pdp_to_send_.end(), change) == pdp_to_send_.end()) { - EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Addind DATA(p) to send: " + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Adding DATA(p) to send: " << change->instanceHandle); pdp_to_send_.push_back(change); return true; @@ -2340,7 +2340,7 @@ bool DiscoveryDataBase::add_edp_publications_to_send_( edp_publications_to_send_.end(), change) == edp_publications_to_send_.end()) { - EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Addind DATA(w) to send: " + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Adding DATA(w) to send: " << change->instanceHandle); edp_publications_to_send_.push_back(change); return true; @@ -2357,7 +2357,7 @@ bool DiscoveryDataBase::add_edp_subscriptions_to_send_( edp_subscriptions_to_send_.end(), change) == edp_subscriptions_to_send_.end()) { - EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Addind DATA(r) to send: " + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Adding DATA(r) to send: " << change->instanceHandle); edp_subscriptions_to_send_.push_back(change); return true; diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp index 4ff9249dbca..caaea824558 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp @@ -307,11 +307,11 @@ class DiscoveryDataBase std::map& changes_map); // This function erase the last backup and all the changes that has arrived since then and create - // a new backup that shows the actual state of the database + // a new backup that shows the actual state of the database. // This way we can simulate the state of the database from a clean state of json backup, or from // an state in the middle of an routine execution, and every message that has arrived and has not // been process. - // By this, we do not lose any change or information in any case + // By this, we do not lose any change or information in any case. // This function must be called with the incoming datas blocked void clean_backup(); @@ -321,7 +321,7 @@ class DiscoveryDataBase data_queues_mutex_.lock(); } - // Unock the incoming of new data to the DDB queue + // Unlock the incoming of new data to the DDB queue void unlock_incoming_data() { data_queues_mutex_.unlock(); @@ -339,7 +339,7 @@ class DiscoveryDataBase return new_updates_.exchange(0); } - // Check if an participant is stored as local. If the participant does not exist, it returns false + // Check if a participant is stored as local. If the participant does not exist, it returns false bool is_participant_local( const eprosima::fastdds::rtps::GuidPrefix_t& participant_prefix); @@ -354,12 +354,12 @@ class DiscoveryDataBase protected: - // change a cacheChange by update or new disposal + // Change a cacheChange by update or new disposal void update_change_and_unmatch_( fastdds::rtps::CacheChange_t* new_change, ddb::DiscoverySharedInfo& entity); - // update the acks + // Update the acks void add_ack_( const eprosima::fastdds::rtps::CacheChange_t* change, const eprosima::fastdds::rtps::GuidPrefix_t& acked_entity); @@ -524,7 +524,7 @@ class DiscoveryDataBase DBQueue edp_data_queue_; - //! Covenient per-topic mapping of readers and writers to speed-up queries + //! Convenient per-topic mapping of readers and writers to speed-up queries std::map> readers_by_topic_; std::map> writers_by_topic_; diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index 90e10d43fe7..02a742fa33a 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -274,7 +274,7 @@ bool PDPClient::create_ds_pdp_best_effort_reader( ratt.endpoint.ignore_non_matching_locators = pattr.ignore_non_matching_locators; ratt.endpoint.topicKind = WITH_KEY; - // change depending of backup mode + // Change depending of backup mode ratt.endpoint.durabilityKind = VOLATILE; ratt.endpoint.reliabilityKind = BEST_EFFORT; @@ -627,7 +627,7 @@ void PDPClient::removeRemoteEndpoints( if (!should_protect_discovery()) #endif // HAVE_SECURITY { - // rematch but discarding any previous state of the server + // Rematch but discarding any previous state of the server // because we know the server shutdown intentionally auto temp_writer_data = get_temporary_writer_proxies_pool().get(); @@ -674,10 +674,10 @@ bool PDPClient::all_servers_acknowledge_PDP() { auto endpoints = static_cast(builtin_endpoints_.get()); - // check if already initialized + // Check if already initialized assert(endpoints->writer.history_ && endpoints->writer.writer_); - // get a reference to client proxy data + // Get a reference to client proxy data CacheChange_t* pPD; if (endpoints->writer.history_->get_min_change(&pPD)) { @@ -734,9 +734,9 @@ void PDPClient::announceParticipantState( // Add the write params to the sample if (dispose) { - // we must assure when the server is dying that all client are send at least a DATA(p) - // note here we can no longer receive and DATA or ACKNACK from clients. - // In order to avoid that we send the message directly as in the standard stateless PDP + // We must assure when the server is dying that all client are send at least a DATA(p). + // Note here we can no longer receive and DATA or ACKNACK from clients. + // In order to avoid that we send the message directly as in the standard stateless PDP. CacheChange_t* change = nullptr; change = history.create_change( @@ -746,7 +746,7 @@ void PDPClient::announceParticipantState( if (nullptr != change) { - // update the sequence number + // Update the sequence number change->sequenceNumber = history.next_sequence_number(); change->write_params = wp; @@ -764,15 +764,14 @@ void PDPClient::announceParticipantState( // //locators.push_back(ep.multicastLocatorList); //} { - // temporary workaround + // Temporary workaround eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); for (auto& svr : mp_builtin->m_DiscoveryServers) { - // if we are matched to a server report demise + // If we are matched to a server report demise if (svr.is_connected) { - //locators.push_back(svr.metatrafficMulticastLocatorList); locators.push_back(svr.metatrafficUnicastLocatorList); remote_readers.emplace_back(svr.guidPrefix, endpoints->reader.reader_->getGuid().entityId); @@ -786,7 +785,7 @@ void PDPClient::announceParticipantState( } } - // free change + // Free change history.release_change(change); } else @@ -795,7 +794,7 @@ void PDPClient::announceParticipantState( if (!new_change) { - // retrieve the participant discovery data + // Retrieve the participant discovery data CacheChange_t* pPD; if (history.get_min_change(&pPD)) { @@ -805,7 +804,7 @@ void PDPClient::announceParticipantState( for (auto& svr : mp_builtin->m_DiscoveryServers) { - // non-pinging announcements like lease duration ones must be + // Non-pinging announcements like lease duration ones must be // broadcast to all servers if (!svr.is_connected || !_serverPing) { @@ -816,8 +815,8 @@ void PDPClient::announceParticipantState( direct_send(getRTPSParticipant(), locators, *pPD); - // ping done independtly of which triggered the announcement - // note all event callbacks are currently serialized + // Ping done independently of which triggered the announcement. + // Note all event callbacks are currently serialized _serverPing = false; } else @@ -1006,13 +1005,13 @@ bool load_environment_server_info( { if (port > std::numeric_limits::max()) { - throw std::out_of_range("Too large udp port passed into the server's list"); + throw std::out_of_range("Too large port passed into the server's list"); } if (!IPLocator::setPhysicalPort(server, static_cast(port))) { std::stringstream ss; - ss << "Wrong udp port passed into the server's list " << port; + ss << "Wrong port passed into the server's list " << port; throw std::invalid_argument(ss.str()); } }; @@ -1022,13 +1021,13 @@ bool load_environment_server_info( { RemoteServerAttributes server_att; - // add the server to the list + // Add the server to the list if (!get_server_client_default_guidPrefix(id, server_att.guidPrefix)) { throw std::invalid_argument("The maximum number of default discovery servers has been reached"); } - // split multi and unicast locators + // Split multi and unicast locators auto unicast = std::partition(locators.begin(), locators.end(), IPLocator::isMulticast); LocatorList mlist; @@ -1067,13 +1066,13 @@ bool load_environment_server_info( if (sm.matched) { - // now we must parse the inner expression + // Now we must parse the inner expression std::smatch mr; std::string locator(sm); if (locator.empty()) { - // it's intencionally empty to hint us to ignore this server + // It's intencionally empty to hint us to ignore this server } // Try first with IPv4 else if (std::regex_match(locator, mr, ROS2_IPV4_ADDRESSPORT_PATTERN, @@ -1081,7 +1080,7 @@ bool load_environment_server_info( { std::smatch::iterator it = mr.cbegin(); - // traverse submatches + // Traverse submatches if (++it != mr.cend()) { std::string address = it->str(); @@ -1101,17 +1100,17 @@ bool load_environment_server_info( IPLocator::setIPv4(server_locator, "127.0.0.1"); } - // get port if any + // Get port if any int port = DEFAULT_ROS2_SERVER_PORT; if (++it != mr.cend() && it->matched) { port = stoi(it->str()); } - process_port( port, server_locator); + process_port(port, server_locator); } - // add server to the list + // Add server to the list add_server2qos(server_id, std::forward_list{server_locator}, attributes); } // Try IPv6 next @@ -1120,7 +1119,7 @@ bool load_environment_server_info( { std::smatch::iterator it = mr.cbegin(); - // traverse submatches + // Traverse submatches if (++it != mr.cend()) { std::string address = it->str(); @@ -1140,20 +1139,20 @@ bool load_environment_server_info( IPLocator::setIPv6(server_locator, "::1"); } - // get port if any + // Get port if any int port = DEFAULT_ROS2_SERVER_PORT; if (++it != mr.cend() && it->matched) { port = stoi(it->str()); } - process_port( port, server_locator); + process_port(port, server_locator); } - // add server to the list + // Add server to the list add_server2qos(server_id, std::forward_list{server_locator}, attributes); } - // try resolve DNS + // Try resolve DNS else if (std::regex_match(locator, mr, ROS2_DNS_DOMAINPORT_PATTERN, std::regex_constants::match_not_null)) { @@ -1165,7 +1164,7 @@ bool load_environment_server_info( std::ios_base::out | std::ios_base::ate); - // first try the formal notation, add default port if necessary + // First try the formal notation, add default port if necessary if (!mr[2].matched) { new_locator << ":" << DEFAULT_ROS2_SERVER_PORT; @@ -1185,14 +1184,14 @@ bool load_environment_server_info( { std::smatch::iterator it = mr.cbegin(); - // traverse submatches + // Traverse submatches if (++it != mr.cend()) { std::string domain_name = it->str(); std::set ipv4, ipv6; std::tie(ipv4, ipv6) = IPLocator::resolveNameDNS(domain_name); - // get port if any + // Get port if any int port = DEFAULT_ROS2_SERVER_PORT; if (++it != mr.cend() && it->matched) { @@ -1211,7 +1210,7 @@ bool load_environment_server_info( IPLocator::setIPv4(server_locator, "127.0.0.1"); } - process_port( port, server_locator); + process_port(port, server_locator); flist.push_front(server_locator); } @@ -1227,7 +1226,7 @@ bool load_environment_server_info( IPLocator::setIPv6(server_locator, "::1"); } - process_port( port, server_locator); + process_port(port, server_locator); flist.push_front(server_locator); } } @@ -1241,10 +1240,10 @@ bool load_environment_server_info( throw std::invalid_argument(ss.str()); } - // add server to the list + // Add server to the list add_server2qos(server_id, std::move(flist), attributes); } - // try resolve TCP DNS + // Try resolve TCP DNS else if (std::regex_match(locator, mr, ROS2_DNS_DOMAINPORT_PATTERN_TCP, std::regex_constants::match_not_null)) { @@ -1256,7 +1255,7 @@ bool load_environment_server_info( std::ios_base::out | std::ios_base::ate); - // first try the formal notation, add default port if necessary + // First try the formal notation, add default port if necessary if (!mr[2].matched) { new_locator << ":" << DEFAULT_TCP_SERVER_PORT; @@ -1270,21 +1269,21 @@ bool load_environment_server_info( { case LOCATOR_KIND_TCPv4: case LOCATOR_KIND_TCPv6: - IPLocator::setLogicalPort(server_locator, static_cast(server_locator.port)); + IPLocator::setLogicalPort(server_locator, IPLocator::getPhysicalPort(server_locator)); flist.push_front(server_locator); break; case LOCATOR_KIND_INVALID: { std::smatch::iterator it = mr.cbegin(); - // traverse submatches + // Traverse submatches if (++it != mr.cend()) { std::string domain_name = it->str(); std::set ipv4, ipv6; std::tie(ipv4, ipv6) = IPLocator::resolveNameDNS(domain_name); - // get port if any + // Get port if any int port = DEFAULT_TCP_SERVER_PORT; if (++it != mr.cend() && it->matched) { @@ -1303,7 +1302,7 @@ bool load_environment_server_info( IPLocator::setIPv4(server_locator, "127.0.0.1"); } - process_port( port, server_locator); + process_port(port, server_locator); IPLocator::setLogicalPort(server_locator, static_cast(port)); flist.push_front(server_locator); } @@ -1320,7 +1319,7 @@ bool load_environment_server_info( IPLocator::setIPv6(server_locator, "::1"); } - process_port( port, server_locator); + process_port(port, server_locator); IPLocator::setLogicalPort(server_locator, static_cast(port)); flist.push_front(server_locator); } @@ -1335,7 +1334,7 @@ bool load_environment_server_info( throw std::invalid_argument(ss.str()); } - // add server to the list + // Add server to the list add_server2qos(server_id, std::move(flist), attributes); } else @@ -1346,7 +1345,7 @@ bool load_environment_server_info( } } - // advance to the next server if any + // Advance to the next server if any ++server_id; ++server_it; } diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index f8048d43096..1a62948bff6 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -110,7 +110,7 @@ bool PDPServer::init( return false; } - //INIT EDP + // INIT EDP mp_EDP = new EDPServer(this, mp_RTPSParticipant, durability_); if (!mp_EDP->initEDP(m_discovery)) { @@ -196,15 +196,15 @@ ParticipantProxyData* PDPServer::createParticipantProxyData( { std::lock_guard lock(*getMutex()); - // lease duration is controlled for owned clients or linked servers - // other clients liveliness is provided through server's PDP discovery data + // Lease duration is controlled for owned clients or linked servers. + // Other clients liveliness is provided through server's PDP discovery data - // check if the DATA msg is relayed by another server + // Check if the DATA msg is relayed by another server bool do_lease = participant_data.m_guid.guidPrefix == writer_guid.guidPrefix; if (!do_lease) { - // if not a client verify this participant is a server + // If not a client verify this participant is a server { eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); for (auto& svr : mp_builtin->m_DiscoveryServers) @@ -287,7 +287,7 @@ bool PDPServer::create_ds_pdp_best_effort_reader( ratt.endpoint.external_unicast_locators = mp_builtin->m_att.metatraffic_external_unicast_locators; ratt.endpoint.ignore_non_matching_locators = pattr.ignore_non_matching_locators; ratt.endpoint.topicKind = WITH_KEY; - // change depending of backup mode + // Change depending of backup mode ratt.endpoint.durabilityKind = VOLATILE; ratt.endpoint.reliabilityKind = BEST_EFFORT; @@ -375,7 +375,7 @@ bool PDPServer::create_ds_pdp_reliable_endpoints( ratt.endpoint.external_unicast_locators = mp_builtin->m_att.metatraffic_external_unicast_locators; ratt.endpoint.ignore_non_matching_locators = pattr.ignore_non_matching_locators; ratt.endpoint.topicKind = WITH_KEY; - // change depending of backup mode + // Change depending of backup mode ratt.endpoint.durabilityKind = durability_; ratt.endpoint.reliabilityKind = RELIABLE; ratt.times.heartbeat_response_delay = pdp_heartbeat_response_delay; @@ -574,7 +574,7 @@ void PDPServer::match_reliable_pdp_endpoints( bool use_multicast_locators = !mp_RTPSParticipant->getAttributes().builtin.avoid_builtin_multicast || pdata.metatraffic_locators.unicast.empty(); - // only SERVER and CLIENT participants will be received. All builtin must be there + // Only SERVER and CLIENT participants will be received. All builtin must be there uint32_t auxendp = endp & (DISC_BUILTIN_ENDPOINT_PARTICIPANT_ANNOUNCER | DISC_BUILTIN_ENDPOINT_PARTICIPANT_SECURE_ANNOUNCER); @@ -610,7 +610,7 @@ void PDPServer::match_reliable_pdp_endpoints( return; } - // only SERVER and CLIENT participants will be received. All builtin must be there + // Only SERVER and CLIENT participants will be received. All builtin must be there auxendp = endp & (DISC_BUILTIN_ENDPOINT_PARTICIPANT_DETECTOR | DISC_BUILTIN_ENDPOINT_PARTICIPANT_SECURE_DETECTOR); if (0 != auxendp) { @@ -705,7 +705,7 @@ bool PDPServer::pairing_remote_reader_with_local_writer_after_security( void PDPServer::perform_builtin_endpoints_matching( const ParticipantProxyData& pdata) { - //Inform EDP of new RTPSParticipant data: + // Inform EDP of new RTPSParticipant data: if (mp_EDP != nullptr) { mp_EDP->assignRemoteEndpoints(pdata, true); @@ -873,7 +873,7 @@ void PDPServer::announceParticipantState( return; } - // assign identity + // Assign identity change->sequenceNumber = sn; // Create a RemoteLocatorList for metatraffic_locators @@ -913,7 +913,7 @@ void PDPServer::announceParticipantState( } } // Doesn't make sense to send the DATA directly if it hasn't been introduced in the history yet (missing - // sequence number. + // sequence number). return; } else @@ -973,7 +973,7 @@ void PDPServer::announceParticipantState( } else { - // Dispose if already there + // Dispose if already there. // It may happen if the participant is not removed fast enough history.release_change(change); return; @@ -981,7 +981,7 @@ void PDPServer::announceParticipantState( } else { - // failed to create the disposal change + // Failed to create the disposal change EPROSIMA_LOG_ERROR(RTPS_PDP_SERVER, "Server failed to create its DATA(Up)"); return; } @@ -1056,24 +1056,24 @@ bool PDPServer::remove_remote_participant( // Notify the database if (discovery_db_.update(pC, ddb::DiscoveryParticipantChangeData())) { - // assure processing time for the cache + // Assure processing time for the cache awake_routine_thread(); - // the discovery database takes ownership of the CacheChange_t - // henceforth there are no references to the CacheChange_t + // The discovery database takes ownership of the CacheChange_t + // Henceforth, there are no references to the CacheChange_t } else { - // if the database doesn't take the ownership remove + // If the database doesn't take the ownership remove endpoints->reader.reader_->release_cache(pC); } } } - // check if is a server who has been disposed + // Check if is a server who has been disposed awake_server_thread(); - // delegate into the base class for inherited proxy database removal + // Delegate into the base class for inherited proxy database removal return PDP::remove_remote_participant(partGUID, reason); } @@ -1202,7 +1202,7 @@ bool PDPServer::process_writers_acknowledgements() auto endpoints = static_cast(builtin_endpoints_.get()); // Execute first ACK for endpoints because PDP acked changes relevance in EDP, - // which can result in false positives in EDP acknowledgements. + // which can result in false positives in EDP acknowledgements. /* EDP Subscriptions Writer's History */ EDPServer* edp = static_cast(mp_EDP); @@ -1291,8 +1291,8 @@ History::iterator PDPServer::process_change_acknowledgement( // Remove entry from `participants_|writers_|readers_` // AND put the change in changes_release discovery_db_.delete_entity_of_change(c); - // Remove from writer's history - // remove false because the clean of the change is made by release_change of ddb + // Remove from writer's history. + // Remove false because the clean of the change is made by release_change of ddb return writer_history->remove_change(cit, false); } } @@ -1374,7 +1374,7 @@ void PDPServer::process_changes_release_( for (auto ch : changes) { // Check if change owner is this participant. In that case, the change comes from a writer pool (PDP, EDP - // publications or EDP subscriptions) + // publications or EDP subscriptions). // We compare the instance handle, as the only changes from our own server are its owns if (discovery_db().guid_from_change(ch) == endpoints->writer.writer_->getGuid()) { @@ -1591,7 +1591,7 @@ eprosima::fastdds::rtps::ResourceEvent& PDPServer::get_resource_event_thread() bool PDPServer::all_servers_acknowledge_pdp() { - // check if already initialized + // Check if already initialized auto endpoints = static_cast(builtin_endpoints_.get()); static_cast(endpoints); assert(endpoints->writer.history_ && endpoints->writer.writer_); @@ -1618,7 +1618,7 @@ void PDPServer::ping_remote_servers() auto server_it = std::find(ack_pending_servers.begin(), ack_pending_servers.end(), server.guidPrefix); if (server_it != ack_pending_servers.end()) { - // get the info to send to this already known locators + // Get the info to send to this already known locators locators.push_back(server.metatrafficUnicastLocatorList); } } @@ -1788,16 +1788,16 @@ bool PDPServer::process_backup_discovery_database_restore( } } - // deserialize from json to change already created + // Deserialize from json to change already created ddb::from_json(it.value()["change"], *change_aux); changes_map.insert( std::make_pair(change_aux->instanceHandle, change_aux)); // TODO refactor for multiple servers - // should not send the virtual changes by the listener - // should store in DDB if it is local even for endpoints - // call listener to create proxy info for other entities different than server + // Should not send the virtual changes by the listener. + // Should store in DDB if it is local even for endpoints. + // Call listener to create proxy info for other entities different than server. if (change_aux->write_params.sample_identity().writer_guid().guidPrefix != endpoints->writer.writer_->getGuid().guidPrefix && change_aux->kind == fastdds::rtps::ALIVE @@ -1827,13 +1827,13 @@ bool PDPServer::process_backup_discovery_database_restore( } } - // deserialize from json to change already created + // Deserialize from json to change already created ddb::from_json(it.value()["change"], *change_aux); changes_map.insert( std::make_pair(change_aux->instanceHandle, change_aux)); - // call listener to create proxy info for other entities different than server + // Call listener to create proxy info for other entities different than server if (change_aux->write_params.sample_identity().writer_guid().guidPrefix != endpoints->writer.writer_->getGuid().guidPrefix && change_aux->kind == fastdds::rtps::ALIVE @@ -1843,7 +1843,7 @@ bool PDPServer::process_backup_discovery_database_restore( } } - // load database + // Load database discovery_db_.from_json(j, changes_map); } catch (std::ios_base::failure&) diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp index d9950664818..fca8d838ea9 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp @@ -254,7 +254,7 @@ void PDPServerListener::on_new_cache_change_added( pdp_server()->assignRemoteEndpoints(pdata); } } - // Case ParticipantProxyData already exists but was known remotly and now must be local + // Case ParticipantProxyData already exists but was known remotely and now must be local else if (is_local && !was_local) { // Realease PDP mutex diff --git a/src/cpp/rtps/writer/StatefulWriter.cpp b/src/cpp/rtps/writer/StatefulWriter.cpp index aa674715998..717fc92568e 100644 --- a/src/cpp/rtps/writer/StatefulWriter.cpp +++ b/src/cpp/rtps/writer/StatefulWriter.cpp @@ -349,18 +349,18 @@ void StatefulWriter::unsent_change_added_to_history( [this, &should_be_sent, &change, &max_blocking_time](ReaderProxy* reader) { ChangeForReader_t changeForReader(change); - bool is_revelant = reader->rtps_is_relevant(change); + bool is_relevant = reader->rtps_is_relevant(change); if (m_pushMode || !reader->is_reliable() || reader->is_local_reader()) { //ChangeForReader_t construct sets status to UNSENT. - should_be_sent |= is_revelant; + should_be_sent |= is_relevant; } else { changeForReader.setStatus(UNACKNOWLEDGED); } - reader->add_change(changeForReader, is_revelant, false, max_blocking_time); + reader->add_change(changeForReader, is_relevant, false, max_blocking_time); return false; } From 496f3ac32eaec04c936a71ff26a577c087ec2a27 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Fri, 22 Mar 2024 10:36:43 +0100 Subject: [PATCH 02/51] Refs #20629: Random GUID function Signed-off-by: cferreiragonz --- .../fastdds/rtps/attributes/ServerAttributes.hpp | 7 +++++++ .../builtin/discovery/participant/PDPClient.cpp | 15 +++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/include/fastdds/rtps/attributes/ServerAttributes.hpp b/include/fastdds/rtps/attributes/ServerAttributes.hpp index 7f0bdec3998..d1d6ff43915 100644 --- a/include/fastdds/rtps/attributes/ServerAttributes.hpp +++ b/include/fastdds/rtps/attributes/ServerAttributes.hpp @@ -216,6 +216,13 @@ FASTDDS_EXPORTED_API bool get_server_client_default_guidPrefix( int id, fastdds::rtps::GuidPrefix_t& guid); +/** + * Returns a random generated guidPrefix. + * @param[out] guid reference to the guidPrefix to modify + */ +FASTDDS_EXPORTED_API void set_server_client_random_guidPrefix( + fastrtps::rtps::GuidPrefix_t& guid); + } // rtps } // fastdds } // namespace eprosima diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index 02a742fa33a..2b8eb21dde2 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include @@ -1398,6 +1399,20 @@ bool get_server_client_default_guidPrefix( return false; } +void set_server_client_random_guidPrefix( + GuidPrefix_t& guid) +{ + // Random GUIDs keeps 4 first bytes from the default server GUID + eprosima::fastdds::rtps::GuidUtils::instance().guid_prefix_create(0, guid); + + auto now = std::chrono::high_resolution_clock::now(); + srand(now.time_since_epoch().count()); + for (auto i = 4; i < 12; i++) + { + guid.value[i] = eprosima::fastrtps::rtps::octet(rand() % 254); + } +} + bool PDPClient::remove_remote_participant( const GUID_t& partGUID, ParticipantDiscoveryInfo::DISCOVERY_STATUS reason) From 9f4c58bf22dc975677e44a7d379a418102da8e84 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Thu, 21 Mar 2024 16:36:39 +0100 Subject: [PATCH 03/51] Refs #20629: Update CLI for GUID_LESS DS when no server_id is provided Signed-off-by: cferreiragonz --- ...est_wrong_xml_discovery_server_profile.xml | 18 ------- .../fds/test_xml_discovery_server_profile.xml | 18 +++++++ test/system/tools/fds/tests.py | 21 ++++---- tools/fds/server.cpp | 52 +++++++++++-------- 4 files changed, 59 insertions(+), 50 deletions(-) diff --git a/test/system/tools/fds/test_wrong_xml_discovery_server_profile.xml b/test/system/tools/fds/test_wrong_xml_discovery_server_profile.xml index dcd1cff8ad2..669f00efbc8 100644 --- a/test/system/tools/fds/test_wrong_xml_discovery_server_profile.xml +++ b/test/system/tools/fds/test_wrong_xml_discovery_server_profile.xml @@ -37,22 +37,4 @@ - - - - - - SERVER - - - - -
127.0.0.5
- 11817 - - - - - - diff --git a/test/system/tools/fds/test_xml_discovery_server_profile.xml b/test/system/tools/fds/test_xml_discovery_server_profile.xml index 977097b071e..e96c83eb925 100644 --- a/test/system/tools/fds/test_xml_discovery_server_profile.xml +++ b/test/system/tools/fds/test_xml_discovery_server_profile.xml @@ -88,4 +88,22 @@ + + + + + + SERVER + + + + +
127.0.0.5
+ 11817 + + + + + + diff --git a/test/system/tools/fds/tests.py b/test/system/tools/fds/tests.py index 166cfcce602..56930c7771c 100644 --- a/test/system/tools/fds/tests.py +++ b/test/system/tools/fds/tests.py @@ -499,6 +499,17 @@ def test_fast_discovery_backup(fast_discovery_tool): sys.exit(exit_code) +def test_fast_discovery_no_prefix(fast_discovery_tool): + """Test to set a random GUID when no server ID is provided""" + + XML_file_path = "test_xml_discovery_server_profile.xml" + command = [fast_discovery_tool, '-x', 'UDP_no_prefix@' + XML_file_path] + output, err, exit_code = send_command(command) + msg = "Using random GUID because no server ID was specified (-i option) or defined in the XML file." + exit_code = check_output(output, err, msg, False) + sys.exit(exit_code) + + def test_fast_discovery_no_XML(fast_discovery_tool): """Test that checks output when the XML file provided does not exist""" @@ -528,16 +539,6 @@ def test_fast_discovery_incorrect_participant(fast_discovery_tool): sys.exit(exit_code) -def test_fast_discovery_no_prefix(fast_discovery_tool): - """Test failure when no server ID is provided""" - - XML_file_path = "test_wrong_xml_discovery_server_profile.xml" - command = [fast_discovery_tool, '-x', 'UDP_no_prefix@' + XML_file_path] - output, err, exit_code = send_command(command) - exit_code = check_output(output, err, "Server id is mandatory if not defined in the XML file", True) - sys.exit(exit_code) - - def test_fast_discovery_several_server_ids(fast_discovery_tool): """Test failure when several Server IDs are provided""" diff --git a/tools/fds/server.cpp b/tools/fds/server.cpp index 2addf820910..f010fc2be41 100644 --- a/tools/fds/server.cpp +++ b/tools/fds/server.cpp @@ -165,7 +165,7 @@ int fastdds_discovery_server( } - // Retrieve server Id: is mandatory and only specified once + // Retrieve server ID: is optional and only specified once // Note there is a specific cast to pointer if the Option is valid option::Option* pOp = options[SERVERID]; int server_id = 0; @@ -175,9 +175,10 @@ int fastdds_discovery_server( fastdds::rtps::GuidPrefix_t prefix_cero; if (participantQos.wire_protocol().prefix == prefix_cero) { - std::cout << "Server id is mandatory if not defined in the XML file: use -i or --server-id option." << + std::cout << "Using random GUID because no server ID was specified (-i option) or defined in the XML file." << std::endl; - return 1; + pOp = new option::Option(); + pOp->arg = "-1"; } else if (!(participantQos.wire_protocol().builtin.discovery_config.discoveryProtocol == eprosima::fastdds::rtps::DiscoveryProtocol::SERVER || @@ -194,29 +195,34 @@ int fastdds_discovery_server( std::cout << "Only one server can be created, thus, only one server id can be specified." << std::endl; return 1; } - else + + std::stringstream is; + + // Name the server according with the identifier + if (pOp != nullptr) { - std::stringstream is; + // Cast of option to int has already been checked is << pOp->arg; - - // Validation has been already done - // Name the server according with the identifier - if (!(is >> server_id - && eprosima::fastdds::rtps::get_server_client_default_guidPrefix(server_id, - participantQos.wire_protocol().prefix))) + is >> server_id; + if (server_id == -1) + { + eprosima::fastdds::rtps::set_server_client_random_guidPrefix(participantQos.wire_protocol().prefix); + } + else if(!eprosima::fastdds::rtps::get_server_client_default_guidPrefix(server_id, + participantQos.wire_protocol().prefix)) { - std::cout << "The provided server identifier is not valid" << std::endl; + std::cout << "Failed to set the GUID with the server identifier provided." << std::endl; return 1; } + } - // Clear the std::stringstream state and reset it to an empty string - is.clear(); - is.str(""); + // Clear the std::stringstream state and reset it to an empty string + is.clear(); + is.str(""); - // Set Participant Name - is << "eProsima Default Server number " << server_id; - participantQos.name(is.str().c_str()); - } + // Set Participant Name + is << "eProsima Default Server number " << server_id; + participantQos.name(is.str().c_str()); // Choose the kind of server to create pOp = options[BACKUP]; @@ -572,12 +578,13 @@ int fastdds_discovery_server( } // Print running server attributes + std::string print_server_id = (server_id == -1) ? "Default" : std::to_string(server_id); std::cout << "### Server is running ###" << std::endl; std::cout << " Participant Type: " << participantQos.wire_protocol().builtin.discovery_config.discoveryProtocol << std::endl; std::cout << " Security: " << (has_security ? "YES" : "NO") << std::endl; - std::cout << " Server ID: " << server_id << std::endl; + std::cout << " Server ID: " << print_server_id << std::endl; std::cout << " Server GUID prefix: " << (has_security ? participantQos.wire_protocol().prefix : pServer->guid().guidPrefix) << std::endl; @@ -634,7 +641,7 @@ option::ArgStatus Arg::check_server_id( if (is >> id && is.eof() - && id >= 0 + && id >= -1 && id < 256 ) { return option::ARG_OK; @@ -644,7 +651,8 @@ option::ArgStatus Arg::check_server_id( if (msg) { std::cout << "Option '" << option.name - << "' is mandatory. Should be a key identifier between 0 and 255." << std::endl; + << "' is optional. Should be a key identifier between 0 and 255" + << " or -1 (default) to use a random GUID." << std::endl; } return option::ARG_ILLEGAL; From f292f1b75d99a80bd2946e8b8d2befb37a51e6ba Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Thu, 4 Apr 2024 10:25:13 +0200 Subject: [PATCH 04/51] Refs #20629: Improve CLI Signed-off-by: cferreiragonz --- test/system/tools/fastdds/tests.py | 6 +- test/system/tools/fds/tests.py | 4 +- tools/fastdds/discovery/parser.py | 9 +-- tools/fds/server.cpp | 66 ++++++++-------- tools/fds/server.h | 117 +++++++++++++++-------------- 5 files changed, 103 insertions(+), 99 deletions(-) diff --git a/test/system/tools/fastdds/tests.py b/test/system/tools/fastdds/tests.py index 25f3ad194d6..ea9b4c312f3 100644 --- a/test/system/tools/fastdds/tests.py +++ b/test/system/tools/fastdds/tests.py @@ -109,7 +109,7 @@ def test_fastdds_shm(install_path): def test_fastdds_discovery(install_path, setup_script_path): """Test that discovery command runs.""" - args = ' discovery' + args = ' discovery -h' ret = subprocess.call( cmd(install_path=install_path, setup_script_path=setup_script_path, @@ -123,9 +123,11 @@ def test_fastdds_discovery(install_path, setup_script_path): def test_ros_discovery(install_path, setup_script_path): """Test that discovery command runs.""" + args = ' -h' ret = subprocess.call( cmd(install_path=install_path, - setup_script_path=setup_script_path), + setup_script_path=setup_script_path, + args=args), shell=True) if 0 != ret: diff --git a/test/system/tools/fds/tests.py b/test/system/tools/fds/tests.py index 56930c7771c..84744ff4093 100644 --- a/test/system/tools/fds/tests.py +++ b/test/system/tools/fds/tests.py @@ -505,8 +505,8 @@ def test_fast_discovery_no_prefix(fast_discovery_tool): XML_file_path = "test_xml_discovery_server_profile.xml" command = [fast_discovery_tool, '-x', 'UDP_no_prefix@' + XML_file_path] output, err, exit_code = send_command(command) - msg = "Using random GUID because no server ID was specified (-i option) or defined in the XML file." - exit_code = check_output(output, err, msg, False) + EXPECTED_OUTPUT = "UDPv4:[127.0.0.5]:11817" + exit_code = check_output(output, err, EXPECTED_OUTPUT, False) sys.exit(exit_code) diff --git a/tools/fastdds/discovery/parser.py b/tools/fastdds/discovery/parser.py index 8354a47177c..8446698e89e 100644 --- a/tools/fastdds/discovery/parser.py +++ b/tools/fastdds/discovery/parser.py @@ -30,8 +30,6 @@ class Parser: """Discovery server tool parser.""" - __help_message = 'fastdds discovery \n\n' - def __init__(self, argv): """ Parse the sub-command and dispatch to the appropriate handler. @@ -43,7 +41,7 @@ def __init__(self, argv): try: result = subprocess.run( - [tool_path], + [tool_path, '-h'], stdout=subprocess.PIPE, universal_newlines=True ) @@ -52,7 +50,6 @@ def __init__(self, argv): sys.exit(result.returncode) if ( - len(argv) == 0 or (len(argv) == 1 and argv[0] == '-h') or (len(argv) == 1 and argv[0] == '--help') ): @@ -68,9 +65,7 @@ def __init__(self, argv): pass except BaseException as e: - self.__help_message += str(e) - self.__help_message += '\n fast-discovery-server tool not found!' - print(self.__help_message) + print('\n fast-discovery-server tool not found!') sys.exit(1) def __find_tool_path(self): diff --git a/tools/fds/server.cpp b/tools/fds/server.cpp index f010fc2be41..c61d1302148 100644 --- a/tools/fds/server.cpp +++ b/tools/fds/server.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include #include @@ -78,6 +77,13 @@ int fastdds_discovery_server( return 1; } + for (option::Option* opt = options[UNKNOWN]; opt; opt = opt->next()) + { + EPROSIMA_LOG_ERROR(CLI, "Unknown option: " << opt->name); + option::printUsage(std::cout, usage); + return 1; + } + // No arguments beyond options int noopts = parse.nonOptionsCount(); if (noopts) @@ -96,9 +102,10 @@ int fastdds_discovery_server( } // Show help if asked to - if (options[HELP] || argc == 0) + if (options[HELP]) { option::printUsage(std::cout, usage); + std::cout << EXAMPLES << std::endl; return 0; } @@ -168,17 +175,15 @@ int fastdds_discovery_server( // Retrieve server ID: is optional and only specified once // Note there is a specific cast to pointer if the Option is valid option::Option* pOp = options[SERVERID]; - int server_id = 0; + int server_id = -1; + std::stringstream is; if (nullptr == pOp) { fastdds::rtps::GuidPrefix_t prefix_cero; if (participantQos.wire_protocol().prefix == prefix_cero) { - std::cout << "Using random GUID because no server ID was specified (-i option) or defined in the XML file." << - std::endl; - pOp = new option::Option(); - pOp->arg = "-1"; + eprosima::fastdds::rtps::set_server_client_random_guidPrefix(participantQos.wire_protocol().prefix); } else if (!(participantQos.wire_protocol().builtin.discovery_config.discoveryProtocol == eprosima::fastdds::rtps::DiscoveryProtocol::SERVER || @@ -195,21 +200,13 @@ int fastdds_discovery_server( std::cout << "Only one server can be created, thus, only one server id can be specified." << std::endl; return 1; } - - std::stringstream is; - - // Name the server according with the identifier - if (pOp != nullptr) + else { // Cast of option to int has already been checked is << pOp->arg; is >> server_id; - if (server_id == -1) - { - eprosima::fastdds::rtps::set_server_client_random_guidPrefix(participantQos.wire_protocol().prefix); - } - else if(!eprosima::fastdds::rtps::get_server_client_default_guidPrefix(server_id, - participantQos.wire_protocol().prefix)) + if (!eprosima::fastdds::rtps::get_server_client_default_guidPrefix(server_id, + participantQos.wire_protocol().prefix)) { std::cout << "Failed to set the GUID with the server identifier provided." << std::endl; return 1; @@ -221,7 +218,8 @@ int fastdds_discovery_server( is.str(""); // Set Participant Name - is << "eProsima Default Server number " << server_id; + std::string server_name = (server_id == -1) ? "eProsima Guidless Server" : "eProsima Default Server" + std::to_string(server_id); + is << server_name; participantQos.name(is.str().c_str()); // Choose the kind of server to create @@ -283,7 +281,15 @@ int fastdds_discovery_server( // Retrieve first IP address pOp = options[UDPADDRESS]; + if (pOp != nullptr && pOp->arg == nullptr) + { + pOp->arg = "0.0.0.0"; + } option::Option* pO_tcp = options[TCPADDRESS]; + if (pO_tcp != nullptr && pO_tcp->arg == nullptr) + { + pO_tcp->arg = "0.0.0.0"; + } // Retrieve first TCP port option::Option* pO_tcp_port = options[TCP_PORT]; @@ -386,7 +392,7 @@ int fastdds_discovery_server( else { std::cout << "Warning: the number of specified ports doesn't match the ip" << std::endl - << " addresses provided. Locators share its port number." << std::endl; + << " addresses provided. Locators share their port number." << std::endl; } } @@ -438,8 +444,11 @@ int fastdds_discovery_server( if (nullptr == pO_tcp) { // Only the TCP port has been specified. Use localhost as default interface for TCP - IPLocator::setIPv4(locator_tcp_4, "127.0.0.1"); + IPLocator::setIPv4(locator_tcp_4, "0.0.0.0"); participantQos.wire_protocol().builtin.metatrafficUnicastLocatorList.push_back(locator_tcp_4); + auto tcp_descriptor = std::make_shared(); + tcp_descriptor->add_listener_port(static_cast(locator_tcp_4.port)); + participantQos.transport().user_transports.push_back(tcp_descriptor); } else if (nullptr != pO_tcp) { @@ -578,13 +587,11 @@ int fastdds_discovery_server( } // Print running server attributes - std::string print_server_id = (server_id == -1) ? "Default" : std::to_string(server_id); std::cout << "### Server is running ###" << std::endl; std::cout << " Participant Type: " << participantQos.wire_protocol().builtin.discovery_config.discoveryProtocol << std::endl; std::cout << " Security: " << (has_security ? "YES" : "NO") << std::endl; - std::cout << " Server ID: " << print_server_id << std::endl; std::cout << " Server GUID prefix: " << (has_security ? participantQos.wire_protocol().prefix : pServer->guid().guidPrefix) << std::endl; @@ -641,7 +648,7 @@ option::ArgStatus Arg::check_server_id( if (is >> id && is.eof() - && id >= -1 + && id >= 0 && id < 256 ) { return option::ARG_OK; @@ -650,9 +657,8 @@ option::ArgStatus Arg::check_server_id( if (msg) { - std::cout << "Option '" << option.name - << "' is optional. Should be a key identifier between 0 and 255" - << " or -1 (default) to use a random GUID." << std::endl; + std::cout << "\nOption '" << option.name + << "' is optional. Should be a key identifier between 0 and 255." << std::endl; } return option::ARG_ILLEGAL; @@ -670,7 +676,7 @@ option::ArgStatus Arg::required( if (msg) { - std::cout << "Option '" << option << "' requires an argument" << std::endl; + std::cout << "\nOption '" << option.desc->longopt << "' requires an argument." << std::endl; } return option::ARG_ILLEGAL; } @@ -699,7 +705,7 @@ option::ArgStatus Arg::check_udp_port( if (msg) { - std::cout << "Option '" << option.name + std::cout << "\nOption '" << option.name << "' value should be an UDP port between 1025 and 65535." << std::endl; } @@ -729,7 +735,7 @@ option::ArgStatus Arg::check_tcp_port( if (msg) { - std::cout << "Option '" << option.name + std::cout << "\nOption '" << option.name << "' value should be an TCP port between 1025 and 65535." << std::endl; } diff --git a/tools/fds/server.h b/tools/fds/server.h index 6e47e50846d..14fd0ea9d82 100644 --- a/tools/fds/server.h +++ b/tools/fds/server.h @@ -17,6 +17,7 @@ // Parsing setup #include +#include namespace option = eprosima::option; @@ -43,6 +44,10 @@ struct Arg : public option::Arg const option::Option& option, bool msg); + // static option::ArgStatus optional_ip( + // const option::Option& option, + // bool msg); + static option::ArgStatus check_udp_port( const option::Option& option, bool msg); @@ -61,25 +66,21 @@ const option::Descriptor usage[] = { { HELP, 0, "h", "help", Arg::None, " -h \t--help Produce help message.\n" }, - { SERVERID, 0, "i", "server-id", Arg::check_server_id, - " -i \t--server-id Unique server identifier. Specifies zero based server\n" - "\t position in ROS_DISCOVERY_SERVER environment variable.\n" }, - - { UDPADDRESS, 0, "l", "udp-address", Arg::required, + { UDPADDRESS, 0, "l", "udp-address", Arg::OptionalAny, " -l \t--udp-address IPv4/IPv6 address chosen to listen the clients. Defaults\n" "\t to any (0.0.0.0/::0). Instead of an address, a name can\n" "\t be specified.\n"}, { UDP_PORT, 0, "p", "udp-port", Arg::check_udp_port, - " -p \t--udp-port UDP port chosen to listen the clients. Defaults to 11811\n" }, + " -p \t--udp-port UDP port chosen to listen the clients. Defaults to 11811.\n" }, - { TCPADDRESS, 0, "t", "tcp-address", Arg::required, + { TCPADDRESS, 0, "t", "tcp-address", Arg::OptionalAny, " -t \t--tcp-address IPv4/IPv6 address chosen to listen the clients using\n" "\t TCP transport. Defaults to any (0.0.0.0/::0). Instead of an \n" "\t address, a name can be specified.\n"}, { TCP_PORT, 0, "q", "tcp-port", Arg::check_tcp_port, - " -q \t--tcp-port TCP port chosen to listen the clients. Defaults to 42100\n" }, + " -q \t--tcp-port TCP port chosen to listen the clients. Defaults to 42100.\n" }, { BACKUP, 0, "b", "backup", Arg::None, " -b \t--backup Creates a server with a backup file associated.\n" }, @@ -91,58 +92,58 @@ const option::Descriptor usage[] = { "\t the profile with \"is_default_profile=\"true\"\" unless \n" "\t another profile using uri with \"@\" character is defined.\n"}, - { UNKNOWN, 0, "", "", Arg::None, - "Examples:\n" - - "\t1. Launch a default server with id 0 (first on ROS_DISCOVERY_SERVER)\n" - "\t listening on all available interfaces on UDP port 11811. Only one\n" - "\t server can use default values per machine.\n\n" - "\t $ " FAST_SERVER_BINARY " -i 0\n\n" - - "\t2. Launch a default server with id 1 (second on ROS_DISCOVERY_SERVER)\n" - "\t listening on localhost with UDP port 14520. Only localhost clients\n" - "\t can reach the server using ROS_DISCOVERY_SERVER=;127.0.0.1:14520\n\n" - "\t $ " FAST_SERVER_BINARY " -i 1 -l 127.0.0.1 -p 14520\n\n" - - "\t3. Launch a default server with id 1 (second on ROS_DISCOVERY_SERVER)\n" - "\t listening on UDPv6 address with UDP port 14520.\n\n" - "\t $ " FAST_SERVER_BINARY " -i 1 -l 2a02:ec80:600:ed1a::3 -p 14520\n\n" - - "\t4. Launch a default server with id 2 (third on ROS_DISCOVERY_SERVER)\n" - "\t listening on Wi-Fi (192.163.6.34) and Ethernet (172.20.96.1) local\n" - "\t interfaces with UDP ports 8783 and 51083 respectively\n" - "\t (addresses and ports are made up for the example).\n\n" - "\t $ " FAST_SERVER_BINARY " -i 2 -l 192.163.6.34 -p 8783 -l 172.20.96.1 -p 51083\n\n" - - "\t5. Launch a default server with id 3 (fourth on ROS_DISCOVERY_SERVER)\n" - "\t listening on 172.31.44.1 with UDP port 12345 and provided with a\n" - "\t backup file. If the server crashes it will automatically restore its\n" - "\t previous state when reenacted.\n\n" - "\t $ " FAST_SERVER_BINARY " -i 3 -l 172.31.44.1 -p 12345 -b\n\n" - - "\t6. Launch a default server with id 0 (first on ROS_DISCOVERY_SERVER)\n" - "\t listening on localhost with UDP port 14520. Only localhost clients\n" - "\t can reach the server using ROS_DISCOVERY_SERVER=localhost:14520\n\n" - "\t $ " FAST_SERVER_BINARY " -i 0 -l localhost -p 14520\n\n" - - "\t7. Launch a server with id 0 (first on ROS_DISCOVERY_SERVER) reading\n" - "\t default configuration from XML file.\n\n" - "\t $ " FAST_SERVER_BINARY " -i 0 -x config.xml\n\n" - - "\t8. Launch a server with id 0 (first on ROS_DISCOVERY_SERVER) reading\n" - "\t specific profile_name configuration from XML file.\n\n" - "\t $ " FAST_SERVER_BINARY " -i 0 -x profile_name@config.xml\n\n" - - "\t9. Launch a server with id 0 (first on ROS_DISCOVERY_SERVER) listening\n" - "\t on localhost with default TCP port 42100.\n\n" - "\t $ " FAST_SERVER_BINARY " -i 0 -t 127.0.0.1\n\n" - - "\t10. Launch a server with id 0 (first on ROS_DISCOVERY_SERVER) listening\n" - "\t on localhost and Wi-Fi (192.163.6.34). Two TCP ports need to be\n" - "\t specified because TCP Transports cannot share ports.\n\n" - "\t $ " FAST_SERVER_BINARY " -i 0 -t 127.0.0.1 -q 42100 -t 192.163.6.34 -q 42101"}, + { SERVERID, 0, "i", "server-id", Arg::check_server_id, + " -i \t--server-id Unique server identifier. Its functionality its\n" + "\t deprecated. It can be used to select a fixed GUID.\n" }, { 0, 0, 0, 0, 0, 0 } }; +const std::string EXAMPLES = + + "Examples:\n" + + "\t1. Launch a default server listening on all available interfaces on\n" + "\t UDP port 11811. Only one server can use default values per machine.\n\n" + "\t $ " FAST_SERVER_BINARY "\n\n" + + "\t2. Launch a default server listening on localhost on UDP port 14520.\n" + "\t Only localhost clients can reach the server, for example using\n" + "\t ROS_DISCOVERY_SERVER=127.0.0.1:14520\n\n" + "\t $ " FAST_SERVER_BINARY " -l 127.0.0.1 -p 14520\n\n" + + "\t3. Launch a default server listening on all available interfaces on\n" + "\t TCP port 42100. Only one server can use default values per machine.\n\n" + "\t $ " FAST_SERVER_BINARY " -t\n\n" + + "\t4. Launch a default server with GUID corresponding to id 1 (deprecated\n" + "\t parameter) listening on IPv6 address on UDP port 14520.\n\n" + "\t $ " FAST_SERVER_BINARY " -i 1 -l 2a02:ec80:600:ed1a::3 -p 14520\n\n" + + "\t5. Launch a default server listening on WiFi (192.168.36.34) and Ethernet\n" + "\t (172.20.96.1) local interfaces on UDP ports 8783 and 51083 \n" + "\t respectively (addresses and ports are made up for the example).\n\n" + "\t $ " FAST_SERVER_BINARY " -l 192.163.6.34 -p 8783 -l 172.20.96.1\n" + "\t -p 51083\n\n" + + "\t6. Launch a default server listening on 172.31.44.1 on UDP port 12345\n" + "\t and provided with a backup file. If the server crashes it will\n" + "\t automatically restore its previous state when reenacted.\n\n" + "\t $ " FAST_SERVER_BINARY " -l 172.31.44.1 -p 12345 -b\n\n" + + "\t7. Launch a server reading default configuration from XML file.\n\n" + "\t $ " FAST_SERVER_BINARY " -x config.xml\n\n" + + "\t8. Launch a server reading a profile_name configuration from XML file.\n\n" + "\t $ " FAST_SERVER_BINARY " -x profile_name@config.xml\n\n" + + "\t9. Launch a server listening on localhost with default TCP port 42100\n\n" + "\t $ " FAST_SERVER_BINARY " -t 127.0.0.1\n\n" + + "\t10. Launch a server listening on localhost and Wi-Fi (192.163.6.34).\n" + "\t Two TCP ports need to be specified because TCP Transports cannot\n" + "\t share ports.\n\n" + "\t $ " FAST_SERVER_BINARY " -t 127.0.0.1 -q 42100 -t 192.163.6.34 \n" + "\t -q 42101"; + #endif // FASTDDS_SERVER_SERVER_H_ From f18377c2ae6d154f6f299605747dcdc5d4eab7e7 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Thu, 4 Apr 2024 12:59:56 +0200 Subject: [PATCH 05/51] Refs #20629: GUID less env.var. Signed-off-by: cferreiragonz --- .../discovery/participant/PDPClient.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index 2b8eb21dde2..50ed1405d76 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -1018,15 +1018,11 @@ bool load_environment_server_info( }; // Add new server - auto add_server2qos = [](int id, std::forward_list&& locators, RemoteServerList_t& attributes) + auto add_server2qos = [](std::forward_list&& locators, RemoteServerList_t& attributes) { RemoteServerAttributes server_att; - // Add the server to the list - if (!get_server_client_default_guidPrefix(id, server_att.guidPrefix)) - { - throw std::invalid_argument("The maximum number of default discovery servers has been reached"); - } + // To set the GUID is no necessary // Split multi and unicast locators auto unicast = std::partition(locators.begin(), locators.end(), IPLocator::isMulticast); @@ -1052,7 +1048,6 @@ bool load_environment_server_info( { // Do the parsing and populate the list Locator_t server_locator(LOCATOR_KIND_UDPv4, DEFAULT_ROS2_SERVER_PORT); - int server_id = 0; std::sregex_iterator server_it( list.begin(), @@ -1112,7 +1107,7 @@ bool load_environment_server_info( } // Add server to the list - add_server2qos(server_id, std::forward_list{server_locator}, attributes); + add_server2qos(std::forward_list{server_locator}, attributes); } // Try IPv6 next else if (std::regex_match(locator, mr, ROS2_IPV6_ADDRESSPORT_PATTERN, @@ -1151,7 +1146,7 @@ bool load_environment_server_info( } // Add server to the list - add_server2qos(server_id, std::forward_list{server_locator}, attributes); + add_server2qos(std::forward_list{server_locator}, attributes); } // Try resolve DNS else if (std::regex_match(locator, mr, ROS2_DNS_DOMAINPORT_PATTERN, @@ -1242,7 +1237,7 @@ bool load_environment_server_info( } // Add server to the list - add_server2qos(server_id, std::move(flist), attributes); + add_server2qos(std::move(flist), attributes); } // Try resolve TCP DNS else if (std::regex_match(locator, mr, ROS2_DNS_DOMAINPORT_PATTERN_TCP, @@ -1336,7 +1331,7 @@ bool load_environment_server_info( } // Add server to the list - add_server2qos(server_id, std::move(flist), attributes); + add_server2qos(std::move(flist), attributes); } else { @@ -1347,7 +1342,6 @@ bool load_environment_server_info( } // Advance to the next server if any - ++server_id; ++server_it; } From c15f0a63595c8a0b9b244f9c112979e706bf6dff Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Tue, 26 Mar 2024 11:51:45 +0100 Subject: [PATCH 06/51] Refs #20629: Turn servers_ into vector so it is modifyable Signed-off-by: cferreiragonz --- .../builtin/discovery/database/DiscoveryDataBase.cpp | 11 +++++++---- .../builtin/discovery/database/DiscoveryDataBase.hpp | 4 ++-- .../rtps/builtin/discovery/participant/PDPServer.cpp | 6 +++--- .../rtps/builtin/discovery/participant/PDPServer.hpp | 2 +- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index ab08c3ea0e0..50f63cd6a01 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -38,7 +38,7 @@ namespace ddb { DiscoveryDataBase::DiscoveryDataBase( fastdds::rtps::GuidPrefix_t server_guid_prefix, - std::set servers) + std::vector servers) : server_guid_prefix_(server_guid_prefix) , server_acked_by_all_(servers.size() == 0) , servers_(servers) @@ -66,7 +66,10 @@ void DiscoveryDataBase::add_server( fastdds::rtps::GuidPrefix_t server) { EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Server " << server << " added"); - servers_.insert(server); + if (std::find(servers_.begin(), servers_.end(), server) == servers_.end()) + { + servers_.push_back(server); + } } void DiscoveryDataBase::remove_related_alive_from_history_nts( @@ -1646,7 +1649,7 @@ bool DiscoveryDataBase::server_acked_by_my_servers() assert(this_server != participants_.end()); - for (auto prefix : servers_) + for (const auto prefix : servers_) { if (!this_server->second.is_matched(prefix)) { @@ -1663,7 +1666,7 @@ std::vector DiscoveryDataBase::ack_pending_servers( std::vector ack_pending_servers; // Find the server's participant and check whether all its servers have ACKed the server's DATA(p) auto this_server = participants_.find(server_guid_prefix_); - for (auto prefix : servers_) + for (const auto prefix : servers_) { if (!this_server->second.is_matched(prefix)) { diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp index caaea824558..b7145b5354d 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp @@ -118,7 +118,7 @@ class DiscoveryDataBase DiscoveryDataBase( fastdds::rtps::GuidPrefix_t server_guid_prefix, - std::set servers); + std::vector servers); ~DiscoveryDataBase(); @@ -568,7 +568,7 @@ class DiscoveryDataBase std::atomic server_acked_by_all_; //! List of GUID prefixes of the remote servers - std::set servers_; + std::vector servers_; // The virtual topic associated with virtual writers and readers const std::string virtual_topic_ = "eprosima_server_virtual_topic"; diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index 1a62948bff6..75a5016b842 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -1571,15 +1571,15 @@ bool PDPServer::pending_ack() return ret; } -std::set PDPServer::servers_prefixes() +std::vector PDPServer::servers_prefixes() { std::lock_guard lock(*getMutex()); - std::set servers; + std::vector servers; eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - servers.insert(it.guidPrefix); + servers.push_back(it.guidPrefix); } return servers; } diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp index 792b2b64f52..69d4b253f4c 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp @@ -286,7 +286,7 @@ class PDPServer : public fastdds::rtps::PDP nlohmann::json& ddb_json, std::vector& new_changes); - std::set servers_prefixes(); + std::vector servers_prefixes(); // General file name for the prefix of every backup file std::ostringstream get_persistence_file_name_() const; From d6690a01020e6a860836d792a40002ec6b0fa9f4 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Mon, 1 Apr 2024 11:04:42 +0200 Subject: [PATCH 07/51] Refs #20629: Check participant type from ParameterPropertyList Signed-off-by: cferreiragonz --- .../builtin/discovery/participant/PDP.cpp | 65 +++++++++++++++++++ .../rtps/builtin/discovery/participant/PDP.h | 10 +++ 2 files changed, 75 insertions(+) diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp index 971bf9ef3f2..ea7852df49d 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp @@ -226,6 +226,71 @@ bool PDP::data_matches_with_prefix( return ret_val; } +std::string PDP::check_participant_type( + const eprosima::fastrtps::ParameterPropertyList_t properties) +{ + auto participant_type = std::find_if( + properties.begin(), + properties.end(), + [](const fastdds::dds::ParameterProperty_t& property) + { + return property.first() == fastdds::dds::parameter_property_participant_type; + }); + + if (participant_type != properties.end()) + { + if (participant_type->second() == fastdds::rtps::ParticipantType::SERVER) + { + return fastdds::rtps::ParticipantType::SERVER; + } + else if (participant_type->second() == fastdds::rtps::ParticipantType::BACKUP) + { + return fastdds::rtps::ParticipantType::BACKUP; + } + else if (participant_type->second() == fastdds::rtps::ParticipantType::SIMPLE) + { + return fastdds::rtps::ParticipantType::SIMPLE; + } + else if (participant_type->second() == fastdds::rtps::ParticipantType::CLIENT) + { + return fastdds::rtps::ParticipantType::CLIENT; + } + else if (participant_type->second() == fastdds::rtps::ParticipantType::SUPER_CLIENT) + { + return fastdds::rtps::ParticipantType::SUPER_CLIENT; + } + else + { + EPROSIMA_LOG_ERROR(RTPS_PDP_LISTENER, "Wrong " << fastdds::dds::parameter_property_participant_type << ": " + << participant_type->second()); + return fastdds::rtps::ParticipantType::UNKNOWN; + } + } + else + { + EPROSIMA_LOG_INFO(RTPS_PDP_LISTENER, fastdds::dds::parameter_property_participant_type << " is not set"); + // Fallback to checking whether participant is a SERVER looking for the persistence GUID + auto persistence_guid = std::find_if( + properties.begin(), + properties.end(), + [](const fastdds::dds::ParameterProperty_t& property) + { + return property.first() == fastdds::dds::parameter_property_persistence_guid; + }); + // The presence of persistence GUID property suggests a SERVER. This assumption is made to keep + // backwards compatibility with Discovery Server v1.0. However, any participant that has been configured + // as persistent will have this property. + if (persistence_guid != properties.end()) + { + return fastdds::rtps::ParticipantType::SERVER; + } + else + { + return fastdds::rtps::ParticipantType::CLIENT; + } + } +} + void PDP::initializeParticipantProxyData( ParticipantProxyData* participant_data) { diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.h b/src/cpp/rtps/builtin/discovery/participant/PDP.h index ed95dedfe42..5a49f20c9d9 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDP.h +++ b/src/cpp/rtps/builtin/discovery/participant/PDP.h @@ -572,6 +572,16 @@ class PDP : public fastdds::statistics::rtps::IProxyQueryable const GuidPrefix_t& guid_prefix, const ParticipantProxyData& participant_data); + /** + * Obtains the participant type based on a Parameters Properties list. + * + * @param properties Parameter Properties list to check + * + * @return struct of ParticipantType indicating the type. + */ + std::string check_participant_type( + const eprosima::fastrtps::ParameterPropertyList_t properties); + /** * Gets the key of a participant proxy data. * From 8ce59b3ba1995891049630d17e9392da4a158ad5 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Fri, 5 Apr 2024 09:41:22 +0200 Subject: [PATCH 08/51] Refs #20629: Refactor PDPClient Signed-off-by: cferreiragonz --- .../rtps/attributes/ServerAttributes.hpp | 4 - .../discovery/participant/PDPClient.cpp | 116 ++++++++++-------- .../builtin/discovery/participant/PDPClient.h | 9 ++ .../participant/timedevent/DSClientEvent.cpp | 17 ++- 4 files changed, 82 insertions(+), 64 deletions(-) diff --git a/include/fastdds/rtps/attributes/ServerAttributes.hpp b/include/fastdds/rtps/attributes/ServerAttributes.hpp index d1d6ff43915..eeeb39f61f9 100644 --- a/include/fastdds/rtps/attributes/ServerAttributes.hpp +++ b/include/fastdds/rtps/attributes/ServerAttributes.hpp @@ -56,7 +56,6 @@ class RemoteServerAttributes guidPrefix = fastdds::rtps::GuidPrefix_t::unknown(); metatrafficUnicastLocatorList.clear(); metatrafficMulticastLocatorList.clear(); - is_connected = false; } FASTDDS_EXPORTED_API fastdds::rtps::GUID_t GetParticipant() const; @@ -78,9 +77,6 @@ class RemoteServerAttributes //!Guid prefix fastdds::rtps::GuidPrefix_t guidPrefix; - // Whether connection has been established - bool is_connected = false; - // Check if there are specific transport locators associated // the template parameter is the locator kind (e.g. LOCATOR_KIND_UDPv4) template bool requires_transport() const diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index 50ed1405d76..a868536a6f8 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -189,17 +189,10 @@ ParticipantProxyData* PDPClient::createParticipantProxyData( // Verify if this participant is a server bool is_server = false; - + std::string part_type = check_participant_type(participant_data.m_properties); + if (part_type == ParticipantType::SERVER || part_type == ParticipantType::BACKUP) { - eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - - for (auto& svr : mp_builtin->m_DiscoveryServers) - { - if (data_matches_with_prefix(svr.guidPrefix, participant_data)) - { - is_server = true; - } - } + is_server = true; } ParticipantProxyData* pdata = add_participant_proxy_data(participant_data.m_guid, is_server, &participant_data); @@ -441,8 +434,7 @@ bool PDPClient::create_ds_pdp_reliable_endpoints( return false; } - // Perform matching with remote servers and ensure output channels are open in the transport for the corresponding - // locators + // Ensure output channels are open in the transport for the corresponding locators { eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); @@ -452,23 +444,11 @@ bool PDPClient::create_ds_pdp_reliable_endpoints( it.metatrafficUnicastLocatorList, it.metatrafficMulticastLocatorList); mp_RTPSParticipant->createSenderResources(entry); -#if HAVE_SECURITY - if (!mp_RTPSParticipant->is_secure()) - { - match_pdp_writer_nts_(it); - match_pdp_reader_nts_(it); - } - else if (!is_discovery_protected) - { - BaseReader::downcast(endpoints.reader.reader_)->allow_unknown_writers(); - } -#else + // If SECURITY is disabled, this condition is ALWAYS true if (!is_discovery_protected) { - match_pdp_writer_nts_(it); - match_pdp_reader_nts_(it); + BaseReader::downcast(endpoints.reader.reader_)->allow_unknown_writers(); } -#endif // HAVE_SECURITY } } @@ -486,12 +466,30 @@ void PDPClient::assignRemoteEndpoints( { eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - // Verify if this participant is a server - for (auto& svr : mp_builtin->m_DiscoveryServers) + std::string part_type = check_participant_type(pdata->m_properties); + if (part_type == ParticipantType::SERVER || part_type == ParticipantType::BACKUP) { - if (data_matches_with_prefix(svr.guidPrefix, *pdata)) + // Add new servers to the connected list + EPROSIMA_LOG_INFO(RTPS_PDP_CLIENT, "Server [" << pdata->m_guid.guidPrefix << "] matched."); + RemoteServerAttributes server; + server.guidPrefix = pdata->m_guid.guidPrefix; + for (const Locator_t& locator : pdata->metatraffic_locators.multicast) { - svr.is_connected = true; + server.metatrafficMulticastLocatorList.push_back(locator); + } + for (const Locator_t& locator : pdata->metatraffic_locators.unicast) + { + server.metatrafficUnicastLocatorList.push_back(locator); + } + connected_servers_.push_back(server); + + // Match incoming server +#if HAVE_SECURITY + if (!should_protect_discovery()) +#endif // HAVE_SECURITY + { + match_pdp_writer_nts_(server); + match_pdp_reader_nts_(server); } } } @@ -503,6 +501,10 @@ void PDPClient::assignRemoteEndpoints( perform_builtin_endpoints_matching(*pdata); } } + else + { + EPROSIMA_LOG_INFO(RTPS_PDP, "Ignoring new participant " << pdata->m_guid); + } } void PDPClient::notifyAboveRemoteEndpoints( @@ -595,12 +597,12 @@ void PDPClient::removeRemoteEndpoints( eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); // Verify if this participant is a server - for (auto& svr : mp_builtin->m_DiscoveryServers) + for (auto it = connected_servers_.begin(); it != connected_servers_.end(); ++it) { - if (svr.guidPrefix == pdata->m_guid.guidPrefix) + if (it->guidPrefix == pdata->m_guid.guidPrefix) { std::unique_lock lock(*getMutex()); - svr.is_connected = false; + it = connected_servers_.erase(it); is_server = true; mp_sync->restart_timer(); // enable announcement and sync mechanism till this server reappears } @@ -768,15 +770,11 @@ void PDPClient::announceParticipantState( // Temporary workaround eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - for (auto& svr : mp_builtin->m_DiscoveryServers) + for (auto& svr: connected_servers_) { - // If we are matched to a server report demise - if (svr.is_connected) - { - locators.push_back(svr.metatrafficUnicastLocatorList); - remote_readers.emplace_back(svr.guidPrefix, - endpoints->reader.reader_->getGuid().entityId); - } + locators.push_back(svr.metatrafficUnicastLocatorList); + remote_readers.emplace_back(svr.guidPrefix, + endpoints->reader.reader_->getGuid().entityId); } } @@ -803,11 +801,24 @@ void PDPClient::announceParticipantState( eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - for (auto& svr : mp_builtin->m_DiscoveryServers) + // An already connected server will be pinged again if there exists a non-connected server (2 or more servers scenario). + // This is because we no longer use the GUID to match servers, so we cannot discern which servers are connected + // and which are not. We cannot map servers in m_DiscoveryServers to connected_servers_. + + // Ping not-connected servers + if (connected_servers_.size() < mp_builtin->m_DiscoveryServers.size()) + { + for (auto& svr : mp_builtin->m_DiscoveryServers) + { + locators.push_back(svr.metatrafficMulticastLocatorList); + locators.push_back(svr.metatrafficUnicastLocatorList); + } + } + + // Announce liveliness (lease duration) to all servers + if (!_serverPing) { - // Non-pinging announcements like lease duration ones must be - // broadcast to all servers - if (!svr.is_connected || !_serverPing) + for (auto& svr : connected_servers_) { locators.push_back(svr.metatrafficMulticastLocatorList); locators.push_back(svr.metatrafficUnicastLocatorList); @@ -855,15 +866,7 @@ void PDPClient::update_remote_servers_list() mp_RTPSParticipant->createSenderResources(entry); } - if (!endpoints->reader.reader_->matched_writer_is_matched(it.GetPDPWriter())) - { - match_pdp_writer_nts_(it); - } - - if (!endpoints->writer.writer_->matched_reader_is_matched(it.GetPDPReader())) - { - match_pdp_reader_nts_(it); - } + endpoints->reader.reader_->enableMessagesFromUnkownWriters(true); } } mp_sync->restart_timer(); @@ -1435,6 +1438,11 @@ bool PDPClient::remove_remote_participant( return false; } +const std::list& PDPClient::connected_servers() +{ + return connected_servers_; +} + } /* namespace rtps */ } /* namespace fastdds */ } /* namespace eprosima */ diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.h b/src/cpp/rtps/builtin/discovery/participant/PDPClient.h index 96aecb2c4f1..446c9f838fe 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.h +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.h @@ -152,6 +152,12 @@ class PDPClient : public PDP */ void update_remote_servers_list(); + /** + * Get the list of remote servers to which the client is already connected. + * @return A reference to the list of RemoteServerAttributes + */ + const std::list& connected_servers(); + protected: void update_builtin_locators() override; @@ -253,6 +259,9 @@ class PDPClient : public PDP //! flag to know this client must use super client participant type bool _super_client; + + //! List of real connected servers + std::list connected_servers_; }; } /* namespace rtps */ diff --git a/src/cpp/rtps/builtin/discovery/participant/timedevent/DSClientEvent.cpp b/src/cpp/rtps/builtin/discovery/participant/timedevent/DSClientEvent.cpp index d11d0e35363..0dcb38b3bb4 100644 --- a/src/cpp/rtps/builtin/discovery/participant/timedevent/DSClientEvent.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/timedevent/DSClientEvent.cpp @@ -60,14 +60,12 @@ bool DSClientEvent::event() // Iterate over remote servers to check for new unmatched servers ParticipantProxyData* part_proxy_data; eprosima::shared_lock lock(mp_PDP->mp_builtin->getDiscoveryMutex()); - for (auto server: mp_PDP->remote_server_attributes()) + + for (auto server: mp_PDP->connected_servers()) { std::unique_lock pdp_lock(*mp_PDP->getMutex()); // Get the participant proxy data of the server part_proxy_data = mp_PDP->get_participant_proxy_data(server.guidPrefix); - - // If the server is known (meaning the client has a proxy for it), it means that this client has - // received the server's DATA(p), which in turn means that the server has received the client's DATA(p) if (nullptr != part_proxy_data) { // Match EDP endpoints with this server if necessary @@ -76,13 +74,20 @@ bool DSClientEvent::event() mp_EDP->assignRemoteEndpoints(*(part_proxy_data), true); } } - // If the server is not known, we need to run the event again else { - restart = true; + EPROSIMA_LOG_ERROR(CLIENT_PDP_THREAD, + "Error while retrieving Participant Proxy Data of a connected server: " + << server.guidPrefix); } } + if (mp_PDP->connected_servers().size() < mp_PDP->remote_server_attributes().size()) + { + // If there are pending servers, we need to run the event again + restart = true; + } + // If we are still not connected to all servers, we need to keep pinging the unmatched ones if (restart) { From cb08aba7c9858fc790a8385800eedaa530be2bf5 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Fri, 5 Apr 2024 09:58:07 +0200 Subject: [PATCH 09/51] Refs #20629: Refactor PDPServer Signed-off-by: cferreiragonz --- .../discovery/database/DiscoveryDataBase.cpp | 21 ++- .../discovery/database/DiscoveryDataBase.hpp | 9 +- .../discovery/participant/PDPServer.cpp | 128 ++++++++++-------- .../discovery/participant/PDPServer.hpp | 18 ++- 4 files changed, 101 insertions(+), 75 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index 50f63cd6a01..60cfc078f99 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -37,11 +37,9 @@ namespace rtps { namespace ddb { DiscoveryDataBase::DiscoveryDataBase( - fastdds::rtps::GuidPrefix_t server_guid_prefix, - std::vector servers) + fastdds::rtps::GuidPrefix_t server_guid_prefix) : server_guid_prefix_(server_guid_prefix) - , server_acked_by_all_(servers.size() == 0) - , servers_(servers) + , server_acked_by_all_(true) , enabled_(true) , new_updates_(0) , processing_backup_(false) @@ -72,6 +70,21 @@ void DiscoveryDataBase::add_server( } } +void DiscoveryDataBase::remove_server( + fastrtps::rtps::GuidPrefix_t server) +{ + auto remove_it = std::find(servers_.begin(), servers_.end(), server); + if (remove_it != servers_.end()) + { + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Removing server " << server); + servers_.erase(remove_it); + } + else + { + EPROSIMA_LOG_ERROR(DISCOVERY_DATABASE, "Tried to remove " << server << " but it was not found"); + } +} + void DiscoveryDataBase::remove_related_alive_from_history_nts( fastdds::rtps::WriterHistory* writer_history, const fastdds::rtps::GuidPrefix_t& entity_guid_prefix) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp index b7145b5354d..9644b7d2bf4 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp @@ -117,8 +117,7 @@ class DiscoveryDataBase friend class AckedFunctor; DiscoveryDataBase( - fastdds::rtps::GuidPrefix_t server_guid_prefix, - std::vector servers); + fastdds::rtps::GuidPrefix_t server_guid_prefix); ~DiscoveryDataBase(); @@ -347,6 +346,10 @@ class DiscoveryDataBase void add_server( fastdds::rtps::GuidPrefix_t server); + //! Remove a server from the list of remote servers + void remove_server( + fastrtps::rtps::GuidPrefix_t server); + // Removes all the changes whose original sender was entity_guid_prefix from writer_history void remove_related_alive_from_history_nts( fastdds::rtps::WriterHistory* writer_history, @@ -567,7 +570,7 @@ class DiscoveryDataBase //! Is own server DATA(p) acked by all other clients std::atomic server_acked_by_all_; - //! List of GUID prefixes of the remote servers + //! List of GUID prefixes of the connected remote servers std::vector servers_; // The virtual topic associated with virtual writers and readers diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index 75a5016b842..6d42fd75f7c 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -61,8 +61,7 @@ PDPServer::PDPServer( : PDP(builtin, allocation) , routine_(nullptr) , ping_(nullptr) - , discovery_db_(builtin->mp_participantImpl->getGuid().guidPrefix, - servers_prefixes()) + , discovery_db_(builtin->mp_participantImpl->getGuid().guidPrefix) , durability_ (durability_kind) { // Add remote servers from environment variable @@ -79,7 +78,6 @@ PDPServer::PDPServer( mp_builtin->m_DiscoveryServers.push_back(server); } m_discovery.discovery_config.m_DiscoveryServers.push_back(server); - discovery_db_.add_server(server.guidPrefix); } } } @@ -186,6 +184,7 @@ void PDPServer::pre_enable_actions() if (durability_ == TRANSIENT) { // This vector is empty till backup queue is implemented + // TODO Carlos (OPT-IMPROVE): delete this? process_backup_restore_queue(backup_queue); } } @@ -205,15 +204,10 @@ ParticipantProxyData* PDPServer::createParticipantProxyData( if (!do_lease) { // If not a client verify this participant is a server + std::string part_type = check_participant_type(participant_data.m_properties); + if (part_type == ParticipantType::SERVER || part_type == ParticipantType::BACKUP) { - eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - for (auto& svr : mp_builtin->m_DiscoveryServers) - { - if (data_matches_with_prefix(svr.guidPrefix, participant_data)) - { - do_lease = true; - } - } + do_lease = true; } } @@ -354,6 +348,7 @@ bool PDPServer::create_ds_pdp_reliable_endpoints( DiscoveryServerPDPEndpoints& endpoints, bool secure) { + static_cast(secure); const RTPSParticipantAttributes& pattr = mp_RTPSParticipant->getRTPSParticipantAttributes(); /*********************************** @@ -498,8 +493,7 @@ bool PDPServer::create_ds_pdp_reliable_endpoints( // TODO check if this should be done here or before this point in creation endpoints.writer.history_->remove_all_changes(); - // Perform matching with remote servers and ensure output channels are open in the transport for the corresponding - // locators + // Ensure output channels are open in the transport for the corresponding locators { eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); @@ -508,12 +502,6 @@ bool PDPServer::create_ds_pdp_reliable_endpoints( auto entry = LocatorSelectorEntry::create_fully_selected_entry( it.metatrafficUnicastLocatorList, it.metatrafficMulticastLocatorList); mp_RTPSParticipant->createSenderResources(entry); - - if (!secure) - { - match_pdp_writer_nts_(it); - match_pdp_reader_nts_(it); - } } } @@ -647,8 +635,31 @@ void PDPServer::match_reliable_pdp_endpoints( void PDPServer::assignRemoteEndpoints( ParticipantProxyData* pdata) { - EPROSIMA_LOG_INFO(RTPS_PDP_SERVER, "Assigning remote endpoint for RTPSParticipant: " << pdata->m_guid.guidPrefix); + { + eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); + + // If the received participant GUID is from a server, update the servers list and DB and match the endpoints + std::string part_type = check_participant_type(pdata->m_properties); + if (part_type == ParticipantType::SERVER || part_type == ParticipantType::BACKUP) + { + EPROSIMA_LOG_INFO(RTPS_PDP_SERVER, "Assigning remote endpoint for SERVER: " << pdata->m_guid.guidPrefix); + + // Update DisvoveryDataBase + discovery_db_.add_server(pdata->m_guid.guidPrefix); +#if HAVE_SECURITY + if (!should_protect_discovery()) +#endif // HAVE_SECURITY + { + match_pdp_writer_nts_(*pdata); + match_pdp_reader_nts_(*pdata); + } + } + else + { + EPROSIMA_LOG_INFO(RTPS_PDP_SERVER, "Assigning remote endpoint for CLIENT: " << pdata->m_guid.guidPrefix); + } + } match_reliable_pdp_endpoints(*pdata); #if HAVE_SECURITY @@ -750,6 +761,14 @@ void PDPServer::removeRemoteEndpoints( << " did not send information about builtin readers"); return; } + + // Remove remote server + std::string part_type = check_participant_type(pdata->m_properties); + if (part_type == ParticipantType::SERVER || part_type == ParticipantType::BACKUP) + { + eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); + discovery_db_.remove_server(pdata->m_guid.guidPrefix); + } } std::ostringstream PDPServer::get_persistence_file_name_() const @@ -1174,23 +1193,9 @@ void PDPServer::update_remote_servers_list() it.metatrafficUnicastLocatorList, it.metatrafficMulticastLocatorList); mp_RTPSParticipant->createSenderResources(entry); } - - if (!endpoints->reader.reader_->matched_writer_is_matched(it.GetPDPWriter())) - { - match_pdp_writer_nts_(it); - } - - if (!endpoints->writer.writer_->matched_reader_is_matched(it.GetPDPReader())) - { - match_pdp_reader_nts_(it); - } - } - - for (auto server : mp_builtin->m_DiscoveryServers) - { - discovery_db_.add_server(server.guidPrefix); } + // TODO Carlos (PINGS): right now this is never shut down. ¿Enter into ping with high frequency loop? // Need to reactivate the server thread to send the DATA(p) to the new servers awake_server_thread(); } @@ -1571,19 +1576,6 @@ bool PDPServer::pending_ack() return ret; } -std::vector PDPServer::servers_prefixes() -{ - std::lock_guard lock(*getMutex()); - std::vector servers; - eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) - { - servers.push_back(it.guidPrefix); - } - return servers; -} - eprosima::fastdds::rtps::ResourceEvent& PDPServer::get_resource_event_thread() { return resource_event_thread_; @@ -2006,35 +1998,55 @@ void PDPServer::process_backup_store() } void PDPServer::match_pdp_writer_nts_( - const eprosima::fastdds::rtps::RemoteServerAttributes& server_att) + const ParticipantProxyData& pdata) { auto endpoints = static_cast(builtin_endpoints_.get()); const NetworkFactory& network = mp_RTPSParticipant->network_factory(); auto temp_writer_data = get_temporary_writer_proxies_pool().get(); temp_writer_data->clear(); - temp_writer_data->guid(server_att.GetPDPWriter()); - temp_writer_data->set_multicast_locators(server_att.metatrafficMulticastLocatorList, network); - temp_writer_data->set_remote_unicast_locators(server_att.metatrafficUnicastLocatorList, network); + temp_writer_data->guid({ pdata.m_guid.guidPrefix, endpoints->writer.writer_->getGuid().entityId }); + temp_writer_data->set_remote_locators(pdata.metatraffic_locators, network, true); temp_writer_data->m_qos.m_durability.durabilityKind(durability_); temp_writer_data->m_qos.m_reliability.kind = dds::RELIABLE_RELIABILITY_QOS; - endpoints->reader.reader_->matched_writer_add(*temp_writer_data); +#if HAVE_SECURITY + if (should_protect_discovery()) + { + mp_RTPSParticipant->security_manager().discovered_builtin_writer( + endpoints->reader.reader_->getGuid(), { pdata.m_guid.guidPrefix, c_EntityId_RTPSParticipant }, + *temp_writer_data, endpoints->reader.reader_->getAttributes().security_attributes()); + } + else +#endif // HAVE_SECURITY + { + endpoints->reader.reader_->matched_writer_add(*temp_writer_data); + } } void PDPServer::match_pdp_reader_nts_( - const eprosima::fastdds::rtps::RemoteServerAttributes& server_att) + const ParticipantProxyData& pdata) { auto endpoints = static_cast(builtin_endpoints_.get()); const NetworkFactory& network = mp_RTPSParticipant->network_factory(); auto temp_reader_data = get_temporary_reader_proxies_pool().get(); temp_reader_data->clear(); - temp_reader_data->guid(server_att.GetPDPReader()); - temp_reader_data->set_multicast_locators(server_att.metatrafficMulticastLocatorList, network); - temp_reader_data->set_remote_unicast_locators(server_att.metatrafficUnicastLocatorList, network); + temp_reader_data->guid({ pdata.m_guid.guidPrefix, endpoints->reader.reader_->getGuid().entityId }); + temp_reader_data->set_remote_locators(pdata.metatraffic_locators, network, true); temp_reader_data->m_qos.m_durability.kind = dds::TRANSIENT_LOCAL_DURABILITY_QOS; temp_reader_data->m_qos.m_reliability.kind = dds::RELIABLE_RELIABILITY_QOS; - endpoints->writer.writer_->matched_reader_add(*temp_reader_data); +#if HAVE_SECURITY + if (should_protect_discovery()) + { + mp_RTPSParticipant->security_manager().discovered_builtin_reader( + endpoints->writer.writer_->getGuid(), { pdata.m_guid.guidPrefix, c_EntityId_RTPSParticipant }, + *temp_reader_data, endpoints->writer.writer_->getAttributes().security_attributes()); + } + else +#endif // HAVE_SECURITY + { + endpoints->writer.writer_->matched_reader_add(*temp_reader_data); + } } void PDPServer::release_change_from_writer( diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp index 69d4b253f4c..acfc63481a7 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp @@ -286,8 +286,6 @@ class PDPServer : public fastdds::rtps::PDP nlohmann::json& ddb_json, std::vector& new_changes); - std::vector servers_prefixes(); - // General file name for the prefix of every backup file std::ostringstream get_persistence_file_name_() const; @@ -299,20 +297,20 @@ class PDPServer : public fastdds::rtps::PDP void process_backup_store(); /** - * Manually match the local PDP reader with the PDP writer of a given server. The function is - * not thread safe (nts) in the sense that it does not take the PDP mutex. It does however take - * temp_data_lock_ + * Manually match the local PDP reader with the PDP writer of a given partipant of type server. + * The function is not thread safe (nts) in the sense that it does not take the PDP mutex. + * It does however take temp_data_lock_ */ void match_pdp_writer_nts_( - const eprosima::fastdds::rtps::RemoteServerAttributes& server_att); + const fastrtps::rtps::ParticipantProxyData& pdata); /** - * Manually match the local PDP writer with the PDP reader of a given server. The function is - * not thread safe (nts) in the sense that it does not take the PDP mutex. It does however take - * temp_data_lock_ + * Manually match the local PDP writer with the PDP reader of a given partipant of type server. + * The function is not thread safe (nts) in the sense that it does not take the PDP mutex. + * It does however take temp_data_lock_ */ void match_pdp_reader_nts_( - const eprosima::fastdds::rtps::RemoteServerAttributes& server_att); + const fastrtps::rtps::ParticipantProxyData& pdata); /** * Release a change from the history of the PDP writer. From 1675e01800b8e1f54b5b3f3a86d3088e157aa5bd Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Fri, 5 Apr 2024 10:00:57 +0200 Subject: [PATCH 10/51] Refs #20629: Send Data(p) of servers to create a mesh topology Signed-off-by: cferreiragonz --- .../discovery/database/DiscoveryDataBase.cpp | 39 +++++++++++++++++++ .../participant/PDPServerListener.cpp | 4 +- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index 60cfc078f99..0a867c2fdd4 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -619,6 +619,45 @@ void DiscoveryDataBase::match_new_server_( assert(our_data_it != participants_.end()); add_pdp_to_send_(our_data_it->second.change()); + // To obtain a mesh topology, we need to: + // - Make all known servers relevant to the new server + // - Make the new server relevant to all known servers + // - Send DATA(p) of all known servers to the new server + // - Send Data(p) of the new server to all other servers + for (auto& part : participants_) + { + if (part.first != server_guid_prefix_ && !part.second.is_client()) + { + if (part.first == participant_prefix) + { + bool resend_new_pdp = false; + for (auto& server: servers_) + { + if (server != participant_prefix) + { + // Make all known servers relevant to the new server, but not matched + part.second.add_or_update_ack_participant(server, false); + resend_new_pdp = true; + } + } + if (resend_new_pdp) + { + // Send DATA(p) of the new server to all other servers. + add_pdp_to_send_(part.second.change()); + } + } + else + { + // Make the new server relevant to all known servers + part.second.add_or_update_ack_participant(participant_prefix, false); + // Send DATA(p) of all known servers to the new participant + add_pdp_to_send_(part.second.change()); + } + } + } + // The resources needed for TCP new connections are created during the matching process when the + // DATA(p) is receieved by each server. + // Create virtual endpoints create_virtual_endpoints_(participant_prefix); } diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp index fca8d838ea9..1a6d19a010c 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp @@ -248,8 +248,8 @@ void PDPServerListener::on_new_cache_change_added( lock.unlock(); // All local builtins are connected, the database will avoid any EDP DATA to be send before having PDP - // DATA acknowledgement - if (pdata && is_local) + // DATA acknowledgement. Non-local SERVERs will also be connected + if ( pdata && (is_local || !is_client)) { pdp_server()->assignRemoteEndpoints(pdata); } From 2daf24eef7792a7321435b0de4f51ee8b9b51aa5 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Fri, 5 Apr 2024 10:06:16 +0200 Subject: [PATCH 11/51] Refs #20629: Do not send Data(p) of remote clients Signed-off-by: cferreiragonz --- .../builtin/discovery/database/DiscoveryDataBase.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index 0a867c2fdd4..4abf1308dc7 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -1148,9 +1148,9 @@ void DiscoveryDataBase::match_writer_reader_( { // Writer virtual - // If reader is virtual do not exchange info - // If not, writer needs all the info from this endpoint - if (!reader_info.is_virtual()) + // If reader is virtual OR not local, do not exchange info. Servers do not redirect Data(p) of remote clients. + // Otherwise, writer needs all the info from this endpoint + if (!reader_info.is_virtual() && reader_participant_info.is_local()) { // Only if they do not have the info yet if (!reader_participant_info.is_relevant_participant(writer_guid.guidPrefix)) @@ -1228,9 +1228,9 @@ void DiscoveryDataBase::match_writer_reader_( { // Writer external - // if reader is external do not exchange info - // if not, reader needs all the info from this endpoint - if (reader_participant_info.is_local()) + // If reader is external OR virtual, do not exchange info. Servers do not redirect Data(p) of remote clients. + // Otherwise, reader needs all the info from this endpoint + if (reader_participant_info.is_local() && !reader_info.is_virtual()) { // Only if they do not have the info yet if (!writer_participant_info.is_relevant_participant(reader_guid.guidPrefix)) From b5dcd4aafa4e496531ac99acb3ab7bb98648cc8c Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Mon, 8 Apr 2024 08:25:00 +0200 Subject: [PATCH 12/51] Refs #20629: Update Servers' ping routine Signed-off-by: cferreiragonz --- .../discovery/database/DiscoveryDataBase.cpp | 42 ------------- .../discovery/database/DiscoveryDataBase.hpp | 4 -- .../builtin/discovery/participant/PDP.cpp | 10 ++++ .../rtps/builtin/discovery/participant/PDP.h | 5 ++ .../discovery/participant/PDPServer.cpp | 60 +++++-------------- .../discovery/participant/PDPServer.hpp | 16 +---- .../participant/timedevent/DServerEvent.cpp | 35 ----------- .../participant/timedevent/DServerEvent.hpp | 23 ------- 8 files changed, 32 insertions(+), 163 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index 4abf1308dc7..82075fc37b7 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -1686,48 +1686,6 @@ const std::vector DiscoveryDataBase::direct_clients return direct_clients_and_servers; } -bool DiscoveryDataBase::server_acked_by_my_servers() -{ - std::lock_guard guard(mutex_); - - if (servers_.size() == 0) - { - return true; - } - - // Find the server's participant and check whether all its servers have ACKed the server's DATA(p) - auto this_server = participants_.find(server_guid_prefix_); - // check it is always there - - assert(this_server != participants_.end()); - - for (const auto prefix : servers_) - { - if (!this_server->second.is_matched(prefix)) - { - return false; - } - } - return true; -} - -std::vector DiscoveryDataBase::ack_pending_servers() -{ - std::lock_guard guard(mutex_); - - std::vector ack_pending_servers; - // Find the server's participant and check whether all its servers have ACKed the server's DATA(p) - auto this_server = participants_.find(server_guid_prefix_); - for (const auto prefix : servers_) - { - if (!this_server->second.is_matched(prefix)) - { - ack_pending_servers.push_back(prefix); - } - } - return ack_pending_servers; -} - LocatorList DiscoveryDataBase::participant_metatraffic_locators( fastdds::rtps::GuidPrefix_t participant_guid_prefix) { diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp index 9644b7d2bf4..432ac4904e7 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp @@ -288,10 +288,6 @@ class DiscoveryDataBase server_acked_by_all_.store(s); } - bool server_acked_by_my_servers(); - - std::vector ack_pending_servers(); - //////////// // Data structures utils diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp index ea7852df49d..cde17efdd98 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp @@ -1493,6 +1493,16 @@ void PDP::set_initial_announcement_interval() set_next_announcement_interval(); } +void PDP::resend_ininitial_announcements() +{ + if (enabled_) + { + initial_announcements_ = m_discovery.discovery_config.initial_announcements; + set_next_announcement_interval(); + resetParticipantAnnouncement(); + } +} + void PDP::set_external_participant_properties_( ParticipantProxyData* participant_data) { diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.h b/src/cpp/rtps/builtin/discovery/participant/PDP.h index 5a49f20c9d9..12d05a5a7a6 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDP.h +++ b/src/cpp/rtps/builtin/discovery/participant/PDP.h @@ -616,6 +616,11 @@ class PDP : public fastdds::statistics::rtps::IProxyQueryable ParticipantProxyData* pdata, bool& should_be_ignored); + /** + * Restores the `initial_announcements_` configuration to resend the initial announcements again. + */ + void resend_ininitial_announcements(); + #ifdef FASTDDS_STATISTICS std::atomic proxy_observer_; diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index 6d42fd75f7c..27c329f5112 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -60,7 +60,6 @@ PDPServer::PDPServer( DurabilityKind_t durability_kind /* TRANSIENT_LOCAL */) : PDP(builtin, allocation) , routine_(nullptr) - , ping_(nullptr) , discovery_db_(builtin->mp_participantImpl->getGuid().guidPrefix) , durability_ (durability_kind) { @@ -87,14 +86,12 @@ PDPServer::~PDPServer() { // Stop timed events routine_->cancel_timer(); - ping_->cancel_timer(); // Disable database discovery_db_.disable(); // Delete timed events delete(routine_); - delete(ping_); // Clear ddb and release its changes process_changes_release_(discovery_db_.clear()); @@ -136,15 +133,6 @@ bool PDPServer::init( TimeConv::Duration_t2MilliSecondsDouble( m_discovery.discovery_config.discoveryServer_client_syncperiod)); - /* - Given the fact that a participant is either a client or a server the - discoveryServer_client_syncperiod parameter has a context defined meaning. - */ - ping_ = new DServerPingEvent(this, - TimeConv::Duration_t2MilliSecondsDouble( - m_discovery.discovery_config.discoveryServer_client_syncperiod)); - ping_->restart_timer(); - return true; } @@ -953,6 +941,9 @@ void PDPServer::announceParticipantState( return; } } + + // Ping remote servers. It is done separately to avoid setting wrong remote_readers + ping_remote_servers(); } else { @@ -1089,8 +1080,8 @@ bool PDPServer::remove_remote_participant( } } - // Check if is a server who has been disposed - awake_server_thread(); + // Resend participant announcements to try to reconnect faster + resend_ininitial_announcements(); // Delegate into the base class for inherited proxy database removal return PDP::remove_remote_participant(partGUID, reason); @@ -1111,11 +1102,6 @@ void PDPServer::awake_routine_thread( routine_->restart_timer(); } -void PDPServer::awake_server_thread() -{ - ping_->restart_timer(); -} - bool PDPServer::server_update_routine() { // There is pending work to be done by the server if there are changes that have not been acknowledged. @@ -1195,9 +1181,8 @@ void PDPServer::update_remote_servers_list() } } - // TODO Carlos (PINGS): right now this is never shut down. ¿Enter into ping with high frequency loop? // Need to reactivate the server thread to send the DATA(p) to the new servers - awake_server_thread(); + resend_ininitial_announcements(); } bool PDPServer::process_writers_acknowledgements() @@ -1581,41 +1566,28 @@ eprosima::fastdds::rtps::ResourceEvent& PDPServer::get_resource_event_thread() return resource_event_thread_; } -bool PDPServer::all_servers_acknowledge_pdp() -{ - // Check if already initialized - auto endpoints = static_cast(builtin_endpoints_.get()); - static_cast(endpoints); - assert(endpoints->writer.history_ && endpoints->writer.writer_); - - return discovery_db_.server_acked_by_my_servers(); -} - void PDPServer::ping_remote_servers() { - // Get the servers that have not ACKed this server's DATA(p) - std::vector ack_pending_servers = discovery_db_.ack_pending_servers(); + LocatorList locators_ping; std::vector remote_readers; - LocatorList locators; // Iterate over the list of servers { std::lock_guard lock(*getMutex()); eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - for (auto& server : mp_builtin->m_DiscoveryServers) { - - // If the server is the the ack_pending list, then add its GUID and locator to send the announcement - auto server_it = std::find(ack_pending_servers.begin(), ack_pending_servers.end(), server.guidPrefix); - if (server_it != ack_pending_servers.end()) - { - // Get the info to send to this already known locators - locators.push_back(server.metatrafficUnicastLocatorList); - } + // Get the info to send to this already known locators + locators_ping.push_back(server.metatrafficUnicastLocatorList); } } - send_announcement(discovery_db().cache_change_own_participant(), remote_readers, locators); + + if (!locators_ping.empty()) + { + EPROSIMA_LOG_INFO(RTPS_PDP_SERVER, + "Server " << getRTPSParticipant()->getGuid() << " PDP announcement"); + send_announcement(discovery_db().cache_change_own_participant(), remote_readers, locators_ping); + } } void PDPServer::send_announcement( diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp index acfc63481a7..e93e9950705 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp @@ -127,7 +127,7 @@ class PDPServer : public fastdds::rtps::PDP bool dispose = false, fastdds::rtps::WriteParams& wparams = fastdds::rtps::WriteParams::WRITE_PARAM_DEFAULT) override; - // Force the sending of our DATA(p) to those servers that has not acked yet + // Force the sending of our DATA(p) to those servers in the initial server list void ping_remote_servers(); // send a specific Data to specific locators @@ -179,15 +179,6 @@ class PDPServer : public fastdds::rtps::PDP void awake_routine_thread( double interval_ms = 0); - void awake_server_thread(); - - /** - * Check if all servers have acknowledge this server PDP data - * This method must be called from a mutex protected context. - * @return True if all can reach the client - */ - bool all_servers_acknowledge_pdp(); - /* The server's main routine. This includes all the discovery related tasks that the server needs to run * periodically to keep the discovery graph updated. * @return: True if there is pending work, false otherwise. @@ -380,11 +371,6 @@ class PDPServer : public fastdds::rtps::PDP */ DServerRoutineEvent* routine_; - /** - * TimedEvent for server ping to other servers - */ - DServerPingEvent* ping_; - //! Discovery database fastdds::rtps::ddb::DiscoveryDataBase discovery_db_; diff --git a/src/cpp/rtps/builtin/discovery/participant/timedevent/DServerEvent.cpp b/src/cpp/rtps/builtin/discovery/participant/timedevent/DServerEvent.cpp index 2ed27e6b299..be796e85b68 100644 --- a/src/cpp/rtps/builtin/discovery/participant/timedevent/DServerEvent.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/timedevent/DServerEvent.cpp @@ -61,41 +61,6 @@ bool DServerRoutineEvent::server_routine_event() return pending_work; } -DServerPingEvent::DServerPingEvent( - PDPServer* pdp, - double interval) - : TimedEvent(pdp->getRTPSParticipant()->getEventResource(), - [this]() - { - return server_ping_event(); - }, interval) - , pdp_(pdp) -{ - -} - -DServerPingEvent::~DServerPingEvent() -{ -} - -bool DServerPingEvent::server_ping_event() -{ - // Check if all servers received my discovery data - if (!pdp_->all_servers_acknowledge_pdp()) - { - // Not all servers have yet received our DATA(p) thus resend - pdp_->ping_remote_servers(); - EPROSIMA_LOG_INFO(SERVER_PING_THREAD, - "Server " << pdp_->getRTPSParticipant()->getGuid() << " PDP announcement"); - - // restart - return true; - } - - // do not restart - return false; -} - } // namespace rtps } // namespace fastdds } // namespace eprosima diff --git a/src/cpp/rtps/builtin/discovery/participant/timedevent/DServerEvent.hpp b/src/cpp/rtps/builtin/discovery/participant/timedevent/DServerEvent.hpp index 5b3b91507a6..7a48b03fe0a 100644 --- a/src/cpp/rtps/builtin/discovery/participant/timedevent/DServerEvent.hpp +++ b/src/cpp/rtps/builtin/discovery/participant/timedevent/DServerEvent.hpp @@ -60,29 +60,6 @@ class DServerRoutineEvent : public TimedEvent double server_routine_period_; }; -class DServerPingEvent : public TimedEvent -{ -public: - - /** - * Constructor. - * @param pdp Pointer to the PDPServer. - * @param interval Interval in ms. - */ - DServerPingEvent( - PDPServer* pdp, - double interval); - ~DServerPingEvent(); - - /** - * Method invoked when the server routine event occurs. - */ - bool server_ping_event(); - - //!Pointer to the PDPServer object. - PDPServer* pdp_; -}; - } // namespace rtps } // namespace fastdds } // namespace eprosima From c9daac56ea07514ea7c7e4edd9b11ed0a93b8357 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Mon, 8 Apr 2024 08:59:03 +0200 Subject: [PATCH 13/51] Refs #20629: Refactor update_attributes Signed-off-by: cferreiragonz --- .../fastdds/domain/DomainParticipantImpl.cpp | 26 ------ .../rtps/participant/RTPSParticipantImpl.cpp | 89 ++----------------- 2 files changed, 5 insertions(+), 110 deletions(-) diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp index e9b72d13718..ade365b2378 100644 --- a/src/cpp/fastdds/domain/DomainParticipantImpl.cpp +++ b/src/cpp/fastdds/domain/DomainParticipantImpl.cpp @@ -1788,32 +1788,6 @@ bool DomainParticipantImpl::can_qos_be_updated( EPROSIMA_LOG_WARNING(RTPS_QOS_CHECK, "WireProtocolConfigQos cannot be changed after the participant is enabled, " << "with the exception of builtin.discovery_config.m_DiscoveryServers"); } - else - { - // This means that the only change is in wire_protocol().builtin.discovery_config.m_DiscoveryServers - // In that case, we need to ensure that the current list (to) is strictly contained in the incoming - // list (from). For that, we check that every server in the current list (to) is also in the incoming one - // (from) - for (auto existing_server : to.wire_protocol().builtin.discovery_config.m_DiscoveryServers) - { - bool contained = false; - for (auto incoming_server : from.wire_protocol().builtin.discovery_config.m_DiscoveryServers) - { - if (existing_server.guidPrefix == incoming_server.guidPrefix) - { - contained = true; - break; - } - } - if (!contained) - { - updatable = false; - EPROSIMA_LOG_WARNING(RTPS_QOS_CHECK, - "Discovery Servers cannot be removed from the list; they can only be added"); - break; - } - } - } } if (!(to.transport() == from.transport())) { diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp index 0b29d33ac7e..b16d7f3be2f 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp @@ -1489,8 +1489,6 @@ void RTPSParticipantImpl::update_attributes( || local_interfaces_changed) { update_pdp = true; - std::vector modified_servers; - LocatorList_t modified_locators; // Update RTPSParticipantAttributes members m_att.userData = patt.userData; @@ -1516,47 +1514,6 @@ void RTPSParticipantImpl::update_attributes( } } - // Check that the remote servers list is consistent: all the already known remote servers must be included in - // the list and either new remote servers are added or remote server listening locator is modified. - for (auto existing_server : m_att.builtin.discovery_config.m_DiscoveryServers) - { - bool contained = false; - for (auto incoming_server : converted_discovery_servers) - { - if (existing_server.guidPrefix == incoming_server.guidPrefix) - { - for (auto incoming_locator : incoming_server.metatrafficUnicastLocatorList) - { - bool locator_contained = false; - for (auto existing_locator : existing_server.metatrafficUnicastLocatorList) - { - if (incoming_locator == existing_locator) - { - locator_contained = true; - break; - } - } - if (!locator_contained) - { - modified_servers.emplace_back(incoming_server.GetParticipant()); - modified_locators.push_back(incoming_locator); - EPROSIMA_LOG_INFO(RTPS_QOS_CHECK, - "DS Server: " << incoming_server.guidPrefix << " has modified its locators: " - << incoming_locator << " being added"); - } - } - contained = true; - break; - } - } - if (!contained) - { - EPROSIMA_LOG_ERROR(RTPS_QOS_CHECK, - "Discovery Servers cannot be removed from the list; they can only be added"); - return; - } - } - { std::lock_guard lock(*pdp->getMutex()); @@ -1586,10 +1543,6 @@ void RTPSParticipantImpl::update_attributes( createSenderResources(m_att.builtin.metatrafficUnicastLocatorList); createSenderResources(m_att.defaultUnicastLocatorList); } - if (!modified_locators.empty()) - { - createSenderResources(modified_locators); - } // Update remote servers list if (m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::CLIENT || @@ -1597,33 +1550,11 @@ void RTPSParticipantImpl::update_attributes( m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::SERVER || m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::BACKUP) { - // Add incoming servers if we don't know about them already or the listening locator has been modified - for (auto incoming_server : converted_discovery_servers) - { - eprosima::fastdds::rtps::RemoteServerList_t::iterator server_it; - for (server_it = m_att.builtin.discovery_config.m_DiscoveryServers.begin(); - server_it != m_att.builtin.discovery_config.m_DiscoveryServers.end(); server_it++) - { - if (server_it->guidPrefix == incoming_server.guidPrefix) - { - // Check if the listening locators have been modified - for (auto guid : modified_servers) - { - if (guid == incoming_server.GetParticipant()) - { - server_it->metatrafficUnicastLocatorList = - incoming_server.metatrafficUnicastLocatorList; - break; - } - } - break; - } - } - if (server_it == m_att.builtin.discovery_config.m_DiscoveryServers.end()) - { - m_att.builtin.discovery_config.m_DiscoveryServers.push_back(incoming_server); - } - } + // Update list of Discovery Servers. The participant will remain connected to the servers of the + // previous list but will cease pinging servers that are not included in the new list. + // Liveliness will be maintained until the old server is removed from the participant, ensuring + // that existing connections are unaffected by the list update. + m_att.builtin.discovery_config.m_DiscoveryServers = converted_discovery_servers; // Update the servers list in builtin protocols { @@ -1637,11 +1568,6 @@ void RTPSParticipantImpl::update_attributes( { fastdds::rtps::PDPServer* pdp_server = static_cast(pdp); pdp_server->update_remote_servers_list(); - for (auto remote_server : modified_servers) - { - pdp_server->remove_remote_participant(remote_server, - ParticipantDiscoveryInfo::DISCOVERY_STATUS::DROPPED_PARTICIPANT); - } } // Notify PDPClient else if (m_att.builtin.discovery_config.discoveryProtocol == DiscoveryProtocol::CLIENT || @@ -1649,11 +1575,6 @@ void RTPSParticipantImpl::update_attributes( { fastdds::rtps::PDPClient* pdp_client = static_cast(pdp); pdp_client->update_remote_servers_list(); - for (auto remote_server : modified_servers) - { - pdp_client->remove_remote_participant(remote_server, - ParticipantDiscoveryInfo::DISCOVERY_STATUS::DROPPED_PARTICIPANT); - } } } } From b79ce66e6e8c0f76a2c97ab433812256636d05a6 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Mon, 8 Apr 2024 16:11:28 +0200 Subject: [PATCH 14/51] Refs #20629: Avoid trying to create extra resources Signed-off-by: cferreiragonz --- src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index a868536a6f8..646eca5be59 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -1433,7 +1433,8 @@ bool PDPClient::remove_remote_participant( rguid.entityId = endpoints->reader.reader_->getGuid().entityId; endpoints->writer.writer_->matched_reader_remove(rguid); - update_remote_servers_list(); + // Reactivate ping routine + mp_sync->restart_timer(); return false; } From 9f8242188570156576f2b42beefb4cabd963a88e Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Mon, 8 Apr 2024 16:29:59 +0200 Subject: [PATCH 15/51] Refs #20629: Support match with security Signed-off-by: cferreiragonz --- .../discovery/participant/PDPClient.cpp | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index 646eca5be59..0b727afd9b2 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -516,22 +516,26 @@ void PDPClient::notifyAboveRemoteEndpoints( { eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - // Verify if this participant is a server - for (auto& svr : mp_builtin->m_DiscoveryServers) + std::string part_type = check_participant_type(pdata.m_properties); + if (part_type == ParticipantType::SERVER || part_type == ParticipantType::BACKUP) { - if (data_matches_with_prefix(svr.guidPrefix, pdata)) + // Add new servers to the connected list + EPROSIMA_LOG_INFO(RTPS_PDP_CLIENT, "Secure Server [" << pdata.m_guid.guidPrefix << "] matched."); + RemoteServerAttributes server; + server.guidPrefix = pdata.m_guid.guidPrefix; + for (const Locator_t& locator : pdata.metatraffic_locators.multicast) { - if (!svr.is_connected && nullptr != get_participant_proxy_data(svr.guidPrefix)) - { - //! mark proxy as connected from an unmangled prefix in case - //! it could not be done in assignRemoteEndpoints() - svr.is_connected = true; - } - - match_pdp_reader_nts_(svr, pdata.m_guid.guidPrefix); - match_pdp_writer_nts_(svr, pdata.m_guid.guidPrefix); - break; + server.metatrafficMulticastLocatorList.push_back(locator); } + for (const Locator_t& locator : pdata.metatraffic_locators.unicast) + { + server.metatrafficUnicastLocatorList.push_back(locator); + } + connected_servers_.push_back(server); + + // Match incoming server + match_pdp_writer_nts_(server); + match_pdp_reader_nts_(server); } } #endif // HAVE_SECURITY From 476f29d13026d51757870662779f6dacdf0c02a9 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Tue, 9 Apr 2024 16:57:40 +0200 Subject: [PATCH 16/51] Refs #20629: Modify DS example to work wo/ GUID Signed-off-by: cferreiragonz --- .../DiscoveryServerPublisher.cpp | 20 +++++----- .../DiscoveryServerPublisher.h | 1 - .../DiscoveryServerServer.cpp | 27 ++++++-------- .../DiscoveryServerServer.h | 4 +- .../DiscoveryServerSubscriber.cpp | 20 +++++----- .../DiscoveryServerSubscriber.h | 1 - .../DiscoveryServer_main.cpp | 33 ++--------------- .../cpp/dds/DiscoveryServerExample/README.md | 13 ------- .../arg_configuration.h | 37 ------------------- 9 files changed, 34 insertions(+), 122 deletions(-) diff --git a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerPublisher.cpp b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerPublisher.cpp index 2a83c809b50..ae9e061e35b 100644 --- a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerPublisher.cpp +++ b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerPublisher.cpp @@ -62,7 +62,6 @@ bool HelloWorldPublisher::init( const std::string& topic_name, const std::string& server_address, unsigned short server_port, - unsigned short server_id, TransportKind transport) { hello_.index(0); @@ -142,7 +141,14 @@ bool HelloWorldPublisher::init( server_locator.kind = LOCATOR_KIND_TCPv6; eprosima::fastdds::rtps::IPLocator::setLogicalPort(server_locator, server_port); - eprosima::fastdds::rtps::IPLocator::setIPv6(server_locator, ip_server_address); + if (eprosima::fastdds::rtps::IPLocator::isIPv6(ip_server_address)) + { + eprosima::fastdds::rtps::IPLocator::setIPv6(server_locator, ip_server_address); + } + else + { + eprosima::fastdds::rtps::IPLocator::setIPv6(server_locator, "::1"); + } break; } @@ -154,15 +160,8 @@ bool HelloWorldPublisher::init( pqos.wire_protocol().builtin.discovery_config.discoveryProtocol = eprosima::fastdds::rtps::DiscoveryProtocol::CLIENT; - // Set SERVER's GUID prefix - RemoteServerAttributes remote_server_att; - remote_server_att.guidPrefix = get_discovery_server_guid_from_id(server_id); - - // Set SERVER's listening locator for PDP - remote_server_att.metatrafficUnicastLocatorList.push_back(server_locator); - // Add remote SERVER to CLIENT's list of SERVERs - pqos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.push_back(remote_server_att); + pqos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.push_back(server_locator); // Add descriptor pqos.transport().user_transports.push_back(descriptor); @@ -179,7 +178,6 @@ bool HelloWorldPublisher::init( "Publisher Participant " << pqos.name() << " created with GUID " << participant_->guid() << " connecting to server <" << server_locator << "> " << - " with Guid: <" << remote_server_att.guidPrefix << "> " << std::endl; // REGISTER THE TYPE diff --git a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerPublisher.h b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerPublisher.h index a3e9b5e01d8..8397dd5472b 100644 --- a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerPublisher.h +++ b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerPublisher.h @@ -46,7 +46,6 @@ class HelloWorldPublisher const std::string& topic_name, const std::string& server_address, unsigned short server_port, - unsigned short server_id, TransportKind transport); //! Publish a sample diff --git a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerServer.cpp b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerServer.cpp index 60a915899fb..d2dccf17aa7 100644 --- a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerServer.cpp +++ b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerServer.cpp @@ -57,12 +57,10 @@ void DiscoveryServer::stop() bool DiscoveryServer::init( const std::string& server_address, unsigned short server_port, - unsigned short server_id, TransportKind transport, bool has_connection_server, const std::string& connection_server_address, - unsigned short connection_server_port, - unsigned short connection_server_id) + unsigned short connection_server_port) { DomainParticipantQos pqos; pqos.name("DS-Server"); @@ -163,7 +161,14 @@ bool DiscoveryServer::init( listening_locator.kind = LOCATOR_KIND_TCPv6; eprosima::fastdds::rtps::IPLocator::setLogicalPort(listening_locator, server_port); - eprosima::fastdds::rtps::IPLocator::setIPv6(listening_locator, ip_listening_address); + if (eprosima::fastdds::rtps::IPLocator::isIPv6(ip_listening_address)) + { + eprosima::fastdds::rtps::IPLocator::setIPv6(listening_locator, ip_listening_address); + } + else + { + eprosima::fastdds::rtps::IPLocator::setIPv6(listening_locator, "::1"); + } connection_locator.kind = LOCATOR_KIND_TCPv6; eprosima::fastdds::rtps::IPLocator::setIPv6(connection_locator, ip_connection_address); eprosima::fastdds::rtps::IPLocator::setLogicalPort(connection_locator, connection_server_port); @@ -182,7 +187,7 @@ bool DiscoveryServer::init( eprosima::fastdds::rtps::DiscoveryProtocol::SERVER; // Set SERVER's GUID prefix - pqos.wire_protocol().prefix = get_discovery_server_guid_from_id(server_id); + set_server_client_random_guidPrefix(pqos.wire_protocol().prefix); // Set SERVER's listening locator for PDP pqos.wire_protocol().builtin.metatrafficUnicastLocatorList.push_back(listening_locator); @@ -191,17 +196,10 @@ bool DiscoveryServer::init( // Configure Connection address /////////////////////////////// - RemoteServerAttributes remote_server_att; if (has_connection_server) { - // Set SERVER's GUID prefix - remote_server_att.guidPrefix = get_discovery_server_guid_from_id(connection_server_id); - - // Set SERVER's listening locator for PDP - remote_server_att.metatrafficUnicastLocatorList.push_back(connection_locator); - // Add remote SERVER to CLIENT's list of SERVERs - pqos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.push_back(remote_server_att); + pqos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.push_back(connection_locator); } @@ -224,8 +222,7 @@ bool DiscoveryServer::init( "Server Participant " << pqos.name() << " created with GUID " << participant_->guid() << " listening in address <" << listening_locator << "> " << - " connecting with Discovery Server <" << remote_server_att.guidPrefix << "> " - " with address <" << connection_locator << "> " << + " connecting with Discovery Server <" << connection_locator << "> " << std::endl; } else diff --git a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerServer.h b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerServer.h index 4fd8b6d79f7..0520eb2dc00 100644 --- a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerServer.h +++ b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerServer.h @@ -44,12 +44,10 @@ class DiscoveryServer bool init( const std::string& server_address, unsigned short server_port, - unsigned short server_id, TransportKind transport, bool has_connection_server, const std::string& connection_server_address, - unsigned short connection_server_port, - unsigned short connection_server_id); + unsigned short connection_server_port); //! Run void run( diff --git a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerSubscriber.cpp b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerSubscriber.cpp index ed0dea34efe..8cfb7f3d194 100644 --- a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerSubscriber.cpp +++ b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerSubscriber.cpp @@ -67,7 +67,6 @@ bool HelloWorldSubscriber::init( uint32_t max_messages, const std::string& server_address, unsigned short server_port, - unsigned short server_id, TransportKind transport) { DomainParticipantQos pqos; @@ -145,7 +144,14 @@ bool HelloWorldSubscriber::init( server_locator.kind = LOCATOR_KIND_TCPv6; eprosima::fastdds::rtps::IPLocator::setLogicalPort(server_locator, server_port); - eprosima::fastdds::rtps::IPLocator::setIPv4(server_locator, ip_server_address); + if (eprosima::fastdds::rtps::IPLocator::isIPv6(ip_server_address)) + { + eprosima::fastdds::rtps::IPLocator::setIPv6(server_locator, ip_server_address); + } + else + { + eprosima::fastdds::rtps::IPLocator::setIPv6(server_locator, "::1"); + } break; } @@ -157,15 +163,8 @@ bool HelloWorldSubscriber::init( pqos.wire_protocol().builtin.discovery_config.discoveryProtocol = eprosima::fastdds::rtps::DiscoveryProtocol::CLIENT; - // Set SERVER's GUID prefix - RemoteServerAttributes remote_server_att; - remote_server_att.guidPrefix = get_discovery_server_guid_from_id(server_id); - - // Set SERVER's listening locator for PDP - remote_server_att.metatrafficUnicastLocatorList.push_back(server_locator); - // Add remote SERVER to CLIENT's list of SERVERs - pqos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.push_back(remote_server_att); + pqos.wire_protocol().builtin.discovery_config.m_DiscoveryServers.push_back(server_locator); // Add descriptor pqos.transport().user_transports.push_back(descriptor); @@ -183,7 +182,6 @@ bool HelloWorldSubscriber::init( "Subscriber Participant " << pqos.name() << " created with GUID " << participant_->guid() << " connecting to server <" << server_locator << "> " << - " with Guid: <" << remote_server_att.guidPrefix << "> " << std::endl; // REGISTER THE TYPE diff --git a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerSubscriber.h b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerSubscriber.h index 2b2c83ceb12..cc81aad5f6a 100644 --- a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerSubscriber.h +++ b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServerSubscriber.h @@ -49,7 +49,6 @@ class HelloWorldSubscriber uint32_t max_messages, const std::string& server_address, unsigned short server_port, - unsigned short server_id, TransportKind transport); //! RUN the subscriber until number samples are received diff --git a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServer_main.cpp b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServer_main.cpp index e1badf3b661..e71d6b04fe0 100644 --- a/examples/cpp/dds/DiscoveryServerExample/DiscoveryServer_main.cpp +++ b/examples/cpp/dds/DiscoveryServerExample/DiscoveryServer_main.cpp @@ -64,13 +64,11 @@ int main( // Discovery Server connection std::string connection_address = "127.0.0.1"; // default ip address uint16_t connection_port = 16166; // default physical port - uint16_t connection_ds_id = 0; // default DS id bool id_ds_set = false; // Discovery Server listening std::string listening_address = "127.0.0.1"; // default ip address uint16_t listening_port = 16166; // default physical port - uint16_t listening_ds_id = 0; // default DS id uint32_t timeout = 0; // default DS id if (argc > 1) @@ -190,16 +188,13 @@ int main( } case optionIndex::CONNECTION_PORT: + id_ds_set = true; connection_port = static_cast(strtol(opt.arg, nullptr, 10)); break; case optionIndex::CONNECTION_ADDRESS: - connection_address = opt.arg; - break; - - case optionIndex::CONNECTION_DISCOVERY_SERVER_ID: id_ds_set = true; - connection_ds_id = static_cast(strtol(opt.arg, nullptr, 10)); + connection_address = opt.arg; break; case optionIndex::LISTENING_PORT: @@ -221,15 +216,6 @@ int main( break; - case optionIndex::LISTENING_DISCOVERY_SERVER_ID: - if (type != EntityKind::SERVER) - { - print_warning("server", opt.name); - break; - } - listening_ds_id = static_cast(strtol(opt.arg, nullptr, 10)); - break; - case optionIndex::TIMEOUT: if (type != EntityKind::SERVER) { @@ -268,15 +254,6 @@ int main( // return 1; // } - // Check that a DS has not same id itself and connection - if (id_ds_set && type == EntityKind::SERVER && listening_ds_id == connection_ds_id) - { - std::cerr << "ERROR: Discovery Servers ids must be different, " - << " cannot connect to a server with same id " << listening_ds_id << std::endl; - option::printUsage(fwrite, stdout, usage, columns); - return 1; - } - // Check that a DS has not same ip and port in listening and connection if (id_ds_set && type == EntityKind::SERVER && @@ -299,7 +276,6 @@ int main( topic_name, connection_address, connection_port, - connection_ds_id, transport)) { mypub.run(static_cast(count), static_cast(sleep)); @@ -319,7 +295,6 @@ int main( static_cast(count), connection_address, connection_port, - connection_ds_id, transport)) { mysub.run(static_cast(count)); @@ -337,12 +312,10 @@ int main( if (myserver.init( listening_address, listening_port, - listening_ds_id, transport, id_ds_set, connection_address, - connection_port, - connection_ds_id)) + connection_port)) { myserver.run(timeout); } diff --git a/examples/cpp/dds/DiscoveryServerExample/README.md b/examples/cpp/dds/DiscoveryServerExample/README.md index 5d3448e3274..ef236617f4e 100644 --- a/examples/cpp/dds/DiscoveryServerExample/README.md +++ b/examples/cpp/dds/DiscoveryServerExample/README.md @@ -35,9 +35,6 @@ Publisher options: Server listening port (Default port: 16166). --transport= Use Transport Protocol [udpv4|udpv6|tcpv4|tcpv6] (Default: udpv4). - -d --connection-discovery-server-id - Id of the Discovery Server to connect with. GUID will be - calculated from id (Default: 0). Subscriber options: -t --topic= @@ -51,16 +48,10 @@ Subscriber options: Server listening port (Default port: 16166). --transport= Use Transport Protocol [udpv4|udpv6|tcpv4|tcpv6] (Default: udpv4). - -d --connection-discovery-server-id - Id of the Discovery Server to connect with. GUID will be - calculated from id (Default: 0). DiscoveryServer options: --listening-address= Server address (Default address: 127.0.0.1). - --id= - Id of this Discovery Server. - GUID will be calculated from id (Default: 0). --listening-port= Server listening port (Default port: 16166). --transport= @@ -69,10 +60,6 @@ DiscoveryServer options: Server address (Default address: 127.0.0.1). -p --connection-port= Server listening port (Default port: 16166). - -d --connection-discovery-server-id - Id of the Discovery Server to connect with. GUID will be - calculated from id (if not set, this DS will not connect - to other server). -z --timeout Number of seconds before finish the process (Default: 0 = till ^C). ``` diff --git a/examples/cpp/dds/DiscoveryServerExample/arg_configuration.h b/examples/cpp/dds/DiscoveryServerExample/arg_configuration.h index ad01e206847..d7a888df0af 100644 --- a/examples/cpp/dds/DiscoveryServerExample/arg_configuration.h +++ b/examples/cpp/dds/DiscoveryServerExample/arg_configuration.h @@ -169,11 +169,9 @@ enum optionIndex CONNECTION_ADDRESS, CONNECTION_PORT, - CONNECTION_DISCOVERY_SERVER_ID, LISTENING_ADDRESS, LISTENING_PORT, - LISTENING_DISCOVERY_SERVER_ID, TIMEOUT, }; @@ -239,15 +237,6 @@ const option::Descriptor usage[] = { Arg::Transport, " \t--transport \tUse Transport Protocol [udpv4|udpv6|tcpv4|tcpv6] (UDPv4 by default)." }, - { - CONNECTION_DISCOVERY_SERVER_ID, - 0, - "d", - "discovery-server-id", - Arg::Numeric, - " -d \t--connection-discovery-server-id \tId of the Discovery Server to connect with. " - "GUID will be calculated from id (0 by default)." - }, /// SUBSCRIBER OPTIONS {UNKNOWN_OPT, 0, "", "", Arg::None, "\nSubscriber options:"}, @@ -291,15 +280,6 @@ const option::Descriptor usage[] = { Arg::Transport, " \t--transport \tUse Transport Protocol [udpv4|udpv6|tcpv4|tcpv6] (UDPv4 by default)." }, - { - CONNECTION_DISCOVERY_SERVER_ID, - 0, - "d", - "discovery-server-id", - Arg::Numeric, - " -d \t--connection-discovery-server-id \tId of the Discovery Server to connect with. " - "GUID will be calculated from id (0 by default)." - }, /// SERVER OPTIONS {UNKNOWN_OPT, 0, "", "", Arg::None, "\nDiscovery Server options:"}, @@ -311,14 +291,6 @@ const option::Descriptor usage[] = { Arg::String, " \t--listening-address= \tServer address (Default address: 127.0.0.1)." }, - { - LISTENING_DISCOVERY_SERVER_ID, - 0, - "", - "id", - Arg::Numeric, - " \t--id \tId of this Discovery Server. GUID will be calculated from id (0 by default)." - }, { LISTENING_PORT, 0, @@ -351,15 +323,6 @@ const option::Descriptor usage[] = { Arg::String, " -c \t--connection-address= \tServer address (Default address: 127.0.0.1)." }, - { - CONNECTION_DISCOVERY_SERVER_ID, - 0, - "d", - "connection-discovery-server-id", - Arg::Numeric, - " -d \t--connection-discovery-server-id \tId of the Discovery Server to connect with. " - "GUID will be calculated from id (if not set, this DS will not connect to other server)." - }, { TIMEOUT, 0, From b1419eaa0271f3dbb70a2d36d4aeb40d01bbd03a Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Wed, 10 Apr 2024 08:09:12 +0200 Subject: [PATCH 17/51] Refs #20629: Use LocatorList for remote servers Signed-off-by: cferreiragonz --- .../attributes/RTPSParticipantAttributes.hpp | 2 +- .../rtps/attributes/ServerAttributes.hpp | 14 +- include/fastdds/rtps/common/LocatorList.hpp | 7 + .../rtps/common/LocatorSelectorEntry.hpp | 15 ++ src/cpp/rtps/RTPSDomain.cpp | 128 +++++++++--------- src/cpp/rtps/builtin/BuiltinProtocols.cpp | 22 ++- src/cpp/rtps/builtin/BuiltinProtocols.h | 2 +- .../builtin/discovery/participant/PDP.cpp | 2 +- .../rtps/builtin/discovery/participant/PDP.h | 6 +- .../discovery/participant/PDPClient.cpp | 84 ++++-------- .../discovery/participant/PDPServer.cpp | 37 ++--- .../discovery/participant/PDPServer.hpp | 6 +- .../participant/timedevent/DSClientEvent.cpp | 2 +- .../rtps/participant/RTPSParticipantImpl.cpp | 81 ++++------- 14 files changed, 179 insertions(+), 229 deletions(-) diff --git a/include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp b/include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp index e534f505c7d..f36b2fcb258 100644 --- a/include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp +++ b/include/fastdds/rtps/attributes/RTPSParticipantAttributes.hpp @@ -277,7 +277,7 @@ class DiscoverySettings Duration_t discoveryServer_client_syncperiod = { 0, 450 * 1000000}; // 450 milliseconds //! Discovery Server initial connections, needed if `discoveryProtocol` = CLIENT | SUPER_CLIENT | SERVER | BACKUP - eprosima::fastdds::rtps::RemoteServerList_t m_DiscoveryServers; + eprosima::fastdds::rtps::LocatorList m_DiscoveryServers; //! Filtering participants out depending on location ParticipantFilteringFlags ignoreParticipantFlags = ParticipantFilteringFlags::NO_FILTER; diff --git a/include/fastdds/rtps/attributes/ServerAttributes.hpp b/include/fastdds/rtps/attributes/ServerAttributes.hpp index eeeb39f61f9..5cc89e20868 100644 --- a/include/fastdds/rtps/attributes/ServerAttributes.hpp +++ b/include/fastdds/rtps/attributes/ServerAttributes.hpp @@ -165,30 +165,28 @@ const char* const ROS_SUPER_CLIENT = "ROS_SUPER_CLIENT"; /** * Retrieves a semicolon-separated list of locators from a string, and - * populates a RemoteServerList_t mapping list position to default guid. + * populates a LocatorList_t. * @param[in] list servers listening locator list. - * @param[out] attributes reference to a RemoteServerList_t to populate. + * @param[out] servers_list reference to a LocatorList_t to populate. * @return true if parsing succeeds, false otherwise (or if the list is empty) */ FASTDDS_EXPORTED_API bool load_environment_server_info( const std::string& list, - RemoteServerList_t& attributes); + LocatorList& servers_list); /** * Retrieves a semicolon-separated list of locators from DEFAULT_ROS2_MASTER_URI environment variable, and - * populates a RemoteServerList_t mapping list position to default guid. + * populates a LocatorList_t. * * The environment variable can be read from an environment file (which allows runtime modification of the remote * servers list) or directly from the environment. * The value contained in the file takes precedence over the environment value (if both are set). - * This is to avoid conflicts because only new servers can be added to the list (containing thus all the previously - * known servers). * - * @param[out] attributes reference to a RemoteServerList_t to populate. + * @param[out] servers_list reference to a LocatorList_t to populate. * @return true if parsing succeeds, false otherwise */ FASTDDS_EXPORTED_API bool load_environment_server_info( - RemoteServerList_t& attributes); + LocatorList& servers_list); /** * Get the value of environment variable DEFAULT_ROS2_MASTER_URI diff --git a/include/fastdds/rtps/common/LocatorList.hpp b/include/fastdds/rtps/common/LocatorList.hpp index a466f121d59..149eac8e10b 100644 --- a/include/fastdds/rtps/common/LocatorList.hpp +++ b/include/fastdds/rtps/common/LocatorList.hpp @@ -166,6 +166,13 @@ class LocatorList return false; } + /// Not equal to operator + FASTDDS_EXPORTED_API bool operator !=( + const LocatorList& locator_list) const + { + return !(*this == locator_list); + } + /** * @brief Return an iterator to the beginning. * diff --git a/include/fastdds/rtps/common/LocatorSelectorEntry.hpp b/include/fastdds/rtps/common/LocatorSelectorEntry.hpp index abe08d666b4..cb53cd4bcd5 100644 --- a/include/fastdds/rtps/common/LocatorSelectorEntry.hpp +++ b/include/fastdds/rtps/common/LocatorSelectorEntry.hpp @@ -123,6 +123,21 @@ struct LocatorSelectorEntry return entry; } + static LocatorSelectorEntry create_fully_selected_entry( + const LocatorList_t& unicast_locators) + { + // Create an entry with space for all locators + LocatorSelectorEntry entry(unicast_locators.size(), 0); + // Add and select unicast locators + for (const Locator_t& locator : unicast_locators) + { + entry.state.unicast.push_back(entry.unicast.size()); + entry.unicast.push_back(locator); + } + // Return created entry + return entry; + } + //! GUID of the remote entity. GUID_t remote_guid; //! List of unicast locators to send data to the remote entity. diff --git a/src/cpp/rtps/RTPSDomain.cpp b/src/cpp/rtps/RTPSDomain.cpp index 779d1feacd9..3a5506420ae 100644 --- a/src/cpp/rtps/RTPSDomain.cpp +++ b/src/cpp/rtps/RTPSDomain.cpp @@ -499,7 +499,7 @@ RTPSParticipant* RTPSDomainImpl::clientServerEnvironmentCreationOverride( RTPSParticipantAttributes client_att(att); // Retrieve the info from the environment variable - RemoteServerList_t& server_list = client_att.builtin.discovery_config.m_DiscoveryServers; + LocatorList_t& server_list = client_att.builtin.discovery_config.m_DiscoveryServers; if (load_environment_server_info(server_list) && server_list.empty()) { // It's not an error, the environment variable may not be set. Any issue with environment @@ -507,89 +507,85 @@ RTPSParticipant* RTPSDomainImpl::clientServerEnvironmentCreationOverride( return nullptr; } - // Check if some server requires the UDPv6, TCPv4 or TCPv6 transport - for (auto& server : server_list) + // Check if some adress requires the UDPv6, TCPv4 or TCPv6 transport + if (server_list.has_kind()) { - if (server.requires_transport()) - { - // Extend builtin transports with the UDPv6 transport - auto descriptor = std::make_shared(); - descriptor->sendBufferSize = client_att.sendSocketBufferSize; - descriptor->receiveBufferSize = client_att.listenSocketBufferSize; - client_att.userTransports.push_back(std::move(descriptor)); - break; - } - if (server.requires_transport()) + // Extend builtin transports with the UDPv6 transport + auto descriptor = std::make_shared(); + descriptor->sendBufferSize = client_att.sendSocketBufferSize; + descriptor->receiveBufferSize = client_att.listenSocketBufferSize; + client_att.userTransports.push_back(std::move(descriptor)); + } + if (server_list.has_kind()) + { + // Check if a TCPv4 transport exists. Otherwise create it + fastdds::rtps::TCPTransportDescriptor* pT = nullptr; + std::shared_ptr p4; + bool no_tcpv4 = true; + + for (auto sp : client_att.userTransports) { - // Check if a TCPv4 transport exists. Otherwise create it - fastdds::rtps::TCPTransportDescriptor* pT = nullptr; - std::shared_ptr p4; - bool no_tcpv4 = true; + pT = dynamic_cast(sp.get()); - for (auto sp : client_att.userTransports) + if (pT != nullptr) { - pT = dynamic_cast(sp.get()); - - if (pT != nullptr) + if (!p4) { - if (!p4) + if ((p4 = std::dynamic_pointer_cast(sp))) { - if ((p4 = std::dynamic_pointer_cast(sp))) - { - // TCPv4 transport already exists - no_tcpv4 = false; - break; - } + // TCPv4 transport already exists + no_tcpv4 = false; + break; } } } - if (no_tcpv4) - { - // Extend builtin transports with the TCPv4 transport - auto descriptor = std::make_shared(); - // Add automatic port - descriptor->add_listener_port(0); - descriptor->sendBufferSize = client_att.sendSocketBufferSize; - descriptor->receiveBufferSize = client_att.listenSocketBufferSize; - client_att.userTransports.push_back(std::move(descriptor)); - } - } - if (server.requires_transport()) + if (no_tcpv4) + { + // Extend builtin transports with the TCPv4 transport + auto descriptor = std::make_shared(); + // Add automatic port + descriptor->add_listener_port(0); + descriptor->sendBufferSize = client_att.sendSocketBufferSize; + descriptor->receiveBufferSize = client_att.listenSocketBufferSize; + client_att.userTransports.push_back(std::move(descriptor)); + } + + } + if (server_list.has_kind()) + { + // Check if a TCPv6 transport exists. Otherwise create it + fastdds::rtps::TCPTransportDescriptor* pT = nullptr; + std::shared_ptr p6; + bool no_tcpv6 = true; + + for (auto sp : client_att.userTransports) { - // Check if a TCPv6 transport exists. Otherwise create it - fastdds::rtps::TCPTransportDescriptor* pT = nullptr; - std::shared_ptr p6; - bool no_tcpv6 = true; + pT = dynamic_cast(sp.get()); - for (auto sp : client_att.userTransports) + if (pT != nullptr) { - pT = dynamic_cast(sp.get()); - - if (pT != nullptr) + if (!p6) { - if (!p6) + // try to find a descriptor matching the listener port setup + if ((p6 = std::dynamic_pointer_cast(sp))) { - // try to find a descriptor matching the listener port setup - if ((p6 = std::dynamic_pointer_cast(sp))) - { - // TCPv6 transport already exists - no_tcpv6 = false; - break; - } + // TCPv6 transport already exists + no_tcpv6 = false; + break; } } } - if (no_tcpv6) - { - // Extend builtin transports with the TCPv6 transport - auto descriptor = std::make_shared(); - // Add automatic port - descriptor->add_listener_port(0); - descriptor->sendBufferSize = client_att.sendSocketBufferSize; - descriptor->receiveBufferSize = client_att.listenSocketBufferSize; - client_att.userTransports.push_back(std::move(descriptor)); - } + } + if (no_tcpv6) + { + // Extend builtin transports with the TCPv6 transport + auto descriptor = std::make_shared(); + // Add automatic port + descriptor->add_listener_port(0); + descriptor->sendBufferSize = client_att.sendSocketBufferSize; + descriptor->receiveBufferSize = client_att.listenSocketBufferSize; + client_att.userTransports.push_back(std::move(descriptor)); } } diff --git a/src/cpp/rtps/builtin/BuiltinProtocols.cpp b/src/cpp/rtps/builtin/BuiltinProtocols.cpp index 289f8951b02..640a7f1e1ba 100644 --- a/src/cpp/rtps/builtin/BuiltinProtocols.cpp +++ b/src/cpp/rtps/builtin/BuiltinProtocols.cpp @@ -166,22 +166,20 @@ void BuiltinProtocols::filter_server_remote_locators( { eprosima::shared_lock disc_lock(getDiscoveryMutex()); - for (eprosima::fastdds::rtps::RemoteServerAttributes& rs : m_DiscoveryServers) + LocatorList_t allowed_locators; + + for (auto loc : m_DiscoveryServers) { - LocatorList_t allowed_locators; - for (Locator_t& loc : rs.metatrafficUnicastLocatorList) + if (nf.is_locator_remote_or_allowed(loc)) + { + allowed_locators.push_back(loc); + } + else { - if (nf.is_locator_remote_or_allowed(loc)) - { - allowed_locators.push_back(loc); - } - else - { - EPROSIMA_LOG_WARNING(RTPS_PDP, "Ignoring remote server locator " << loc << " : not allowed."); - } + EPROSIMA_LOG_WARNING(RTPS_PDP, "Ignoring remote server locator " << loc << " : not allowed."); } - rs.metatrafficUnicastLocatorList.swap(allowed_locators); } + m_DiscoveryServers.swap(allowed_locators); } bool BuiltinProtocols::addLocalWriter( diff --git a/src/cpp/rtps/builtin/BuiltinProtocols.h b/src/cpp/rtps/builtin/BuiltinProtocols.h index 8efd402314d..07b79135977 100644 --- a/src/cpp/rtps/builtin/BuiltinProtocols.h +++ b/src/cpp/rtps/builtin/BuiltinProtocols.h @@ -123,7 +123,7 @@ class BuiltinProtocols //! Initial peers LocatorList_t m_initialPeersList; //! Known discovery and backup server container - std::list m_DiscoveryServers; + LocatorList_t m_DiscoveryServers; /** * Add a local Writer to the BuiltinProtocols. diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp index cde17efdd98..16a6d58cf2e 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp @@ -1435,7 +1435,7 @@ ParticipantProxyData* PDP::get_participant_proxy_data( return nullptr; } -std::list& PDP::remote_server_attributes() +fastdds::rtps::LocatorList& PDP::remote_server_locators() { return mp_builtin->m_DiscoveryServers; } diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.h b/src/cpp/rtps/builtin/discovery/participant/PDP.h index 12d05a5a7a6..3af54b68894 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDP.h +++ b/src/cpp/rtps/builtin/discovery/participant/PDP.h @@ -432,10 +432,10 @@ class PDP : public fastdds::statistics::rtps::IProxyQueryable const GuidPrefix_t& guid_prefix); /** - * Get the list of remote servers to which the client should connect - * @return A reference to the list of RemoteServerAttributes + * Get the list of remote servers' locators to which the client should connect + * @return A reference to the LocatorList */ - std::list& remote_server_attributes(); + fastdds::rtps::LocatorList& remote_server_locators(); /** * Access the temporary proxy pool for reader proxies diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index 0b727afd9b2..6198880f42e 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -438,17 +438,14 @@ bool PDPClient::create_ds_pdp_reliable_endpoints( { eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) - { - auto entry = LocatorSelectorEntry::create_fully_selected_entry( - it.metatrafficUnicastLocatorList, it.metatrafficMulticastLocatorList); - mp_RTPSParticipant->createSenderResources(entry); + auto entry = LocatorSelectorEntry::create_fully_selected_entry( + mp_builtin->m_DiscoveryServers); + mp_RTPSParticipant->createSenderResources(entry); - // If SECURITY is disabled, this condition is ALWAYS true - if (!is_discovery_protected) - { - BaseReader::downcast(endpoints.reader.reader_)->allow_unknown_writers(); - } + // If SECURITY is disabled, this condition is ALWAYS true + if (!is_discovery_protected) + { + BaseReader::downcast(endpoints.reader.reader_)->allow_unknown_writers(); } } @@ -812,11 +809,7 @@ void PDPClient::announceParticipantState( // Ping not-connected servers if (connected_servers_.size() < mp_builtin->m_DiscoveryServers.size()) { - for (auto& svr : mp_builtin->m_DiscoveryServers) - { - locators.push_back(svr.metatrafficMulticastLocatorList); - locators.push_back(svr.metatrafficUnicastLocatorList); - } + locators = mp_builtin->m_DiscoveryServers; } // Announce liveliness (lease duration) to all servers @@ -860,18 +853,13 @@ void PDPClient::update_remote_servers_list() { eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) - { - if (!endpoints->reader.reader_->matched_writer_is_matched(it.GetPDPWriter()) || - !endpoints->writer.writer_->matched_reader_is_matched(it.GetPDPReader())) - { - auto entry = LocatorSelectorEntry::create_fully_selected_entry( - it.metatrafficUnicastLocatorList, it.metatrafficMulticastLocatorList); - mp_RTPSParticipant->createSenderResources(entry); - } + // Create resources for remote servers. If a sender resource is already created, this step will be skipped for + // that locator. + auto entry = LocatorSelectorEntry::create_fully_selected_entry( + mp_builtin->m_DiscoveryServers); + mp_RTPSParticipant->createSenderResources(entry); - endpoints->reader.reader_->enableMessagesFromUnkownWriters(true); - } + BaseReader::downcast(endpoints->reader.reader_)->allow_unknown_writers(); } mp_sync->restart_timer(); } @@ -979,16 +967,16 @@ const std::string& ros_discovery_server_env() } bool load_environment_server_info( - RemoteServerList_t& attributes) + LocatorList_t& servers_list) { - return load_environment_server_info(ros_discovery_server_env(), attributes); + return load_environment_server_info(ros_discovery_server_env(), servers_list); } bool load_environment_server_info( const std::string& list, - RemoteServerList_t& attributes) + LocatorList_t& servers_list) { - attributes.clear(); + servers_list.clear(); if (list.empty()) { return true; @@ -1025,30 +1013,12 @@ bool load_environment_server_info( }; // Add new server - auto add_server2qos = [](std::forward_list&& locators, RemoteServerList_t& attributes) + auto add_server2qos = [](std::forward_list&& locators, LocatorList_t& servers_list) { - RemoteServerAttributes server_att; - - // To set the GUID is no necessary - - // Split multi and unicast locators - auto unicast = std::partition(locators.begin(), locators.end(), IPLocator::isMulticast); - - LocatorList mlist; - std::copy(locators.begin(), unicast, std::back_inserter(mlist)); - if (!mlist.empty()) + for (auto& server_address : locators) { - server_att.metatrafficMulticastLocatorList.push_back(std::move(mlist)); + servers_list.push_back(std::move(server_address)); } - - LocatorList ulist; - std::copy(unicast, locators.end(), std::back_inserter(ulist)); - if (!ulist.empty()) - { - server_att.metatrafficUnicastLocatorList.push_back(std::move(ulist)); - } - - attributes.push_back(std::move(server_att)); }; try @@ -1114,7 +1084,7 @@ bool load_environment_server_info( } // Add server to the list - add_server2qos(std::forward_list{server_locator}, attributes); + add_server2qos(std::forward_list{server_locator}, servers_list); } // Try IPv6 next else if (std::regex_match(locator, mr, ROS2_IPV6_ADDRESSPORT_PATTERN, @@ -1153,7 +1123,7 @@ bool load_environment_server_info( } // Add server to the list - add_server2qos(std::forward_list{server_locator}, attributes); + add_server2qos(std::forward_list{server_locator}, servers_list); } // Try resolve DNS else if (std::regex_match(locator, mr, ROS2_DNS_DOMAINPORT_PATTERN, @@ -1244,7 +1214,7 @@ bool load_environment_server_info( } // Add server to the list - add_server2qos(std::move(flist), attributes); + add_server2qos(std::move(flist), servers_list); } // Try resolve TCP DNS else if (std::regex_match(locator, mr, ROS2_DNS_DOMAINPORT_PATTERN_TCP, @@ -1338,7 +1308,7 @@ bool load_environment_server_info( } // Add server to the list - add_server2qos(std::move(flist), attributes); + add_server2qos(std::move(flist), servers_list); } else { @@ -1353,7 +1323,7 @@ bool load_environment_server_info( } // Check for server info - if (attributes.empty()) + if (servers_list.empty()) { throw std::invalid_argument("No default server locators were provided."); } @@ -1361,7 +1331,7 @@ bool load_environment_server_info( catch (std::exception& e) { EPROSIMA_LOG_ERROR(SERVER_CLIENT_DISCOVERY, e.what()); - attributes.clear(); + servers_list.clear(); return false; } diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index 27c329f5112..f3b5bfd0976 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -64,7 +64,7 @@ PDPServer::PDPServer( , durability_ (durability_kind) { // Add remote servers from environment variable - RemoteServerList_t env_servers; + LocatorList_t env_servers; { std::lock_guard lock(*getMutex()); @@ -485,12 +485,9 @@ bool PDPServer::create_ds_pdp_reliable_endpoints( { eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) - { - auto entry = LocatorSelectorEntry::create_fully_selected_entry( - it.metatrafficUnicastLocatorList, it.metatrafficMulticastLocatorList); - mp_RTPSParticipant->createSenderResources(entry); - } + auto entry = LocatorSelectorEntry::create_fully_selected_entry( + mp_builtin->m_DiscoveryServers); + mp_RTPSParticipant->createSenderResources(entry); } return true; @@ -1168,17 +1165,14 @@ void PDPServer::update_remote_servers_list() std::lock_guard lock(*getMutex()); - eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) { - if (!endpoints->reader.reader_->matched_writer_is_matched(it.GetPDPWriter()) || - !endpoints->writer.writer_->matched_reader_is_matched(it.GetPDPReader())) - { - auto entry = LocatorSelectorEntry::create_fully_selected_entry( - it.metatrafficUnicastLocatorList, it.metatrafficMulticastLocatorList); - mp_RTPSParticipant->createSenderResources(entry); - } + eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); + + // Create resources for remote servers. If a sender resource is already created, this step will be skipped for + // that locator. + auto entry = LocatorSelectorEntry::create_fully_selected_entry( + mp_builtin->m_DiscoveryServers); + mp_RTPSParticipant->createSenderResources(entry); } // Need to reactivate the server thread to send the DATA(p) to the new servers @@ -1444,7 +1438,7 @@ fastdds::rtps::ddb::DiscoveryDataBase& PDPServer::discovery_db() return discovery_db_; } -const RemoteServerList_t& PDPServer::servers() +const LocatorList_t& PDPServer::servers() { return mp_builtin->m_DiscoveryServers; } @@ -1575,11 +1569,8 @@ void PDPServer::ping_remote_servers() { std::lock_guard lock(*getMutex()); eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - for (auto& server : mp_builtin->m_DiscoveryServers) - { - // Get the info to send to this already known locators - locators_ping.push_back(server.metatrafficUnicastLocatorList); - } + // Get the info to send to this already known locators + locators_ping = mp_builtin->m_DiscoveryServers; } if (!locators_ping.empty()) diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp index e93e9950705..9d6a6f52538 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.hpp @@ -193,12 +193,12 @@ class PDPServer : public fastdds::rtps::PDP fastdds::rtps::ddb::DiscoveryDataBase& discovery_db(); /** - * Access to the remote servers list + * Access to the remote servers locators list * This method is not thread safe. * The return reference may be invalidated if the user modifies simultaneously the remote server list. - * @return constant reference to the remote servers list + * @return constant reference to the remote servers locators list */ - const RemoteServerList_t& servers(); + const LocatorList& servers(); protected: diff --git a/src/cpp/rtps/builtin/discovery/participant/timedevent/DSClientEvent.cpp b/src/cpp/rtps/builtin/discovery/participant/timedevent/DSClientEvent.cpp index 0dcb38b3bb4..6d258646117 100644 --- a/src/cpp/rtps/builtin/discovery/participant/timedevent/DSClientEvent.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/timedevent/DSClientEvent.cpp @@ -82,7 +82,7 @@ bool DSClientEvent::event() } } - if (mp_PDP->connected_servers().size() < mp_PDP->remote_server_attributes().size()) + if (mp_PDP->connected_servers().size() < mp_PDP->remote_server_locators().size()) { // If there are pending servers, we need to run the event again restart = true; diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp index b16d7f3be2f..2aeac35ffb0 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp @@ -334,21 +334,18 @@ RTPSParticipantImpl::RTPSParticipantImpl( } }); } - for (fastdds::rtps::RemoteServerAttributes& it : m_att.builtin.discovery_config.m_DiscoveryServers) - { - std::for_each(it.metatrafficUnicastLocatorList.begin(), - it.metatrafficUnicastLocatorList.end(), [&](Locator_t& locator) + std::for_each(m_att.builtin.discovery_config.m_DiscoveryServers.begin(), + m_att.builtin.discovery_config.m_DiscoveryServers.end(), [&](Locator_t& locator) + { + // TCP DS default logical port is the same as the physical one + if (locator.kind == LOCATOR_KIND_TCPv4 || locator.kind == LOCATOR_KIND_TCPv6) { - // TCP DS default logical port is the same as the physical one - if (locator.kind == LOCATOR_KIND_TCPv4 || locator.kind == LOCATOR_KIND_TCPv6) + if (IPLocator::getLogicalPort(locator) == 0) { - if (IPLocator::getLogicalPort(locator) == 0) - { - IPLocator::setLogicalPort(locator, IPLocator::getPhysicalPort(locator)); - } + IPLocator::setLogicalPort(locator, IPLocator::getPhysicalPort(locator)); } - }); - } + } + }); } } break; @@ -367,21 +364,18 @@ RTPSParticipantImpl::RTPSParticipantImpl( " tries to create a TCP client for discovery server without providing a proper listening port." << " No TCP participants will be able to connect to this participant, but it will be able make connections."); } - for (fastdds::rtps::RemoteServerAttributes& it : m_att.builtin.discovery_config.m_DiscoveryServers) - { - std::for_each(it.metatrafficUnicastLocatorList.begin(), - it.metatrafficUnicastLocatorList.end(), [&](Locator_t& locator) + std::for_each(m_att.builtin.discovery_config.m_DiscoveryServers.begin(), + m_att.builtin.discovery_config.m_DiscoveryServers.end(), [&](Locator_t& locator) + { + // TCP DS default logical port is the same as the physical one + if (locator.kind == LOCATOR_KIND_TCPv4 || locator.kind == LOCATOR_KIND_TCPv6) { - // TCP DS default logical port is the same as the physical one - if (locator.kind == LOCATOR_KIND_TCPv4 || locator.kind == LOCATOR_KIND_TCPv6) + if (IPLocator::getLogicalPort(locator) == 0) { - if (IPLocator::getLogicalPort(locator) == 0) - { - IPLocator::setLogicalPort(locator, IPLocator::getPhysicalPort(locator)); - } + IPLocator::setLogicalPort(locator, IPLocator::getPhysicalPort(locator)); } - }); - } + } + }); } } default: @@ -1455,30 +1449,24 @@ void RTPSParticipantImpl::update_attributes( bool update_pdp = false; // Check if discovery servers need to be updated - eprosima::fastdds::rtps::RemoteServerList_t converted_discovery_servers = + eprosima::fastdds::rtps::LocatorList_t converted_discovery_servers = patt.builtin.discovery_config.m_DiscoveryServers; - if (patt.builtin.discovery_config.m_DiscoveryServers != m_att.builtin.discovery_config.m_DiscoveryServers) + if (converted_discovery_servers != m_att.builtin.discovery_config.m_DiscoveryServers) { for (auto& transportDescriptor : m_att.userTransports) { TCPTransportDescriptor* pT = dynamic_cast(transportDescriptor.get()); if (pT) { - for (fastdds::rtps::RemoteServerAttributes& it : converted_discovery_servers) - { - std::for_each(it.metatrafficUnicastLocatorList.begin(), - it.metatrafficUnicastLocatorList.end(), [&](Locator_t& locator) + // TCP DS default logical port is the same as the physical one + std::for_each(converted_discovery_servers.begin(), + converted_discovery_servers.end(), [&](Locator_t& locator) + { + if (IPLocator::getLogicalPort(locator) == 0) { - // TCP DS default logical port is the same as the physical one - if (locator.kind == LOCATOR_KIND_TCPv4 || locator.kind == LOCATOR_KIND_TCPv6) - { - if (IPLocator::getLogicalPort(locator) == 0) - { - IPLocator::setLogicalPort(locator, IPLocator::getPhysicalPort(locator)); - } - } - }); - } + IPLocator::setLogicalPort(locator, IPLocator::getPhysicalPort(locator)); + } + }); } } } @@ -2704,19 +2692,6 @@ bool RTPSParticipantImpl::ignore_participant( EPROSIMA_LOG_WARNING(RTPS_PARTICIPANT, "A participant is unable to ignore itself"); return false; } - { - shared_lock _(mp_builtinProtocols->getDiscoveryMutex()); - - for (auto server_it = m_att.builtin.discovery_config.m_DiscoveryServers.begin(); - server_it != m_att.builtin.discovery_config.m_DiscoveryServers.end(); server_it++) - { - if (server_it->guidPrefix == participant_guid) - { - EPROSIMA_LOG_WARNING(RTPS_PARTICIPANT, "Cannot ignore one of this participant Discovery Servers"); - return false; - } - } - } { std::unique_lock _(ignored_mtx_); ignored_participants_.insert(participant_guid); From 2acdc9baaabe0bef19ed60da227391fcf84443c3 Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Wed, 10 Apr 2024 08:10:30 +0200 Subject: [PATCH 18/51] Refs #20629: Use LocatorList for remote servers - XML Signed-off-by: cferreiragonz --- resources/xsd/fastdds_profiles.xsd | 22 ++----------------- src/cpp/xmlparser/XMLElementParser.cpp | 4 ++-- ...ple_secure_msg_crypto_ds_1_sub_profile.xml | 16 +++++--------- ...s_simple_secure_msg_crypto_pub_profile.xml | 16 +++++--------- ...s_simple_secure_msg_crypto_sub_profile.xml | 16 +++++--------- ...ry_no_rtps_protection_ds_1_sub_profile.xml | 16 +++++--------- ...scovery_no_rtps_protection_pub_profile.xml | 16 +++++--------- ...scovery_no_rtps_protection_sub_profile.xml | 16 +++++--------- ..._discovery_protection_ds_1_sub_profile.xml | 16 +++++--------- ...re_no_discovery_protection_pub_profile.xml | 16 +++++--------- ...re_no_discovery_protection_sub_profile.xml | 16 +++++--------- ...re_no_rtps_protection_ds_1_sub_profile.xml | 16 +++++--------- ..._secure_no_rtps_protection_pub_profile.xml | 16 +++++--------- ..._secure_no_rtps_protection_sub_profile.xml | 16 +++++--------- .../secure_simple_ds_server_idx_1_profile.xml | 16 +++++--------- ...overy_no_rtps_protection_idx_1_profile.xml | 16 +++++--------- ..._no_discovery_protection_idx_1_profile.xml | 16 +++++--------- ...erver_no_rtps_protection_idx_1_profile.xml | 16 +++++--------- .../tools/xmlvalidation/all_profile.xml | 16 +++++--------- .../domainParticipant_profile.xml | 16 +++++--------- 20 files changed, 112 insertions(+), 202 deletions(-) diff --git a/resources/xsd/fastdds_profiles.xsd b/resources/xsd/fastdds_profiles.xsd index e85067db314..5b4409368ce 100644 --- a/resources/xsd/fastdds_profiles.xsd +++ b/resources/xsd/fastdds_profiles.xsd @@ -740,11 +740,7 @@