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: Implement Keep-alive support #66316
net: tcp: Implement Keep-alive support #66316
Conversation
390279e
to
b2dfe8c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I noticed one simple optimization possibility in get/set option.
When a TCP connection is established, if there is no data exchange between the two parties within the set time, the side that enables TCP Keep-alive will send a TCP probe packet with the same sequence number as the previous TCP packet. This TCP probe packet is an empty ACK packet (the specification recommends that it should not contain any data, but can also contain 1 nonsense byte, such as 0x00.). If there is no response from the other side after several consecutive probe packets are sent, it is determined that the tcp connection has failed, and the connection is closed. The keep-alive default parameters are aligned with Linux defaults. Signed-off-by: Horse Ma <mawei@coltsmart.com> Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
The number of Kconfig options for the TCP stack grew considerably, therefore it makes sense to move them to a separate file not to bloat the Kconfig file with generic networking options. Take this opportunity to reorder TCP options, so that protocol parameters (timings/buffer sizes) are not mixed up with optional protocol features (fast retransmit/congestion avoidance etc.). Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
Verify that TCP keep alive options can be set properly and that TCP connections time out correctly when keep-alive probes fail to get replies. Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
b2dfe8c
to
7e38282
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR implements TCP keep-alive support. The PR is based on #46716, however the code has been refactored to address unresolved review feedback and to reduce ifdef slicing of the TCP code.
Additionally, I've added a couple of tests to verify that keep-alive works as expected. I've kept it to a bare minimum though, as testing keep-alive is time consuming (the timeouts are counted in seconds).
Finally, I've decided to create a separte Kconfig page for TCP-related configs. Over time, the number of TCP Kconfig entries increased a lot so it makes sense to group them in a separate page, instead of bloating the generic config space.
FYI @maweicoltsmart. I've kept your signoff on the original commit, to give you credit for the work you've done.
Resolves #34044