Skip to content

Commit 0eb0c8b

Browse files
cferreiragonzmergify[bot]
cferreiragonz
authored and
mergify[bot]
committed
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> (cherry picked from commit f2a55e9) # Conflicts: # include/fastrtps/utils/IPLocator.h # src/cpp/rtps/transport/TCPTransportInterface.cpp
1 parent 14c66c7 commit 0eb0c8b

6 files changed

Lines changed: 122 additions & 12 deletions

File tree

‎include/fastrtps/utils/IPLocator.h‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,32 @@ class IPLocator
9797
RTPS_DllAPI static std::string toIPv4string(
9898
const Locator_t& locator);
9999

100+
<<<<<<< HEAD:include/fastrtps/utils/IPLocator.h
100101
//! Copies locator's IPv4.
101102
RTPS_DllAPI static bool copyIPv4(
103+
=======
104+
/**
105+
* @brief Copies locator's IPv4 to a destination array.
106+
* @param locator Locator from which to copy the IPv4.
107+
* @param dest Destination array where the IPv4 will be copied.
108+
* @return true if the copy was successful, false otherwise.
109+
*/
110+
FASTDDS_EXPORTED_API static bool copyIPv4(
111+
>>>>>>> f2a55e9a (Avoid calling setIPv4 in TCPTransportInterface (#5492)):include/fastdds/utils/IPLocator.hpp
102112
const Locator_t& locator,
103113
unsigned char* dest);
104114

115+
/**
116+
* @brief Copies locator's IPv4 to a destination locator.
117+
* It only copies the IPv4 part (last 4 bytes), leaving other parts unchanged.
118+
* @param locator Locator from which to copy the IPv4.
119+
* @param dest Destination locator where the IPv4 will be copied.
120+
* @return true if the copy was successful, false otherwise.
121+
*/
122+
FASTDDS_EXPORTED_API static bool copyIPv4(
123+
const Locator_t& locator,
124+
Locator_t& dest);
125+
105126
// IPv6
106127
//! Sets locator's IPv6.
107128
RTPS_DllAPI static bool setIPv6(
@@ -252,6 +273,16 @@ class IPLocator
252273
const Locator_t& loc2,
253274
bool fullAddress = false);
254275

276+
/**
277+
* Copies the whole address from one locator to another.
278+
* @param loc1 Locator to copy from.
279+
* @param loc2 Locator to copy to.
280+
* @return True if the copy was successful.
281+
*/
282+
FASTDDS_EXPORTED_API static bool copy_address(
283+
const Locator_t& loc1,
284+
Locator_t& loc2);
285+
255286
//! Checks if a both locators has the same IP address and physical port (as in RTCP protocol).
256287
RTPS_DllAPI static bool compareAddressAndPhysicalPort(
257288
const Locator_t& loc1,

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

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -324,14 +324,28 @@ ResponseCode TCPTransportInterface::bind_socket(
324324

325325
std::vector<fastrtps::rtps::IPFinder::info_IP> local_interfaces;
326326
// Check if the locator is from an owned interface to link all local interfaces to the channel
327-
is_own_interface(channel->locator(), local_interfaces);
328-
if (!local_interfaces.empty())
327+
// Note: Only applicable for TCPv4 until TCPv6 scope selection is implemented
328+
if (channel->locator().kind != LOCATOR_KIND_TCPv6)
329329
{
330-
Locator local_locator(channel->locator());
331-
for (auto& interface_it : local_interfaces)
330+
is_own_interface(channel->locator(), local_interfaces);
331+
if (!local_interfaces.empty())
332332
{
333+
<<<<<<< HEAD
333334
IPLocator::setIPv4(local_locator, interface_it.locator);
334335
channel_resources_.insert(decltype(channel_resources_)::value_type{local_locator, channel});
336+
=======
337+
Locator local_locator(channel->locator());
338+
for (auto& interface_it : local_interfaces)
339+
{
340+
IPLocator::copy_address(interface_it.locator, local_locator);
341+
const auto insert_ret_local = channel_resources_.insert(
342+
decltype(channel_resources_)::value_type{local_locator, channel});
343+
if (!insert_ret_local.first->second->connection_established())
344+
{
345+
insert_ret_local.first->second = channel;
346+
}
347+
}
348+
>>>>>>> f2a55e9a (Avoid calling setIPv4 in TCPTransportInterface (#5492))
335349
}
336350
}
337351
return ret;
@@ -1031,14 +1045,18 @@ bool TCPTransportInterface::CreateInitialConnect(
10311045

10321046
std::vector<fastrtps::rtps::IPFinder::info_IP> local_interfaces;
10331047
// Check if the locator is from an owned interface to link all local interfaces to the channel
1034-
is_own_interface(physical_locator, local_interfaces);
1035-
if (!local_interfaces.empty())
1048+
// Note: Only applicable for TCPv4 until TCPv6 scope selection is implemented
1049+
if (physical_locator.kind != LOCATOR_KIND_TCPv6)
10361050
{
1037-
Locator local_locator(physical_locator);
1038-
for (auto& interface_it : local_interfaces)
1051+
is_own_interface(physical_locator, local_interfaces);
1052+
if (!local_interfaces.empty())
10391053
{
1040-
IPLocator::setIPv4(local_locator, interface_it.locator);
1041-
channel_resources_[local_locator] = channel;
1054+
Locator local_locator(physical_locator);
1055+
for (auto& interface_it : local_interfaces)
1056+
{
1057+
IPLocator::copy_address(interface_it.locator, local_locator);
1058+
channel_resources_[local_locator] = channel;
1059+
}
10421060
}
10431061
}
10441062

@@ -1345,7 +1363,8 @@ bool TCPTransportInterface::Receive(
13451363
do
13461364
{
13471365
header_found = receive_header(channel, tcp_header, ec);
1348-
} while (!header_found && !ec && channel->connection_status());
1366+
}
1367+
while (!header_found && !ec && channel->connection_status());
13491368

13501369
if (ec)
13511370
{

‎src/cpp/utils/IPLocator.cpp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,13 @@ bool IPLocator::copyIPv4(
178178
return true;
179179
}
180180

181+
bool IPLocator::copyIPv4(
182+
const Locator_t& locator,
183+
Locator_t& dest)
184+
{
185+
return copyIPv4(locator, &(dest.address[12]));
186+
}
187+
181188
// IPv6
182189
bool IPLocator::setIPv6(
183190
Locator_t& locator,
@@ -911,6 +918,27 @@ bool IPLocator::compareAddress(
911918
}
912919
}
913920

921+
bool IPLocator::copy_address(
922+
const Locator_t& loc1,
923+
Locator_t& loc2)
924+
{
925+
if (loc1.kind != loc2.kind)
926+
{
927+
return false;
928+
}
929+
930+
if (loc1.kind == LOCATOR_KIND_UDPv4 || loc1.kind == LOCATOR_KIND_TCPv4)
931+
{
932+
copyIPv4(loc1, loc2);
933+
return true;
934+
}
935+
else if (loc1.kind == LOCATOR_KIND_UDPv6 || loc1.kind == LOCATOR_KIND_TCPv6)
936+
{
937+
return copyIPv6(loc1, loc2.address);
938+
}
939+
return false;
940+
}
941+
914942
bool IPLocator::compareAddressAndPhysicalPort(
915943
const Locator_t& loc1,
916944
const Locator_t& loc2)

‎test/unittest/transport/TCPv4Tests.cpp‎

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

19561956
std::this_thread::sleep_for(std::chrono::milliseconds(100));
19571957

1958+
EXPECT_GT(receiveTransportUnderTest.get_channel_resources().size(), 2u);
19581959
std::set<std::shared_ptr<TCPChannelResource>> channels_created;
19591960
for (const auto& channel_resource : receiveTransportUnderTest.get_channel_resources())
19601961
{

‎test/unittest/transport/TCPv6Tests.cpp‎

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

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

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

373373
#ifndef _WIN32

‎test/unittest/utils/LocatorTests.cpp‎

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

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

0 commit comments

Comments
 (0)