Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16133] Fix corner cases on external locators selection #3079

Merged
merged 8 commits into from
Nov 11, 2022
Merged

[16133] Fix corner cases on external locators selection #3079

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fde4e12
Refs #16104. Added regression test.
MiguelCompany Nov 10, 2022
29de239
Refs #16104. Do not drop UDP locators when compatible SHM locators ar…
MiguelCompany Nov 7, 2022
eb1a689
Refs #16104. Static method Host::compute_id.
MiguelCompany Nov 7, 2022
1442c94
Refs #16104. Using metatraffic_external_unicast_locators for host id …
MiguelCompany Nov 7, 2022
d466bb0
Refs #16104. Automagically match localhost locators.
MiguelCompany Nov 10, 2022
e88bda5
Refs #16104. Linters.
MiguelCompany Nov 10, 2022
4f1b440
Refs #16133. Add comment on host id hashing.
MiguelCompany Nov 11, 2022
88b9472
Refs #16133. Remove unused variables.
MiguelCompany Nov 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/cpp/rtps/RTPSDomain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include <rtps/participant/RTPSParticipantImpl.h>

#include <rtps/common/GuidUtils.hpp>
#include <rtps/network/ExternalLocatorsProcessor.hpp>
#include <utils/Host.hpp>
#include <utils/SystemInfo.hpp>

