RIOT
RIOT copied to clipboard
examples: add TCP echo server & client from documentation
Contribution description
The TCP sock API comes with an TCP echo example server & client. But since this is never compiled and run it contained hidden bugs and is generally inconvenient as no Makefile is included.
Move this code to a proper example to it can be easily build & run for LWIP and GNRC TCP implementations.
Testing procedure
You can run all the tests on native
using nc
as the other end:
server mode
> ifconfig
Iface 5 HWaddr: 7A:37:FC:7D:1A:AF
L2-PDU:1500 MTU:1500 HL:64 Source address length: 6
Link type: wired
inet6 addr: fe80::7837:fcff:fe7d:1aaf scope: link VAL
inet6 group: ff02::1
inet6 group: ff02::1:ff7d:1aaf
> listen 1234
Listening on port 1234
Send some data from Linux
echo -n "Hello from Linux" | nc fe80::7837:fcff:fe7d:1aaf%tapbr0 1234
Reading data
Read: "Hello from Linux"
> Disconnected
client mode
Start a server on Linux
% ip a s tapbr0
3: tapbr0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
link/ether 92:a7:a6:4b:2e:32 brd ff:ff:ff:ff:ff:ff
inet6 fe80::90a7:a6ff:fe4b:2e32/64 scope link
valid_lft forever preferred_lft forever
% echo -n "Hello Back" | nc -6 -l 1234
Send some data in RIOT
> send fe80::90a7:a6ff:fe4b:2e32 1234 "Hello from RIOT"
Read: "Hello Back"
Receive it on Linux
Hello from RIOT
Issues/PRs references
depend on and includes #16741 #16494
Why single out the TCP example? Why not the UDP, IP, or DTLS examples?
Well we should have those as runnable code too, but we got to start somewhere :wink:
IMHO this could also be one application.
(and we should call it sock_tcp_echo
to distiguish it from posix_sockets
)
@brummer-simon I just noticed something now that client and server are combined into the same application: After starting the server, I can no longer open any client connections:
2021-09-08 17:02:30,857 # listen 1234
2021-09-08 17:02:30,859 # Listening on port 1234
> send 2001:16b8:45bb:3d00:b8f3:77d9:fa25:2b01 1234 Test
2021-09-08 17:02:46,663 # send 2001:16b8:45bb:3d00:b8f3:77d9:fa25:2b01 1234 Test
2021-09-08 17:02:46,667 # Error connecting sock: Not enough space
With LWIP=1
this is working fine:
> listen 1234
2021-09-08 17:06:24,483 # listen 1234
2021-09-08 17:06:24,485 # Listening on port 1234
> send 2001:16b8:45bb:3d00:b8f3:77d9:fa25:2b01 1234 Test
2021-09-08 17:07:09,175 # send 2001:16b8:45bb:3d00:b8f3:77d9:fa25:2b01 1234 Test
2021-09-08 17:07:09,178 # Read: "Hello from Linux
2021-09-08 17:07:09,178 # "
@brummer-simon I just noticed something now that client and server are combined into the same application: After starting the server, I can no longer open any client connections:
2021-09-08 17:02:30,857 # listen 1234 2021-09-08 17:02:30,859 # Listening on port 1234 > send 2001:16b8:45bb:3d00:b8f3:77d9:fa25:2b01 1234 Test 2021-09-08 17:02:46,663 # send 2001:16b8:45bb:3d00:b8f3:77d9:fa25:2b01 1234 Test 2021-09-08 17:02:46,667 # Error connecting sock: Not enough space
With
LWIP=1
this is working fine:> listen 1234 2021-09-08 17:06:24,483 # listen 1234 2021-09-08 17:06:24,485 # Listening on port 1234 > send 2001:16b8:45bb:3d00:b8f3:77d9:fa25:2b01 1234 Test 2021-09-08 17:07:09,175 # send 2001:16b8:45bb:3d00:b8f3:77d9:fa25:2b01 1234 Test 2021-09-08 17:07:09,178 # Read: "Hello from Linux 2021-09-08 17:07:09,178 # "
This makes sence. By default GNRC_TCP has a preallocated memory for a single receive buffer. If you want to maintain multiple connections at the same time you must increase CONFIG_GNRC_TCP_RCV_BUFFERS to something above 1.
With IPv6 usage each receive buffer in use tries to allocate have at least 1220 Byte from the preallocated memory. To keep memory consumption low GNRC_TCP preallocates only enough space for a single connection by default.
Shouldn't gnrc_pktbuf
take care of holding the memory of the packet? Why do we need to copy it to a second buffer?
Shouldn't
gnrc_pktbuf
take care of holding the memory of the packet? Why do we need to copy it to a second buffer?
IIRC, the current gnrc_tcp
implementation is not zero-copy. At some point you need to reassemble the TCP segments, so I guess this is where the second buffer comes in. Why gnrc_pktbuf
is not used as the space where this happens, I cannot remember.
Shouldn't
gnrc_pktbuf
take care of holding the memory of the packet? Why do we need to copy it to a second buffer?IIRC, the current
gnrc_tcp
implementation is not zero-copy. At some point you need to reassemble the TCP segments, so I guess this is where the second buffer comes in. Whygnrc_pktbuf
is not used as the space where this happens, I cannot remember.
Exactly. The so called receive buffer holds the received payload until a user reads it. gnrc_pktbuf is only used to for packet data in transit. Keeping both seperate reduces the risk of running out of memory in gnrc_pktbuf in cases where the user does not read received data. This is important then multiple connections are used in parallel.
Mhhh not sure that reasoning makes sense. Yes it is called pktbuf
, but nothing prevents you to use it to store non packets ;-). Regarding running out of buffer space: that can also happen with an additional buffer. From user perspective it would make more sense to increase the size of one buffer instead of now needing to gauge the size of two distinct buffers.
It'd be cool to explore zero-copying TCP (as I understand @miri64's comment) and just keeping the frames around to keep the data. Two concerns:
- If 3 processes run and one doesn't fetch its data, right now the 2 others keep working and the blocked one just propagates backpressure to the peer. With shared buffers, all would create backpressure. (Which is especially bad if the stalled process is waiting for the others to make progress).
- Mitigation would be that each TCP connection does bookkeeping of ifs allocated frames, and applies backpressure when the equivalent of the current per-socket buffer is applied and new frames are NACKed.
- Overcommitting could still stall things, but the buffer sizes can be chosen appropriately: If we now have, say, 1k per socket, then when we'd switch that'd go away and the calculation for the network buffer would grow by the 1k * max tcp sockets.
- If a peer wants to clog a RIOT device, it might send very small TCP frames and clog the buffer: if the above 1k is filled with TCP packets with 4 bytes payload each, we'd still keep tons of headers around, and exceed limitations.
- Mitigation could be defragmentation, possibly into repurposed header fields, but suddenly this is not trivial any more.
(But maybe this is OT for here...)
@chrysn allowing a peer to drain gnrc_pktbuf is a real problem here the receive buffer helps to prevent. Zero copy would be nice and all but we had already memory issues in the past (https://www.opencve.io/cve/CVE-2019-15134).
Thanks for the clarification (and reminding of that vuln) @brummer-simon.
What's left to do here?
i tested with native tap on the other side of the connection i went for nc
listening 12345 :heavy_check_mark:
connecting with nc :heavy_check_mark: nc fe80::xxxxxxxx:13e2%tapbr0 12345
close and reconnect with nc :heavy_check_mark:
server nc: nc -6 -l 23456
send fe80::xxxxxxx:6dfa 23456 hallo
send fe80::58ca:8ff:fe05:6dfa 23456 hallo
Error connecting sock: Cannot allocate memory
restart and send without listening :heavy_check_mark:
seems as if this is not able to send while listening even though there is
CONFIG_GNRC_TCP_RCV_BUFFERS=2
AFAIU this is the intended behavior Try with
CFLAGS += -DCONFIG_GNRC_TCP_EXPERIMENTAL_DYN_MSL_EN=1
or LWIP=1
if you want fast reconnects.
seems as if this is not able to send while listening even though there is
CONFIG_GNRC_TCP_RCV_BUFFERS=2
hm yea I can't really explain that - maybe @brummer-simon knows more?
Since the behaviour (RIOT is either listening or sending) is intended i consider the tests successful. I think this example shows what it intends to show (how to do TCP with Riot).
with LWIP=1 listening and sending in the same RIOT works -> example is working as intended
I don't see any reviews that suggest changes -> please squash
maybe transfer a newline with the sendpkg
seems as if this is not able to send while listening even though there is
CONFIG_GNRC_TCP_RCV_BUFFERS=2
hm yea I can't really explain that - maybe @brummer-simon knows more?>
I did't really follow this PR but maybe I can help. Sending data and listening for incomming data in parallel, should be no issue and I think it is covered in the test suite (although I am not entirely sure on that). Could you provide me with detailed log output, maybe I can give more insight on the issue.
EDIT: I took a short look into the test suite and maintaining both modes in parallel is not tested there. That being said GNRC_TCP is not designed in a way, that active and passive connection handling are mutually exclusive so it should be no problem. We have untested teritory that might be broken. We should make an Issue out of it and It would be great if @kfessel could provide me with a detailed log output.
@brummer-simon
all tests are done with tap network
setup: terminal 1 (T1):
$ nc -6 -l 23456
terminal 2 (T2):
examples/sock_tcp_echo$ make all
Building application "sock_tcp_echo" for "native" with MCU "native"
...
test: T2:
...../examples/sock_tcp_echo/bin/native/sock_tcp_echo.elf tap0
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.
NETOPT_RX_END_IRQ not implemented by driver
main(): This is RIOT! (Version: 2021.10-devel-596-g0904b-HEAD)
RIOT TCP client example application
All up, running the shell now
> listen 12345
listen 12345
Listening on port 12345
> send fe80::XXXX:6dfa 23456 hallo
send fe80::XXXX:6dfa 23456 hallo
Error connecting sock: Cannot allocate memory
>
with LWIP setup: same setup for T1 as for gnrc T2:
examples/sock_tcp_echo$ LWIP=1 make
Building application "sock_tcp_echo" for "native" with MCU "native".
test T2:
examples/sock_tcp_echo/bin/native/sock_tcp_echo.elf tap0
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.
main(): This is RIOT! (Version: 2021.10-devel-596-g0904b-HEAD)
RIOT TCP client example application
All up, running the shell now
> listen 12345
listen 12345
Listening on port 12345
> send fe80::XXXX:6dfa 23456 hallo
send fe80::XXXX:6dfa 23456 hallo
#change tab#
Read: "jim
"
T1:
hallo#change tab#jim
# nc terminates
@kfessel: Interesting. I'll try to replicate It in the evening. Maybe I am able to figure something out. For now it looks like faulty behavior to me
@benpicco, @kfessel - I think I found the Issue here. For some reason unknown to me, the configuration in "app.gnrc.config" is not applied.
If I add "CFLAGS += -DCONFIG_GNRC_TCP_RCV_BUFFERS=2" directly in the makefile, the error does not occur anymore. If this setting is not applied, it is obvious why GNRC_TCP runs out of space.
Rebase + fix after #17714 broke the build.
But now send
will hang indefinitely.
RIOT native interrupts/signals initialized.
RIOT native board initialized.
RIOT native hardware initialization complete.
NETOPT_RX_END_IRQ not implemented by driver
main(): This is RIOT! (Version: 2022.04-devel-577-gb3297-examples/tcp_echo)
RIOT TCP client example application
All up, running the shell now
> ifconfig
ifconfig
Iface 5 HWaddr: 7A:37:FC:7D:1A:AF
L2-PDU:1500 MTU:1500 HL:64 Source address length: 6
Link type: wired
inet6 addr: fe80::7837:fcff:fe7d:1aaf scope: link VAL
inet6 group: ff02::1
inet6 group: ff02::1:ff7d:1aaf
> send ff02::1 1234 test
send ff02::1 1234 test
ps
ps
ps
ifconfig
edit ah I guess making a TCP connection with a broadcast address is not going to work (still should not hang IMHO), with a normal address it works (but still with a really long timeout after the other side closes the connection - I just used nc -6 -l 1234
)
> send fe80::90a7:a6ff:fe4b:2e32 1234 test
send fe80::90a7:a6ff:fe4b:2e32 1234 test
Read: "Hello from Linux!
"
ps
ps
ps
ifconfig
> ps
pid | name | state Q | pri | stack ( used) ( free) | base addr | current
- | isr_stack | - - | - | 8192 ( -1) ( 8193) | 0x807f820 | 0x807f820
1 | idle | pending Q | 15 | 8192 ( 612) ( 7580) | 0x8078120 | 0x8079f80
2 | main | running Q | 7 | 12288 ( 3132) ( 9156) | 0x807a120 | 0x807cf80
3 | ipv6 | bl rx _ | 4 | 8192 ( 1776) ( 6416) | 0x8083bc0 | 0x8085a20
4 | gnrc_tcp | bl rx _ | 5 | 8192 ( 1680) ( 6512) | 0x8089880 | 0x808b6e0
5 | gnrc_netdev_tap | bl rx _ | 2 | 8192 ( 2492) ( 5700) | 0x8085fe0 | 0x8087e40
| SUM | | | 53248 ( 9692) (43556)
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.
Murdock results
:heavy_check_mark: PASSED
f34ed11219b6c8ca4cd8e03c4f2d5ca4b401f28a examples/sock_tcp_echo: add TCP echo client / server
Success | Failures | Total | Runtime |
---|---|---|---|
17 | 0 | 17 | 01m:15s |
Artifacts
Ping @benpicco
seems like this is mergable just some small improvements to the coding style