Problem:
When tcp_close_work starts to run in LPWORK, if another event comes and
calls tcp_free before tcp_close_work takes net_lock, the tcp_free will
be called twice and cause double free.
Signed-off-by: Zhe Weng <wengzhe@xiaomi.com>
When do socket bind, if the connection domain is not equal to the bound address type, this will cause the stack-buffer-overflow.
Signed-off-by: liqinhui <liqinhui@xiaomi.com>
- NewReno congestion control algorithm is used to solve the problem
of network congestion breakdown. NewReno congestion control includes
slow start, collision avoidance, fast retransmission, and fast
recovery. The implementation refers to RFC6582 and RFC5681.
- In addition, we optimize the congestion algorithm. In the conflict
avoidance stage, the maximum congestion window max_cwnd is used to
limit the excessive growth of cwnd and prevent network jitter
caused by congestion. Maximum congestion window max_cwnd is updated
with the current congestion window cwnd and the update weight is
0.875 when an RTO timeout occurs.
Signed-off-by: liqinhui <liqinhui@xiaomi.com>
If TCP_FIN is received before the user calls accept, the later accept will
report an error, then the previously received data cannot be read.
operation flow:
tcp_server_listen -> tcp_client_connect -> tcp_client_send ->
tcp_client_close -> tcp_server_accept(fix this error) -> tcp_server_recv
Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
We have a case that an http server gives out-of-ordered ACKs, and NuttX client makes `ofoseg`s with length 0, trying to rebuild / put them into `ofosegs` array, which is not intended (no available data and should be skipped). This breaks later logic and finally crashed in `tcp_ofoseg_bufsize` (`ofosegs[i].data` is `NULL`, which should never happen in normal logic).
Note:
- `iob_trimhead` won't return `NULL` when it's applying on normal IOB.
- Keep `dev->d_iob == NULL` to avoid `iob_trimhead` changed.
- `iob_free_chain` will do nothing when applied to `NULL`.
Signed-off-by: Zhe Weng <wengzhe@xiaomi.com>
According to RFC 2525, Section 2.17:
"When an application closes a connection in such a way that it can no longer read any received data, the TCP SHOULD, per section 4.2.2.13 of RFC 1122, send a RST if there is any unread received data, or if any new data is received."
When our TCP socket is closed (even when the thread has exited), the peer can keep sending data and NuttX keeps replying ACK (we've tried for ~12h). This is not a good behavior (also different from Linux), so send RST instead of ACK for data receiving in FIN_WAIT.
Signed-off-by: Zhe Weng <wengzhe@xiaomi.com>
The priorities for finding a network adapter are as follows:
1. if laddr is not ANY, use laddr to find device;
2. if laddr is ANY, and bound index is not 0, use bound index
to find device;
3. if laddr is ANY and no device is bound, use raddr to find
device.
Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
Receiving an ACK indicating TCP Window Update will not set ACKDATA flag (because tx_unacked is 0) in our TCP stack. Then this ACK won't let us send anything after receiving it, even if it updates snd_wnd. So we need to check whether we can send data immediately when our snd_wnd is updated (especially from 0), otherwise we will only send next data after timer expiry.
Signed-off-by: Zhe Weng <wengzhe@xiaomi.com>
NuttX kernel should not use the syscall functions, especially after
enabling CONFIG_SCHED_INSTRUMENTATION_SYSCALL, all system functions
will be traced to backend, which will impact system performance.
Signed-off-by: chao an <anchao@xiaomi.com>
Issue:
TCP rx buffer is freed after 4-way handshake with current design.
3 socket's rx buffer might be consumed during ffmpeg switch music procedure,
and this might cause IOB exhausted.
Solution:
free TCP rx buffer immediately in tcp_close to make sure IOB won't be
exhausted.
Signed-off-by: 梁超众 <liangchaozhong@xiaomi.com>
Signed-off-by: chao an <anchao@xiaomi.com>
This reverts commit fbe641a916.
This issue already fixed by below change:
| commit 715785245c
| Author: chao an <anchao@xiaomi.com>
| Date: Mon Jan 16 12:37:44 2023 +0800
|
| net/tcp: fix potential busy loop in tcp_send_buffered.c
|
| if the wrbuffer does not have enough space to send the rest of
| the data, then the send function will loop infinitely in nonblock
| mode, add send timeout check to avoid this issue.
|
| Signed-off-by: chao an <anchao@xiaomi.com>
Signed-off-by: chao an <anchao@xiaomi.com>
tcp_input:
If we didn't find an active connection that expected the packet,
If the SYN flag isn't set,
It is an old packet and we send a RST.
conn is NULL when this case
Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
psock_tcp_send will enter busyloop state when no IOB keeps at unavailable state
when the socket is blocking socket and this will cause WDT.
According to socket's manpage, -1 should be returned while errno set to EAGAIN
if send/recv timeout was set.
Here's the description:
SO_RCVTIMEO and SO_SNDTIMEO
Specify the receiving or sending timeouts until reporting an error. The argument is a struct timeval. If an input or output function blocks for this period of time, and data has been sent or received, the return value of that function will be the amount of data transferred; if no data has been transferred and the timeout has been reached then -1 is returned with errno set to EAGAIN or EWOULDBLOCK, or EINPROGRESS (for connect(2)) just as if the socket was specified to be nonblocking. If the timeout is set to zero (the default) then the operation will never timeout. Timeouts only have effect for system calls that perform socket I/O (e.g., read(2), recvmsg(2), send(2), sendmsg(2)); timeouts have no effect for select(2), poll(2), epoll_wait(2), and so on.
Signed-off-by: liangchaozhong <liangchaozhong@xiaomi.com>
1. Remove tcp_txdrain() from close() to avoid indefinitely block
2. Send TCP_RST immediately if linger timeout
Signed-off-by: chao an <anchao@xiaomi.com>
Merge conflicts lead to code being placed in thre wrong place
The debug code should placed in tcp_send() not tcp_synack()
Signed-off-by: chao an <anchao@xiaomi.com>
if the wrbuffer does not have enough space to send the rest of
the data, then the send function will loop infinitely in nonblock
mode, add send timeout check to avoid this issue.
Signed-off-by: chao an <anchao@xiaomi.com>
Allocate the device buffer only if the protocol really need to send data.
not all protocols require the driver to prepare additional iob before
sending, especially UDP, each iob reserves l2/l3 header in advance
after prepare write buffer, net device could reuse this entry to send directly
Signed-off-by: chao an <anchao@xiaomi.com>
Fragmentation of network data will intensify iob consumption, if
the device receives a message storm of fragmented packets, the iob
cache will not be effectively used, this is not allowed on iot devices
since the resources of such devices are limited. Of course, this
also takes some disadvantages: data needs to be copied.
This option will brings some balance on resource-constrained devices,
enable this config to reduce the consumption of iob, the received iob
buffers will be merged into the contiguous iob chain.
Signed-off-by: chao an <anchao@xiaomi.com>
l3/l4 stack will decouple the reference of d_buf gradually, Only legacy
devices still retain d_buf support, new net devices will use d_iob
Signed-off-by: chao an <anchao@xiaomi.com>
fix build break if enable CONFIG_NET_IPv6 only
In file included from tcp/tcp_sendfile.c:38:
tcp/tcp_sendfile.c: In function ‘sendfile_eventhandler’:
tcp/tcp_sendfile.c:173:27: error: ‘struct tcp_conn_s’ has no member named ‘domain’
173 | DEBUGASSERT(conn->domain == PF_INET6);
| ^~
Signed-off-by: chao an <anchao@xiaomi.com>
Issue:
recv return 0 means peer side has already closed the connection according to
man page's description.
0 is returned without set errno when TCP rx timeout happens with current
design.
Solution:
return result instead of ir_result when ir_result is 0, then -1 will be
returned with errno set to EAGAIN for tcp rx buffer empty case.
Signed-off-by: liangchaozhong <liangchaozhong@xiaomi.com>
devif/ipv6_input.c: In function 'ipv6_input':
devif/ipv6_input.c:59:33: error: 'TCPIPv6BUF' undeclared (first use in this function); did you mean 'IPv6BUF'?
59 | #define PAYLOAD ((FAR uint8_t *)TCPIPv6BUF)
| ^~~~~~~~~~
devif/ipv6_input.c:302:14: note: in expansion of macro 'PAYLOAD'
302 | payload = PAYLOAD; /* Assume payload starts right after IPv6 header */
|
Signed-off-by: chao an <anchao@xiaomi.com>
Fixed by commit:
| net: remove pvconn reference from all devif callback
|
| Do not use 'pvconn' argument to get the connection pointer since
| pvconn is normally NULL for some events like NETDEV_DOWN.
| Instead, the connection pointer can be reliably obtained from the
| corresponding private pointer.
Signed-off-by: chao an <anchao@xiaomi.com>
I noticed that the conn instance will leak during stress test,
The close work queued from tcp_close_eventhandler() will be canceled
by tcp_timer() immediately:
Breakpoint 1, tcp_close_eventhandler (dev=0x565cd338 <up_irq_restore+108>, pvpriv=0x5655e6ff <getpid+12>, flags=0) at tcp/tcp_close.c:71
(gdb) bt
| #0 tcp_close_eventhandler (dev=0x565cd338 <up_irq_restore+108>, pvpriv=0x5655e6ff <getpid+12>, flags=0) at tcp/tcp_close.c:71
| #1 0x5658bf1e in devif_conn_event (dev=0x5660bd80 <g_sim_dev>, flags=512, list=0x5660d558 <g_cbprealloc+312>) at devif/devif_callback.c:508
| #2 0x5658a219 in tcp_callback (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>, flags=512) at tcp/tcp_callback.c:167
| #3 0x56589253 in tcp_timer (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_timer.c:378
| #4 0x5658dd47 in tcp_poll (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_devpoll.c:95
| #5 0x5658b95f in devif_poll_tcp_connections (dev=0x5660bd80 <g_sim_dev>, callback=0x565770f2 <netdriver_txpoll>) at devif/devif_poll.c:601
| #6 0x5658b9ea in devif_poll (dev=0x5660bd80 <g_sim_dev>, callback=0x565770f2 <netdriver_txpoll>) at devif/devif_poll.c:722
| #7 0x56577230 in netdriver_txavail_work (arg=0x5660bd80 <g_sim_dev>) at sim/up_netdriver.c:308
| #8 0x5655999e in work_thread (argc=2, argv=0xf3db5dd0) at wqueue/kwork_thread.c:178
| #9 0x5655983f in nxtask_start () at task/task_start.c:129
(gdb) c
Continuing.
Breakpoint 2, tcp_update_timer (conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_timer.c:178
(gdb) bt
| #0 tcp_update_timer (conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_timer.c:178
| #1 0x5658952a in tcp_timer (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_timer.c:708
| #2 0x5658dd47 in tcp_poll (dev=0x5660bd80 <g_sim_dev>, conn=0x5660c4a0 <g_tcp_connections>) at tcp/tcp_devpoll.c:95
| #3 0x5658b95f in devif_poll_tcp_connections (dev=0x5660bd80 <g_sim_dev>, callback=0x565770f2 <netdriver_txpoll>) at devif/devif_poll.c:601
| #4 0x5658b9ea in devif_poll (dev=0x5660bd80 <g_sim_dev>, callback=0x565770f2 <netdriver_txpoll>) at devif/devif_poll.c:722
| #5 0x56577230 in netdriver_txavail_work (arg=0x5660bd80 <g_sim_dev>) at sim/up_netdriver.c:308
| #6 0x5655999e in work_thread (argc=2, argv=0xf3db5dd0) at wqueue/kwork_thread.c:178
| #7 0x5655983f in nxtask_start () at task/task_start.c:129
Since a separate work will add 24 bytes to each conn instance,
but in order to support the feature of asynchronous close(),
I can not find a better way than adding a separate work,
for resource constraints, I recommend the developers to enable
CONFIG_NET_ALLOC_CONNS, which will reduce the ram usage.
Signed-off-by: chao an <anchao@xiaomi.com>
Do not use 'pvconn' argument to get the connection pointer since
pvconn is normally NULL for some events like NETDEV_DOWN.
Instead, the connection pointer can be reliably obtained from the
corresponding private pointer.
Signed-off-by: chao.an <anchao@xiaomi.com>
Otherwise, when a long test triggers multiple timeout retransmissions,
the late timeout retransmissions are always delayed between 24 and 48 seconds
Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
since it is impossible to track producer and consumer
correctly if TCP/IP stack pass IOB directly to netdev
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
fix regression of invalid update the rexmit_seq in buffer mode
rexmit_seq should not be used instead of sndseq in fast retransmission,
sndseq of retransmission in the packet does not need to be re-updated
Signed-off-by: chao.an <anchao@xiaomi.com>
Net poll teardown is not protected by net lock, if the conn is released
before teardown, the assertion failure will be triggered during free dev
callback, this patch will add the net lock around net poll teardown to
fix race condition
nuttx/libs/libc/assert/lib_assert.c:36
nuttx/net/devif/devif_callback.c:85
nuttx/net/tcp/tcp_netpoll.c:405
nuttx/fs/vfs/fs_poll.c:244
nuttx/fs/vfs/fs_poll.c:500
Signed-off-by: chao.an <anchao@xiaomi.com>
When the free connection list is unenough to alloc a new instance,
the TCP stack will reuse the currently closed connection, but if
the handle is not released by the user via close(2), the reference
count of the connection remains in a non-zero value, it will cause
the assertion to fail, so when the handle is not released we should
not use such a conn instance when being actively closed, and ensure
that the reference count is assigned within the net lock protection
|(gdb) bt
|#0 up_assert (filename=0x565c78f7 "tcp/tcp_conn.c", lineno=771) at sim/up_assert.c:75
|#1 0x56566177 in _assert (filename=0x565c78f7 "tcp/tcp_conn.c", linenum=771) at assert/lib_assert.c:36
|#2 0x5657d620 in tcp_free (conn=0x565fb3e0 <g_tcp_connections>) at tcp/tcp_conn.c:771
|#3 0x5657d5a1 in tcp_alloc (domain=2 '\002') at tcp/tcp_conn.c:700
|#4 0x565b1f50 in inet_tcp_alloc (psock=0xf3dea150) at inet/inet_sockif.c:144
|#5 0x565b2082 in inet_setup (psock=0xf3dea150, protocol=0) at inet/inet_sockif.c:253
|#6 0x565b1bf0 in psock_socket (domain=2, type=1, protocol=0, psock=0xf3dea150) at socket/socket.c:121
|#7 0x56588f5f in socket (domain=2, type=1, protocol=0) at socket/socket.c:278
|#8 0x565b11c0 in hello_main (argc=1, argv=0xf3dfab10) at hello_main.c:35
|#9 0x56566631 in nxtask_startup (entrypt=0x565b10ef <hello_main>, argc=1, argv=0xf3dfab10) at sched/task_startup.c:70
|#10 0x565597fa in nxtask_start () at task/task_start.c:134
Signed-off-by: chao.an <anchao@xiaomi.com>
This reverts commit b88a1fd7fd. [1]
Because:
* It casues assertion failures like [2].
* I don't understand what it attempted to fix.
[1]
```
commit b88a1fd7fd
Author: chao.an <anchao@xiaomi.com>
Date: Sat Jul 2 13:17:41 2022 +0800
net/tcp: discard connect reference before free
connect reference should be set to 0 before free
Signed-off-by: chao.an <anchao@xiaomi.com>
```
[2]
```
#0 up_assert (filename=0x5516d0 "tcp/tcp_conn.c", lineno=771) at sim/up_assert.c:75
#1 0x000000000040a4bb in _assert (filename=0x5516d0 "tcp/tcp_conn.c", linenum=771) at assert/lib_assert.c:36
#2 0x000000000042a2ad in tcp_free (conn=0x597fe0 <g_tcp_connections+384>) at tcp/tcp_conn.c:771
#3 0x000000000053bdc2 in tcp_close_disconnect (psock=0x7f58d1abbd80) at tcp/tcp_close.c:331
#4 0x000000000053bc69 in tcp_close (psock=0x7f58d1abbd80) at tcp/tcp_close.c:366
#5 0x000000000052eefe in inet_close (psock=0x7f58d1abbd80) at inet/inet_sockif.c:1689
#6 0x000000000052eb9b in psock_close (psock=0x7f58d1abbd80) at socket/net_close.c:102
#7 0x0000000000440495 in sock_file_close (filep=0x7f58d1b35f40) at socket/socket.c:115
#8 0x000000000043b8b6 in file_close (filep=0x7f58d1b35f40) at vfs/fs_close.c:74
#9 0x000000000043ab22 in nx_close (fd=9) at inode/fs_files.c:544
#10 0x000000000043ab7f in close (fd=9) at inode/fs_files.c:578
```
Retransmit only one the earliest not acknowledged segment
(according to RFC 6298 (5.4)). The issue is the same as it was
in tcp_send_unbuffered.c and tcp_sendfile.c.
Signed-off-by: chao.an <anchao@xiaomi.com>
The time consuming of tcp waving hands(close(2)) will be affected
by network jitter, especially the wireless device cannot receive
the last-ack under worst environment, in this change we move the
tcp close callback into background and invoke the resource free
from workqueue, which will avoid the user application from being
blocked for a long time and unable to return in the call of close
Signed-off-by: chao.an <anchao@xiaomi.com>
since the code could map the unsupported work to the
supported one and remove select SCHED_WORKQUEUE from
Kconfig since SCHED_[L|H]PWORK already do the selection
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
