Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v3.7] TCP Delay ACK in case no PSH flag is present causing extremly low download throughput for some clients #76471

Closed
fengming-ye opened this issue Jul 30, 2024 · 3 comments
Assignees
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: high High impact/importance bug
Milestone

Comments

@fengming-ye
Copy link
Contributor

fengming-ye commented Jul 30, 2024

Caused by commit 5442e47
Delay an ACK in case no PSH flag is present in the data packet

This improves TCP download throughput in some cases or for some clients.
But we detect extremly low TCP download throughput 6Mbps for some other clients,
which barely send us PSH frame.
In this case we do not send back ACK frame either on tcp_data_received(net process) or on sock_recvmsg_vmeth(after application process).
Thus ACK can only be triggered by 100ms timer, causing low throughput.

Previous code logic of tcp_data_received and sock_recvmsg_vmeth seems to reach balance of ACK.
When tcp_data_received finds receive window short, it will delay ACK and let application reads socket buffer.
And when sock_recvmsg_vmeth reads buffer, the receive window increases. And when it increases from short window to long window, it will send ACK.

But with PSH logic, when tcp_data_received delay ACK if PSH not present, receive window will quickly reduce to short window.
After that even with PSH flag present, we still delay ACK. But sock_recvmsg_vmeth in zsock_recv_stream->net_context_update_recv_wnd->tcp_update_recv_wnd, short_win_before and short_win_after are both true. So application will not send ACK either.

Above all this PSH commit is a nice improvement for throughput for some clients.
But we need to figure out fix on this compatibility issue.

@fengming-ye fengming-ye added the RFC Request For Comments: want input from the community label Jul 30, 2024
Copy link

Hi @fengming-ye! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

@jukkar
Copy link
Member

jukkar commented Jul 30, 2024

The #76418 should improve the throughput, please try that.

@awojasinski awojasinski removed their assignment Jul 30, 2024
@dleach02 dleach02 added the priority: high High impact/importance bug label Jul 31, 2024
@dleach02 dleach02 added this to the v3.7.1 milestone Jul 31, 2024
@dleach02 dleach02 added bug The issue is a bug, or the PR is fixing a bug and removed RFC Request For Comments: want input from the community labels Jul 31, 2024
@dleach02 dleach02 removed this from RFC Backlog Jul 31, 2024
@carlescufi carlescufi changed the title TCP Delay ACK in case no PSH flag is present causing extremly low download throughput for some clients [v3.7] TCP Delay ACK in case no PSH flag is present causing extremly low download throughput for some clients Aug 7, 2024
@carlescufi
Copy link
Member

Backport is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: high High impact/importance bug
Projects
None yet
Development

No branches or pull requests

6 participants