Merge branch 'lukas/erts/fix_udp_realloc_bug' into maint

* lukas/erts/fix_udp_realloc_bug:
  erts: Limit the automatic max buffer for UDP to 2^16
  erts: Free udp buffer when getting EAGAIN

3 files changed, 50 insertions, 20 deletions

diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c
index 5e9afdc5ca..2048d0f625 100644
--- a/erts/emulator/drivers/common/inet_drv.c
+++ b/erts/emulator/drivers/common/inet_drv.c
@@ -1549,6 +1549,8 @@ static void *realloc_wrapper(void *current, ErlDrvSizeT size){
 #   define LOAD_ASSOC_ID        LOAD_UINT
 #   define LOAD_ASSOC_ID_CNT    LOAD_UINT_CNT
 #   define SCTP_ANC_BUFF_SIZE   INET_DEF_BUFFER/2 /* XXX: not very good... */
+#else
+#   define IS_SCTP(desc) 0
 #endif
 
 #ifdef HAVE_UDP
@@ -1785,6 +1787,7 @@ static void release_buffer(ErlDrvBinary* buf)
 #ifdef HAVE_UDP
 static ErlDrvBinary* realloc_buffer(ErlDrvBinary* buf, ErlDrvSizeT newsz)
 {
+    DEBUGF(("realloc_buffer: %ld -> %ld\r\n", (buf==NULL) ? 0 : buf->orig_size, newsz));
     return driver_realloc_binary(buf, newsz);
 }
 #endif
@@ -6435,7 +6438,12 @@ static int inet_set_opts(inet_descriptor* desc, char* ptr, int len)
 		(long)desc->port, desc->s, res));
 	if (type == SO_RCVBUF) {
 	    /* make sure we have desc->bufsz >= SO_RCVBUF */
-	    if (ival > desc->bufsz)
+            if (ival > (1 << 16) && desc->stype == SOCK_DGRAM && !IS_SCTP(desc))
+                /* For UDP we don't want to automatically
+                   set the buffer size to be larger than
+                   the theoretical max MTU */
+                desc->bufsz = 1 << 16;
+	    else if (ival > desc->bufsz)
 		desc->bufsz = ival;
 	}
     }
@@ -12042,15 +12050,11 @@ static int packet_inet_input(udp_descriptor* udesc, HANDLE event)
 	sys_memzero((char *) &other, sizeof(other));
 
 	/* udesc->i_buf is only kept between SCTP fragments */
-	if (udesc->i_buf == NULL) {
-	    udesc->i_bufsz = desc->bufsz + len;
-	    if ((udesc->i_buf = alloc_buffer(udesc->i_bufsz)) == NULL)
-		return packet_error(udesc, ENOMEM);
-	    /* pointer to message start */
-	    udesc->i_ptr = udesc->i_buf->orig_bytes + len;
-	} else {
-	    ErlDrvBinary* tmp;
+#ifdef HAVE_SCTP
+	if (udesc->i_buf != NULL) {
+            ErlDrvBinary* tmp;
 	    int bufsz;
+            ASSERT(IS_SCTP(desc));
 	    bufsz = desc->bufsz + (udesc->i_ptr - udesc->i_buf->orig_bytes);
 	    if ((tmp = realloc_buffer(udesc->i_buf, bufsz)) == NULL) {
 		release_buffer(udesc->i_buf);
@@ -12062,6 +12066,15 @@ static int packet_inet_input(udp_descriptor* udesc, HANDLE event)
 		udesc->i_buf = tmp;
 		udesc->i_bufsz = bufsz;
 	    }
+	} else
+#endif
+        {
+            ASSERT(udesc->i_buf == NULL);
+	    udesc->i_bufsz = desc->bufsz + len;
+	    if ((udesc->i_buf = alloc_buffer(udesc->i_bufsz)) == NULL)
+		return packet_error(udesc, ENOMEM);
+	    /* pointer to message start */
+	    udesc->i_ptr = udesc->i_buf->orig_bytes + len;
 	}
 
 	/* Note: On Windows NT, recvfrom() fails if the socket is connected. */
@@ -12120,6 +12133,14 @@ static int packet_inet_input(udp_descriptor* udesc, HANDLE event)
 		) {
 		sock_select(desc,FD_READ,1);
 	    }
