search

checkpoint-restore / criu

Checkpoint/Restore tool
criu.org
Other
2.93k stars 585 forks source link

Allow skipping iptables/nftables invocation. #2197

Closed osctobe closed 1 year ago

osctobe commented 1 year ago

Make it possible to skip network lock to enable uses that break connections anyway to work without iptables/nftables being present. This is used in Google's environment where we drop all connections and reconnect after migration.

adrianreber commented 1 year ago

I like this change. That would be also useful for containers I think. It needs additional changes in cr-service.c I think and I also think that the name NETWORK_LOCK_DUMMY is not the best choice. Maybe a name that includes NO_LOCK or so would be better.

osctobe commented 1 year ago

I like this change. That would be also useful for containers I think. It needs additional changes in cr-service.c I think

Done.

and I also think that the name NETWORK_LOCK_DUMMY is not the best choice. Maybe a name that includes NO_LOCK or so would be better.

Can we have more votes on the name? I think NETWORK_LOCK_DUMMY reads better than NETWORK_LOCK_NO_LOCK, unless we're willing to have NETWORK_NO_LOCK or similar.

osctobe commented 1 year ago

Would NONE instead of DUMMY be better? (Keeping it a single-word.)

rst0git commented 1 year ago

Would NONE instead of DUMMY be better? (Keeping it a single-word.)

NETWORK_LOCK_NONE looks better. It is similar to the --manage-cgroups none option.

rst0git commented 1 year ago

Make it possible to skip network lock to enable uses that break connections anyway to work without iptables/nftables being present.

How does this option relate to --tcp-close? Should these two options be used together?

osctobe commented 1 year ago

Make it possible to skip network lock to enable uses that break connections anyway to work without iptables/nftables being present.

How does this option relate to --tcp-close? Should these two options be used together?

It makes --tcp-close cheaper. Added a hint to the doc.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 14.28% and project coverage change: -0.02 :warning:

Comparison is base (9301aba) 70.71% compared to head (5e265a5) 70.69%.

:exclamation: Current head 5e265a5 differs from pull request most recent head da85519. Consider uploading reports for the commit da85519 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## criu-dev #2197 +/- ## ============================================ - Coverage 70.71% 70.69% -0.02% ============================================ Files 133 133 Lines 33238 33251 +13 ============================================ + Hits 23504 23508 +4 - Misses 9734 9743 +9 ``` | [Impacted Files](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2197?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [criu/config.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2197?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y3JpdS9jb25maWcuYw==) | `81.78% <0.00%> (-0.32%)` | :arrow_down: | | [criu/cr-service.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2197?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y3JpdS9jci1zZXJ2aWNlLmM=) | `63.53% <0.00%> (-0.27%)` | :arrow_down: | | [criu/sk-tcp.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2197?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y3JpdS9zay10Y3AuYw==) | `75.93% <0.00%> (-1.66%)` | :arrow_down: | | [lib/c/criu.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2197?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-bGliL2MvY3JpdS5j) | `37.08% <0.00%> (ø)` | | | [criu/net.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2197?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y3JpdS9uZXQuYw==) | `76.68% <100.00%> (+0.02%)` | :arrow_up: | ... and [7 files with indirect coverage changes](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2197/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.