From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Alexander Turenko Subject: [PATCH 1/3] socket: evaluate buffer size in recv / recvfrom Date: Fri, 24 Aug 2018 05:47:37 +0300 Message-Id: <36b23f07009f3cd467c1eb7e54e982b1f907908d.1535076888.git.alexander.turenko@tarantool.org> In-Reply-To: References: In-Reply-To: References: To: Vladimir Davydov Cc: Alexander Turenko , tarantool-patches@freelists.org, Yaroslav Dynnikov List-ID: When size parameter is not passed to socket:recv() or socket:recvfrom() it will call a) or b) on the socket to evaluate size of the buffer to store the receiving datagram. Before this commit a datagram will be truncated to 512 bytes in the case. a) Linux: recv(fd, NULL, 0 , MSG_TRUNC | MSG_PEEK) b) Mac OS: getsockopt(fd, SOL_SOCKET, SO_NREAD, &val, &len) It is recommended to set 'size' parameter (size of the input buffer) explicitly based on known message format and known network conditions (say, set it less then MTU to prevent IP fragmentation, which can be inefficient) or pass it from a configuration option of a library / an application. The reason is that explicit buffer size provided allows to avoid extra recv syscall on Linux. Added socket:is_udp() and socket:dgram_length() methods to the public API. Part of #3619. --- src/lua/socket.c | 3 + src/lua/socket.lua | 57 +++++- test/app/socket.result | 401 +++++++++++++++++++++++++++++++++++++++ test/app/socket.test.lua | 154 +++++++++++++++ 4 files changed, 613 insertions(+), 2 deletions(-) diff --git a/src/lua/socket.c b/src/lua/socket.c index 14d49d106..bf23eae1f 100644 --- a/src/lua/socket.c +++ b/src/lua/socket.c @@ -268,6 +268,9 @@ static const struct lbox_sockopt_reg so_opts[] = { #ifdef SO_PROTOCOL {"SO_PROTOCOL", SO_PROTOCOL, 1, 0, }, #endif +#ifdef SO_NREAD /* available on Darwin */ + {"SO_NREAD", SO_NREAD, 1, 0, }, +#endif {"SO_TYPE", SO_TYPE, 1, 0, }, diff --git a/src/lua/socket.lua b/src/lua/socket.lua index 06306eae2..946e11e9e 100644 --- a/src/lua/socket.lua +++ b/src/lua/socket.lua @@ -770,6 +770,41 @@ local function socket_send(self, octets, flags) return tonumber(res) end +local function socket_is_udp(self) + local fd = check_socket(self) + if self.itype == nil then + self.itype = self:getsockopt('SOL_SOCKET', 'SO_TYPE') + end + return self.itype == internal.SO_TYPE['SOCK_DGRAM'] +end + +-- returns nil and sets EAGAIN when there are no datagrams in the receive queue +-- as well as when zero-length datagram is the next one +local function socket_dgram_length(self) + local fd = check_socket(self) + if not self:is_udp() then + error("socket:dgram_length() supports only a UDP socket") + end + + local len + if jit.os == 'OSX' then + self._errno = nil + len = self:getsockopt('SOL_SOCKET', 'SO_NREAD') + else + local iflags = get_iflags(internal.SEND_FLAGS, {'MSG_TRUNC', 'MSG_PEEK'}) + assert(iflags ~= nil) + self._errno = nil + len = tonumber(ffi.C.recv(fd, nil, 0, iflags)) + end + + if len == -1 or len == 0 then + self._errno = boxerrno() + return nil + end + + return len +end + local function socket_recv(self, size, flags) local fd = check_socket(self) local iflags = get_iflags(internal.SEND_FLAGS, flags) @@ -778,7 +813,15 @@ local function socket_recv(self, size, flags) return nil end - size = size or 512 + if self:is_udp() then + size = size or self:dgram_length() + if size == nil then + return nil + end + else + size = size or 512 + end + self._errno = nil local buf = ffi.new("char[?]", size) @@ -799,7 +842,15 @@ local function socket_recvfrom(self, size, flags) return nil end - size = size or 512 + if self:is_udp() then + size = size or self:dgram_length() + if size == nil then + return nil + end + else + size = size or 512 + end + self._errno = nil local res, from = internal.recvfrom(fd, size, iflags) if res == nil then @@ -1165,6 +1216,8 @@ socket_mt = { read = socket_read; write = socket_write; send = socket_send; + is_udp = socket_is_udp; + dgram_length = socket_dgram_length; recv = socket_recv; recvfrom = socket_recvfrom; sendto = socket_sendto; diff --git a/test/app/socket.result b/test/app/socket.result index b4d4a9a78..07cd24c15 100644 --- a/test/app/socket.result +++ b/test/app/socket.result @@ -1721,6 +1721,7 @@ test_run:cmd("setopt delimiter ''"); -------------------------------------------------------------------------------- -- Lua Socket Emulation -------------------------------------------------------------------------------- +-- {{{ test_run:cmd("push filter 'fd=([0-9]+)' to 'fd='") --- - true @@ -2230,6 +2231,406 @@ s:close() --- - 1 ... +-- }}} +-- s:is_udp() {{{ +s = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +s:is_udp() -- expected true +--- +- true +... +s:close() +--- +- true +... +s = socket('AF_INET', 'SOCK_STREAM', 0) +--- +... +s:is_udp() -- expected false +--- +- false +... +s:close() +--- +- true +... +-- }}} +-- s:dgram_length() {{{ +-- case: tcp socket is not supported +s = socket('AF_INET', 'SOCK_STREAM', 0) +--- +... +s:dgram_length() -- expected an error +--- +- error: 'builtin/socket.lua: socket:dgram_length() supports only a UDP socket' +... +s:close() +--- +- true +... +receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +receiving_socket:bind('127.0.0.1', 0) +--- +- true +... +receiving_socket_port = receiving_socket:name().port +--- +... +sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +-- case: no datagram +received_len = receiving_socket:dgram_length() +--- +... +e = receiving_socket:errno() +--- +... +received_len == nil -- expected true +--- +- true +... +received_len +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +message_len = 0 +--- +... +message = '' +--- +... +-- case: zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +received_len = receiving_socket:dgram_length() +--- +... +e = receiving_socket:errno() +--- +... +received_len == nil -- expected true +--- +- true +... +received_len +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +message_len = 1025 +--- +... +message = string.rep('x', message_len) +--- +... +-- case: non-zero length datagram +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- 1025 +... +received_len = receiving_socket:dgram_length() +--- +... +e = receiving_socket:errno() +--- +... +received_len == message_len -- expected true +--- +- true +... +received_len +--- +- 1025 +... +e == 0 -- expected true +--- +- true +... +e +--- +- 0 +... +receiving_socket:close() +--- +- true +... +sending_socket:close() +--- +- true +... +-- }}} +-- gh-3619: socket.recvfrom crops UDP packets {{{ +receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +receiving_socket:bind('127.0.0.1', 0) +--- +- true +... +receiving_socket_port = receiving_socket:name().port +--- +... +sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +-- case: recv, no datagram +received_message = receiving_socket:recv() +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +-- case: recvfrom, no datagram +received_message, from = receiving_socket:recvfrom() +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +from == nil -- expected true +--- +- true +... +from +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +message_len = 0 +--- +... +message = '' +--- +... +-- case: recv, zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +received_message = receiving_socket:recv() +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +-- case: recvfrom, zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +received_message, from = receiving_socket:recvfrom() +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +from == nil -- expected true +--- +- true +... +from +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +-- case: recv, zero-length datagram (the same as no datagram), explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +received_message = receiving_socket:recv(512) +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +-- case: recvfrom, zero-length datagram (the same as no datagram), explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +received_message, from = receiving_socket:recvfrom(512) +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +from == nil -- expected true +--- +- true +... +from +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +message_len = 1025 +--- +... +message = string.rep('x', message_len) +--- +... +-- case: recv, non-zero length datagram +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- 1025 +... +received_message = receiving_socket:recv() +--- +... +e = receiving_socket:errno() +--- +... +received_message == message -- expected true +--- +- true +... +received_message:len() +--- +- 1025 +... +received_message +--- +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +... +e == 0 -- expected true +--- +- true +... +e +--- +- 0 +... +-- case: recvfrom, non-zero length datagram +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- 1025 +... +received_message, from = receiving_socket:recvfrom() +--- +... +e = receiving_socket:errno() +--- +... +received_message == message -- expected true +--- +- true +... +received_message:len() +--- +- 1025 +... +received_message +--- +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +... +from ~= nil -- expected true +--- +- true +... +from.host == '127.0.0.1' -- expected true +--- +- true +... +e == 0 -- expected true +--- +- true +... +e +--- +- 0 +... +receiving_socket:close() +--- +- true +... +sending_socket:close() +--- +- true +... +-- }}} test_run:cmd("clear filter") --- - true diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua index f6539438b..c2dc7ea90 100644 --- a/test/app/socket.test.lua +++ b/test/app/socket.test.lua @@ -591,6 +591,7 @@ test_run:cmd("setopt delimiter ''"); -------------------------------------------------------------------------------- -- Lua Socket Emulation -------------------------------------------------------------------------------- +-- {{{ test_run:cmd("push filter 'fd=([0-9]+)' to 'fd='") @@ -736,4 +737,157 @@ sc:connect(host, port) sc:close() s:close() +-- }}} + +-- s:is_udp() {{{ + +s = socket('AF_INET', 'SOCK_DGRAM', 'udp') +s:is_udp() -- expected true +s:close() + +s = socket('AF_INET', 'SOCK_STREAM', 0) +s:is_udp() -- expected false +s:close() + +-- }}} + +-- s:dgram_length() {{{ + +-- case: tcp socket is not supported +s = socket('AF_INET', 'SOCK_STREAM', 0) +s:dgram_length() -- expected an error +s:close() + +receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +receiving_socket:bind('127.0.0.1', 0) +receiving_socket_port = receiving_socket:name().port +sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') + +-- case: no datagram +received_len = receiving_socket:dgram_length() +e = receiving_socket:errno() +received_len == nil -- expected true +received_len +e == errno.EAGAIN -- expected true + +message_len = 0 +message = '' + +-- case: zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_len = receiving_socket:dgram_length() +e = receiving_socket:errno() +received_len == nil -- expected true +received_len +e == errno.EAGAIN -- expected true + +message_len = 1025 +message = string.rep('x', message_len) + +-- case: non-zero length datagram +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_len = receiving_socket:dgram_length() +e = receiving_socket:errno() +received_len == message_len -- expected true +received_len +e == 0 -- expected true +e + +receiving_socket:close() +sending_socket:close() + +-- }}} + +-- gh-3619: socket.recvfrom crops UDP packets {{{ + +receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +receiving_socket:bind('127.0.0.1', 0) +receiving_socket_port = receiving_socket:name().port +sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') + +-- case: recv, no datagram +received_message = receiving_socket:recv() +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +e == errno.EAGAIN -- expected true + +-- case: recvfrom, no datagram +received_message, from = receiving_socket:recvfrom() +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +from == nil -- expected true +from +e == errno.EAGAIN -- expected true + +message_len = 0 +message = '' + +-- case: recv, zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message = receiving_socket:recv() +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +e == errno.EAGAIN -- expected true + +-- case: recvfrom, zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message, from = receiving_socket:recvfrom() +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +from == nil -- expected true +from +e == errno.EAGAIN -- expected true + +-- case: recv, zero-length datagram (the same as no datagram), explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message = receiving_socket:recv(512) +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +e == errno.EAGAIN -- expected true + +-- case: recvfrom, zero-length datagram (the same as no datagram), explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message, from = receiving_socket:recvfrom(512) +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +from == nil -- expected true +from +e == errno.EAGAIN -- expected true + +message_len = 1025 +message = string.rep('x', message_len) + +-- case: recv, non-zero length datagram +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message = receiving_socket:recv() +e = receiving_socket:errno() +received_message == message -- expected true +received_message:len() +received_message +e == 0 -- expected true +e + +-- case: recvfrom, non-zero length datagram +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message, from = receiving_socket:recvfrom() +e = receiving_socket:errno() +received_message == message -- expected true +received_message:len() +received_message +from ~= nil -- expected true +from.host == '127.0.0.1' -- expected true +e == 0 -- expected true +e + +receiving_socket:close() +sending_socket:close() + +-- }}} + test_run:cmd("clear filter") -- 2.17.1