[22208] The SocketTransportDescriptor::min_send_buffer_size() method returns too large value when exceeded net.core.wmem_max

[i-and]

Is there an already existing issue for this?

• I have searched the existing issues

Expected behavior

The values sendBufferSize and receiveBufferSize from the SocketTransportDescriptor adequately reflect the buffer sizes of the created UDP sockets.

Current behavior

The values sendBufferSize and receiveBufferSize from the SocketTransportDescriptor and SocketTransportDescriptor::min_send_buffer_size() method return larger values than the actual size of the UDP socket buffers.

Steps to reproduce

Using the fields sendBufferSize and receiveBufferSize from the SocketTransportDescriptor , you must set a value greater than net.core.wmem_max or net.core.rmem_max.

Fast DDS version/commit

v2.13.0

Platform/Architecture

Ubuntu Focal 20.04 amd64

Transport layer

UDPv4

Additional context

No response

XML configuration file

No response

Relevant log output

No response

Network traffic capture

No response

[i-and]

Below is a possible patch.

diff --git a/src/cpp/rtps/transport/UDPTransportInterface.cpp b/src/cpp/rtps/transport/UDPTransportInterface.cpp
index 820056e26..9e9b05ea3 100644
--- a/src/cpp/rtps/transport/UDPTransportInterface.cpp
+++ b/src/cpp/rtps/transport/UDPTransportInterface.cpp
@@ -151,6 +151,53 @@ bool UDPTransportInterface::init(
             }
         }
     }
