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

net/tcp(unbuffered): removed excessive overwrites of conn->sndseq #5102

Merged
merged 1 commit into from
Dec 29, 2021

Conversation

a-lunev
Copy link
Contributor

@a-lunev a-lunev commented Dec 28, 2021

Summary

conn->sndseq was updated in multiple places that was unreasonable and complicated.

Impact

TCP

Testing

(conn->sndseq was updated in multiple places that was unreasonable and complicated).
@xiaoxiang781216 xiaoxiang781216 merged commit 2b60468 into apache:master Dec 29, 2021
@a-lunev a-lunev deleted the sndseq_control_optimization branch December 29, 2021 13:05
@masayuki2009
Copy link
Contributor

@a-lunev

I noticed this PR has a side effect.
For example, if I try the wget command with spresnse:rndis + CONFIG_NET_TCP_WRITE_BUFFERS=n, it failed.
Actually, it works if I revert this PR.

Please see the wireshark logs.

spresense-rndis-wget-logs-20220102.zip

@a-lunev
Copy link
Contributor Author

a-lunev commented Jan 2, 2022

@a-lunev

I noticed this PR has a side effect. For example, if I try the wget command with spresnse:rndis + CONFIG_NET_TCP_WRITE_BUFFERS=n, it failed. Actually, it works if I revert this PR.

Please see the wireshark logs.

spresense-rndis-wget-logs-20220102.zip

Thank you for letting me know. I was able to reproduce the issue based on ftpc command. As I can see, tcp_sendfile is affected in unbuffered mode. I'm investigating the issue.

@a-lunev
Copy link
Contributor Author

a-lunev commented Jan 2, 2022

@masayuki2009 could you please review and test #5138 for the found issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants