From 82d589f3cbd144ec1ec3e0759beffa1f56e6c2ea Mon Sep 17 00:00:00 2001 From: cferreiragonz Date: Tue, 12 Nov 2024 08:40:14 +0100 Subject: [PATCH] Refs #22056: Avoid sending machine_id in Data(r/w) Signed-off-by: cferreiragonz --- src/cpp/rtps/builtin/data/ReaderProxyData.cpp | 126 +++++++----------- src/cpp/rtps/builtin/data/ReaderProxyData.hpp | 14 ++ src/cpp/rtps/builtin/data/WriterProxyData.cpp | 101 +++++++------- src/cpp/rtps/builtin/data/WriterProxyData.hpp | 14 ++ .../discovery/endpoint/EDPSimpleListeners.cpp | 6 +- 5 files changed, 128 insertions(+), 133 deletions(-) diff --git a/src/cpp/rtps/builtin/data/ReaderProxyData.cpp b/src/cpp/rtps/builtin/data/ReaderProxyData.cpp index 874202416b6..e8c7adc16c3 100644 --- a/src/cpp/rtps/builtin/data/ReaderProxyData.cpp +++ b/src/cpp/rtps/builtin/data/ReaderProxyData.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -192,13 +193,6 @@ uint32_t ReaderProxyData::get_serialized_size( // PID_NETWORK_CONFIGURATION_SET ret_val += 4 + PARAMETER_NETWORKCONFIGSET_LENGTH; - if (m_host_id.size() > 0) - { - // PID_HOST_ID - ret_val += - fastdds::dds::ParameterSerializer::cdr_serialized_size(m_host_id); - } - // PID_UNICAST_LOCATOR ret_val += static_cast((4 + PARAMETER_LOCATOR_LENGTH) * remote_locators_.unicast.size()); @@ -389,15 +383,6 @@ bool ReaderProxyData::writeToCDRMessage( } } - if (m_host_id.size() > 0) - { - ParameterString_t p(fastdds::dds::PID_HOST_ID, 0, m_host_id); - if (!fastdds::dds::ParameterSerializer::add_to_cdr_message(p, msg)) - { - return false; - } - } - for (const Locator_t& locator : remote_locators_.unicast) { ParameterLocator_t p(fastdds::dds::PID_UNICAST_LOCATOR, PARAMETER_LOCATOR_LENGTH, locator); @@ -897,33 +882,6 @@ bool ReaderProxyData::readFromCDRMessage( m_networkConfiguration = p.netconfigSet; break; } - case fastdds::dds::PID_HOST_ID: - { - VendorId_t local_vendor_id = source_vendor_id; - if (c_VendorId_Unknown == local_vendor_id) - { - local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id); - } - - // Ignore custom PID when coming from other vendors - if (c_VendorId_eProsima != local_vendor_id) - { - EPROSIMA_LOG_INFO(RTPS_PROXY_DATA, - "Ignoring custom PID" << pid << " from vendor " << local_vendor_id); - return true; - } - - ParameterString_t p(pid, plength); - if (!fastdds::dds::ParameterSerializer::read_from_cdr_message( - p, msg, - plength)) - { - return false; - } - - m_host_id = p.getName(); - break; - } case fastdds::dds::PID_UNICAST_LOCATOR: { ParameterLocator_t p(pid, plength); @@ -933,23 +891,7 @@ bool ReaderProxyData::readFromCDRMessage( return false; } - if (!should_filter_locators) - { - remote_locators_.add_unicast_locator(p.locator); - } - else - { - Locator_t temp_locator; - if (network.transform_remote_locator(p.locator, temp_locator, m_networkConfiguration, - check_same_host())) - { - ProxyDataFilters::filter_locators( - network, - remote_locators_, - temp_locator, - true); - } - } + remote_locators_.add_unicast_locator(p.locator); break; } case fastdds::dds::PID_MULTICAST_LOCATOR: @@ -961,23 +903,7 @@ bool ReaderProxyData::readFromCDRMessage( return false; } - if (!should_filter_locators) - { - remote_locators_.add_multicast_locator(p.locator); - } - else - { - Locator_t temp_locator; - if (network.transform_remote_locator(p.locator, temp_locator, m_networkConfiguration, - check_same_host())) - { - ProxyDataFilters::filter_locators( - network, - remote_locators_, - temp_locator, - false); - } - } + remote_locators_.add_unicast_locator(p.locator); break; } case fastdds::dds::PID_EXPECTS_INLINE_QOS: @@ -1193,6 +1119,52 @@ bool ReaderProxyData::readFromCDRMessage( return false; } +void ReaderProxyData::setup_locators( + const ReaderProxyData* rdata, + NetworkFactory& network, + const ParticipantProxyData& participant_data) +{ + if (this == rdata) + { + return; + } + + machine_id = participant_data.machine_id; + + if (has_locators()) + { + // Get the transformed remote locators for the ReaderProxyData received + remote_locators_.unicast.clear(); + remote_locators_.multicast.clear(); + for (const Locator_t& locator : rdata->remote_locators_.unicast) + { + Locator_t temp_locator; + if (network.transform_remote_locator(locator, temp_locator, m_networkConfiguration, + is_from_this_host())) + { + ProxyDataFilters::filter_locators(network, remote_locators_, temp_locator, true); + } + } + for (const Locator_t& locator : rdata->remote_locators_.multicast) + { + Locator_t temp_locator; + if (network.transform_remote_locator(locator, temp_locator, m_networkConfiguration, + is_from_this_host())) + { + ProxyDataFilters::filter_locators(network, remote_locators_, temp_locator, true); + } + } + auto locators = remote_locators_; + set_remote_locators(locators, network, true); + } + else + { + // Get the remote locators from the participant_data + set_remote_locators(participant_data.default_locators, network, true); + } + +} + bool ReaderProxyData::is_from_this_host() { bool same_host = false; diff --git a/src/cpp/rtps/builtin/data/ReaderProxyData.hpp b/src/cpp/rtps/builtin/data/ReaderProxyData.hpp index 5a0e1942f3c..03999ce32ff 100644 --- a/src/cpp/rtps/builtin/data/ReaderProxyData.hpp +++ b/src/cpp/rtps/builtin/data/ReaderProxyData.hpp @@ -35,6 +35,7 @@ namespace rtps { struct CDRMessage_t; class NetworkFactory; +class ParticipantProxyData; /** * Class ReaderProxyData, used to represent all the information on a Reader (both local and remote) with the purpose of @@ -434,6 +435,19 @@ class ReaderProxyData bool should_filter_locators, fastdds::rtps::VendorId_t source_vendor_id = c_VendorId_eProsima); + /** + * Transform and set the remote locators from the remote_locators_ of another ReaderProxyData. + * If the received WriterProxyData has no locators, remote locators will be extracted from the + * ParticipantProxyData. + * @param rdata ReaderProxyData to get the locators from + * @param network NetworkFactory to transform locators + * @param participant_data ParticipantProxyData to get the locators from + */ + void setup_locators( + const ReaderProxyData* wdata, + NetworkFactory& network, + const ParticipantProxyData& participant_data); + /** * Check if the host is the same as the one that sent the data. * It tries to use the machine_id. If it is not available, it will compare GUIDs. diff --git a/src/cpp/rtps/builtin/data/WriterProxyData.cpp b/src/cpp/rtps/builtin/data/WriterProxyData.cpp index cfab52e59ff..df0298a9804 100644 --- a/src/cpp/rtps/builtin/data/WriterProxyData.cpp +++ b/src/cpp/rtps/builtin/data/WriterProxyData.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -184,13 +185,6 @@ uint32_t WriterProxyData::get_serialized_size( // PID_NETWORK_CONFIGURATION_SET ret_val += 4 + PARAMETER_NETWORKCONFIGSET_LENGTH; - if (m_host_id.size() > 0) - { - // PID_HOST_ID - ret_val += - fastdds::dds::ParameterSerializer::cdr_serialized_size(m_host_id); - } - // PID_UNICAST_LOCATOR ret_val += static_cast((4 + PARAMETER_LOCATOR_LENGTH) * remote_locators_.unicast.size()); @@ -371,15 +365,6 @@ bool WriterProxyData::writeToCDRMessage( } } - if (m_host_id.size() > 0) - { - ParameterString_t p(fastdds::dds::PID_HOST_ID, 0, m_host_id); - if (!fastdds::dds::ParameterSerializer::add_to_cdr_message(p, msg)) - { - return false; - } - } - for (const Locator_t& locator : remote_locators_.unicast) { ParameterLocator_t p(fastdds::dds::PID_UNICAST_LOCATOR, PARAMETER_LOCATOR_LENGTH, locator); @@ -903,23 +888,7 @@ bool WriterProxyData::readFromCDRMessage( return false; } - if (!should_filter_locators) - { - remote_locators_.add_unicast_locator(p.locator); - } - else - { - Locator_t temp_locator; - if (network.transform_remote_locator(p.locator, temp_locator, m_networkConfiguration, - check_same_host())) - { - ProxyDataFilters::filter_locators( - network, - remote_locators_, - temp_locator, - true); - } - } + remote_locators_.add_unicast_locator(p.locator); break; } case fastdds::dds::PID_MULTICAST_LOCATOR: @@ -930,23 +899,7 @@ bool WriterProxyData::readFromCDRMessage( return false; } - if (!should_filter_locators) - { - remote_locators_.add_multicast_locator(p.locator); - } - else - { - Locator_t temp_locator; - if (network.transform_remote_locator(p.locator, temp_locator, m_networkConfiguration, - check_same_host())) - { - ProxyDataFilters::filter_locators( - network, - remote_locators_, - temp_locator, - false); - } - } + remote_locators_.add_multicast_locator(p.locator); break; } case fastdds::dds::PID_KEY_HASH: @@ -1134,7 +1087,53 @@ bool WriterProxyData::readFromCDRMessage( return false; } -bool WriterProxyData::check_same_host() +void WriterProxyData::setup_locators( + const WriterProxyData* wdata, + NetworkFactory& network, + const ParticipantProxyData& participant_data) +{ + if (this == wdata) + { + return; + } + + machine_id = participant_data.machine_id; + + if (has_locators()) + { + // Get the transformed remote locators for the WriterProxyData received + remote_locators_.unicast.clear(); + remote_locators_.multicast.clear(); + for (const Locator_t& locator : wdata->remote_locators_.unicast) + { + Locator_t temp_locator; + if (network.transform_remote_locator(locator, temp_locator, m_networkConfiguration, + is_from_this_host())) + { + ProxyDataFilters::filter_locators(network, remote_locators_, temp_locator, true); + } + } + for (const Locator_t& locator : wdata->remote_locators_.multicast) + { + Locator_t temp_locator; + if (network.transform_remote_locator(locator, temp_locator, m_networkConfiguration, + is_from_this_host())) + { + ProxyDataFilters::filter_locators(network, remote_locators_, temp_locator, true); + } + } + auto locators = remote_locators_; + set_remote_locators(locators, network, true); + } + else + { + // Get the remote locators from the participant_data + set_remote_locators(participant_data.default_locators, network, true); + } + +} + +bool WriterProxyData::is_from_this_host() { bool same_host = false; if (machine_id.size() > 0) diff --git a/src/cpp/rtps/builtin/data/WriterProxyData.hpp b/src/cpp/rtps/builtin/data/WriterProxyData.hpp index 949ef1a410b..3969e5b56ee 100644 --- a/src/cpp/rtps/builtin/data/WriterProxyData.hpp +++ b/src/cpp/rtps/builtin/data/WriterProxyData.hpp @@ -37,6 +37,7 @@ namespace rtps { struct CDRMessage_t; class NetworkFactory; +class ParticipantProxyData; /** **@ingroup BUILTIN_MODULE @@ -455,6 +456,19 @@ class WriterProxyData bool should_filter_locators, fastdds::rtps::VendorId_t source_vendor_id = c_VendorId_eProsima); + /** + * Transform and set the remote locators from the remote_locators_ of another WriterProxyData. + * If the received WriterProxyData has no locators, remote locators will be extracted from the + * ParticipantProxyData. + * @param wdata WriterProxyData to get the locators from + * @param network NetworkFactory to transform locators + * @param participant_data ParticipantProxyData to get the locators from + */ + void setup_locators( + const WriterProxyData* wdata, + NetworkFactory& network, + const ParticipantProxyData& participant_data); + /** * Check if the host is the same as the one that sent the data. * It tries to use the machine_id. If it is not available, it will compare GUIDs. diff --git a/src/cpp/rtps/builtin/discovery/endpoint/EDPSimpleListeners.cpp b/src/cpp/rtps/builtin/discovery/endpoint/EDPSimpleListeners.cpp index dd81e438fdb..fb5a972673f 100644 --- a/src/cpp/rtps/builtin/discovery/endpoint/EDPSimpleListeners.cpp +++ b/src/cpp/rtps/builtin/discovery/endpoint/EDPSimpleListeners.cpp @@ -95,11 +95,6 @@ void EDPBasePUBListener::add_writer_from_change( bool updating, const ParticipantProxyData& participant_data) { - if (!temp_writer_data->has_locators()) - { - temp_writer_data->set_remote_locators(participant_data.default_locators, network, - true); - } if (updating && !data->is_update_allowed(*temp_writer_data)) { @@ -108,6 +103,7 @@ void EDPBasePUBListener::add_writer_from_change( data->guid()); } *data = *temp_writer_data; + data->setup_locators(temp_writer_data, network, participant_data); if (request_ret_status != fastdds::dds::RETCODE_OK) {