Expand Down Expand Up @@ -167,6 +168,16 @@ RTPSParticipant* RTPSDomain::createParticipant(
// Generate a new GuidPrefix_t
GuidPrefix_t guidP;
guid_prefix_create(ID, guidP);
if (!PParam.builtin.metatraffic_external_unicast_locators.empty())
EduPonz marked this conversation as resolved.
Show resolved Hide resolved
{
fastdds::rtps::LocatorList locators;
fastrtps::rtps::IPFinder::getIP4Address(&locators);
fastdds::rtps::ExternalLocatorsProcessor::add_external_locators(locators,
PParam.builtin.metatraffic_external_unicast_locators);
uint16_t host_id = Host::compute_id(locators);
guidP.value[2] = static_cast<octet>(host_id & 0xFF);
guidP.value[3] = static_cast<octet>((host_id >> 8) & 0xFF);
}

RTPSParticipant* p = new RTPSParticipant(nullptr);
RTPSParticipantImpl* pimpl = nullptr;
Expand Down
16 changes: 4 additions & 12 deletions src/cpp/rtps/builtin/data/ParticipantProxyData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,7 @@ bool ParticipantProxyData::readFromCDRMessage(
{
ProxyDataFilters::filter_locators(
is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_metatraffic_locators_present,
&metatraffic_locators,
metatraffic_locators,
temp_locator,
false);
}
Expand All @@ -490,9 +488,7 @@ bool ParticipantProxyData::readFromCDRMessage(
{
ProxyDataFilters::filter_locators(
is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_metatraffic_locators_present,
&metatraffic_locators,
metatraffic_locators,
temp_locator,
true);
}
Expand All @@ -512,9 +508,7 @@ bool ParticipantProxyData::readFromCDRMessage(
{
ProxyDataFilters::filter_locators(
is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_default_locators_present,
&default_locators,
default_locators,
temp_locator,
true);
}
Expand All @@ -534,9 +528,7 @@ bool ParticipantProxyData::readFromCDRMessage(
{
ProxyDataFilters::filter_locators(
is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_default_locators_present,
&default_locators,
default_locators,
temp_locator,
false);
}
Expand Down
66 changes: 11 additions & 55 deletions src/cpp/rtps/builtin/data/ProxyDataFilters.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,79 +30,35 @@ class ProxyDataFilters
public:

/**
* As locator are parsed, when a CDR encapsulated proxydata message is received,
* this function decides whether SHM communication is possible, in that case only
* SHM locators are stored in the target_locator_list. If SHM communication is
* not possible SHM locators are not stored in the list.
* This function filters out SHM locators when they cannot be used for communication on the local host.
* @param[in] is_shm_transport_available Indicates whether the participant has SHM transport enabled.
* @param[in/out] is_shm_transport_possible Is true when at least a SHM locator from the local host has
* been parsed
* @param[in/out] are_shm_locators_present True when SHM locators has been parsed
* @param[in/out] target_locators_list List where parsed locators are stored
* @param[in,out] target_locators_list List where parsed locators are stored
* @param[in] temp_locator New locator to parse
* @param[in] is_unicast true if temp_locator is unicast, false if it is multicast
*/
static void filter_locators(
bool is_shm_transport_available,
bool* is_shm_transport_possible,
bool* are_shm_locators_present,
RemoteLocatorList* target_locators_list,
RemoteLocatorList& target_locators_list,
const Locator_t& temp_locator,
bool is_unicast)
{
using SHMLocator = eprosima::fastdds::rtps::SHMLocator;

if (is_shm_transport_available && !(*is_shm_transport_possible))
bool can_use_locator = LOCATOR_KIND_SHM != temp_locator.kind;
if (!can_use_locator)
{
*is_shm_transport_possible = SHMLocator::is_shm_and_from_this_host(temp_locator);
can_use_locator = is_shm_transport_available && SHMLocator::is_shm_and_from_this_host(temp_locator);
}

if (*is_shm_transport_possible)
if (can_use_locator)
{
if (temp_locator.kind == LOCATOR_KIND_SHM)
if (is_unicast)
{
// First SHM locator
if (!(*are_shm_locators_present))
{
// Remove previously added locators from other transports
target_locators_list->unicast.clear();
target_locators_list->multicast.clear();
*are_shm_locators_present = true;
}

if (is_unicast)
{
target_locators_list->add_unicast_locator(temp_locator);
}
else
{
target_locators_list->add_multicast_locator(temp_locator);
}
}
else if (!(*are_shm_locators_present))
{
if (is_unicast)
{
target_locators_list->add_unicast_locator(temp_locator);
}
else
{
target_locators_list->add_multicast_locator(temp_locator);
}
target_locators_list.add_unicast_locator(temp_locator);
}
}
else
{
if (temp_locator.kind != LOCATOR_KIND_SHM)
else
{
if (is_unicast)
{
target_locators_list->add_unicast_locator(temp_locator);
}
else
{
target_locators_list->add_multicast_locator(temp_locator);
}
target_locators_list.add_multicast_locator(temp_locator);
}
}
}
Expand Down
8 changes: 2 additions & 6 deletions src/cpp/rtps/builtin/data/ReaderProxyData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -840,9 +840,7 @@ bool ReaderProxyData::readFromCDRMessage(
{
ProxyDataFilters::filter_locators(
is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_default_locators_present,
&remote_locators_,
remote_locators_,
temp_locator,
true);
}
Expand All @@ -862,9 +860,7 @@ bool ReaderProxyData::readFromCDRMessage(
{
ProxyDataFilters::filter_locators(
is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_default_locators_present,
&remote_locators_,
remote_locators_,
temp_locator,
false);
}
Expand Down
8 changes: 2 additions & 6 deletions src/cpp/rtps/builtin/data/WriterProxyData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,9 +844,7 @@ bool WriterProxyData::readFromCDRMessage(
{
ProxyDataFilters::filter_locators(
is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_default_locators_present,
&remote_locators_,
remote_locators_,
temp_locator,
true);
}
Expand All @@ -866,9 +864,7 @@ bool WriterProxyData::readFromCDRMessage(
{
ProxyDataFilters::filter_locators(
is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_default_locators_present,
&remote_locators_,
remote_locators_,
temp_locator,
false);
}
Expand Down
39 changes: 39 additions & 0 deletions src/cpp/rtps/network/ExternalLocatorsProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <fastdds/rtps/builtin/data/WriterProxyData.h>
#include <fastdds/rtps/common/LocatorList.hpp>
#include <fastdds/rtps/common/LocatorSelectorEntry.hpp>
#include <fastrtps/utils/IPLocator.h>

namespace eprosima {
namespace fastdds {
Expand Down Expand Up @@ -110,6 +111,27 @@ static void perform_add_external_locators(
}
}

template<>
void perform_add_external_locators<LocatorList>(
LocatorList& locators,
const ExternalLocators& external_locators)
{
for (const auto& externality_item : external_locators)
{
// Only add locators with externality greater than 0
if (externality_item.first > 0)
{
for (const auto& cost_item : externality_item.second)
{
for (const auto& locator : cost_item.second)
{
locators.push_back(locator);
}
}
}
EduPonz marked this conversation as resolved.
Show resolved Hide resolved
}
}

void add_external_locators(
ParticipantProxyData& data,
const ExternalLocators& metatraffic_external_locators,
Expand All @@ -133,6 +155,13 @@ void add_external_locators(
perform_add_external_locators(data, external_locators);
}

void add_external_locators(
LocatorList& list,
const ExternalLocators& external_locators)
{
perform_add_external_locators(list, external_locators);
}

/**
* Checks if the first significant bits of two locator addresses match.
*
Expand Down Expand Up @@ -207,6 +236,16 @@ static uint64_t heuristic(
const ExternalLocators& external_locators,
bool ignore_non_matching)
{
if (LOCATOR_KIND_SHM == remote_locator.kind)
{
return heuristic_value(0, 0);
}

if (fastrtps::rtps::IPLocator::isLocal(remote_locator))
{
return heuristic_value(0, 1);
}

for (const auto& externality : external_locators)
{
for (const auto& cost : externality.second)
Expand Down
10 changes: 10 additions & 0 deletions src/cpp/rtps/network/ExternalLocatorsProcessor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,16 @@ void add_external_locators(
ReaderProxyData& data,
const ExternalLocators& external_locators);

/**
* Adds external locators to a list of locators.
*
* @param [in,out] list LocatorList to be updated.
* @param [in] external_locators The external locators collection with the external locators to be announced.
*/
void add_external_locators(
LocatorList& list,
const ExternalLocators& external_locators);

/**
* Filters the locators of a remote participant according to the matching algorithm.
*
Expand Down
55 changes: 34 additions & 21 deletions src/cpp/utils/Host.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,35 +60,48 @@ class Host
return singleton;
}

private:

Host()
static inline uint16_t compute_id(
const fastdds::rtps::LocatorList& loc)
{
// Compute the host id
fastdds::rtps::LocatorList loc;
fastrtps::rtps::IPFinder::getIP4Address(&loc);
uint16_t ret_val = 0;

if (loc.size() > 0)
{
if (loc.size() > 0)
MD5 md5;
for (auto& l : loc)
{
MD5 md5;
for (auto& l : loc)
{
md5.update(l.address, sizeof(l.address));
}
md5.finalize();
id_ = 0;
for (size_t i = 0; i < sizeof(md5.digest); i += 2)
{
id_ ^= ((md5.digest[i] << 8) | md5.digest[i + 1]);
}
md5.update(l.address, sizeof(l.address));
}
else
md5.finalize();

// Hash the 16-bytes md5.digest into a uint16_t
ret_val = 0;
for (size_t i = 0; i < sizeof(md5.digest); i += 2)
{
reinterpret_cast<uint8_t*>(&id_)[0] = 127;
reinterpret_cast<uint8_t*>(&id_)[1] = 1;
// Treat the next two bytes as a big-endian uint16_t and
// hash them into ret_val.
uint16_t tmp = static_cast<uint16_t>(md5.digest[i]);
tmp = (tmp << 8) | static_cast<uint16_t>(md5.digest[i + 1]);
ret_val ^= tmp;
EduPonz marked this conversation as resolved.
Show resolved Hide resolved
}
}
else
{
reinterpret_cast<uint8_t*>(&ret_val)[0] = 127;
reinterpret_cast<uint8_t*>(&ret_val)[1] = 1;
}

return ret_val;
}

private:

Host()
{
// Compute the host id
fastdds::rtps::LocatorList loc;
fastrtps::rtps::IPFinder::getIP4Address(&loc);
id_ = compute_id(loc);

// Compute the MAC id
std::vector<fastrtps::rtps::IPFinder::info_MAC> macs;
Expand Down
Loading