nuttx icon indicating copy to clipboard operation
nuttx copied to clipboard

net/tcp/udp: Modify calls of net_iob*alloc calls to be unthrottled for sends, throttled for receives.

Open mennovf opened this issue 1 month ago • 9 comments

Note: Please adhere to Contributing Guidelines.

Summary

Addresses this issue: https://github.com/apache/nuttx/issues/17299

The Kconfig option CONFIG_IOB_THROTTLE is used to limit the amount allocated by TCP (and UDP) receives as to not starve sends. This distinction is made via a 'throttle' argument passed to the iob_*alloc functions, which are wrapped by net_iob*alloc. Previously the udp/tcp_wrbuffer_write functions incorrectly allocate with throttle=true, effectively making the IOB_THROTTLE option useless.

This patch modifies the calls in udp/tcp_wrbuffer_write to allocate unthrottled.

There were also several locations in the receive path that incorrectly allocated unthrottled.

Impact

This should not have an effect during normal operation. This change is only in effect when CONFIG_IOB_THROTTLE > 0 and there's high reception load. In that case the system should keep operating and not deadlock on a sendto() call on a blocking socket without timeout.

Testing

I ran iperf -s -B 10.0.1.2 -u & in the simulator target with the other end iperf -c -b 100M ... on the host machine while periodically executing cat /proc/iobinfo to check the state of the IOB MM. The sim target is compiled with CONFIG_IOB_THROTTLE=128

When compiled from master, I get (worst-case):

nsh> cat /proc/iobinfo   3.01-   6.02 sec   29767500 Bytes   79.12 Mbits/sec

    ntotal     nfree     nwait nthrottle
      1024         5         0         0
nsh> cat /proc/iobinfo
    ntotal     nfree     nwait nthrottle
      1024         5         0         0
nsh> cat /proc/iobinfo
    ntotal     nfree     nwait nthrottle
      1024         5         0         0
nsh> cat /proc/iobinfo
    ntotal     nfree     nwait nthrottle
      1024         5         0         0

Here you can see that nfree < CONFIG_IOB_THROTTLE(=128) even though only the receive path is exercised.

After the changes the worst-case is:

nsh> cat /proc/iobinfo
    ntotal     nfree     nwait nthrottle
      1024      1024         0       896
nsh> cat /proc/iobinfo   3.01-   6.02 sec   26019000 Bytes   69.15 Mbits/sec

    ntotal     nfree     nwait nthrottle
      1024       134         0         6
nsh> cat /proc/iobinfo
    ntotal     nfree     nwait nthrottle
      1024       134         0         6

So nfree > CONFIG_IOB_THROTTLE.

There are still several places in other drivers that don't respect the throttle parameter e.g. drivers/wireless/ieee802154/xbee/xbee.c:265

mennovf avatar Nov 20 '25 14:11 mennovf

This looks very promising! Thanks @mennovf. Sadly I won't be able to test this until next week :(

fdcavalcanti avatar Nov 20 '25 15:11 fdcavalcanti

In practical work scenarios, if we don't intentionally not read the packet, the IOB in readahead will always be consumed, and ioc_alloc_committed can ensure that the sending thread that was previously in a waiting state obtains the IOB. If the sending direction is unthrottled, during the TCP sending process, if the buffer in the write queue is full of IOB, the driver will be unable to receive any new packet (especially TCP-ACK), and the buffer in the write queue will never be released, causing the entire IP protocol stack to hang. This is a fatal problem. So for the overall robustness of the IP protocol stack, it is recommended to use throttled IOB for the sending direction and unthrottled IOB for the receiving direction. If you agree with my viewpoint, we should modify the description in kconfig. This is my understanding of this issue, which can be used as a reference for you.

zhhyu7 avatar Nov 20 '25 15:11 zhhyu7

Yes that's the dual problem of what I'm experiencing. I think the essential issue is that neither write nor read should be able to starve the other's memory, so compiling with either NET_SEND_BUFSIZE or RECV_SEND_BUFSIZE too small to contain a packet is just not correct. Both the receive and transmit end should always have enough memory to hold at least one packet to ensure progress. This issue is exacerbated with other net resources allocating from the same iob pool.

mennovf avatar Nov 20 '25 16:11 mennovf

Does this close/resolve #17299 or is it just one piece of the puzzle?

linguini1 avatar Nov 21 '25 01:11 linguini1

@mennovf suggestion: since you faced these issues with net iob throttle, maybe you could include a small section at Documentation/components/net/netdriver.rst talking about this feature, to give an overview for someone willing to use it. And the implications do disabling it and/or read-ahead. Maybe add some testing examples at https://nuttx.apache.org/docs/latest/guides/testingtcpip.html

This is just a suggestion, because NuttX documentation is very shy! So all opportunities we have to improve it we need to use. :-)

acassis avatar Nov 21 '25 12:11 acassis

Yes that's the dual problem of what I'm experiencing. I think the essential issue is that neither write nor read should be able to starve the other's memory, so compiling with either NET_SEND_BUFSIZE or RECV_SEND_BUFSIZE too small to contain a packet is just not correct. Both the receive and transmit end should always have enough memory to hold at least one packet to ensure progress. This issue is exacerbated with other net resources allocating from the same iob pool.

