dump/restore: Support ipsets

Submitted by Valeriy Vdovin on Feb. 3, 2020, 4:27 p.m.

Details

Message ID 1580747223-935825-2-git-send-email-valeriy.vdovin@virtuozzo.com
State New
Series "dump/restore: Support ipsets"
Headers show

Commit Message

Valeriy Vdovin Feb. 3, 2020, 4:27 p.m.
https://jira.sw.ru/browse/PSBM-100083

Added ipset dump/restore functionality. At dump operation it calls
'ipset save' and stores result into raw text image. At restore
it restores ipset by calling 'ipset restore'. This is done prior
to restoring iptables.

Signed-off-by: Valeriy Vdovin <valeriy.vdovin@virtuozzo.com>
---
 criu/image-desc.c                 |  1 +
 criu/include/image-desc.h         |  1 +
 criu/include/magic.h              |  1 +
 criu/net.c                        | 51 +++++++++++++++++++++++++++++++++++
 test/zdtm/static/Makefile         |  1 +
 test/zdtm/static/netns-ipset.c    | 56 +++++++++++++++++++++++++++++++++++++++
 test/zdtm/static/netns-ipset.desc | 14 ++++++++++
 7 files changed, 125 insertions(+)
 create mode 100644 test/zdtm/static/netns-ipset.c
 create mode 100644 test/zdtm/static/netns-ipset.desc

Patch hide | download patch | download mbox