+#ifdef HAVE_SCTP
+            if (!short_recv) {
+#endif
+                release_buffer(udesc->i_buf);
+                udesc->i_buf = NULL;
+#ifdef HAVE_SCTP
+            }
+#endif
 	    return count;		/* strange, not ready */
 	}
 
diff --git a/lib/kernel/doc/src/inet.xml b/lib/kernel/doc/src/inet.xml
index e6a7962c5a..f281d61459 100644
--- a/lib/kernel/doc/src/inet.xml
+++ b/lib/kernel/doc/src/inet.xml
@@ -734,22 +734,23 @@ get_tcpi_sacked(Sock) ->
           </item>
           <tag><c>{buffer, Size}</c></tag>
           <item>
-            <p>The size of the user-level software buffer used by
-              the driver. 
-              Not to be confused with options <c>sndbuf</c>
+            <p>The size of the user-level buffer used by
+              the driver. Not to be confused with options <c>sndbuf</c>
               and <c>recbuf</c>, which correspond to the
-              Kernel socket buffers. It is recommended
-              to have <c>val(buffer) &gt;= max(val(sndbuf),val(recbuf))</c> to
+              Kernel socket buffers. For TCP it is recommended
+              to have <c>val(buffer) &gt;= val(recbuf)</c> to
               avoid performance issues because of unnecessary copying.
+              For UDP the same recommendation applies, but the max should
+              not be larger than the MTU of the network path.
               <c>val(buffer)</c> is automatically set to the above
-              maximum when values <c>sndbuf</c> or <c>recbuf</c> are set.
-              However, as the sizes set for <c>sndbuf</c> and <c>recbuf</c>
+              maximum when <c>recbuf</c> is set.
+              However, as the size set for <c>recbuf</c>
               usually become larger, you are encouraged to use
               <seealso marker="#getopts/2"><c>getopts/2</c></seealso>
               to analyze the behavior of your operating system.</p>
             <p>Note that this is also the maximum amount of data that can be
-	       received from a single recv call. If you are using higher than 
-	       normal MTU consider setting buffer higher.</p> 
+	       received from a single recv call. If you are using higher than
+	       normal MTU consider setting buffer higher.</p>
           </item>
           <tag><c>{delay_send, Boolean}</c></tag>
           <item>
diff --git a/lib/kernel/test/gen_udp_SUITE.erl b/lib/kernel/test/gen_udp_SUITE.erl
index b39399b18a..3acfff929e 100644
--- a/lib/kernel/test/gen_udp_SUITE.erl
+++ b/lib/kernel/test/gen_udp_SUITE.erl
@@ -34,7 +34,7 @@
 -export([init_per_testcase/2, end_per_testcase/2]).
 
 -export([send_to_closed/1, active_n/1,
-	 buffer_size/1, binary_passive_recv/1, bad_address/1,
+	 buffer_size/1, binary_passive_recv/1, max_buffer_size/1, bad_address/1,
 	 read_packets/1, open_fd/1, connect/1, implicit_inet6/1,
 	 local_basic/1, local_unbound/1,
 	 local_fdopen/1, local_fdopen_unbound/1, local_abstract/1]).
@@ -44,7 +44,7 @@ suite() ->
      {timetrap,{minutes,1}}].
 
 all() -> 
-    [send_to_closed, buffer_size, binary_passive_recv,
+    [send_to_closed, buffer_size, binary_passive_recv, max_buffer_size,
      bad_address, read_packets, open_fd, connect,
      implicit_inet6, active_n,
      {group, local}].
@@ -237,6 +237,14 @@ buffer_size_server_recv(Socket, IP, Port, Cnt) ->
     end.
 
 
+%%-------------------------------------------------------------
+%% OTP-15206: Keep buffer small for udp
+%%-------------------------------------------------------------
+max_buffer_size(Config) when is_list(Config) ->
+    {ok, Socket}  = gen_udp:open(0, [binary]),
+    ok = inet:setopts(Socket,[{recbuf, 1 bsl 20}]),
+    {ok, [{buffer, 65536}]} = inet:getopts(Socket,[buffer]),
+    gen_udp:close(Socket).
 
 %%-------------------------------------------------------------
 %% OTP-3823 gen_udp:recv does not return address in binary mode