@mennovf with your change how to handle the case described by @zhhyu7 :

If the sending direction is unthrottled, during the TCP sending process, if the buffer in the write queue is full of IOB, the driver will be unable to receive any new packet (especially TCP-ACK), and the buffer in the write queue will never be released, causing the entire IP protocol stack to hang. This is a fatal problem.

xiaoxiang781216 avatar Nov 21 '25 13:11 xiaoxiang781216

Tried running the IPERF test and it works at first, but then it fails on iob_add_queue.c as shown below.

nsh> iperf -s &
iperf [9:100]
nsh>      IP: 192.168.0.127

 mode=tcp-server sip=192.168.0.127:5001,dip=0.0.0.0:5001, interval=3, time=0
accept: 192.168.0.125:39578

           Interval         Transfer         Bandwidth

   0.00-   3.01 sec    2874800 Bytes    7.64 Mbits/sec
   3.01-   6.02 sec    1865880 Bytes    4.96 Mbits/sec
   6.02-   9.03 sec    2638220 Bytes    7.01 Mbits/sec
   9.03-  12.04 sec    2486380 Bytes    6.61 Mbits/sec
dump_assert_info: Current Version: NuttX  10.4.0 a0dc172c62 Nov 25 2025 10:34:44 risc-v
dump_assert_info: Assertion failed iobq->qh_tail: at file: iob/iob_add_queue.c:73 task: wifi process: Kernel 0x4080924a

edit: it is also failing on master. I think it is still starving the IOBs, but I'm not sure.

fdcavalcanti avatar Nov 25 '25 13:11 fdcavalcanti

@mennovf with your change how to handle the case described by @zhhyu7 :

If the sending direction is unthrottled, during the TCP sending process, if the buffer in the write queue is full of IOB, the driver will be unable to receive any new packet (especially TCP-ACK), and the buffer in the write queue will never be released, causing the entire IP protocol stack to hang. This is a fatal problem.

Yes, the TCP sending will be blocked. Note that this is already an issue if _IOB_THROTTLE=0, regardless whether sending or receiving is marked "throttled". There is a more fundamental issue with the current networking stack: ideally each socket's send & receive end should allocate some minimum amount of memory on creation that's used to ensure forward progress.

@fdcavalcanti I can't reproduce it. What relevant options are you using?

I did seemingly run into another bug using iperf where the socket doesn't seem to get cleaned up properly on ctr-c.

mennovf avatar Nov 27 '25 16:11 mennovf

@mennovf with your change how to handle the case described by @zhhyu7 : If the sending direction is unthrottled, during the TCP sending process, if the buffer in the write queue is full of IOB, the driver will be unable to receive any new packet (especially TCP-ACK), and the buffer in the write queue will never be released, causing the entire IP protocol stack to hang. This is a fatal problem.

Yes, the TCP sending will be blocked. Note that this is already an issue if _IOB_THROTTLE=0, regardless whether sending or receiving is marked "throttled". As I said, this is a more fundamental issue with the current networking stack. Ideally each socket's send & receive end should allocate some minimum amount of memory on creation that's used to ensure forward progress.

@fdcavalcanti I can't reproduce it. What relevant options are you using?

I did seemingly run into another bug using iperf where the socket doesn't seem to get cleaned up properly on ctr-c.

I use the default defconfig on esp32c6-devkits:wifi. Some relevant options are:

CONFIG_IOB_BUFSIZE=128
CONFIG_IOB_NBUFFERS=160
CONFIG_IOB_THROTTLE=24
CONFIG_NETUTILS_IPERF=y
CONFIG_NET_BROADCAST=y
CONFIG_NET_ETH_PKTSIZE=1514
CONFIG_NET_ICMP_SOCKET=y
CONFIG_NET_TCP=y
CONFIG_NET_TCP_DELAYED_ACK=y
CONFIG_NET_TCP_KEEPALIVE=y
CONFIG_NET_TCP_WRITE_BUFFERS=y
CONFIG_NET_UDP=y

fdcavalcanti avatar Nov 28 '25 11:11 fdcavalcanti

I've seen this IOB deadlock issue too. I've been working around it by simply increasing the number of buffers available, but that isn't a good solution. Are you guys planning to move this pull request ahead, or are you parking it for now?

sastel avatar Dec 17 '25 16:12 sastel

I think the THROTTLE option is a dead-end due to the TCP remarks. IMO the memory management needs an overhaul. In the meantime I can resolve my issue by fixing RECV/SEND_BUFSIZE in a fork and correctly configuring the system with these parameters.

mennovf avatar Dec 17 '25 21:12 mennovf

I think the THROTTLE option is a dead-end due to the TCP remarks. IMO the memory management needs an overhaul. In the meantime I can resolve my issue by fixing RECV/SEND_BUFSIZE in a fork and correctly configuring the system with these parameters.

Can you show an example of this approach?

fdcavalcanti avatar Dec 18 '25 13:12 fdcavalcanti