From ac13492e32cbc7561ff27b36787ad27f74723e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Ferreira=20Gonz=C3=A1lez?= Date: Fri, 14 Jun 2024 14:03:30 +0200 Subject: [PATCH] Fix DS servers not connecting due to ports logic (#4941) * Refs #21170: Add DS servers connection test Signed-off-by: cferreiragonz * Refs #21170: Support DS servers connection Signed-off-by: cferreiragonz * Refs #21170: Revision Signed-off-by: cferreiragonz * Refs #21170: Uncrustify Signed-off-by: cferreiragonz * Refs #21170: Fix Windows build & comment Signed-off-by: cferreiragonz --------- Signed-off-by: cferreiragonz (cherry picked from commit b441560f9f9617ce64b12f1b91036c20aaf87dad) --- .../discovery/participant/PDPServer.cpp | 10 -- .../rtps/participant/RTPSParticipantImpl.cpp | 15 +++ .../api/dds-pim/PubSubParticipant.hpp | 19 +++ .../common/DDSBlackboxTestsDiscovery.cpp | 124 +++++++++++++++--- 4 files changed, 137 insertions(+), 31 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index 0c77f1e6015..228cdc770cc 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -489,16 +489,6 @@ bool PDPServer::create_ds_pdp_reliable_endpoints( wout->reader_data_filter(pdp_filter); // Enable separate sending so the filter can be called for each change and reader proxy wout->set_separate_sending(true); - - if (!secure) - { - eprosima::shared_lock disc_lock(mp_builtin->getDiscoveryMutex()); - - for (const eprosima::fastdds::rtps::RemoteServerAttributes& it : mp_builtin->m_DiscoveryServers) - { - match_pdp_reader_nts_(it); - } - } } // Could not create PDP Writer, so return false else diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp index 6cc0e28ee52..921257b3926 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp @@ -332,6 +332,21 @@ RTPSParticipantImpl::RTPSParticipantImpl( } }); } + for (fastdds::rtps::RemoteServerAttributes& it : m_att.builtin.discovery_config.m_DiscoveryServers) + { + std::for_each(it.metatrafficUnicastLocatorList.begin(), + it.metatrafficUnicastLocatorList.end(), [&](Locator_t& locator) + { + // TCP DS default logical port is the same as the physical one + if (locator.kind == LOCATOR_KIND_TCPv4 || locator.kind == LOCATOR_KIND_TCPv6) + { + if (IPLocator::getLogicalPort(locator) == 0) + { + IPLocator::setLogicalPort(locator, IPLocator::getPhysicalPort(locator)); + } + } + }); + } } } break; diff --git a/test/blackbox/api/dds-pim/PubSubParticipant.hpp b/test/blackbox/api/dds-pim/PubSubParticipant.hpp index b4e5b193aa0..bc4afb9800a 100644 --- a/test/blackbox/api/dds-pim/PubSubParticipant.hpp +++ b/test/blackbox/api/dds-pim/PubSubParticipant.hpp @@ -891,6 +891,25 @@ class PubSubParticipant on_participant_qos_update_ = f; } + PubSubParticipant& fill_server_qos( + eprosima::fastdds::dds::WireProtocolConfigQos& qos, + eprosima::fastrtps::rtps::GuidPrefix_t& guid, + eprosima::fastrtps::rtps::Locator_t& locator_server, + uint16_t port, + uint32_t kind) + { + qos.builtin.discovery_config.discoveryProtocol = eprosima::fastrtps::rtps::DiscoveryProtocol_t::SERVER; + qos.prefix = guid; + // Generate server's listening locator + eprosima::fastrtps::rtps::IPLocator::setIPv4(locator_server, 127, 0, 0, 1); + eprosima::fastrtps::rtps::IPLocator::setPhysicalPort(locator_server, port); + locator_server.kind = kind; + // Leave logical port as 0 to use TCP DS default logical port + qos.builtin.metatrafficUnicastLocatorList.push_back(locator_server); + + return wire_protocol(qos); + } + private: PubSubParticipant& operator =( diff --git a/test/blackbox/common/DDSBlackboxTestsDiscovery.cpp b/test/blackbox/common/DDSBlackboxTestsDiscovery.cpp index 4ce5fabfdce..7defcecf58e 100644 --- a/test/blackbox/common/DDSBlackboxTestsDiscovery.cpp +++ b/test/blackbox/common/DDSBlackboxTestsDiscovery.cpp @@ -208,13 +208,12 @@ TEST(DDSDiscovery, AddDiscoveryServerToListTCP) using namespace eprosima::fastrtps::rtps; // TCP default DS port - std::string W_UNICAST_PORT_RANDOM_NUMBER_STR = "42100"; + constexpr uint16_t W_UNICAST_PORT_RANDOM_NUMBER_STR = 42100; /* Create first server */ PubSubParticipant server_1(0u, 0u, 0u, 0u); // Set participant as server WireProtocolConfigQos server_1_qos; - server_1_qos.builtin.discovery_config.discoveryProtocol = DiscoveryProtocol_t::SERVER; // Generate random GUID prefix srand(static_cast(time(nullptr))); GuidPrefix_t server_1_prefix; @@ -222,20 +221,15 @@ TEST(DDSDiscovery, AddDiscoveryServerToListTCP) { server_1_prefix.value[i] = eprosima::fastrtps::rtps::octet(rand() % 254); } - server_1_qos.prefix = server_1_prefix; - // Generate server's listening locator + uint16_t server_1_port = W_UNICAST_PORT_RANDOM_NUMBER_STR; Locator_t locator_server_1; - IPLocator::setIPv4(locator_server_1, 127, 0, 0, 1); - uint16_t server_1_port = static_cast(stoi(W_UNICAST_PORT_RANDOM_NUMBER_STR)); - IPLocator::setPhysicalPort(locator_server_1, server_1_port); - locator_server_1.kind = LOCATOR_KIND_TCPv4; - // Leave logical port as 0 to use TCP DS default logical port - server_1_qos.builtin.metatrafficUnicastLocatorList.push_back(locator_server_1); // Add TCP transport auto descriptor_1 = std::make_shared(); descriptor_1->add_listener_port(server_1_port); + // Init server - ASSERT_TRUE(server_1.wire_protocol(server_1_qos) + ASSERT_TRUE(server_1.fill_server_qos(server_1_qos, server_1_prefix, locator_server_1, server_1_port, + LOCATOR_KIND_TCPv4) .disable_builtin_transport() .add_user_transport_to_pparams(descriptor_1) .init_participant()); @@ -244,25 +238,17 @@ TEST(DDSDiscovery, AddDiscoveryServerToListTCP) PubSubParticipant server_2(0u, 0u, 0u, 0u); // Set participant as server WireProtocolConfigQos server_2_qos; - server_2_qos.builtin.discovery_config.discoveryProtocol = DiscoveryProtocol_t::SERVER; - // Generate random GUID prefix GuidPrefix_t server_2_prefix = server_1_prefix; server_2_prefix.value[11]++; - server_2_qos.prefix = server_2_prefix; - // Generate server's listening locator Locator_t locator_server_2; - IPLocator::setIPv4(locator_server_2, 127, 0, 0, 1); uint16_t server_2_port = server_1_port + 1; - IPLocator::setPhysicalPort(locator_server_2, server_2_port); - locator_server_2.kind = LOCATOR_KIND_TCPv4; - // Leave logical port as 0 to use TCP DS default logical port - server_2_qos.builtin.metatrafficUnicastLocatorList.push_back(locator_server_2); // Add TCP transport auto descriptor_2 = std::make_shared(); descriptor_2->add_listener_port(server_2_port); // Init server - ASSERT_TRUE(server_2.wire_protocol(server_2_qos) + ASSERT_TRUE(server_2.fill_server_qos(server_2_qos, server_2_prefix, locator_server_2, server_2_port, + LOCATOR_KIND_TCPv4) .disable_builtin_transport() .add_user_transport_to_pparams(descriptor_2) .init_participant()); @@ -331,6 +317,102 @@ TEST(DDSDiscovery, AddDiscoveryServerToListTCP) server_2.wait_discovery(std::chrono::seconds::zero(), 2, true); // Knows client1 and server1 } +TEST(DDSDiscovery, ServersConnectionTCP) +{ + using namespace eprosima; + using namespace eprosima::fastdds::dds; + using namespace eprosima::fastrtps::rtps; + + // TCP default DS port + constexpr uint16_t W_UNICAST_PORT_RANDOM_NUMBER_STR = 41100; + + /* Create first server */ + PubSubParticipant server_1(0u, 0u, 0u, 0u); + // Set participant as server + WireProtocolConfigQos server_1_qos; + // Generate random GUID prefix + srand(static_cast(time(nullptr))); + GuidPrefix_t server_1_prefix; + for (auto i = 0; i < 12; i++) + { + server_1_prefix.value[i] = eprosima::fastrtps::rtps::octet(rand() % 254); + } + Locator_t locator_server_1; + uint16_t server_1_port = W_UNICAST_PORT_RANDOM_NUMBER_STR; + // Add TCP transport + auto descriptor_1 = std::make_shared(); + descriptor_1->add_listener_port(server_1_port); + // Init server + ASSERT_TRUE(server_1.fill_server_qos(server_1_qos, server_1_prefix, locator_server_1, server_1_port, + LOCATOR_KIND_TCPv4) + .disable_builtin_transport() + .add_user_transport_to_pparams(descriptor_1) + .init_participant()); + + /* Create second server */ + PubSubParticipant server_2(0u, 0u, 0u, 0u); + // Set participant as server + WireProtocolConfigQos server_2_qos; + GuidPrefix_t server_2_prefix = server_1_prefix; + server_2_prefix.value[11]++; + Locator_t locator_server_2; + uint16_t server_2_port = server_1_port + 1; + // Add TCP transport + auto descriptor_2 = std::make_shared(); + descriptor_2->add_listener_port(server_2_port); + + // Connect to first server + RemoteServerAttributes server_1_att; + server_1_att.guidPrefix = server_1_prefix; + server_1_att.metatrafficUnicastLocatorList.push_back(Locator_t(locator_server_1)); + server_2_qos.builtin.discovery_config.m_DiscoveryServers.push_back(server_1_att); + + // Init server + ASSERT_TRUE(server_2.fill_server_qos(server_2_qos, server_2_prefix, locator_server_2, server_2_port, + LOCATOR_KIND_TCPv4) + .disable_builtin_transport() + .add_user_transport_to_pparams(descriptor_2) + .init_participant()); + + /* Create third server */ + PubSubParticipant server_3(0u, 0u, 0u, 0u); + // Set participant as server + WireProtocolConfigQos server_3_qos; + GuidPrefix_t server_3_prefix = server_1_prefix; + server_3_prefix.value[11]--; + Locator_t locator_server_3; + uint16_t server_3_port = server_1_port - 1; + // Add TCP transport + auto descriptor_3 = std::make_shared(); + descriptor_3->add_listener_port(server_3_port); + // Connect to first server + server_3_qos.builtin.discovery_config.m_DiscoveryServers.push_back(server_1_att); + + // Init server + ASSERT_TRUE(server_3.fill_server_qos(server_3_qos, server_3_prefix, locator_server_3, server_3_port, + LOCATOR_KIND_TCPv4) + .disable_builtin_transport() + .add_user_transport_to_pparams(descriptor_3) + .init_participant()); + + // Check adding servers before initialization + server_1.wait_discovery(std::chrono::seconds::zero(), 2, true); // Knows server2 and server3 + server_2.wait_discovery(std::chrono::seconds::zero(), 1, true); // Knows server1 + server_3.wait_discovery(std::chrono::seconds::zero(), 1, true); // Knows server1 + + /* Add server_3 to server_2 */ + RemoteServerAttributes server_3_att; + server_3_att.guidPrefix = server_3_prefix; + server_3_att.metatrafficUnicastLocatorList.push_back(Locator_t(locator_server_3)); + server_2_qos.builtin.discovery_config.m_DiscoveryServers.push_back(server_3_att); + ASSERT_TRUE(server_2.update_wire_protocol(server_2_qos)); + + // Check adding servers after initialization + server_1.wait_discovery(std::chrono::seconds::zero(), 2, true); // Knows server2 and server3 + server_2.wait_discovery(std::chrono::seconds::zero(), 2, true); // Knows server1 and server3 + server_3.wait_discovery(std::chrono::seconds::zero(), 2, true); // Knows server1 and server2 +} + /** * This test checks the addition of network interfaces at run-time. *