gvanem / Watt-32

Watt-32 TCP/IP library and samples.
https://www.watt-32.net/
18 stars 8 forks source link

Many empty ACKs transmitted #100

Closed jwt27 closed 7 months ago

jwt27 commented 7 months ago

On Wireshark I see Watt-32 transmits many duplicate ACKs. About every 25ms, it sends out 30-100 of these in a row. In transferring 10MB of data, these account for about 45% of all packets sent.

image

From googling I gather that this may be used to request a retransmit. However it begins sending these within nanoseconds after the last data packet, and even when the remote then sends an ACK, it continues to send more of these for a while.

Transfer speed is not great, ~1600 KB/s. I suspect all these empty packets are hurting performance.

jwt27 commented 7 months ago

From wattcp.dbg it seems Watt32 sees a different order of events.

It sends out many packets containing data, and periodically processes all received ACKs. But for every packet received, it sends an empty one back for no reason.

So this looks like:

Transmitted: len 1500
Transmitted: len 1500
Transmitted: len 1500
Transmitted: len 1500
Transmitted: len 1500
Transmitted: len 1500
[ etc ... ]
Received: len 40
Transmitted: len 40
Received: len 40
Transmitted: len 40
Received: len 40
Transmitted: len 40
[ etc ... ]
gvanem commented 7 months ago

Probably some timer going crazy again. But the timers can be tuned with (in wattcp.cfg). This is what I have:

tcp.timer.rto_add   = 10
tcp.timer.rto_base  = 5
tcp.timer.rto_scale = 64
tcp.timer.retran_to = 10
tcp.timer.reset_to  = 0
tcp.timer.max_vjsa = 2000
tcp.timer.max_vjsd = 1000
tcp.recv_win  = 32768

Not sure which is causing your issue. But try to experiment with them.

jwt27 commented 7 months ago

Thanks, I tried those numbers. Transfer speed maybe improves a little bit, but the empty acks remain.

These packets are sent from here: https://github.com/gvanem/Watt-32/blob/d9c197e65c5cfc07772883e9d3f6dd489716115d/src/tcp_fsm.c#L404-L426

Related debug snippet:

Received: pctcp.c (782), time 00:00:15.159
IP4:   10.0.0.10 -> 10.0.0.2
       IHL 20, ver 4, tos 0, len 40, ttl 64, prot TCP, chksum 18CC (ok)
       id 5AAC, ofs 0, DF
TCP:   10.0.0.10 (56574) -> 10.0.0.2 (80), sock 00596FC0
       flags ACK, win 65535, chksum 1A26 (ok), urg 0
                  SEQ  443692597,  ACK  179390716
       ESTAB    (dSEQ          0, dACK       1460), MS 0/0, Unhappy
       RCV.NXT 443692597, SND.NXT 179389256, SND.UNA 58398
       KC 0, vjSA 62, vjSD 56, CW 30, WW 25, RTO 45, RTT-diff 145
tcp_process_data (1032): len 0, ldiff 0
tcp_estab_state (418): FastACK: ldiff 1460, UNA 56938, MS-right 0

Transmitted: tcp_fsm.c (425), time 00:00:15.159
IP4:   10.0.0.2 -> 10.0.0.10
       IHL 20, ver 4, tos 0, len 40, ttl 254, prot TCP, chksum 15A5 (ok)
       id 03AF, ofs 0
TCP:   10.0.0.2 (80) -> 10.0.0.10 (56574), sock 00596FC0
       flags ACK PSH, win 16383, chksum FBB3 (ok), urg 0
                  SEQ  179447654,  ACK  443692597
       ESTAB    (dSEQ       1458, dACK          0), MS 0/0, Unhappy
       RCV.NXT 443692597, SND.NXT 179390716, SND.UNA 56938
       KC 0, vjSA 55, vjSD 49, CW 30, WW 26, RTO 41, RTT-diff 145

I suppose one could work around it with a huge tx buffer, then it would have something to send along with the ACK (but SO_SNDBUF code is disabled, for some reason).

On a somewhat related note, maybe there could be some upper limit on the number of packets produced by _tcp_send()? That way we end up in tcp_tick() more often. But I see this has to do with the congestion algorithm, which I don't fully understand. Possible concern is that non-blocking sockets are... not very "non-blocking", if it has to push 40+ segments through the slow packet driver on every _tcp_send().

jwt27 commented 7 months ago

What does help, is setting sockopt TCP_NOPUSH. This encourages the remote to use delayed-ACK, so it has fewer ACKs to "reply" to. Transfer speed improves to ~2100 KB/s. So it may be worth looking into the PSH flag logic, I think it only makes sense to set this if the tx buffer is empty. But it's not a direct solution for this issue.

gvanem commented 7 months ago

Yes, like the comment above * We need a better criteria for doing Fast-ACK. says. I cannot remember if I wrote that. Surely do not understand much of it now.

jwt27 commented 7 months ago

I think (ldiff > 0 && s->tx_datalen > 0) is meant to trigger only if there is data remaining to be sent. But tx_datalen includes unacked data that was already sent.

So maybe it should be:

if ((ldiff > 0 && (s->tx_datalen - s->send_una) > 0) || len > 0)

?

gvanem commented 7 months ago

if ((ldiff > 0 && (s->tx_datalen - s->send_una) > 0) || len > 0)

Perhaps. I'm rather rusty on this. Try it to see if it helps or something else breaks.

jwt27 commented 7 months ago

Seems to solve the problem, and no mysterious new bugs so far.

It is hard to tell exactly what the original intention was. However, I found in an old version of Wattcp a comment that confirms my initial guess:

/* S. Lawson - added per below
 *   Eliminate the spurious ACK messages bug.
 *   For the window update, the length should be the
 *   data length only, so exclude the TCP header size
 *    -- Joe <jdhagen@itis.com> (this helped alot -gv)
 */
            len -= tcp_GetDataOffset(tp) << 2;

// S. Lawson - don't send on ACK unless there's more to send
//          if ( diff > 0 || len > 0 ) {
            if ( (diff > 0 && s->datalen) || len > 0 ) {   // S. Lawson
                /* need to update window, but how urgent ??? */
// S. Lawson            if ( diff > 0 || (len > (s->mss >> 1))) {
                if (s->frag[0] || (diff > 0 && s->datalen) || (len > (s->mss >> 1))) { // S. Lawson
                    tcp_send( s, __LINE__ );
                } else
                    tcp_sendsoon( s );

            }

BTW, I see a few more instances where tx_datalen (unsent + unacked) is used, but where likely only "unsent" data was intended. Eg:

https://github.com/gvanem/Watt-32/blob/15273818345f49376e92ab215db75e41147a7575/src/pctcp.c#L1903-L1913

https://github.com/gvanem/Watt-32/blob/15273818345f49376e92ab215db75e41147a7575/src/pctcp.c#L3083-L3097