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: remember the name of the lock chain (nftables) #2550

Open
wants to merge 1 commit into
base: criu-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions criu/image.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ bool img_common_magic = true;
TaskKobjIdsEntry *root_ids;
u32 root_cg_set;
Lsmtype image_lsm;
uint64_t dump_criu_run_id;

struct inventory_plugin {
struct list_head node;
Expand Down Expand Up @@ -120,6 +121,15 @@ int check_img_inventory(bool restore)
goto out_err;
}
}

/**
* This contains the criu_run_id during dumping of the process.
* For things like removing network locking (nftables) this
* information is needed to identify the name of the network
* locking table.
*/
dump_criu_run_id = he->dump_criu_run_id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (he->has_dump_criu_run_id) {
	dump_criu_run_id = he->dump_criu_run_id;
}

pr_info("Dump CRIU run id = %#" PRIx64 "\n", dump_criu_run_id);
}

ret = 0;
Expand Down Expand Up @@ -367,6 +377,15 @@ int prepare_inventory(InventoryEntry *he)
he->has_network_lock_method = true;
he->network_lock_method = opts.network_lock_method;

/**
* This contains the criu_run_id during dumping of the process.
* For things like removing network locking (nftables) this
* information is needed to identify the name of the network
* locking table.
*/
he->has_dump_criu_run_id = true;
he->dump_criu_run_id = criu_run_id;

return 0;
}

Expand Down
2 changes: 1 addition & 1 deletion criu/include/netfilter.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ extern void preload_netfilter_modules(void);

extern int nftables_init_connection_lock(void);
extern int nftables_lock_connection(struct inet_sk_desc *);
extern int nftables_get_table(char *table, int n);
extern int nftables_get_table(char *table, int n, uint64_t id);