diff --git a/criu/image-desc.c b/criu/image-desc.c
index 04e827d..475d176 100644
--- a/criu/image-desc.c
+++ b/criu/image-desc.c
@@ -74,6 +74,7 @@  struct cr_fd_desc_tmpl imgset_template[CR_FD_MAX] = {
 	FD_ENTRY_F(ROUTE,	"route-%u", O_NOBUF),
 	FD_ENTRY_F(ROUTE6,	"route6-%u", O_NOBUF),
 	FD_ENTRY_F(RULE,	"rule-%u", O_NOBUF),
+	FD_ENTRY_F(IPSET,	"ipset-%u", O_NOBUF),
 	FD_ENTRY_F(IPTABLES,	"iptables-%u", O_NOBUF),
 	FD_ENTRY_F(IP6TABLES,	"ip6tables-%u", O_NOBUF),
 	FD_ENTRY_F(NFTABLES,	"nftables-%u", O_NOBUF),
diff --git a/criu/include/image-desc.h b/criu/include/image-desc.h
index 1015191..d875722 100644
--- a/criu/include/image-desc.h
+++ b/criu/include/image-desc.h
@@ -40,6 +40,7 @@  enum {
 	CR_FD_ROUTE,
 	CR_FD_ROUTE6,
 	CR_FD_RULE,
+	CR_FD_IPSET,
 	CR_FD_IPTABLES,
 	CR_FD_IP6TABLES,
 	CR_FD_NFTABLES,
diff --git a/criu/include/magic.h b/criu/include/magic.h
index 1a583f4..32421ed 100644
--- a/criu/include/magic.h
+++ b/criu/include/magic.h
@@ -101,6 +101,7 @@ 
 #define RULE_MAGIC		RAW_IMAGE_MAGIC
 #define TMPFS_IMG_MAGIC		RAW_IMAGE_MAGIC
 #define TMPFS_DEV_MAGIC		RAW_IMAGE_MAGIC
+#define IPSET_MAGIC		RAW_IMAGE_MAGIC
 #define IPTABLES_MAGIC		RAW_IMAGE_MAGIC
 #define IP6TABLES_MAGIC		RAW_IMAGE_MAGIC
 #define NFTABLES_MAGIC		RAW_IMAGE_MAGIC
diff --git a/criu/net.c b/criu/net.c
index 8ee560c..7f57509 100644
--- a/criu/net.c
+++ b/criu/net.c
@@ -1775,6 +1775,26 @@  static int restore_links()
 	return 0;
 }
 
+static int run_ipset_tool(char *arg1, char *arg2, char *arg3, int fdin, int fdout)
+{
+	char *cmd;
+	int ret;
+
+	pr_debug("\tRunning ipset %s %s %s \n", arg1, arg2, arg3 ? : "");
+
+	cmd = getenv("CR_IPSET_TOOL");
+	if (!cmd)
+		cmd = "ipset";
+
+	ret = cr_system(fdin, fdout, -1, cmd,
+				(char *[]) { cmd, arg1, arg2, arg3, NULL }, 0);
+	if (ret) {
+		pr_err("ipset tool failed on %s %s %s \n", arg1, arg2, arg3 ? : "");
+		return -1;
+	}
+
+	return 0;
+}
 
 static int run_ip_tool(char *arg1, char *arg2, char *arg3, char *arg4, int fdin, int fdout, unsigned flags)
 {
@@ -1891,6 +1911,16 @@  static inline int dump_rule(struct cr_imgset *fds)
 	return 0;
 }
 
+static inline int dump_ipset(struct cr_imgset *fds)
+{
+	int ret;
+	struct cr_img *img;
+	img = img_from_set(fds, CR_FD_IPSET);
+	ret = run_ipset_tool("save", 0, 0, -1, img_raw_fd(img));
+	close_image(img);
+	return ret;
+}
+
 static inline int dump_iptables(struct cr_imgset *fds)
 {
 	struct cr_img *img;
@@ -2129,6 +2159,23 @@  static int prepare_xtable_lock()
 	return 0;
 }
 
+static inline int restore_ipset(int pid)
+{
+	int ret;
+	struct cr_img *img;
+	img = open_image(CR_FD_IPSET, O_RSTR, pid);
+	if (img == NULL)
+		return -1;
+	if (empty_image(img)) {
+		ret = 0;
+		goto out;
+	}
+	ret = run_ipset_tool("restore", 0, 0, img_raw_fd(img), -1);
+out:
+	close_image(img);
+	return ret;
+}
+
 static inline int restore_iptables(int pid)
 {
 	int ret = -1;
@@ -2446,6 +2493,8 @@  int dump_net_ns(struct ns_id *ns)
 		if (!ret)
 			ret = dump_rule(fds);
 		if (!ret)
+			ret = dump_ipset(fds);
+		if (!ret)
 			ret = dump_iptables(fds);
 		if (!ret)
 			ret = dump_nftables(fds);
@@ -2541,6 +2590,8 @@  static int prepare_net_ns_second_stage(struct ns_id *ns)
 		if (!ret)
 			ret = restore_rule(nsid);
 		if (!ret)
+			ret = restore_ipset(nsid);
+		if (!ret)
 			ret = restore_iptables(nsid);
 		if (!ret)
 			ret = restore_nftables(nsid);
diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
index 28717b1..bdef4d0 100644
--- a/test/zdtm/static/Makefile
+++ b/test/zdtm/static/Makefile
@@ -143,6 +143,7 @@  TST_NOFILE	:=				\
 		poll				\
 		mountpoints			\
 		netns				\
+		netns-ipset			\
 		netns-dev			\
 		session01			\
 		session02			\
diff --git a/test/zdtm/static/netns-ipset.c b/test/zdtm/static/netns-ipset.c
new file mode 100644
index 0000000..301f0d9
--- /dev/null
+++ b/test/zdtm/static/netns-ipset.c
@@ -0,0 +1,56 @@ 
+#include <string.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "zdtmtst.h"
+
+const char *test_doc	= "Check that ipset are dumped and restored correctly";
+
+#define RUN_OR_ERR(cmd, failmsg) if (system(cmd)) { pr_perror(failmsg); return -1; }
+#define RUN_OR_FAIL(cmd, failmsg) if (system(cmd)) { fail(failmsg); return -1; }
+
+int main(int argc, char **argv)
+{
+	char dump_ipset_old[]    = "ipset save > /tmp/ipset.old";
+	char dump_ipset_new[]    = "ipset save > /tmp/ipset.new";
+	char dump_iptables_old[] = "iptables -L INPUT 1 > /tmp/iptables.old";
+	char dump_iptables_new[] = "iptables -L INPUT 1 > /tmp/iptables.new";
+	char cmp_ipset[]         = "diff /tmp/ipset.old /tmp/ipset.new";
+	char cmp_iptables[]      = "diff /tmp/iptables.old /tmp/iptables.new";
+	char rm_ipset_files[]    = "rm -fv /tmp/ipset.old /tmp/ipset.new";
+	char rm_iptables_files[] = "rm -fv /tmp/iptables.old /tmp/iptables.new";
+
+	test_init(argc, argv);
+
+	/* create ipset group and add some ip addresses to it */
+	RUN_OR_ERR("ipset create testgroup nethash", "Can't create test ipset");
+	RUN_OR_ERR("ipset add testgroup 127.0.0.1/8", "Can't add ip addresses to ipset group");
+
+	/* Use testgroup in iptables rule */
+	RUN_OR_ERR("iptables -I INPUT 1 -p tcp -m set --match-set testgroup src,dst -j ACCEPT",
+		"Failed to setup iptables rule with ipset group");
+
+	/* dump ipset and iptables states to text files */
+	RUN_OR_ERR(dump_iptables_old, "Can't save iptables rules.");
+	RUN_OR_ERR(dump_ipset_old   , "Can't save ipset list.");
+
+	test_daemon();
+	test_waitsig();
+
+	/* again dump ipset and iptables states to other text files */
+	RUN_OR_ERR(dump_iptables_new, "Can't dump restored iptables rules.");
+	RUN_OR_ERR(dump_ipset_new   , "Can't save restored ipset list to file.");
+
+	/* compare original and restored iptables rules */
+	RUN_OR_FAIL(cmp_iptables, "iptables rules differ");
+
+	/* compare original and restored ipset rules */
+	RUN_OR_FAIL(cmp_ipset, "ipset lists differ");
+
+	RUN_OR_ERR(rm_ipset_files, "Can't remove ipset files");
+	RUN_OR_ERR(rm_iptables_files, "Can't remove iptables files");
+
+	pass();
+	return 0;
+}
diff --git a/test/zdtm/static/netns-ipset.desc b/test/zdtm/static/netns-ipset.desc
new file mode 100644
index 0000000..d6eefa3
--- /dev/null
+++ b/test/zdtm/static/netns-ipset.desc
@@ -0,0 +1,14 @@ 
+{
+	'flavor': 'h ns uns',
+	'flags': 'suid',
+	'deps': [
+		'/usr/bin/rm',
+		'/usr/bin/sh',
+		'/usr/bin/diff',
+		'/usr/sbin/ipset',
+		'/usr/sbin/iptables',
+		'/lib64/libipset.so.13',
+		'/usr/lib64/xtables/libxt_set.so',
+                '/usr/lib64/xtables/libxt_standard.so|/lib/xtables/libxt_standard.so|/usr/lib/powerpc64le-linux-gnu/xtables/libxt_standard.so|/usr/lib/x86_64-linux-gnu/xtables/libxt_standard.so|/usr/lib/xtables/libxt_standard.so'
+		]
+}

Comments

Pavel Tikhomirov Feb. 11, 2020, 2:07 p.m.
note: cover-letter is only required for series with multiple patches

On 2/3/20 7:27 PM, Valeriy Vdovin wrote:
> https://jira.sw.ru/browse/PSBM-100083
> 
> Added ipset dump/restore functionality. At dump operation it calls
> 'ipset save' and stores result into raw text image. At restore
> it restores ipset by calling 'ipset restore'. This is done prior
> to restoring iptables.
> 
> Signed-off-by: Valeriy Vdovin <valeriy.vdovin@virtuozzo.com>
> ---vz7-u13
>   criu/image-desc.c                 |  1 +
>   criu/include/image-desc.h         |  1 +
>   criu/include/magic.h              |  1 +
>   criu/net.c                        | 51 +++++++++++++++++++++++++++++++++++

Please split c/r and zdtm parts to two different patches, that is a 
common style in criu patches.

>   test/zdtm/static/Makefile         |  1 +
>   test/zdtm/static/netns-ipset.c    | 56 +++++++++++++++++++++++++++++++++++++++
>   test/zdtm/static/netns-ipset.desc | 14 ++++++++++
>   7 files changed, 125 insertions(+)
>   create mode 100644 test/zdtm/static/netns-ipset.c
>   create mode 100644 test/zdtm/static/netns-ipset.desc
> 
> diff --git a/criu/image-desc.c b/criu/image-desc.c
> index 04e827d..475d176 100644
> --- a/criu/image-desc.c
> +++ b/criu/image-desc.c
> @@ -74,6 +74,7 @@ struct cr_fd_desc_tmpl imgset_template[CR_FD_MAX] = {
>   	FD_ENTRY_F(ROUTE,	"route-%u", O_NOBUF),
>   	FD_ENTRY_F(ROUTE6,	"route6-%u", O_NOBUF),
>   	FD_ENTRY_F(RULE,	"rule-%u", O_NOBUF),
> +	FD_ENTRY_F(IPSET,	"ipset-%u", O_NOBUF),
>   	FD_ENTRY_F(IPTABLES,	"iptables-%u", O_NOBUF),
>   	FD_ENTRY_F(IP6TABLES,	"ip6tables-%u", O_NOBUF),
>   	FD_ENTRY_F(NFTABLES,	"nftables-%u", O_NOBUF),
> diff --git a/criu/include/image-desc.h b/criu/include/image-desc.h
> index 1015191..d875722 100644
> --- a/criu/include/image-desc.h
> +++ b/criu/include/image-desc.h
> @@ -40,6 +40,7 @@ enum {
>   	CR_FD_ROUTE,
>   	CR_FD_ROUTE6,
>   	CR_FD_RULE,
> +	CR_FD_IPSET,
>   	CR_FD_IPTABLES,
>   	CR_FD_IP6TABLES,
>   	CR_FD_NFTABLES,
> diff --git a/criu/include/magic.h b/criu/include/magic.h
> index 1a583f4..32421ed 100644
> --- a/criu/include/magic.h
> +++ b/criu/include/magic.h
> @@ -101,6 +101,7 @@
>   #define RULE_MAGIC		RAW_IMAGE_MAGIC
>   #define TMPFS_IMG_MAGIC		RAW_IMAGE_MAGIC
>   #define TMPFS_DEV_MAGIC		RAW_IMAGE_MAGIC
> +#define IPSET_MAGIC		RAW_IMAGE_MAGIC
>   #define IPTABLES_MAGIC		RAW_IMAGE_MAGIC
>   #define IP6TABLES_MAGIC		RAW_IMAGE_MAGIC
>   #define NFTABLES_MAGIC		RAW_IMAGE_MAGIC
> diff --git a/criu/net.c b/criu/net.c
> index 8ee560c..7f57509 100644
> --- a/criu/net.c
> +++ b/criu/net.c
> @@ -1775,6 +1775,26 @@ static int restore_links()
>   	return 0;
>   }
>   
> +static int run_ipset_tool(char *arg1, char *arg2, char *arg3, int fdin, int fdout)

Do we actually need arg2 and arg3 in this function? Please remove.

> +{
> +	char *cmd;
> +	int ret;
> +
> +	pr_debug("\tRunning ipset %s %s %s \n", arg1, arg2, arg3 ? : "");
> +
> +	cmd = getenv("CR_IPSET_TOOL");
> +	if (!cmd)
> +		cmd = "ipset";
> +
> +	ret = cr_system(fdin, fdout, -1, cmd,
> +				(char *[]) { cmd, arg1, arg2, arg3, NULL }, 0);
> +	if (ret) {
> +		pr_err("ipset tool failed on %s %s %s \n", arg1, arg2, arg3 ? : "");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
>   
>   static int run_ip_tool(char *arg1, char *arg2, char *arg3, char *arg4, int fdin, int fdout, unsigned flags)
>   {
> @@ -1891,6 +1911,16 @@ static inline int dump_rule(struct cr_imgset *fds)
>   	return 0;
>   }
>   
> +static inline int dump_ipset(struct cr_imgset *fds)
> +{
> +	int ret;
> +	struct cr_img *img;
> +	img = img_from_set(fds, CR_FD_IPSET);
> +	ret = run_ipset_tool("save", 0, 0, -1, img_raw_fd(img));
> +	close_image(img);

Here we should not close image.

> +	return ret;
> +}
> +
>   static inline int dump_iptables(struct cr_imgset *fds)
>   {
>   	struct cr_img *img;
> @@ -2129,6 +2159,23 @@ static int prepare_xtable_lock()
>   	return 0;
>   }
>   
> +static inline int restore_ipset(int pid)
> +{
> +	int ret;
> +	struct cr_img *img;
> +	img = open_image(CR_FD_IPSET, O_RSTR, pid);
> +	if (img == NULL)
> +		return -1;
> +	if (empty_image(img)) {
> +		ret = 0;
> +		goto out;
> +	}
> +	ret = run_ipset_tool("restore", 0, 0, img_raw_fd(img), -1);
> +out:
> +	close_image(img);
> +	return ret;
> +}
> +
>   static inline int restore_iptables(int pid)
>   {
>   	int ret = -1;
> @@ -2446,6 +2493,8 @@ int dump_net_ns(struct ns_id *ns)
>   		if (!ret)
>   			ret = dump_rule(fds);
>   		if (!ret)
> +			ret = dump_ipset(fds);
> +		if (!ret)
>   			ret = dump_iptables(fds);
>   		if (!ret)
>   			ret = dump_nftables(fds);
> @@ -2541,6 +2590,8 @@ static int prepare_net_ns_second_stage(struct ns_id *ns)
>   		if (!ret)
>   			ret = restore_rule(nsid);
>   		if (!ret)
> +			ret = restore_ipset(nsid);
> +		if (!ret)
>   			ret = restore_iptables(nsid);
>   		if (!ret)
>   			ret = restore_nftables(nsid);
> diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
> index 28717b1..bdef4d0 100644
> --- a/test/zdtm/static/Makefile
> +++ b/test/zdtm/static/Makefile
> @@ -143,6 +143,7 @@ TST_NOFILE	:=				\
>   		poll				\
>   		mountpoints			\
>   		netns				\
> +		netns-ipset			\
>   		netns-dev			\
>   		session01			\
>   		session02			\
> diff --git a/test/zdtm/static/netns-ipset.c b/test/zdtm/static/netns-ipset.c
> new file mode 100644
> index 0000000..301f0d9
> --- /dev/null
> +++ b/test/zdtm/static/netns-ipset.c
> @@ -0,0 +1,56 @@
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "zdtmtst.h"
> +
> +const char *test_doc	= "Check that ipset are dumped and restored correctly";

Please add an author line:

const char *test_author = ""

> +
> +#define RUN_OR_ERR(cmd, failmsg) if (system(cmd)) { pr_perror(failmsg); return -1; }
> +#define RUN_OR_FAIL(cmd, failmsg) if (system(cmd)) { fail(failmsg); return -1; }
> +
> +int main(int argc, char **argv)
> +{
> +	char dump_ipset_old[]    = "ipset save > /tmp/ipset.old";

Lets better put these files relative to the cwd. See 
test/zdtm/static/netns-nf.c

> +	char dump_ipset_new[]    = "ipset save > /tmp/ipset.new";
> +	char dump_iptables_old[] = "iptables -L INPUT 1 > /tmp/iptables.old";
> +	char dump_iptables_new[] = "iptables -L INPUT 1 > /tmp/iptables.new";
> +	char cmp_ipset[]         = "diff /tmp/ipset.old /tmp/ipset.new";
> +	char cmp_iptables[]      = "diff /tmp/iptables.old /tmp/iptables.new";
> +	char rm_ipset_files[]    = "rm -fv /tmp/ipset.old /tmp/ipset.new";
> +	char rm_iptables_files[] = "rm -fv /tmp/iptables.old /tmp/iptables.new";
> +
> +	test_init(argc, argv);
> +
> +	/* create ipset group and add some ip addresses to it */
> +	RUN_OR_ERR("ipset create testgroup nethash", "Can't create test ipset");

Please rename testgroup to something matching the test name e.g.: 
zdtm-netns-ipset or netns-ipset.

> +	RUN_OR_ERR("ipset add testgroup 127.0.0.1/8", "Can't add ip addresses to ipset group");
> +
> +	/* Use testgroup in iptables rule */
> +	RUN_OR_ERR("iptables -I INPUT 1 -p tcp -m set --match-set testgroup src,dst -j ACCEPT",
> +		"Failed to setup iptables rule with ipset group");
> +
> +	/* dump ipset and iptables states to text files */
> +	RUN_OR_ERR(dump_iptables_old, "Can't save iptables rules.");
> +	RUN_OR_ERR(dump_ipset_old   , "Can't save ipset list.");
> +
> +	test_daemon();
> +	test_waitsig();
> +
> +	/* again dump ipset and iptables states to other text files */
> +	RUN_OR_ERR(dump_iptables_new, "Can't dump restored iptables rules.");
> +	RUN_OR_ERR(dump_ipset_new   , "Can't save restored ipset list to file.");
> +
> +	/* compare original and restored iptables rules */
> +	RUN_OR_FAIL(cmp_iptables, "iptables rules differ");
> +
> +	/* compare original and restored ipset rules */
> +	RUN_OR_FAIL(cmp_ipset, "ipset lists differ");
> +
> +	RUN_OR_ERR(rm_ipset_files, "Can't remove ipset files");
> +	RUN_OR_ERR(rm_iptables_files, "Can't remove iptables files");
> +
> +	pass();
> +	return 0;
> +}
> diff --git a/test/zdtm/static/netns-ipset.desc b/test/zdtm/static/netns-ipset.desc
> new file mode 100644
> index 0000000..d6eefa3
> --- /dev/null
> +++ b/test/zdtm/static/netns-ipset.desc
> @@ -0,0 +1,14 @@
> +{
> +	'flavor': 'h ns uns',
> +	'flags': 'suid',
> +	'deps': [
> +		'/usr/bin/rm',
> +		'/usr/bin/sh',
> +		'/usr/bin/diff',
> +		'/usr/sbin/ipset',
> +		'/usr/sbin/iptables',

> +		'/lib64/libipset.so.13',
> +		'/usr/lib64/xtables/libxt_set.so',
> +                '/usr/lib64/xtables/libxt_standard.so|/lib/xtables/libxt_standard.so|/usr/lib/powerpc64le-linux-gnu/xtables/libxt_standard.so|/usr/lib/x86_64-linux-gnu/xtables/libxt_standard.so|/usr/lib/xtables/libxt_standard.so'

Hm, what if libraries will change path due to version change?

> +		]
> +}
>