Skip to content

Commit f2a55e9

Browse files
cferreiragonzcferreiragonz
authored
Avoid calling setIPv4 in TCPTransportInterface (#5492)
* Refs #22469: Regression Tests for IPv6 Signed-off-by: cferreiragonz <carlosferreira@eprosima.com> * Refs #22469: New method copyAddress Signed-off-by: cferreiragonz <carlosferreira@eprosima.com> * Refs #22469: Fix TCPTransportInterface Signed-off-by: cferreiragonz <carlosferreira@eprosima.com> * Refs #22469: Apply Review Signed-off-by: cferreiragonz <carlosferreira@eprosima.com> * Refs #22469: Disable grouping of locators in single channel for TCPv6 Signed-off-by: cferreiragonz <carlosferreira@eprosima.com> * Refs #22469: Uncrustify Signed-off-by: cferreiragonz <carlosferreira@eprosima.com> * Refs #22469: Apply review Signed-off-by: cferreiragonz <carlosferreira@eprosima.com> * Refs #22469: Allow to copyIPv4 between two locators Signed-off-by: cferreiragonz <carlosferreira@eprosima.com> * Refs #22469: Apply Review 2 Signed-off-by: cferreiragonz <carlosferreira@eprosima.com> --------- Signed-off-by: cferreiragonz <carlosferreira@eprosima.com>
1 parent 6bae07c commit f2a55e9

6 files changed

Lines changed: 113 additions & 18 deletions

File tree

‎include/fastdds/utils/IPLocator.hpp‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,27 @@ class IPLocator
9797
FASTDDS_EXPORTED_API static std::string toIPv4string(
9898
const Locator_t& locator);
9999

100-
//! Copies locator's IPv4.
100+
/**
101+
* @brief Copies locator's IPv4 to a destination array.
102+
* @param locator Locator from which to copy the IPv4.
103+
* @param dest Destination array where the IPv4 will be copied.
104+
* @return true if the copy was successful, false otherwise.
105+
*/
101106
FASTDDS_EXPORTED_API static bool copyIPv4(
102107
const Locator_t& locator,
103108
unsigned char* dest);
104109

110+
/**
111+
* @brief Copies locator's IPv4 to a destination locator.
112+
* It only copies the IPv4 part (last 4 bytes), leaving other parts unchanged.
113+
* @param locator Locator from which to copy the IPv4.
114+
* @param dest Destination locator where the IPv4 will be copied.
115+
* @return true if the copy was successful, false otherwise.
116+
*/
117+
FASTDDS_EXPORTED_API static bool copyIPv4(
118+
const Locator_t& locator,
119+
Locator_t& dest);
120+
105121
// IPv6
106122
//! Sets locator's IPv6.
107123
FASTDDS_EXPORTED_API static bool setIPv6(
@@ -252,6 +268,16 @@ class IPLocator
252268
const Locator_t& loc2,
253269
bool fullAddress = false);
254270

271+
/**
272+
* Copies the whole address from one locator to another.
273+
* @param loc1 Locator to copy from.
274+
* @param loc2 Locator to copy to.
275+
* @return True if the copy was successful.
276+
*/
277+
FASTDDS_EXPORTED_API static bool copy_address(
278+
const Locator_t& loc1,
279+
Locator_t& loc2);
280+
255281
//! Checks if a both locators has the same IP address and physical port (as in RTCP protocol).
256282
FASTDDS_EXPORTED_API static bool compareAddressAndPhysicalPort(
257283
const Locator_t& loc1,

‎src/cpp/rtps/transport/TCPTransportInterface.cpp‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -319,18 +319,22 @@ ResponseCode TCPTransportInterface::bind_socket(
319319

320320
std::vector<fastdds::rtps::IPFinder::info_IP> local_interfaces;
321321
// Check if the locator is from an owned interface to link all local interfaces to the channel
322-
is_own_interface(channel->locator(), local_interfaces);
323-
if (!local_interfaces.empty())
322+
// Note: Only applicable for TCPv4 until TCPv6 scope selection is implemented
323+
if (channel->locator().kind != LOCATOR_KIND_TCPv6)
324324
{
325-
Locator local_locator(channel->locator());
326-
for (auto& interface_it : local_interfaces)
325+
is_own_interface(channel->locator(), local_interfaces);
326+
if (!local_interfaces.empty())
327327
{
328-
IPLocator::setIPv4(local_locator, interface_it.locator);
329-
const auto insert_ret_local = channel_resources_.insert(
330-
decltype(channel_resources_)::value_type{local_locator, channel});
331-
if (!insert_ret_local.first->second->connection_established())
328+
Locator local_locator(channel->locator());
329+
for (auto& interface_it : local_interfaces)
332330
{
333-
insert_ret_local.first->second = channel;
331+
IPLocator::copy_address(interface_it.locator, local_locator);
332+
const auto insert_ret_local = channel_resources_.insert(
333+
decltype(channel_resources_)::value_type{local_locator, channel});
334+
if (!insert_ret_local.first->second->connection_established())
335+
{
336+
insert_ret_local.first->second = channel;
337+
}
334338
}
335339
}
336340
}
@@ -1035,14 +1039,18 @@ bool TCPTransportInterface::CreateInitialConnect(
10351039

10361040
std::vector<fastdds::rtps::IPFinder::info_IP> local_interfaces;
10371041
// Check if the locator is from an owned interface to link all local interfaces to the channel
1038-
is_own_interface(physical_locator, local_interfaces);
1039-
if (!local_interfaces.empty())
1042+
// Note: Only applicable for TCPv4 until TCPv6 scope selection is implemented
1043+
if (physical_locator.kind != LOCATOR_KIND_TCPv6)
10401044
{
1041-
Locator local_locator(physical_locator);
1042-
for (auto& interface_it : local_interfaces)
1045+
is_own_interface(physical_locator, local_interfaces);
1046+
if (!local_interfaces.empty())
10431047
{
1044-
IPLocator::setIPv4(local_locator, interface_it.locator);
1045-
channel_resources_[local_locator] = channel;
1048+
Locator local_locator(physical_locator);
1049+
for (auto& interface_it : local_interfaces)
1050+
{
1051+
IPLocator::copy_address(interface_it.locator, local_locator);
1052+
channel_resources_[local_locator] = channel;
1053+
}
10461054
}
10471055
}
10481056

@@ -1349,7 +1357,8 @@ bool TCPTransportInterface::Receive(
13491357
do
13501358
{
13511359
header_found = receive_header(channel, tcp_header, ec);
1352-
} while (!header_found && !ec && channel->connection_status());
1360+
}
1361+
while (!header_found && !ec && channel->connection_status());
13531362

13541363
if (ec)
13551364
{

‎src/cpp/utils/IPLocator.cpp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,13 @@ bool IPLocator::copyIPv4(
202202
return true;
203203
}
204204

205+
bool IPLocator::copyIPv4(
206+
const Locator_t& locator,
207+
Locator_t& dest)
208+
{
209+
return copyIPv4(locator, &(dest.address[12]));
210+
}
211+
205212
// IPv6
206213
bool IPLocator::setIPv6(
207214
Locator_t& locator,
@@ -985,6 +992,27 @@ bool IPLocator::compareAddress(
985992
}
986993
}
987994

995+
bool IPLocator::copy_address(
996+
const Locator_t& loc1,
997+
Locator_t& loc2)
998+
{
999+
if (loc1.kind != loc2.kind)
1000+
{
1001+
return false;
1002+
}
1003+
1004+
if (loc1.kind == LOCATOR_KIND_UDPv4 || loc1.kind == LOCATOR_KIND_TCPv4)
1005+
{
1006+
copyIPv4(loc1, loc2);
1007+
return true;
1008+
}
1009+
else if (loc1.kind == LOCATOR_KIND_UDPv6 || loc1.kind == LOCATOR_KIND_TCPv6)
1010+
{
1011+
return copyIPv6(loc1, loc2.address);
1012+
}
1013+
return false;
1014+
}
1015+
9881016
bool IPLocator::compareAddressAndPhysicalPort(
9891017
const Locator_t& loc1,
9901018
const Locator_t& loc2)

‎test/unittest/transport/TCPv4Tests.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2035,6 +2035,7 @@ TEST_F(TCPv4Tests, client_announced_local_port_uniqueness)
20352035

20362036
std::this_thread::sleep_for(std::chrono::milliseconds(100));
20372037

2038+
EXPECT_GT(receiveTransportUnderTest.get_channel_resources().size(), 2u);
20382039
std::set<std::shared_ptr<TCPChannelResource>> channels_created;
20392040
for (const auto& channel_resource : receiveTransportUnderTest.get_channel_resources())
20402041
{

‎test/unittest/transport/TCPv6Tests.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ TEST_F(TCPv6Tests, client_announced_local_port_uniqueness)
368368

369369
std::this_thread::sleep_for(std::chrono::milliseconds(100));
370370

371-
ASSERT_EQ(receiveTransportUnderTest.get_channel_resources().size(), 2u);
371+
EXPECT_EQ(receiveTransportUnderTest.get_channel_resources().size(), 2u);
372372
}
373373

374374
#ifndef _WIN32

‎test/unittest/utils/LocatorTests.cpp‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,37 @@ TEST_F(IPLocatorTests, copyIPv6)
767767
ASSERT_EQ(arr[15], 1u);
768768
}
769769

770+
/*
771+
* Check to copy an address
772+
*/
773+
TEST_F(IPLocatorTests, copy_address)
774+
{
775+
// Copy IPv4
776+
Locator_t locator1(LOCATOR_KIND_UDPv4);
777+
Locator_t locator2(LOCATOR_KIND_UDPv4);
778+
IPLocator::setIPv4(locator1, ipv4_lo_address);
779+
ASSERT_FALSE(IPLocator::compareAddress(locator1, locator2));
780+
ASSERT_TRUE(IPLocator::copy_address(locator1, locator2));
781+
ASSERT_TRUE(IPLocator::compareAddress(locator1, locator2));
782+
783+
// Check cannot copy between different kinds
784+
locator1.kind = LOCATOR_KIND_UDPv6;
785+
ASSERT_FALSE(IPLocator::copy_address(locator1, locator2));
786+
787+
// Copy IPv6
788+
locator2.kind = LOCATOR_KIND_UDPv6;
789+
IPLocator::setIPv6(locator1, ipv6_lo_address);
790+
ASSERT_FALSE(IPLocator::compareAddress(locator1, locator2));
791+
ASSERT_TRUE(IPLocator::copy_address(locator1, locator2));
792+
ASSERT_TRUE(IPLocator::compareAddress(locator1, locator2));
793+
794+
// Check cannot copy between SHM locators
795+
locator1.kind = LOCATOR_KIND_SHM;
796+
Locator_t locator3(LOCATOR_KIND_SHM);
797+
ASSERT_FALSE(IPLocator::copy_address(locator1, locator3));
798+
ASSERT_FALSE(IPLocator::compareAddress(locator1, locator3));
799+
}
800+
770801
/*
771802
* Check to set ip of any kind
772803
*/

0 commit comments

Comments
 (0)