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 all 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
26 changes: 6 additions & 20 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 @@ -376,14 +376,8 @@ bool ParticipantProxyData::readFromCDRMessage(
const NetworkFactory& network,
bool is_shm_transport_available)
{
bool are_shm_metatraffic_locators_present = false;
bool are_shm_default_locators_present = false;
bool is_shm_transport_possible = false;

auto param_process = [this, &network, &is_shm_transport_possible,
&are_shm_metatraffic_locators_present,
&are_shm_default_locators_present,
&is_shm_transport_available](CDRMessage_t* msg, const ParameterId_t& pid, uint16_t plength)
auto param_process = [this, &network, &is_shm_transport_available](
CDRMessage_t* msg, const ParameterId_t& pid, uint16_t plength)
{
switch (pid)
{
Expand Down Expand Up @@ -468,9 +462,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 +482,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 +502,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 +522,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
17 changes: 4 additions & 13 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 @@ -620,13 +620,8 @@ bool ReaderProxyData::readFromCDRMessage(
const NetworkFactory& network,
bool is_shm_transport_available)
{
bool are_shm_default_locators_present = false;
bool is_shm_transport_possible = false;

auto param_process = [this, &network,
&is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_default_locators_present](CDRMessage_t* msg, const ParameterId_t& pid, uint16_t plength)
auto param_process = [this, &network, &is_shm_transport_available](
CDRMessage_t* msg, const ParameterId_t& pid, uint16_t plength)
{
switch (pid)
{
Expand Down Expand Up @@ -840,9 +835,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 +855,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
17 changes: 4 additions & 13 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 @@ -606,13 +606,8 @@ bool WriterProxyData::readFromCDRMessage(
const NetworkFactory& network,
bool is_shm_transport_available)
{
bool are_shm_default_locators_present = false;
bool is_shm_transport_possible = false;

auto param_process = [this, &network,
&is_shm_transport_available,
&is_shm_transport_possible,
&are_shm_default_locators_present](CDRMessage_t* msg, const ParameterId_t& pid, uint16_t plength)
auto param_process = [this, &network, &is_shm_transport_available](
CDRMessage_t* msg, const ParameterId_t& pid, uint16_t plength)
{
switch (pid)
{
Expand Down Expand Up @@ -844,9 +839,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 +859,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
Loading