From 83445d0de88a1ad15066e6c415b2c2d9aa93d6a1 Mon Sep 17 00:00:00 2001 From: Lukas Larsson Date: Wed, 20 Jun 2018 17:54:24 +0200 Subject: erts: Free udp buffer when getting EAGAIN Only SCPT should keep the recv buffer when going into select. If UDP does it, it will result in many more memory allocations than there should be which can be very detrimental to performance. --- erts/emulator/drivers/common/inet_drv.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c index 5e9afdc5ca..833befefd1 100644 --- a/erts/emulator/drivers/common/inet_drv.c +++ b/erts/emulator/drivers/common/inet_drv.c @@ -1785,6 +1785,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 @@ -12042,15 +12043,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 +12059,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 +12126,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 */ } -- cgit v1.2.3 From a9d361b4be7d6329bedd7b37d454c1abf4f27233 Mon Sep 17 00:00:00 2001 From: Lukas Larsson Date: Thu, 21 Jun 2018 11:00:25 +0200 Subject: erts: Limit the automatic max buffer for UDP to 2^16 There is no reason to have a larger buffer than this as the recvmsg call will never return more data. OTP-15206 --- erts/emulator/drivers/common/inet_drv.c | 9 ++++++++- lib/kernel/doc/src/inet.xml | 19 ++++++++++--------- lib/kernel/test/gen_udp_SUITE.erl | 12 ++++++++++-- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c index 833befefd1..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 @@ -6436,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; } } 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) -> {buffer, Size} -

The size of the user-level software buffer used by - the driver. - Not to be confused with options sndbuf +

The size of the user-level buffer used by + the driver. Not to be confused with options sndbuf and recbuf, which correspond to the - Kernel socket buffers. It is recommended - to have val(buffer) >= max(val(sndbuf),val(recbuf)) to + Kernel socket buffers. For TCP it is recommended + to have val(buffer) >= val(recbuf) 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. val(buffer) is automatically set to the above - maximum when values sndbuf or recbuf are set. - However, as the sizes set for sndbuf and recbuf + maximum when recbuf is set. + However, as the size set for recbuf usually become larger, you are encouraged to use getopts/2 to analyze the behavior of your operating system.

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.

+ received from a single recv call. If you are using higher than + normal MTU consider setting buffer higher.

 {delay_send, Boolean} 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 -- cgit v1.2.3