+     
+    // Checking the possibility of setting buffer sizes
+    if (mSendBufferSize != 0 || mReceiveBufferSize != 0)
+    {
+        ip::udp::socket socket(io_service_);
+        socket.open(generate_protocol());
+
+        if (mSendBufferSize != 0)
+        {
+            assert(configuration()->sendBufferSize == mSendBufferSize);
+            socket.set_option(socket_base::send_buffer_size(static_cast<int32_t>(mSendBufferSize)));
+            socket_base::send_buffer_size option;
+            socket.get_option(option);
+            uint32_t real_size = static_cast<uint32_t>(option.value());
+            if (real_size != mSendBufferSize) {
+                EPROSIMA_LOG_ERROR(RTPS_MSG_OUT, "sendBufferSize=" << mSendBufferSize <<
+                    " cannot be set (max=" << real_size << ")");
+                set_send_buffer_size(real_size);
+                mSendBufferSize = real_size;
+                // NOTE: I suggest that we consider returning false here (i.e. initialization error). 
+                // This will change the behavior (it was not an error before), on the other hand, 
+                // it will draw the user's attention to the inability to set the specified buffer size 
+                // due to exceeding net.core.wmem_max.
+                // ??? return false; ???
+            }
+        }
+
+        if (mReceiveBufferSize != 0)
+        {
+            assert(configuration()->receiveBufferSize == mReceiveBufferSize);
+            socket.set_option(socket_base::receive_buffer_size(static_cast<int32_t>(mReceiveBufferSize)));
+            socket_base::receive_buffer_size option;
+            socket.get_option(option);
+            uint32_t real_size = static_cast<uint32_t>(option.value());
+            if (real_size != mReceiveBufferSize) {
+                EPROSIMA_LOG_ERROR(RTPS_MSG_OUT, "receiveBufferSize=" << mReceiveBufferSize <<
+                    " cannot be set (max=" << real_size << ")");
+                set_receive_buffer_size(real_size);
+                mReceiveBufferSize = real_size;
+                // NOTE: I suggest that we consider returning false here (i.e. initialization error). 
+                // This will change the behavior (it was not an error before), on the other hand, 
+                // it will draw the user's attention to the inability to set the specified buffer size 
+                // due to exceeding net.core.rmem_max.
+                // ??? return false; ???
+            }
+        }
+    }
 
     if (configuration()->maxMessageSize > s_maximumMessageSize)
     {

[MiguelCompany]

@i-and This might have been solved by #4760. Could you check?

[i-and]

Using the code snippet below, it turned out that the socket.set_option(...,ec) method does not return an error code in ec, when setting the buffer size, more than in the net.core.w(r)mem_max system variable. Instead, OK is returned and the actual buffer size is set to net.core.w(r)mem_max. Thus, analyzing the return code of the method socket.set_option() is not enough, and you need to use the method socket.get_option() to get the actual buffer size value.

diff --git a/src/cpp/rtps/transport/asio_helpers.hpp b/src/cpp/rtps/transport/asio_helpers.hpp
index f9ba81b84..53fb185a8 100644
--- a/src/cpp/rtps/transport/asio_helpers.hpp
+++ b/src/cpp/rtps/transport/asio_helpers.hpp
@@ -53,18 +53,37 @@ struct asio_helpers
         final_buffer_value = initial_buffer_value;
         while (final_buffer_value >= minimum_buffer_value)
         {
+            BufferOptionType option;
+            socket.get_option(option, ec);
+            if (ec)  { std::cout << "ERROR get_option before set_option() for " << final_buffer_value << std::endl; }
+            else { std::cout << "Before set_option() for " << final_buffer_value << " real socket buf size=" << option.value() << std::endl; }
+
             socket.set_option(BufferOptionType(static_cast<int32_t>(final_buffer_value)), ec);
             if (!ec)
             {
+                socket.get_option(option, ec);
+                if (ec)  { std::cout << "ERROR get_option after success set_option() for " << final_buffer_value << std::endl; }
+                else { std::cout << "After SUCCESS set_option() for " << final_buffer_value << " real socket buf size=" << option.value() << std::endl; }
                 return true;
             }
 
             final_buffer_value /= 2;
+            std::cout << "After ERROR set_option() for " << final_buffer_value * 2 << " try to set size=" << final_buffer_value << std::endl;
         }
 
         final_buffer_value = minimum_buffer_value;
         socket.set_option(BufferOptionType(final_buffer_value), ec);
-        return !ec;
+        if (!ec) {
+            BufferOptionType option;
+            socket.get_option(option, ec);
+            if (ec)  { std::cout << "ERROR get_option after success THE LAST set_option() for " << final_buffer_value << std::endl; }
+            else { std::cout << "After SUCCESS THE LAST set_option() for " << final_buffer_value << " real socket buf size=" << option.value() << std::endl; }
+            return true;
+        } else {
+            std::cout << "After ERROR THE LAST set_option() for " << final_buffer_value  << " - ..." << std::endl;
+            return false;
+        }
+        //return !ec;
     }
 
     /**

Before set_option() for 600000 real socket buf size=106496
After SUCCESS set_option() for 600000 real socket buf size=212992
Before set_option() for 700000 real socket buf size=106496
After SUCCESS set_option() for 700000 real socket buf size=212992
Before set_option() for 700000 real socket buf size=106496
After SUCCESS set_option() for 700000 real socket buf size=212992
Before set_option() for 700000 real socket buf size=106496
After SUCCESS set_option() for 700000 real socket buf size=212992
Before set_option() for 700000 real socket buf size=106496
After SUCCESS set_option() for 700000 real socket buf size=212992
Before set_option() for 600000 real socket buf size=106496
After SUCCESS set_option() for 600000 real socket buf size=212992
Before set_option() for 600000 real socket buf size=106496
After SUCCESS set_option() for 600000 real socket buf size=212992
Publisher running. Please press Ctrl+C to stop the Publisher at any time.

net.core.rmem_default = 212992
net.core.rmem_max = 212992
net.core.wmem_default = 212992
net.core.wmem_max = 212992

[MiguelCompany]

Tentative fix in this branch

[MiguelCompany]

@i-and This should have been fixed by #5527

[i-and]

In general, these changes solve the issue.

But there are suggestions related to error handling logic:

1. If the socket.get_option(option, ec) returns an error code, then it is impossible to get the actual size of the buffer - the return value final_buffer_value is undefined. Therefore, in this case, return false.
2. The semantics of the try_setting_buffer_size() implies that the set buffer size is greater than or equal to the minimum_buffer_value. If this is not the case, then return false.

Proposed changes:

diff --git a/src/cpp/rtps/transport/asio_helpers.hpp b/src/cpp/rtps/transport/asio_helpers.hpp
index b57d23d36..6e7f6d604 100644
--- a/src/cpp/rtps/transport/asio_helpers.hpp
+++ b/src/cpp/rtps/transport/asio_helpers.hpp
@@ -70,9 +70,9 @@ struct asio_helpers
                     final_buffer_value = option.value();
                     continue;
                 }
-                // Could not determine the actual value, but the option was set successfully.
-                // Assume the option was set to the desired value.
-                return true;
+                // Could not determine the actual value.
+                // The current buffer size is not defined.
+                return false;
             }
 
             final_buffer_value /= 2;
@@ -87,10 +87,13 @@ struct asio_helpers
             // Last attempt was successful. Get the actual value set.
             BufferOptionType option;
             socket.get_option(option, ec);
-            if (!ec)
+
+            if (ec || option.value() < static_cast<int32_t>(minimum_buffer_value))
             {
-                final_buffer_value = option.value();
+                return false;
             }
+
+            final_buffer_value = option.value();
             return true;
         }
         return false;

[MiguelCompany]

@i-and I think you are right. Would you be so kind to open a PR with your last proposed changes?

Well, actually I would do the last part different:

        // Perform a final attempt to set the minimum value
        final_buffer_value = minimum_buffer_value;
        int32_t value_to_set = static_cast<int32_t>(final_buffer_value);
        socket.set_option(BufferOptionType(value_to_set), ec);
        if (!ec)
        {
            // Last attempt was successful. Get the actual value set.
            BufferOptionType option;
            socket.get_option(option, ec);
            if (!ec && (option.value() >= value_to_set))
            {
                final_buffer_value = option.value();
                return true;
            }
        }
        return false;