sk-inet: Add support for checkpoint/restore of ICMP sockets #2558
Conversation
|
@ss141309 Thank you for opening this pull request! Would you be able to add a ZDTM test for this functionality? Example: |
|
@rst0git oops, it looks like I forgot to add an IP6 version of the test, do I need to create it? |
It would be good to have test for this. CRIU is used in some production environments where only IPv6 addresses are being used. |
|
As far as I remember, ICMP sockets can have attached filters and we need to dump them. Pls take a look at c2cbcaf, maybe some code can be reused. |
|
it seems that the tests are failing because of the GIDs being set in the ping_group_range variable. What should I set them to in the |
The test gid is 58467: Line 507 in 7c66617
Line 444 in 7c66617 I think "58467 58468" is the right range in this case. |
ICMP filters are only attached when using SOCK_RAW, since unprivileged ICMP sockets only accept ICMP_ECHO and ICMP_ECHOREPLY type messages |
ss141309
force-pushed
the
criu-dev
branch
2 times, most recently
from
6f97c64 to
9c54c86
Compare
|
Overall, it looks good to me. We need to move C/R of the sysctl to the proper place and resort patches. I will do all of that this week. Thanks for the contribution. |
|
@ss141309 Would you be able update the pull request to apply the fixup changes into previous commits? |
@rst0git I did the changes, is it now alright? |
|
@ss141309 Would you be able to apply the change from |
We need to integrate it into dump_netns_conf/restore_netns_conf, probably taking as an example ebe3b52353c This value belongs to namespace, not to socket. |
Should I make a new commit or edit the existing one and force push the changes? |
|
@ss141309 I did proper handling of ping_group_range c/r here #2565, you can rebase on top of it when/if it is merged. Machinery of sysctls in CRIU is a bit too complex, I must admit. And so I helped you a bit here, as you can see there is a lot of code to do one more sysctl in the directory which is not yet handled. |
|
A friendly reminder that this PR had no activity for 30 days. |
|
Hello everyone, are there any issues with this PR? |
avagin
added
no-auto-close
Having CTL_FLAGS_IPC_EACCES_SKIP == (CTL_FLAGS_OPTIONAL | CTL_FLAGS_READ_EIO_SKIP) is probably not what we want. So let's make it a real distinct flag. Fixes: 840735a ("ipc_sysctl: Prioritize restoring IPC variables using non usernsd approach") Signed-off-by: Pavel Tikhomirov <[email protected]>
Fixes: f38e588 ("net/sysctl: c/r ipv4/ping_group_range value") Signed-off-by: Pavel Tikhomirov <[email protected]>
Yes, there were issues: PR didn't pass its own tests =) (in host and userns flavors) I did a small rework for ping_group_range c/r: #2679 to make it actually restore in uns flavor. Also I updated patches 2 and 3 of this pr to fix test failures. (note: we can either merge only this PR, or merge #2679 first) Let's see if it will pass all tests now, I hope it would. |
avagin
force-pushed
the
criu-dev
branch
2 times, most recently
from
2af0179 to
8c76625
Compare
avagin
force-pushed
the
criu-dev
branch
3 times, most recently
from
8bd8e2d to
db5e76b
Compare
We have ability to skip sysctl if there is no value, but we still give n requests to sysctl_op, that is not correct and probably can segfault on nullptr access. Fix it by adding ri to count non skipped requests. To be on the safe side, let's add a check that ri == n on read, as we should not do any skips there. While on it lets fix bad error message prefix: s/unix/ipv4/. Remove excess has_iarg set, and add sarg reset to NULL for the case sysctl_op skipped it. Signed-off-by: Andrei Vagin <[email protected]> Signed-off-by: Pavel Tikhomirov <[email protected]>
We dump sysctls from criu user namespace, but restore from restored user namespace. So group id values should be mapped to the restored user namespace gid space to restore correctly. Signed-off-by: Andrei Vagin <[email protected]> Signed-off-by: Pavel Tikhomirov <[email protected]>
Fixes a clang compile-time error: "argument unused during compilation: '-c'". Signed-off-by: Andrei Vagin <[email protected]>
net/unix/max_dgram_qlen can't be tuned from non-root userns before:
v5.17-rc1~170^2~215 ("net: Enable max_dgram_qlen unix sysctl to be
configurable by non-init user namespaces")
Signed-off-by: Andrei Vagin <[email protected]>
Currently there is no option to checkpoint/restore programs that use ICMP sockets, such as `ping`. This patch adds support for the same. Fixes checkpoint-restore#2557 Signed-off-by: समीर सिंह Sameer Singh <[email protected]>
Add ZDTM static tests for IP4/ICMP and IP6/ICMP socket feature. Signed-off-by: समीर सिंह Sameer Singh <[email protected]> Signed-off-by: Andrei Vagin <[email protected]>
Currently there is no option to checkpoint/restore programs that use ICMP sockets, such as
ping. This patch adds support for the same.Fixes #2557