criu icon indicating copy to clipboard operation
criu copied to clipboard

sk-tcp: Improve TCP socket options handling

Open juntongdeng opened this issue 1 year ago • 7 comments

This pull request improves the dumping and restoring of TCP socket options in CRIU.

Currently some of the TCP socket option information is stored in the TcpStreamEntry, but the information in the TcpStreamEntry is only restored after the TCP socket has established connection, which results in these TCP socket options not being restored for unconnected TCP sockets.

Move the TCP socket options from TcpStreamEntry to TcpOptsEntry and add dump_tcp_opts() and restore_tcp_opts() for TCP socket options dump and restore.

Currently some TCP socket option information is stored in SkOptsEntry, which is a little confusing. SkOptsEntry should only contain socket options that are common to all sockets.

Move the TCP-specific socket options from SkOptsEntry to TcpOptsEntry.

Currently there are no socket option test cases for TCP_CORK and TCP_NODELAY, this commit adds related test cases.

The socket option test cases for TCP_KEEPCNT, TCP_KEEPIDLE, and TCP_KEEPINTVL already exist in socket-tcp_keepalive.c, so they are not included in this pull request.

juntongdeng avatar Apr 02 '24 09:04 juntongdeng

Codecov Report

Attention: Patch coverage is 92.30769% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 70.58%. Comparing base (00d7cdc) to head (9076b33).

:exclamation: Current head 9076b33 differs from pull request most recent head 859800b. Consider uploading reports for the commit 859800b to get more accurate results

Files Patch % Lines
criu/sk-inet.c 72.72% 3 Missing :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##           criu-dev    #2380      +/-   ##
============================================
- Coverage     70.73%   70.58%   -0.15%     
============================================
  Files           136      135       -1     
  Lines         32940    33462     +522     
============================================
+ Hits          23299    23620     +321     
- Misses         9641     9842     +201     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Apr 06 '24 10:04 codecov-commenter

I see that code linter fails, but I don't seem to find the EOL whitespace problem in sk-inet.c?

juntongdeng avatar Apr 06 '24 10:04 juntongdeng

It seems that the CRIU Image Streamer Test is stuck in the zdtm/transition/socket_loop00 test case.

But the zdtm/transition/socket_loop00 test case can end normally and pass on my computer.

juntongdeng avatar Apr 06 '24 18:04 juntongdeng

Don't worry. The image streamer tests are known to timeout sometimes.

adrianreber avatar Apr 06 '24 19:04 adrianreber

We continue to support restoring TCP socket options from TcpStreamEntry and SkOptsEntry, but dumping TCP socket options to TcpOptsEntry in new versions of CRIU.

@juntongdeng Could you please add a comment in the code before restoring TcpStreamEntry and SkOptsEntry that this is done for backward comparability, and newer versions of CRIU use TcpOptsEntry?

rst0git avatar Apr 16 '24 10:04 rst0git

@juntongdeng Could you please add a comment in the code before restoring TcpStreamEntry and SkOptsEntry that this is done for backward comparability, and newer versions of CRIU use TcpOptsEntry?

Yes, will do

juntongdeng avatar Apr 16 '24 10:04 juntongdeng

A friendly reminder that this PR had no activity for 30 days.

github-actions[bot] avatar May 17 '24 00:05 github-actions[bot]

We continue to support restoring TCP socket options from TcpStreamEntry and SkOptsEntry, but dumping TCP socket options to TcpOptsEntry in new versions of CRIU.

@juntongdeng Could you please add a comment in the code before restoring TcpStreamEntry and SkOptsEntry that this is done for backward comparability, and newer versions of CRIU use TcpOptsEntry?

fixed

juntongdeng avatar May 19 '24 12:05 juntongdeng

How to remove "stale-pr" label?

juntongdeng avatar May 19 '24 12:05 juntongdeng

Merged. Thanks a lot.

avagin avatar May 21 '24 06:05 avagin