#if defined(CONFIG_HAS_NFTABLES_LIB_API_0)
#define NFT_RUN_CMD(nft, cmd) nft_run_cmd_from_buffer(nft, cmd, strlen(cmd))
Expand Down
6 changes: 6 additions & 0 deletions criu/include/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,12 @@ extern int run_command(char *buf, size_t buf_size, int (*child_fn)(void *), void
* generate resource ID-s to avoid conflicts with other CRIU processes.
*/
extern uint64_t criu_run_id;
/**
* dump_criu_run_id is the criu_run_id which was used during process
* dumping. For network locking this ID is used to unlock the
* network (nftables).
*/
extern uint64_t dump_criu_run_id;
extern void util_init(void);

extern char *resolve_mountpoint(char *path);
Expand Down
44 changes: 42 additions & 2 deletions criu/net.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ static const char *unix_conf_entries[] = {
"max_dgram_qlen",
};

extern char nft_lock_table[32];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, we don't need this anymore.


/*
* MAX_CONF_UNIX_PATH = (sizeof(CONF_UNIX_FMT) - strlen("%s"))
* + MAX_CONF_UNIX_OPT_PATH
Expand Down Expand Up @@ -3073,14 +3075,23 @@ static inline int nftables_lock_network_internal(void)
int ret = 0;
char table[32];
char buf[128];
FILE *fp;

if (nftables_get_table(table, sizeof(table)))
if (nftables_get_table(table, sizeof(table), criu_run_id))
return -1;

nft = nft_ctx_new(NFT_CTX_DEFAULT);
if (!nft)
return -1;

fp = fdopen(log_get_fd(), "w");
if (!fp) {
pr_perror("fdopen() failed");
goto err3;
}
nft_ctx_set_output(nft, fp);
nft_ctx_set_error(nft, fp);

snprintf(buf, sizeof(buf), "create table %s", table);
if (NFT_RUN_CMD(nft, buf))
goto err2;
Expand All @@ -3107,6 +3118,9 @@ static inline int nftables_lock_network_internal(void)
snprintf(buf, sizeof(buf), "delete table %s", table);
NFT_RUN_CMD(nft, buf);
err2:
fflush(fp);
fclose(fp);
err3:
ret = -1;
pr_err("Locking network failed using nftables\n");
out:
Expand Down Expand Up @@ -3171,18 +3185,44 @@ static inline int nftables_network_unlock(void)
struct nft_ctx *nft;
char table[32];
char buf[128];
FILE *fp;
uint64_t locking_run_id;

/**
* Network unlocking can happen during restore, but also
* during checkpointing. It happens during checkpointing
* if the process is kept on running or if there was some
* error. In that case, checkpointing, dump_criu_run_id
* is not set and we have to use criu_run_id.
*/

if (nftables_get_table(table, sizeof(table)))
if (dump_criu_run_id == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to introduce a boolean parameter here to determine if we are on restore/dump codepath as dump_criu_run_id == 0 might be in two different cases: when we deal with an old image (without dump_criu_run_id field) or if we are on the restore codepath.

locking_run_id = criu_run_id;
else
locking_run_id = dump_criu_run_id;

if (nftables_get_table(table, sizeof(table), locking_run_id))
return -1;

nft = nft_ctx_new(NFT_CTX_DEFAULT);
if (!nft)
return -1;

fp = fdopen(log_get_fd(), "w");
if (!fp) {
pr_perror("fdopen() failed");
nft_ctx_free(nft);
return -1;
}
nft_ctx_set_output(nft, fp);
nft_ctx_set_error(nft, fp);

snprintf(buf, sizeof(buf), "delete table %s", table);
if (NFT_RUN_CMD(nft, buf))
ret = -1;

fflush(fp);
fclose(fp);
nft_ctx_free(nft);
return ret;
#else
Expand Down
8 changes: 4 additions & 4 deletions criu/netfilter.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ int nftables_init_connection_lock(void)
int ret = 0;
char table[32];

if (nftables_get_table(table, sizeof(table)))
if (nftables_get_table(table, sizeof(table), criu_run_id))
return -1;

nft = nft_ctx_new(NFT_CTX_DEFAULT);
Expand Down Expand Up @@ -243,7 +243,7 @@ static int nftables_lock_connection_raw(int family, u32 *src_addr, u16 src_port,
char sip[INET_ADDR_LEN], dip[INET_ADDR_LEN];
char table[32];

if (nftables_get_table(table, sizeof(table)))
if (nftables_get_table(table, sizeof(table), criu_run_id))
return -1;

if (family == AF_INET6 && ipv6_addr_mapped(dst_addr)) {
Expand Down Expand Up @@ -297,9 +297,9 @@ int nftables_lock_connection(struct inet_sk_desc *sk)
return ret;
}

int nftables_get_table(char *table, int n)
int nftables_get_table(char *table, int n, uint64_t id)
{
if (snprintf(table, n, "inet CRIU-%d", root_item->pid->real) < 0) {
if (snprintf(table, n, "inet CRIU-%" PRIx64, id) < 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

	/*
	 * Keep compatibility with images
	 * without he->dump_criu_run_id field.
	 */
	if (!id) {
		if (!(root_ns_mask & CLONE_NEWPID)) {
			id = root_item->pid->real;
		} else {
			pr_err("Cannot generate CRIU's nftables table name because of issue #2550\n");
			return -1;
		}
		
	}

	if (snprintf(table, n, "inet CRIU-%" PRIx64, id) < 0) {

What do you think about this?

pr_err("Cannot generate CRIU's nftables table name\n");
return -1;
}
Expand Down
4 changes: 4 additions & 0 deletions images/inventory.proto
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,8 @@ message inventory_entry {
optional bool tcp_close = 10;
optional uint32 network_lock_method = 11;
optional plugins_entry plugins_entry = 12;
// Remember the criu_run_id when CRIU dumped the process.
// This is currently used to delete the correct nftables
// network locking rule.
optional uint64 dump_criu_run_id = 13;
}
Loading