smoltcp-rs / smoltcp

a smol tcp/ip stack
BSD Zero Clause License
3.81k stars 432 forks source link

fix: internal sACK flag not set correctly for client socket #995

Closed XOR-op closed 2 months ago

XOR-op commented 2 months ago

Bug Description

Selective ACK is used for TCP retransmission. In the current implementation, sACK option will be enabled by default for a client TCP socket. However, the remote_has_sack is not correctly set when the server sends a SYN-ACK packet back. Since remote_has_sack is false by default, client TCP sockets will not send sACK when packet loss happens.

However, when the sACK-permitted option is set in the initial SYN packet, the server will (typically) only expect sACK for fast retransmission. Since no sACK is received, the server (as a sender) will wait until retransmission timer expires. This will trigger congestion control, and users will observe an inconsistent download speed even if the network link is relatively stable.

Solution

Set remote_has_sack correctly in SynSent state. Listen state is not affected by this bug.

Dirbaio commented 2 months ago

can you add a test?

XOR-op commented 2 months ago

@Dirbaio Corresponding test has been added.