From eb3381403724b22a0a10131a3e3b5dcb7d9761b7 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 17 May 2024 11:19:56 +0200 Subject: [PATCH 1/2] Handle errors when setting socket buffer sizes (#4760) (#4795) * Refs #20972. Move code into new private methods. Signed-off-by: Miguel Company * Refs #20972. Refactor on configure_send_buffer_size. Signed-off-by: Miguel Company * Refs #20972. Refactor on configure_receive_buffer_size. Signed-off-by: Miguel Company * Refs #20972. Check user configuration at the beginning of init method. Signed-off-by: Miguel Company * Refs #20972. Use maxMessageSize as minimum possible value. Signed-off-by: Miguel Company * Refs #20972. Applying changes on OpenAndBindUnicastOutputSocket. Signed-off-by: Miguel Company * Refs #20972. Add helper header with template method. Signed-off-by: Miguel Company * Refs #20972. Configure methods return boolean. Signed-off-by: Miguel Company * Refs #20972. Configure methods use new template method. Signed-off-by: Miguel Company * Refs #20972. OpenAndBindUnicastOutputSocket uses new template method. Signed-off-by: Miguel Company * Refs #20972. Changes in OpenAndBindInputSocket. Signed-off-by: Miguel Company * Refs #20972.Setting options on TCP channels. Signed-off-by: Miguel Company * Refs #20972. Doxygen. Signed-off-by: Miguel Company * Refs #20972. Check limits of configured sizes. Signed-off-by: Miguel Company * Refs #20972. Add UDP unit tests. Signed-off-by: Miguel Company * Refs #20972. Add TCP unit tests. Signed-off-by: Miguel Company * Refs #20972. Move checks in TCP to beginning of init. Signed-off-by: Miguel Company * Refs #20972. Refactor for common code in UDP. Signed-off-by: Miguel Company * Refs #20972. Refactor for common code in TCP. Signed-off-by: Miguel Company * Refs #20972. Remove unused constants in UDP tests. Signed-off-by: Miguel Company * Refs #20972. Check final configuration on unit tests. Signed-off-by: Miguel Company * Refs #20972. Uncrustify. Signed-off-by: Miguel Company * Refs #20972. Less strict tests. Signed-off-by: Miguel Company * Refs #20972. Remove `s_minimumSocketBuffer` from tests. Signed-off-by: Miguel Company * Refs #20972. Deprecate `s_minimumSocketBuffer`. Signed-off-by: Miguel Company Signed-off-by: Miguel Company --------- Signed-off-by: Miguel Company Signed-off-by: Miguel Company Co-authored-by: Miguel Company (cherry picked from commit 532acfa7471567c90f5d6d362f0065b0e9fcebac) --- .../rtps/transport/TransportInterface.h | 3 +- .../fastrtps/transport/TransportInterface.h | 3 +- src/cpp/rtps/transport/TCPChannelResource.cpp | 49 +++++- src/cpp/rtps/transport/TCPChannelResource.h | 10 ++ .../transport/TCPChannelResourceBasic.cpp | 4 +- .../transport/TCPChannelResourceSecure.cpp | 4 +- .../rtps/transport/TCPTransportInterface.cpp | 75 ++++++++- .../rtps/transport/UDPTransportInterface.cpp | 127 +++++++++++++- .../rtps/transport/UDPTransportInterface.h | 1 + src/cpp/rtps/transport/UDPv4Transport.cpp | 21 ++- src/cpp/rtps/transport/UDPv6Transport.cpp | 17 +- src/cpp/rtps/transport/asio_helpers.hpp | 155 ++++++++++++++++++ test/unittest/transport/TCPv4Tests.cpp | 73 ++++++++- test/unittest/transport/TCPv6Tests.cpp | 75 ++++++++- test/unittest/transport/UDPv4Tests.cpp | 82 ++++++++- test/unittest/transport/UDPv6Tests.cpp | 77 ++++++++- 16 files changed, 742 insertions(+), 34 deletions(-) create mode 100644 src/cpp/rtps/transport/asio_helpers.hpp diff --git a/include/fastdds/rtps/transport/TransportInterface.h b/include/fastdds/rtps/transport/TransportInterface.h index 5f6abca88e8..828032c571b 100644 --- a/include/fastdds/rtps/transport/TransportInterface.h +++ b/include/fastdds/rtps/transport/TransportInterface.h @@ -33,7 +33,8 @@ namespace rtps { constexpr uint32_t s_maximumMessageSize = 65500; //! Default maximum initial peers range constexpr uint32_t s_maximumInitialPeersRange = 4; -//! Default minimum socket buffer +// Default minimum socket buffer +FASTDDS_DEPRECATED_UNTIL(3, s_minimumSocketBuffer, "Minimum socket buffer is now taken from the maximum msg size") constexpr uint32_t s_minimumSocketBuffer = 65536; //! Default IPv4 address static const std::string s_IPv4AddressAny = "0.0.0.0"; diff --git a/include/fastrtps/transport/TransportInterface.h b/include/fastrtps/transport/TransportInterface.h index 9f0a5f88760..22c1386803c 100644 --- a/include/fastrtps/transport/TransportInterface.h +++ b/include/fastrtps/transport/TransportInterface.h @@ -29,7 +29,8 @@ using TransportInterface = fastdds::rtps::TransportInterface; static const uint32_t s_maximumMessageSize = fastdds::rtps::s_maximumMessageSize; static const uint32_t s_maximumInitialPeersRange = fastdds::rtps::s_maximumInitialPeersRange; -static const uint32_t s_minimumSocketBuffer = fastdds::rtps::s_minimumSocketBuffer; +FASTDDS_DEPRECATED_UNTIL(3, s_minimumSocketBuffer, "Minimum socket buffer is now taken from the maximum msg size") +static const uint32_t s_minimumSocketBuffer = 65536; static const std::string s_IPv4AddressAny = fastdds::rtps::s_IPv4AddressAny; static const std::string s_IPv6AddressAny = fastdds::rtps::s_IPv6AddressAny; diff --git a/src/cpp/rtps/transport/TCPChannelResource.cpp b/src/cpp/rtps/transport/TCPChannelResource.cpp index 10bc88cc7b9..ff8a9326a8c 100644 --- a/src/cpp/rtps/transport/TCPChannelResource.cpp +++ b/src/cpp/rtps/transport/TCPChannelResource.cpp @@ -18,6 +18,7 @@ #include #include +#include #include namespace eprosima { @@ -370,6 +371,52 @@ bool TCPChannelResource::check_socket_send_buffer( return true; } +void TCPChannelResource::set_socket_options( + asio::basic_socket& socket, + const TCPTransportDescriptor* options) +{ + uint32_t minimum_value = options->maxMessageSize; + + // Set the send buffer size + { + uint32_t desired_value = options->sendBufferSize; + uint32_t configured_value = 0; + if (!asio_helpers::try_setting_buffer_size( + socket, desired_value, minimum_value, configured_value)) + { + EPROSIMA_LOG_ERROR(TCP_TRANSPORT, + "Couldn't set send buffer size to minimum value: " << minimum_value); + } + else if (desired_value != configured_value) + { + EPROSIMA_LOG_WARNING(TCP_TRANSPORT, + "Couldn't set send buffer size to desired value. " + << "Using " << configured_value << " instead of " << desired_value); + } + } + + // Set the receive buffer size + { + uint32_t desired_value = options->receiveBufferSize; + uint32_t configured_value = 0; + if (!asio_helpers::try_setting_buffer_size( + socket, desired_value, minimum_value, configured_value)) + { + EPROSIMA_LOG_ERROR(TCP_TRANSPORT, + "Couldn't set receive buffer size to minimum value: " << minimum_value); + } + else if (desired_value != configured_value) + { + EPROSIMA_LOG_WARNING(TCP_TRANSPORT, + "Couldn't set receive buffer size to desired value. " + << "Using " << configured_value << " instead of " << desired_value); + } + } + + // Set the TCP_NODELAY option + socket.set_option(asio::ip::tcp::no_delay(options->enable_tcp_nodelay)); +} + } // namespace rtps -} // namespace fastrtps +} // namespace fastdds } // namespace eprosima diff --git a/src/cpp/rtps/transport/TCPChannelResource.h b/src/cpp/rtps/transport/TCPChannelResource.h index c0a9e97f7bb..589ae6221e7 100644 --- a/src/cpp/rtps/transport/TCPChannelResource.h +++ b/src/cpp/rtps/transport/TCPChannelResource.h @@ -234,6 +234,16 @@ class TCPChannelResource : public ChannelResource const size_t& msg_size, const asio::ip::tcp::socket::native_handle_type& socket_native_handle); + /** + * @brief Set descriptor options on a socket. + * + * @param socket Socket on which to set the options. + * @param options Descriptor with the options to set. + */ + static void set_socket_options( + asio::basic_socket& socket, + const TCPTransportDescriptor* options); + TCPConnectionType tcp_connection_type_; friend class TCPTransportInterface; diff --git a/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp b/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp index 427904a2ba3..d58c63beeed 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp +++ b/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp @@ -200,9 +200,7 @@ asio::ip::tcp::endpoint TCPChannelResourceBasic::local_endpoint( void TCPChannelResourceBasic::set_options( const TCPTransportDescriptor* options) { - socket_->set_option(socket_base::receive_buffer_size(options->receiveBufferSize)); - socket_->set_option(socket_base::send_buffer_size(options->sendBufferSize)); - socket_->set_option(ip::tcp::no_delay(options->enable_tcp_nodelay)); + TCPChannelResource::set_socket_options(*socket_, options); } void TCPChannelResourceBasic::cancel() diff --git a/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp b/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp index 1453dc010a6..fd21de6c3df 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp +++ b/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp @@ -282,9 +282,7 @@ asio::ip::tcp::endpoint TCPChannelResourceSecure::local_endpoint( void TCPChannelResourceSecure::set_options( const TCPTransportDescriptor* options) { - secure_socket_->lowest_layer().set_option(socket_base::receive_buffer_size(options->receiveBufferSize)); - secure_socket_->lowest_layer().set_option(socket_base::send_buffer_size(options->sendBufferSize)); - secure_socket_->lowest_layer().set_option(ip::tcp::no_delay(options->enable_tcp_nodelay)); + TCPChannelResource::set_socket_options(secure_socket_->lowest_layer(), options); } void TCPChannelResourceSecure::set_tls_verify_mode( diff --git a/src/cpp/rtps/transport/TCPTransportInterface.cpp b/src/cpp/rtps/transport/TCPTransportInterface.cpp index b9ef852f716..75cbb7e1b3d 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.cpp +++ b/src/cpp/rtps/transport/TCPTransportInterface.cpp @@ -14,17 +14,26 @@ #include -#include +<<<<<<< HEAD #include #include #include #include #include +======= +#include +#include +#include +#include +#include +#include +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include #include #include #include + #include #include #include @@ -33,6 +42,8 @@ #include #include #endif // if TLS_FOUND + +#include #include #include @@ -400,6 +411,42 @@ bool TCPTransportInterface::DoInputLocatorsMatch( bool TCPTransportInterface::init( const fastrtps::rtps::PropertyPolicy* properties) { + uint32_t maximumMessageSize = s_maximumMessageSize; + uint32_t cfg_max_msg_size = configuration()->maxMessageSize; + uint32_t cfg_send_size = configuration()->sendBufferSize; + uint32_t cfg_recv_size = configuration()->receiveBufferSize; + uint32_t max_int_value = static_cast(std::numeric_limits::max()); + + if (cfg_max_msg_size > maximumMessageSize) + { + EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "maxMessageSize cannot be greater than " << maximumMessageSize); + return false; + } + + if (cfg_send_size > max_int_value) + { + EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "sendBufferSize cannot be greater than " << max_int_value); + return false; + } + + if (cfg_recv_size > max_int_value) + { + EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "receiveBufferSize cannot be greater than " << max_int_value); + return false; + } + + if ((cfg_send_size > 0) && (cfg_max_msg_size > cfg_send_size)) + { + EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "maxMessageSize cannot be greater than sendBufferSize"); + return false; + } + + if ((cfg_recv_size > 0) && (cfg_max_msg_size > cfg_recv_size)) + { + EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "maxMessageSize cannot be greater than receiveBufferSize"); + return false; + } + if (!apply_tls_config()) { // TODO decide wether the Transport initialization should keep working after this error @@ -432,8 +479,12 @@ bool TCPTransportInterface::init( } // Check system buffer sizes. - if (configuration()->sendBufferSize == 0) + uint32_t send_size = 0; + uint32_t recv_size = 0; + if (!asio_helpers::configure_buffer_sizes( + *initial_peer_local_locator_socket_, *configuration(), send_size, recv_size)) { +<<<<<<< HEAD socket_base::send_buffer_size option; initial_peer_local_locator_socket_->get_option(option); set_send_buffer_size(option.value()); @@ -459,21 +510,37 @@ bool TCPTransportInterface::init( if (configuration()->maxMessageSize > s_maximumMessageSize) { logError(RTCP_MSG_OUT, "maxMessageSize cannot be greater than 65000"); +======= + EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "Couldn't set buffer sizes to minimum value: " << cfg_max_msg_size); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) return false; } - if (configuration()->maxMessageSize > configuration()->sendBufferSize) + if (cfg_send_size > 0 && send_size != cfg_send_size) { +<<<<<<< HEAD logError(RTCP_MSG_OUT, "maxMessageSize cannot be greater than send_buffer_size"); return false; +======= + EPROSIMA_LOG_WARNING(TRANSPORT_TCP, "UDPTransport sendBufferSize could not be set to the desired value. " + << "Using " << send_size << " instead of " << cfg_send_size); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) } - if (configuration()->maxMessageSize > configuration()->receiveBufferSize) + if (cfg_recv_size > 0 && recv_size != cfg_recv_size) { +<<<<<<< HEAD logError(RTCP_MSG_OUT, "maxMessageSize cannot be greater than receive_buffer_size"); return false; +======= + EPROSIMA_LOG_WARNING(TRANSPORT_TCP, "UDPTransport receiveBufferSize could not be set to the desired value. " + << "Using " << recv_size << " instead of " << cfg_recv_size); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) } + set_send_buffer_size(send_size); + set_receive_buffer_size(recv_size); + if (!rtcp_message_manager_) { rtcp_message_manager_ = std::make_shared(this); diff --git a/src/cpp/rtps/transport/UDPTransportInterface.cpp b/src/cpp/rtps/transport/UDPTransportInterface.cpp index 08742d823e8..1e824bfeae3 100644 --- a/src/cpp/rtps/transport/UDPTransportInterface.cpp +++ b/src/cpp/rtps/transport/UDPTransportInterface.cpp @@ -14,15 +14,18 @@ #include -#include -#include #include #include +#include +#include +#include +#include #include #include -#include #include + +#include #include #include @@ -119,8 +122,15 @@ bool UDPTransportInterface::DoInputLocatorsMatch( bool UDPTransportInterface::init( const fastrtps::rtps::PropertyPolicy*) { - if (configuration()->sendBufferSize == 0 || configuration()->receiveBufferSize == 0) + uint32_t maximumMessageSize = s_maximumMessageSize; + uint32_t cfg_max_msg_size = configuration()->maxMessageSize; + uint32_t cfg_send_size = configuration()->sendBufferSize; + uint32_t cfg_recv_size = configuration()->receiveBufferSize; + uint32_t max_int_value = static_cast(std::numeric_limits::max()); + + if (cfg_max_msg_size > maximumMessageSize) { +<<<<<<< HEAD // Check system buffer sizes. ip::udp::socket socket(io_service_); socket.open(generate_protocol()); @@ -155,25 +165,80 @@ bool UDPTransportInterface::init( if (configuration()->maxMessageSize > s_maximumMessageSize) { logError(RTPS_MSG_OUT, "maxMessageSize cannot be greater than 65000"); +======= + EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "maxMessageSize cannot be greater than " << maximumMessageSize); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) return false; } - if (configuration()->maxMessageSize > configuration()->sendBufferSize) + if (cfg_send_size > max_int_value) { +<<<<<<< HEAD logError(RTPS_MSG_OUT, "maxMessageSize cannot be greater than send_buffer_size"); +======= + EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "sendBufferSize cannot be greater than " << max_int_value); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) return false; } - if (configuration()->maxMessageSize > configuration()->receiveBufferSize) + if (cfg_recv_size > max_int_value) { +<<<<<<< HEAD logError(RTPS_MSG_OUT, "maxMessageSize cannot be greater than receive_buffer_size"); +======= + EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "receiveBufferSize cannot be greater than " << max_int_value); + return false; + } + + if ((cfg_send_size > 0) && (cfg_max_msg_size > cfg_send_size)) + { + EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "maxMessageSize cannot be greater than sendBufferSize"); + return false; + } + + if ((cfg_recv_size > 0) && (cfg_max_msg_size > cfg_recv_size)) + { + EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "maxMessageSize cannot be greater than receiveBufferSize"); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) return false; } // TODO(Ricardo) Create an event that update this list. get_ips(currentInterfaces); - return true; + asio::error_code ec; + ip::udp::socket socket(io_service_); + socket.open(generate_protocol(), ec); + if (!!ec) + { + EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "Error creating socket: " << ec.message()); + return false; + } + + bool ret = asio_helpers::configure_buffer_sizes(socket, *configuration(), mSendBufferSize, mReceiveBufferSize); + if (ret) + { + if (cfg_send_size > 0 && mSendBufferSize != cfg_send_size) + { + EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport sendBufferSize could not be set to the desired value. " + << "Using " << mSendBufferSize << " instead of " << cfg_send_size); + } + + if (cfg_recv_size > 0 && mReceiveBufferSize != cfg_recv_size) + { + EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport receiveBufferSize could not be set to the desired value. " + << "Using " << mReceiveBufferSize << " instead of " << cfg_recv_size); + } + + set_send_buffer_size(mSendBufferSize); + set_receive_buffer_size(mReceiveBufferSize); + } + else + { + EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "Couldn't set buffer sizes to minimum value: " << cfg_max_msg_size); + } + + return ret; } bool UDPTransportInterface::IsInputChannelOpen( @@ -211,8 +276,13 @@ bool UDPTransportInterface::OpenAndBindInputSockets( catch (asio::system_error const& e) { (void)e; +<<<<<<< HEAD logInfo(RTPS_MSG_OUT, "UDPTransport Error binding at port: (" << IPLocator::getPhysicalPort(locator) << ")" << " with msg: " << e.what()); +======= + EPROSIMA_LOG_INFO(TRANSPORT_UDP, "UDPTransport Error binding at port: (" + << IPLocator::getPhysicalPort(locator) << ")" << " with msg: " << e.what()); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) mInputSockets.erase(IPLocator::getPhysicalPort(locator)); return false; } @@ -242,7 +312,18 @@ eProsimaUDPSocket UDPTransportInterface::OpenAndBindUnicastOutputSocket( getSocketPtr(socket)->open(generate_protocol()); if (mSendBufferSize != 0) { - getSocketPtr(socket)->set_option(socket_base::send_buffer_size(static_cast(mSendBufferSize))); + uint32_t configured_value = 0; + if (!asio_helpers::try_setting_buffer_size( + socket, mSendBufferSize, configuration()->maxMessageSize, configured_value)) + { + EPROSIMA_LOG_ERROR(TRANSPORT_UDP, + "Couldn't set send buffer size to minimum value: " << configuration()->maxMessageSize); + } + else if (configured_value != mSendBufferSize) + { + EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport sendBufferSize could not be set to the desired value. " + << "Using " << configured_value << " instead of " << mSendBufferSize); + } } getSocketPtr(socket)->set_option(ip::multicast::hops(configuration()->TTL)); getSocketPtr(socket)->bind(endpoint); @@ -300,7 +381,11 @@ bool UDPTransportInterface::OpenOutputChannel( catch (asio::system_error const& e) { (void)e; +<<<<<<< HEAD logWarning(RTPS_MSG_OUT, "UDPTransport Error binding interface " +======= + EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport Error binding interface " +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) << localhost_name() << " (skipping) with msg: " << e.what()); } } @@ -324,7 +409,11 @@ bool UDPTransportInterface::OpenOutputChannel( catch (asio::system_error const& e) { (void)e; +<<<<<<< HEAD logWarning(RTPS_MSG_OUT, "UDPTransport Error binding interface " +======= + EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport Error binding interface " +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) << (*locIt).name << " (skipping) with msg: " << e.what()); } } @@ -356,7 +445,11 @@ bool UDPTransportInterface::OpenOutputChannel( { (void)e; /* TODO Que hacer? +<<<<<<< HEAD logError(RTPS_MSG_OUT, "UDPTransport Error binding at port: (" << IPLocator::getPhysicalPort(locator) << ")" +======= + EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "UDPTransport Error binding at port: (" << IPLocator::getPhysicalPort(locator) << ")" +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) << " with msg: " << e.what()); for (auto& socket : mOutputSockets) { @@ -501,23 +594,41 @@ bool UDPTransportInterface::send( if ((ec.value() == asio::error::would_block) || (ec.value() == asio::error::try_again)) { +<<<<<<< HEAD logWarning(RTPS_MSG_OUT, "UDP send would have blocked. Packet is dropped."); return true; } logWarning(RTPS_MSG_OUT, ec.message()); +======= + EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDP send would have blocked. Packet is dropped."); + return true; + } + + EPROSIMA_LOG_WARNING(TRANSPORT_UDP, ec.message()); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) return false; } } catch (const std::exception& error) { +<<<<<<< HEAD logWarning(RTPS_MSG_OUT, error.what()); +======= + EPROSIMA_LOG_WARNING(TRANSPORT_UDP, error.what()); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) return false; } (void)bytesSent; +<<<<<<< HEAD logInfo(RTPS_MSG_OUT, "UDPTransport: " << bytesSent << " bytes TO endpoint: " << destinationEndpoint << " FROM " << getSocketPtr(socket)->local_endpoint()); +======= + EPROSIMA_LOG_INFO(TRANSPORT_UDP, + "UDPTransport: " << bytesSent << " bytes TO endpoint: " << destinationEndpoint << + " FROM " << getSocketPtr(socket)->local_endpoint()); +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) success = true; } diff --git a/src/cpp/rtps/transport/UDPTransportInterface.h b/src/cpp/rtps/transport/UDPTransportInterface.h index 7ab22b61291..90d021b9f2c 100644 --- a/src/cpp/rtps/transport/UDPTransportInterface.h +++ b/src/cpp/rtps/transport/UDPTransportInterface.h @@ -282,6 +282,7 @@ class UDPTransportInterface : public TransportInterface bool return_loopback = false); std::atomic_bool rescan_interfaces_ = {true}; + }; } // namespace rtps diff --git a/src/cpp/rtps/transport/UDPv4Transport.cpp b/src/cpp/rtps/transport/UDPv4Transport.cpp index f929a40bc98..7f33f287f4b 100644 --- a/src/cpp/rtps/transport/UDPv4Transport.cpp +++ b/src/cpp/rtps/transport/UDPv4Transport.cpp @@ -26,6 +26,12 @@ #include #include +<<<<<<< HEAD +======= +#include +#include + +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) using namespace std; using namespace asio; @@ -293,7 +299,20 @@ eProsimaUDPSocket UDPv4Transport::OpenAndBindInputSocket( getSocketPtr(socket)->open(generate_protocol()); if (mReceiveBufferSize != 0) { - getSocketPtr(socket)->set_option(socket_base::receive_buffer_size(mReceiveBufferSize)); + uint32_t configured_value = 0; + uint32_t minimum_value = configuration()->maxMessageSize; + if (!asio_helpers::try_setting_buffer_size( + socket, mReceiveBufferSize, minimum_value, configured_value)) + { + EPROSIMA_LOG_ERROR(TRANSPORT_UDPV4, + "Couldn't set receive buffer size to minimum value: " << minimum_value); + } + else if (mReceiveBufferSize != configured_value) + { + EPROSIMA_LOG_WARNING(TRANSPORT_UDPV4, + "Receive buffer size could not be set to the desired value. " + << "Using " << configured_value << " instead of " << mReceiveBufferSize); + } } if (is_multicast) diff --git a/src/cpp/rtps/transport/UDPv6Transport.cpp b/src/cpp/rtps/transport/UDPv6Transport.cpp index e8d070140f8..b020e05e5f0 100644 --- a/src/cpp/rtps/transport/UDPv6Transport.cpp +++ b/src/cpp/rtps/transport/UDPv6Transport.cpp @@ -24,6 +24,8 @@ #include #include +#include + using namespace std; using namespace asio; @@ -297,7 +299,20 @@ eProsimaUDPSocket UDPv6Transport::OpenAndBindInputSocket( getSocketPtr(socket)->open(generate_protocol()); if (mReceiveBufferSize != 0) { - getSocketPtr(socket)->set_option(socket_base::receive_buffer_size(mReceiveBufferSize)); + uint32_t configured_value = 0; + uint32_t minimum_value = configuration()->maxMessageSize; + if (!asio_helpers::asio_helpers::try_setting_buffer_size( + socket, mReceiveBufferSize, minimum_value, configured_value)) + { + EPROSIMA_LOG_ERROR(TRANSPORT_UDPV6, + "Couldn't set receive buffer size to minimum value: " << minimum_value); + } + else if (mReceiveBufferSize != configured_value) + { + EPROSIMA_LOG_WARNING(TRANSPORT_UDPV6, + "Receive buffer size could not be set to the desired value. " + << "Using " << configured_value << " instead of " << mReceiveBufferSize); + } } if (is_multicast) diff --git a/src/cpp/rtps/transport/asio_helpers.hpp b/src/cpp/rtps/transport/asio_helpers.hpp new file mode 100644 index 00000000000..f9ba81b84e0 --- /dev/null +++ b/src/cpp/rtps/transport/asio_helpers.hpp @@ -0,0 +1,155 @@ +// Copyright 2024 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef RTPS_TRANSPORT__ASIO_HELPERS_HPP_ +#define RTPS_TRANSPORT__ASIO_HELPERS_HPP_ + +#include + +#include + +namespace eprosima { +namespace fastdds { +namespace rtps { + +/// Helper functions for asio. +// NOTE: using a struct instead of a namespace to avoid linker errors when using inline free functions. +struct asio_helpers +{ + /** + * @brief Try to set a buffer size on a socket, trying to set the initial value and then halving it until it is + * possible to set it or the minimum value is reached. + * + * @tparam BufferOptionType Type of the buffer option to set. + * @tparam SocketType Type of socket on which to set the buffer size option. + * + * @param socket Socket on which to set the buffer size option. + * @param initial_buffer_value Initial value to try to set. + * @param minimum_buffer_value Minimum value to set. + * @param final_buffer_value Output parameter where the final value set will be stored. + * + * @return true if the buffer size was successfully set, false otherwise. + */ + template + static inline bool try_setting_buffer_size( + SocketType& socket, + const uint32_t initial_buffer_value, + const uint32_t minimum_buffer_value, + uint32_t& final_buffer_value) + { + asio::error_code ec; + + final_buffer_value = initial_buffer_value; + while (final_buffer_value >= minimum_buffer_value) + { + socket.set_option(BufferOptionType(static_cast(final_buffer_value)), ec); + if (!ec) + { + return true; + } + + final_buffer_value /= 2; + } + + final_buffer_value = minimum_buffer_value; + socket.set_option(BufferOptionType(final_buffer_value), ec); + return !ec; + } + + /** + * @brief Configure a buffer size on a socket, using the system default value if the initial value is 0. + * Ensures that the final buffer size is at least the minimum value. + * + * @tparam BufferOptionType Type of the buffer option to set. + * @tparam SocketType Type of socket on which to set the buffer size option. + * + * @param socket Socket on which to set the buffer size option. + * @param initial_buffer_value Initial value to try to set. + * @param minimum_buffer_value Minimum value to set. + * @param final_buffer_value Output parameter where the final value set will be stored. + * + * @return true if the buffer size was successfully set, false otherwise. + */ + template + static inline bool configure_buffer_size( + SocketType& socket, + const uint32_t initial_buffer_value, + const uint32_t minimum_buffer_value, + uint32_t& final_buffer_value) + { + final_buffer_value = initial_buffer_value; + + // If the initial value is 0, try using the system default value + if (initial_buffer_value == 0) + { + asio::error_code ec; + BufferOptionType option; + socket.get_option(option, ec); + if (!ec) + { + final_buffer_value = option.value(); + } + } + + // Ensure the minimum value is used + if (final_buffer_value < minimum_buffer_value) + { + final_buffer_value = minimum_buffer_value; + } + + // Try to set the highest possible value the system allows + return try_setting_buffer_size(socket, final_buffer_value, minimum_buffer_value, + final_buffer_value); + } + + /** + * @brief Configure the send and receive buffer sizes on a socket, using the system default value if the initial + * values are 0. Ensures that the final buffer sizes are at least the minimum value. + * + * @tparam SocketType Type of socket on which to set the buffer size options. + * + * @param socket Socket on which to set the buffer size options. + * @param descriptor Transport descriptor with the buffer sizes to set. + * @param final_send_buffer_size Output parameter where the final send buffer size will be stored. + * @param final_receive_buffer_size Output parameter where the final receive buffer size will be stored. + * + * @return true if the buffer sizes were successfully set, false otherwise. + */ + template + static inline bool configure_buffer_sizes( + SocketType& socket, + const SocketTransportDescriptor& descriptor, + uint32_t& final_send_buffer_size, + uint32_t& final_receive_buffer_size) + { + uint32_t minimum_socket_buffer = descriptor.maxMessageSize; + uint32_t send_buffer_size = descriptor.sendBufferSize; + uint32_t receive_buffer_size = descriptor.receiveBufferSize; + + bool send_buffer_size_set = configure_buffer_size( + socket, send_buffer_size, minimum_socket_buffer, final_send_buffer_size); + bool receive_buffer_size_set = configure_buffer_size( + socket, receive_buffer_size, minimum_socket_buffer, final_receive_buffer_size); + + return send_buffer_size_set && receive_buffer_size_set; + } + +}; + +} // namespace rtps +} // namespace fastdds +} // namespace eprosima + +#endif // RTPS_TRANSPORT__ASIO_HELPERS_HPP_ + diff --git a/test/unittest/transport/TCPv4Tests.cpp b/test/unittest/transport/TCPv4Tests.cpp index 52a7e9e72f8..2a2fceff480 100644 --- a/test/unittest/transport/TCPv4Tests.cpp +++ b/test/unittest/transport/TCPv4Tests.cpp @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include @@ -82,6 +83,74 @@ class TCPv4Tests : public ::testing::Test std::unique_ptr receiverThread; }; +TEST_F(TCPv4Tests, wrong_configuration_values) +{ + // Too big sendBufferSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.sendBufferSize = std::numeric_limits::max(); + TCPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Too big receiveBufferSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.receiveBufferSize = std::numeric_limits::max(); + TCPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Too big maxMessageSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.maxMessageSize = std::numeric_limits::max(); + TCPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // maxMessageSize bigger than receiveBufferSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.maxMessageSize = 10; + wrong_descriptor.receiveBufferSize = 5; + TCPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // maxMessageSize bigger than sendBufferSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.maxMessageSize = 10; + wrong_descriptor.sendBufferSize = 5; + TCPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Buffer sizes automatically decrease + { + auto wrong_descriptor = descriptor; + wrong_descriptor.sendBufferSize = static_cast(std::numeric_limits::max()); + wrong_descriptor.receiveBufferSize = static_cast(std::numeric_limits::max()); + wrong_descriptor.maxMessageSize = 1470; + TCPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_TRUE(transportUnderTest.init()); + auto* final_cfg = transportUnderTest.configuration(); + EXPECT_GE(final_cfg->sendBufferSize, final_cfg->maxMessageSize); + // The system could allow for the send buffer to be MAX_INT, so we cannot check it to be strictly lower + EXPECT_LE(final_cfg->sendBufferSize, wrong_descriptor.sendBufferSize); + EXPECT_GE(final_cfg->receiveBufferSize, final_cfg->maxMessageSize); + // The system could allow for the receive buffer to be MAX_INT, so we cannot check it to be strictly lower + EXPECT_LE(final_cfg->receiveBufferSize, wrong_descriptor.receiveBufferSize); + eprosima::fastdds::dds::Log::Flush(); + } +} + TEST_F(TCPv4Tests, locators_with_kind_1_supported) { // Given @@ -1138,7 +1207,7 @@ TEST_F(TCPv4Tests, secure_non_blocking_send) eprosima::fastdds::dds::Log::SetVerbosity(eprosima::fastdds::dds::Log::Kind::Info); uint16_t port = g_default_port; - uint32_t msg_size = eprosima::fastdds::rtps::s_minimumSocketBuffer; + uint32_t msg_size = 64ul * 1024ul; // Create a TCP Server transport using TLSOptions = TCPTransportDescriptor::TLSConfig::TLSOptions; using TLSVerifyMode = TCPTransportDescriptor::TLSConfig::TLSVerifyMode; @@ -1712,7 +1781,7 @@ TEST_F(TCPv4Tests, client_announced_local_port_uniqueness) TEST_F(TCPv4Tests, non_blocking_send) { uint16_t port = g_default_port; - uint32_t msg_size = eprosima::fastdds::rtps::s_minimumSocketBuffer; + uint32_t msg_size = 64ul * 1024ul; // Create a TCP Server transport TCPv4TransportDescriptor senderDescriptor; senderDescriptor.add_listener_port(port); diff --git a/test/unittest/transport/TCPv6Tests.cpp b/test/unittest/transport/TCPv6Tests.cpp index 59249c33b51..046a62924a5 100644 --- a/test/unittest/transport/TCPv6Tests.cpp +++ b/test/unittest/transport/TCPv6Tests.cpp @@ -12,6 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. +<<<<<<< HEAD +======= +#include +#include +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include #include @@ -73,6 +78,74 @@ class TCPv6Tests : public ::testing::Test std::unique_ptr receiverThread; }; +TEST_F(TCPv6Tests, wrong_configuration_values) +{ + // Too big sendBufferSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.sendBufferSize = std::numeric_limits::max(); + TCPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Too big receiveBufferSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.receiveBufferSize = std::numeric_limits::max(); + TCPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Too big maxMessageSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.maxMessageSize = std::numeric_limits::max(); + TCPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // maxMessageSize bigger than receiveBufferSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.maxMessageSize = 10; + wrong_descriptor.receiveBufferSize = 5; + TCPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // maxMessageSize bigger than sendBufferSize + { + auto wrong_descriptor = descriptor; + wrong_descriptor.maxMessageSize = 10; + wrong_descriptor.sendBufferSize = 5; + TCPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Buffer sizes automatically decrease + { + auto wrong_descriptor = descriptor; + wrong_descriptor.sendBufferSize = static_cast(std::numeric_limits::max()); + wrong_descriptor.receiveBufferSize = static_cast(std::numeric_limits::max()); + wrong_descriptor.maxMessageSize = 1470; + TCPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_TRUE(transportUnderTest.init()); + auto* final_cfg = transportUnderTest.configuration(); + EXPECT_GE(final_cfg->sendBufferSize, final_cfg->maxMessageSize); + // The system could allow for the send buffer to be MAX_INT, so we cannot check it to be strictly lower + EXPECT_LE(final_cfg->sendBufferSize, wrong_descriptor.sendBufferSize); + EXPECT_GE(final_cfg->receiveBufferSize, final_cfg->maxMessageSize); + // The system could allow for the receive buffer to be MAX_INT, so we cannot check it to be strictly lower + EXPECT_LE(final_cfg->receiveBufferSize, wrong_descriptor.receiveBufferSize); + eprosima::fastdds::dds::Log::Flush(); + } +} + TEST_F(TCPv6Tests, conversion_to_ip6_string) { Locator_t locator; @@ -249,7 +322,7 @@ TEST_F(TCPv6Tests, client_announced_local_port_uniqueness) TEST_F(TCPv6Tests, non_blocking_send) { uint16_t port = g_default_port; - uint32_t msg_size = eprosima::fastdds::rtps::s_minimumSocketBuffer; + uint32_t msg_size = 64ul * 1024ul; // Create a TCP Server transport TCPv6TransportDescriptor senderDescriptor; senderDescriptor.add_listener_port(port); diff --git a/test/unittest/transport/UDPv4Tests.cpp b/test/unittest/transport/UDPv4Tests.cpp index 97a7da636eb..932058332f9 100644 --- a/test/unittest/transport/UDPv4Tests.cpp +++ b/test/unittest/transport/UDPv4Tests.cpp @@ -12,12 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. +<<<<<<< HEAD +======= +#include +#include +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include #include #include #include +<<<<<<< HEAD #include +======= + +#include +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include #include #include @@ -29,10 +39,6 @@ using namespace eprosima::fastrtps; using namespace eprosima::fastrtps::rtps; using UDPv4Transport = eprosima::fastdds::rtps::UDPv4Transport; -#ifndef __APPLE__ -const uint32_t ReceiveBufferCapacity = 65536; -#endif // ifndef __APPLE__ - #if defined(_WIN32) #define GET_PID _getpid #else @@ -74,6 +80,74 @@ class UDPv4Tests : public ::testing::Test std::unique_ptr receiverThread; }; +TEST_F(UDPv4Tests, wrong_configuration) +{ + // Too big sendBufferSize + { + UDPv4TransportDescriptor wrong_descriptor; + wrong_descriptor.sendBufferSize = std::numeric_limits::max(); + UDPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Too big receiveBufferSize + { + UDPv4TransportDescriptor wrong_descriptor; + wrong_descriptor.receiveBufferSize = std::numeric_limits::max(); + UDPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Too big maxMessageSize + { + UDPv4TransportDescriptor wrong_descriptor; + wrong_descriptor.maxMessageSize = std::numeric_limits::max(); + UDPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // maxMessageSize bigger than receiveBufferSize + { + UDPv4TransportDescriptor wrong_descriptor; + wrong_descriptor.maxMessageSize = 10; + wrong_descriptor.receiveBufferSize = 5; + UDPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // maxMessageSize bigger than sendBufferSize + { + UDPv4TransportDescriptor wrong_descriptor; + wrong_descriptor.maxMessageSize = 10; + wrong_descriptor.sendBufferSize = 5; + UDPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Buffer sizes automatically decrease + { + UDPv4TransportDescriptor wrong_descriptor; + wrong_descriptor.sendBufferSize = static_cast(std::numeric_limits::max()); + wrong_descriptor.receiveBufferSize = static_cast(std::numeric_limits::max()); + wrong_descriptor.maxMessageSize = 1470; + UDPv4Transport transportUnderTest(wrong_descriptor); + ASSERT_TRUE(transportUnderTest.init()); + auto* final_cfg = transportUnderTest.configuration(); + EXPECT_GE(final_cfg->sendBufferSize, final_cfg->maxMessageSize); + // The system could allow for the send buffer to be MAX_INT, so we cannot check it to be strictly lower + EXPECT_LE(final_cfg->sendBufferSize, wrong_descriptor.sendBufferSize); + EXPECT_GE(final_cfg->receiveBufferSize, final_cfg->maxMessageSize); + // The system could allow for the receive buffer to be MAX_INT, so we cannot check it to be strictly lower + EXPECT_LE(final_cfg->receiveBufferSize, wrong_descriptor.receiveBufferSize); + eprosima::fastdds::dds::Log::Flush(); + } +} + TEST_F(UDPv4Tests, locators_with_kind_1_supported) { // Given diff --git a/test/unittest/transport/UDPv6Tests.cpp b/test/unittest/transport/UDPv6Tests.cpp index bf4cb8261c0..20f310a19d6 100644 --- a/test/unittest/transport/UDPv6Tests.cpp +++ b/test/unittest/transport/UDPv6Tests.cpp @@ -12,6 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. +<<<<<<< HEAD +======= +#include +#include +>>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include #include @@ -29,10 +34,6 @@ using namespace eprosima::fastrtps::rtps; using namespace eprosima::fastrtps; using UDPv6Transport = eprosima::fastdds::rtps::UDPv6Transport; -#ifndef __APPLE__ -const uint32_t ReceiveBufferCapacity = 65536; -#endif // ifndef __APPLE__ - #if defined(_WIN32) #define GET_PID _getpid #else @@ -74,6 +75,74 @@ class UDPv6Tests : public ::testing::Test std::unique_ptr receiverThread; }; +TEST_F(UDPv6Tests, wrong_configuration) +{ + // Too big sendBufferSize + { + UDPv6TransportDescriptor wrong_descriptor; + wrong_descriptor.sendBufferSize = std::numeric_limits::max(); + UDPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Too big receiveBufferSize + { + UDPv6TransportDescriptor wrong_descriptor; + wrong_descriptor.receiveBufferSize = std::numeric_limits::max(); + UDPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Too big maxMessageSize + { + UDPv6TransportDescriptor wrong_descriptor; + wrong_descriptor.maxMessageSize = std::numeric_limits::max(); + UDPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // maxMessageSize bigger than receiveBufferSize + { + UDPv6TransportDescriptor wrong_descriptor; + wrong_descriptor.maxMessageSize = 10; + wrong_descriptor.receiveBufferSize = 5; + UDPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // maxMessageSize bigger than sendBufferSize + { + UDPv6TransportDescriptor wrong_descriptor; + wrong_descriptor.maxMessageSize = 10; + wrong_descriptor.sendBufferSize = 5; + UDPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_FALSE(transportUnderTest.init()); + eprosima::fastdds::dds::Log::Flush(); + } + + // Buffer sizes automatically decrease + { + UDPv6TransportDescriptor wrong_descriptor; + wrong_descriptor.sendBufferSize = static_cast(std::numeric_limits::max()); + wrong_descriptor.receiveBufferSize = static_cast(std::numeric_limits::max()); + wrong_descriptor.maxMessageSize = 1470; + UDPv6Transport transportUnderTest(wrong_descriptor); + ASSERT_TRUE(transportUnderTest.init()); + auto* final_cfg = transportUnderTest.configuration(); + EXPECT_GE(final_cfg->sendBufferSize, final_cfg->maxMessageSize); + // The system could allow for the send buffer to be MAX_INT, so we cannot check it to be strictly lower + EXPECT_LE(final_cfg->sendBufferSize, wrong_descriptor.sendBufferSize); + EXPECT_GE(final_cfg->receiveBufferSize, final_cfg->maxMessageSize); + // The system could allow for the receive buffer to be MAX_INT, so we cannot check it to be strictly lower + EXPECT_LE(final_cfg->receiveBufferSize, wrong_descriptor.receiveBufferSize); + eprosima::fastdds::dds::Log::Flush(); + } +} + TEST_F(UDPv6Tests, conversion_to_ip6_string) { Locator_t locator; From dac08dd0ca79279da42d241d409e92571b8757ab Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 26 Jun 2024 11:09:09 +0200 Subject: [PATCH 2/2] Fix conflicts Signed-off-by: Miguel Company --- src/cpp/rtps/transport/TCPChannelResource.cpp | 8 +- .../rtps/transport/TCPTransportInterface.cpp | 63 ++-------- .../rtps/transport/UDPTransportInterface.cpp | 117 +++--------------- src/cpp/rtps/transport/UDPv4Transport.cpp | 8 +- src/cpp/rtps/transport/UDPv6Transport.cpp | 4 +- test/unittest/transport/TCPv6Tests.cpp | 4 - test/unittest/transport/UDPv4Tests.cpp | 8 +- test/unittest/transport/UDPv6Tests.cpp | 4 - 8 files changed, 37 insertions(+), 179 deletions(-) diff --git a/src/cpp/rtps/transport/TCPChannelResource.cpp b/src/cpp/rtps/transport/TCPChannelResource.cpp index ff8a9326a8c..2ae5b99ada2 100644 --- a/src/cpp/rtps/transport/TCPChannelResource.cpp +++ b/src/cpp/rtps/transport/TCPChannelResource.cpp @@ -384,12 +384,12 @@ void TCPChannelResource::set_socket_options( if (!asio_helpers::try_setting_buffer_size( socket, desired_value, minimum_value, configured_value)) { - EPROSIMA_LOG_ERROR(TCP_TRANSPORT, + logError(TCP_TRANSPORT, "Couldn't set send buffer size to minimum value: " << minimum_value); } else if (desired_value != configured_value) { - EPROSIMA_LOG_WARNING(TCP_TRANSPORT, + logWarning(TCP_TRANSPORT, "Couldn't set send buffer size to desired value. " << "Using " << configured_value << " instead of " << desired_value); } @@ -402,12 +402,12 @@ void TCPChannelResource::set_socket_options( if (!asio_helpers::try_setting_buffer_size( socket, desired_value, minimum_value, configured_value)) { - EPROSIMA_LOG_ERROR(TCP_TRANSPORT, + logError(TCP_TRANSPORT, "Couldn't set receive buffer size to minimum value: " << minimum_value); } else if (desired_value != configured_value) { - EPROSIMA_LOG_WARNING(TCP_TRANSPORT, + logWarning(TCP_TRANSPORT, "Couldn't set receive buffer size to desired value. " << "Using " << configured_value << " instead of " << desired_value); } diff --git a/src/cpp/rtps/transport/TCPTransportInterface.cpp b/src/cpp/rtps/transport/TCPTransportInterface.cpp index 75cbb7e1b3d..55f8b2b794c 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.cpp +++ b/src/cpp/rtps/transport/TCPTransportInterface.cpp @@ -14,20 +14,13 @@ #include -<<<<<<< HEAD -#include -#include -#include -#include -#include -======= #include #include #include #include #include +#include #include ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include #include @@ -419,31 +412,31 @@ bool TCPTransportInterface::init( if (cfg_max_msg_size > maximumMessageSize) { - EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "maxMessageSize cannot be greater than " << maximumMessageSize); + logError(TRANSPORT_TCP, "maxMessageSize cannot be greater than " << maximumMessageSize); return false; } if (cfg_send_size > max_int_value) { - EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "sendBufferSize cannot be greater than " << max_int_value); + logError(TRANSPORT_TCP, "sendBufferSize cannot be greater than " << max_int_value); return false; } if (cfg_recv_size > max_int_value) { - EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "receiveBufferSize cannot be greater than " << max_int_value); + logError(TRANSPORT_TCP, "receiveBufferSize cannot be greater than " << max_int_value); return false; } if ((cfg_send_size > 0) && (cfg_max_msg_size > cfg_send_size)) { - EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "maxMessageSize cannot be greater than sendBufferSize"); + logError(TRANSPORT_TCP, "maxMessageSize cannot be greater than sendBufferSize"); return false; } if ((cfg_recv_size > 0) && (cfg_max_msg_size > cfg_recv_size)) { - EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "maxMessageSize cannot be greater than receiveBufferSize"); + logError(TRANSPORT_TCP, "maxMessageSize cannot be greater than receiveBufferSize"); return false; } @@ -484,58 +477,20 @@ bool TCPTransportInterface::init( if (!asio_helpers::configure_buffer_sizes( *initial_peer_local_locator_socket_, *configuration(), send_size, recv_size)) { -<<<<<<< HEAD - socket_base::send_buffer_size option; - initial_peer_local_locator_socket_->get_option(option); - set_send_buffer_size(option.value()); - - if (configuration()->sendBufferSize < s_minimumSocketBuffer) - { - set_send_buffer_size(s_minimumSocketBuffer); - } - } - - if (configuration()->receiveBufferSize == 0) - { - socket_base::receive_buffer_size option; - initial_peer_local_locator_socket_->get_option(option); - set_receive_buffer_size(option.value()); - - if (configuration()->receiveBufferSize < s_minimumSocketBuffer) - { - set_receive_buffer_size(s_minimumSocketBuffer); - } - } - - if (configuration()->maxMessageSize > s_maximumMessageSize) - { - logError(RTCP_MSG_OUT, "maxMessageSize cannot be greater than 65000"); -======= - EPROSIMA_LOG_ERROR(TRANSPORT_TCP, "Couldn't set buffer sizes to minimum value: " << cfg_max_msg_size); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) + logError(TRANSPORT_TCP, "Couldn't set buffer sizes to minimum value: " << cfg_max_msg_size); return false; } if (cfg_send_size > 0 && send_size != cfg_send_size) { -<<<<<<< HEAD - logError(RTCP_MSG_OUT, "maxMessageSize cannot be greater than send_buffer_size"); - return false; -======= - EPROSIMA_LOG_WARNING(TRANSPORT_TCP, "UDPTransport sendBufferSize could not be set to the desired value. " + logWarning(TRANSPORT_TCP, "UDPTransport sendBufferSize could not be set to the desired value. " << "Using " << send_size << " instead of " << cfg_send_size); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) } if (cfg_recv_size > 0 && recv_size != cfg_recv_size) { -<<<<<<< HEAD - logError(RTCP_MSG_OUT, "maxMessageSize cannot be greater than receive_buffer_size"); - return false; -======= - EPROSIMA_LOG_WARNING(TRANSPORT_TCP, "UDPTransport receiveBufferSize could not be set to the desired value. " + logWarning(TRANSPORT_TCP, "UDPTransport receiveBufferSize could not be set to the desired value. " << "Using " << recv_size << " instead of " << cfg_recv_size); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) } set_send_buffer_size(send_size); diff --git a/src/cpp/rtps/transport/UDPTransportInterface.cpp b/src/cpp/rtps/transport/UDPTransportInterface.cpp index 1e824bfeae3..f63f24f0fb8 100644 --- a/src/cpp/rtps/transport/UDPTransportInterface.cpp +++ b/src/cpp/rtps/transport/UDPTransportInterface.cpp @@ -130,76 +130,31 @@ bool UDPTransportInterface::init( if (cfg_max_msg_size > maximumMessageSize) { -<<<<<<< HEAD - // Check system buffer sizes. - ip::udp::socket socket(io_service_); - socket.open(generate_protocol()); - - if (configuration()->sendBufferSize == 0) - { - socket_base::send_buffer_size option; - socket.get_option(option); - set_send_buffer_size(static_cast(option.value())); - - if (configuration()->sendBufferSize < s_minimumSocketBuffer) - { - set_send_buffer_size(s_minimumSocketBuffer); - mSendBufferSize = s_minimumSocketBuffer; - } - } - - if (configuration()->receiveBufferSize == 0) - { - socket_base::receive_buffer_size option; - socket.get_option(option); - set_receive_buffer_size(static_cast(option.value())); - - if (configuration()->receiveBufferSize < s_minimumSocketBuffer) - { - set_receive_buffer_size(s_minimumSocketBuffer); - mReceiveBufferSize = s_minimumSocketBuffer; - } - } - } - - if (configuration()->maxMessageSize > s_maximumMessageSize) - { - logError(RTPS_MSG_OUT, "maxMessageSize cannot be greater than 65000"); -======= - EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "maxMessageSize cannot be greater than " << maximumMessageSize); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) + logError(TRANSPORT_UDP, "maxMessageSize cannot be greater than " << maximumMessageSize); return false; } if (cfg_send_size > max_int_value) { -<<<<<<< HEAD - logError(RTPS_MSG_OUT, "maxMessageSize cannot be greater than send_buffer_size"); -======= - EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "sendBufferSize cannot be greater than " << max_int_value); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) + logError(TRANSPORT_UDP, "sendBufferSize cannot be greater than " << max_int_value); return false; } if (cfg_recv_size > max_int_value) { -<<<<<<< HEAD - logError(RTPS_MSG_OUT, "maxMessageSize cannot be greater than receive_buffer_size"); -======= - EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "receiveBufferSize cannot be greater than " << max_int_value); + logError(TRANSPORT_UDP, "receiveBufferSize cannot be greater than " << max_int_value); return false; } if ((cfg_send_size > 0) && (cfg_max_msg_size > cfg_send_size)) { - EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "maxMessageSize cannot be greater than sendBufferSize"); + logError(TRANSPORT_UDP, "maxMessageSize cannot be greater than sendBufferSize"); return false; } if ((cfg_recv_size > 0) && (cfg_max_msg_size > cfg_recv_size)) { - EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "maxMessageSize cannot be greater than receiveBufferSize"); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) + logError(TRANSPORT_UDP, "maxMessageSize cannot be greater than receiveBufferSize"); return false; } @@ -211,7 +166,7 @@ bool UDPTransportInterface::init( socket.open(generate_protocol(), ec); if (!!ec) { - EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "Error creating socket: " << ec.message()); + logError(TRANSPORT_UDP, "Error creating socket: " << ec.message()); return false; } @@ -220,13 +175,13 @@ bool UDPTransportInterface::init( { if (cfg_send_size > 0 && mSendBufferSize != cfg_send_size) { - EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport sendBufferSize could not be set to the desired value. " + logWarning(TRANSPORT_UDP, "UDPTransport sendBufferSize could not be set to the desired value. " << "Using " << mSendBufferSize << " instead of " << cfg_send_size); } if (cfg_recv_size > 0 && mReceiveBufferSize != cfg_recv_size) { - EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport receiveBufferSize could not be set to the desired value. " + logWarning(TRANSPORT_UDP, "UDPTransport receiveBufferSize could not be set to the desired value. " << "Using " << mReceiveBufferSize << " instead of " << cfg_recv_size); } @@ -235,7 +190,7 @@ bool UDPTransportInterface::init( } else { - EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "Couldn't set buffer sizes to minimum value: " << cfg_max_msg_size); + logError(TRANSPORT_UDP, "Couldn't set buffer sizes to minimum value: " << cfg_max_msg_size); } return ret; @@ -276,13 +231,8 @@ bool UDPTransportInterface::OpenAndBindInputSockets( catch (asio::system_error const& e) { (void)e; -<<<<<<< HEAD - logInfo(RTPS_MSG_OUT, "UDPTransport Error binding at port: (" << IPLocator::getPhysicalPort(locator) << ")" - << " with msg: " << e.what()); -======= - EPROSIMA_LOG_INFO(TRANSPORT_UDP, "UDPTransport Error binding at port: (" + logInfo(TRANSPORT_UDP, "UDPTransport Error binding at port: (" << IPLocator::getPhysicalPort(locator) << ")" << " with msg: " << e.what()); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) mInputSockets.erase(IPLocator::getPhysicalPort(locator)); return false; } @@ -316,12 +266,12 @@ eProsimaUDPSocket UDPTransportInterface::OpenAndBindUnicastOutputSocket( if (!asio_helpers::try_setting_buffer_size( socket, mSendBufferSize, configuration()->maxMessageSize, configured_value)) { - EPROSIMA_LOG_ERROR(TRANSPORT_UDP, + logError(TRANSPORT_UDP, "Couldn't set send buffer size to minimum value: " << configuration()->maxMessageSize); } else if (configured_value != mSendBufferSize) { - EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport sendBufferSize could not be set to the desired value. " + logWarning(TRANSPORT_UDP, "UDPTransport sendBufferSize could not be set to the desired value. " << "Using " << configured_value << " instead of " << mSendBufferSize); } } @@ -381,11 +331,7 @@ bool UDPTransportInterface::OpenOutputChannel( catch (asio::system_error const& e) { (void)e; -<<<<<<< HEAD - logWarning(RTPS_MSG_OUT, "UDPTransport Error binding interface " -======= - EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport Error binding interface " ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) + logWarning(TRANSPORT_UDP, "UDPTransport Error binding interface " << localhost_name() << " (skipping) with msg: " << e.what()); } } @@ -409,11 +355,7 @@ bool UDPTransportInterface::OpenOutputChannel( catch (asio::system_error const& e) { (void)e; -<<<<<<< HEAD - logWarning(RTPS_MSG_OUT, "UDPTransport Error binding interface " -======= - EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDPTransport Error binding interface " ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) + logWarning(TRANSPORT_UDP, "UDPTransport Error binding interface " << (*locIt).name << " (skipping) with msg: " << e.what()); } } @@ -445,11 +387,7 @@ bool UDPTransportInterface::OpenOutputChannel( { (void)e; /* TODO Que hacer? -<<<<<<< HEAD - logError(RTPS_MSG_OUT, "UDPTransport Error binding at port: (" << IPLocator::getPhysicalPort(locator) << ")" -======= - EPROSIMA_LOG_ERROR(TRANSPORT_UDP, "UDPTransport Error binding at port: (" << IPLocator::getPhysicalPort(locator) << ")" ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) + logError(TRANSPORT_UDP, "UDPTransport Error binding at port: (" << IPLocator::getPhysicalPort(locator) << ")" << " with msg: " << e.what()); for (auto& socket : mOutputSockets) { @@ -594,41 +532,24 @@ bool UDPTransportInterface::send( if ((ec.value() == asio::error::would_block) || (ec.value() == asio::error::try_again)) { -<<<<<<< HEAD - logWarning(RTPS_MSG_OUT, "UDP send would have blocked. Packet is dropped."); - return true; - } - - logWarning(RTPS_MSG_OUT, ec.message()); -======= - EPROSIMA_LOG_WARNING(TRANSPORT_UDP, "UDP send would have blocked. Packet is dropped."); + logWarning(TRANSPORT_UDP, "UDP send would have blocked. Packet is dropped."); return true; } - EPROSIMA_LOG_WARNING(TRANSPORT_UDP, ec.message()); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) + logWarning(TRANSPORT_UDP, ec.message()); return false; } } catch (const std::exception& error) { -<<<<<<< HEAD - logWarning(RTPS_MSG_OUT, error.what()); -======= - EPROSIMA_LOG_WARNING(TRANSPORT_UDP, error.what()); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) + logWarning(TRANSPORT_UDP, error.what()); return false; } (void)bytesSent; -<<<<<<< HEAD - logInfo(RTPS_MSG_OUT, "UDPTransport: " << bytesSent << " bytes TO endpoint: " << destinationEndpoint - << " FROM " << getSocketPtr(socket)->local_endpoint()); -======= - EPROSIMA_LOG_INFO(TRANSPORT_UDP, + logInfo(TRANSPORT_UDP, "UDPTransport: " << bytesSent << " bytes TO endpoint: " << destinationEndpoint << " FROM " << getSocketPtr(socket)->local_endpoint()); ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) success = true; } diff --git a/src/cpp/rtps/transport/UDPv4Transport.cpp b/src/cpp/rtps/transport/UDPv4Transport.cpp index 7f33f287f4b..63214acdfb3 100644 --- a/src/cpp/rtps/transport/UDPv4Transport.cpp +++ b/src/cpp/rtps/transport/UDPv4Transport.cpp @@ -26,12 +26,8 @@ #include #include -<<<<<<< HEAD -======= -#include #include ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) using namespace std; using namespace asio; @@ -304,12 +300,12 @@ eProsimaUDPSocket UDPv4Transport::OpenAndBindInputSocket( if (!asio_helpers::try_setting_buffer_size( socket, mReceiveBufferSize, minimum_value, configured_value)) { - EPROSIMA_LOG_ERROR(TRANSPORT_UDPV4, + logError(TRANSPORT_UDPV4, "Couldn't set receive buffer size to minimum value: " << minimum_value); } else if (mReceiveBufferSize != configured_value) { - EPROSIMA_LOG_WARNING(TRANSPORT_UDPV4, + logWarning(TRANSPORT_UDPV4, "Receive buffer size could not be set to the desired value. " << "Using " << configured_value << " instead of " << mReceiveBufferSize); } diff --git a/src/cpp/rtps/transport/UDPv6Transport.cpp b/src/cpp/rtps/transport/UDPv6Transport.cpp index b020e05e5f0..0815008af1c 100644 --- a/src/cpp/rtps/transport/UDPv6Transport.cpp +++ b/src/cpp/rtps/transport/UDPv6Transport.cpp @@ -304,12 +304,12 @@ eProsimaUDPSocket UDPv6Transport::OpenAndBindInputSocket( if (!asio_helpers::asio_helpers::try_setting_buffer_size( socket, mReceiveBufferSize, minimum_value, configured_value)) { - EPROSIMA_LOG_ERROR(TRANSPORT_UDPV6, + logError(TRANSPORT_UDPV6, "Couldn't set receive buffer size to minimum value: " << minimum_value); } else if (mReceiveBufferSize != configured_value) { - EPROSIMA_LOG_WARNING(TRANSPORT_UDPV6, + logWarning(TRANSPORT_UDPV6, "Receive buffer size could not be set to the desired value. " << "Using " << configured_value << " instead of " << mReceiveBufferSize); } diff --git a/test/unittest/transport/TCPv6Tests.cpp b/test/unittest/transport/TCPv6Tests.cpp index 046a62924a5..be1a4f4b2d2 100644 --- a/test/unittest/transport/TCPv6Tests.cpp +++ b/test/unittest/transport/TCPv6Tests.cpp @@ -12,13 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -<<<<<<< HEAD -======= #include #include ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include -#include #include #include diff --git a/test/unittest/transport/UDPv4Tests.cpp b/test/unittest/transport/UDPv4Tests.cpp index 932058332f9..e26b885f00b 100644 --- a/test/unittest/transport/UDPv4Tests.cpp +++ b/test/unittest/transport/UDPv4Tests.cpp @@ -12,22 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -<<<<<<< HEAD -======= #include #include ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include -#include #include #include -<<<<<<< HEAD + #include -======= #include ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include #include #include diff --git a/test/unittest/transport/UDPv6Tests.cpp b/test/unittest/transport/UDPv6Tests.cpp index 20f310a19d6..6954a0cd24c 100644 --- a/test/unittest/transport/UDPv6Tests.cpp +++ b/test/unittest/transport/UDPv6Tests.cpp @@ -12,13 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -<<<<<<< HEAD -======= #include #include ->>>>>>> 532acfa74 (Handle errors when setting socket buffer sizes (#4760) (#4795)) #include -#include #include #include