empty-ns: Don't C/R iptables too (v2)

Submitted by Pavel Emelianov on Nov. 15, 2016, 2:08 p.m.

Details

Message ID 582B16D4.5060604@virtuozzo.com
State Superseded
Series "empty-ns: Don't C/R iptables too"
Headers show

Commit Message

Pavel Emelianov Nov. 15, 2016, 2:08 p.m.
When C/R-ing a net namespace with --empty-ns net option we should also
skip netfilter rules too (as per https://github.com/docker/docker/issues/27597).

However, there's one thing to be handled -- local TCP blocking rules are
expected to be there on restore by restore_iptables() which is no longer
the case, so put them back manually.

Test included, checked on zdtm/static/socket-tcpbuf-local :)

v2: Full scripts for empty netns setup.

Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>

---

Patch hide | download patch | download mbox

diff --git a/criu/cr-restore.c b/criu/cr-restore.c
index e687a4f..e64fe19 100644
--- a/criu/cr-restore.c
+++ b/criu/cr-restore.c
@@ -1733,6 +1733,19 @@  static int restore_root_task(struct pstree_item *init)
 	if (ret)
 		goto out_kill;
 
+	if (opts.empty_ns & CLONE_NEWNET) {
+		/*
+		 * Local TCP connections were locked by network_lock_internal()
+		 * on dump and normally should have been C/R-ed by respectively
+		 * dump_iptables() and restore_iptables() in net.c. However in
+		 * the '--empty-ns net' mode no iptables C/R is done and we
+		 * need to return these rules by hands.
+		 */
+		ret = network_lock_internal();
+		if (ret)
+			goto out_kill;
+	}
+
 	timing_start(TIME_FORK);
 	ret = restore_switch_stage(CR_STATE_RESTORE_SHARED);
 	if (ret < 0)
diff --git a/criu/include/net.h b/criu/include/net.h
index f42a21c..deac65f 100644
--- a/criu/include/net.h
+++ b/criu/include/net.h
@@ -26,6 +26,7 @@  extern int collect_net_namespaces(bool for_dump);
 
 extern int network_lock(void);
 extern void network_unlock(void);
+extern int network_lock_internal();
 
 extern struct ns_desc net_ns_desc;
 
diff --git a/criu/net.c b/criu/net.c
index 15d3783..329c97c 100644
--- a/criu/net.c
+++ b/criu/net.c
@@ -1650,10 +1650,10 @@  int dump_net_ns(int ns_id)
 			ret = dump_route(fds);
 		if (!ret)
 			ret = dump_rule(fds);
+		if (!ret)
+			ret = dump_iptables(fds);
 	}
 	if (!ret)
-		ret = dump_iptables(fds);
-	if (!ret)
 		ret = dump_nf_ct(fds, CR_FD_NETNF_CT);
 	if (!ret)
 		ret = dump_nf_ct(fds, CR_FD_NETNF_EXP);
@@ -1683,9 +1683,10 @@  int prepare_net_ns(int pid)
 			ret = restore_route(pid);
 		if (!ret)
 			ret = restore_rule(pid);
+		if (!ret)
+			ret = restore_iptables(pid);
 	}
-	if (!ret)
-		ret = restore_iptables(pid);
+
 	if (!ret)
 		ret = restore_nf_ct(pid, CR_FD_NETNF_CT);
 	if (!ret)
@@ -1757,7 +1758,7 @@  err:
 	return ret;
 }
 
-static int network_lock_internal()
+int network_lock_internal()
 {
 	char conf[] =	"*filter\n"
 				":CRIU - [0:0]\n"
diff --git a/test/empty-netns-prep.sh b/test/empty-netns-prep.sh
new file mode 100755
index 0000000..212eb39
--- /dev/null
+++ b/test/empty-netns-prep.sh
@@ -0,0 +1,8 @@ 
+#!/bin/sh
+
+if [ "$CRTOOLS_SCRIPT_ACTION" == "setup-namespaces" ]; then
+	echo "Will up lo at $CRTOOLS_INIT_PID netns"
+	nsenter -t "$CRTOOLS_INIT_PID" --net ip link set up dev lo || exit 1
+fi
+
+exit 0
diff --git a/test/zdtm.py b/test/zdtm.py
index f8f409f..a57cd6e 100755
--- a/test/zdtm.py
+++ b/test/zdtm.py
@@ -767,6 +767,7 @@  class criu:
 		self.__page_server = (opts['page_server'] and True or False)
 		self.__restore_sibling = (opts['sibling'] and True or False)
 		self.__join_ns = (opts['join_ns'] and True or False)
+		self.__empty_ns = (opts['empty_ns'] and True or False)
 		self.__fault = (opts['fault'])
 		self.__script = opts['script']
 		self.__sat = (opts['sat'] and True or False)
@@ -893,6 +894,8 @@  class criu:
 
 		if self.__leave_stopped:
 			a_opts += ['--leave-stopped']
+		if self.__empty_ns:
+			a_opts += ['--empty-ns', 'net']
 
 		self.__criu_act(action, opts = a_opts + opts)
 		if self.__mdedup and self.__iter > 1:
@@ -914,6 +917,9 @@  class criu:
 		if self.__join_ns:
 			r_opts.append("--join-ns")
 			r_opts.append("net:%s" % join_ns_file)
+		if self.__empty_ns:
+			r_opts += ['--empty-ns', 'net']
+			r_opts += ['--action-script', os.getcwd() + '/empty-netns-prep.sh']
 
 		self.__prev_dump_iter = None
 		criu_dir = os.path.dirname(os.getcwd())
@@ -1344,7 +1350,7 @@  class launcher:
 		self.__nr += 1
 		self.__show_progress()
 
-		nd = ('nocr', 'norst', 'pre', 'iters', 'page_server', 'sibling', 'stop',
+		nd = ('nocr', 'norst', 'pre', 'iters', 'page_server', 'sibling', 'stop', 'empty_ns',
 				'fault', 'keep_img', 'report', 'snaps', 'sat', 'script', 'rpc',
 				'join_ns', 'dedup', 'sbs', 'freezecg', 'user', 'dry_run', 'noauto_dedup')
 		arg = repr((name, desc, flavor, {d: self.__opts[d] for d in nd}))
@@ -1590,6 +1596,8 @@  def run_tests(opts):
 			# remove ns and uns flavor in join_ns
 			if opts['join_ns']:
 				run_flavs -= set(['ns', 'uns'])
+			if opts['empty_ns']:
+				run_flavs -= set(['h'])
 
 			if run_flavs:
 				l.run_test(t, tdesc, run_flavs)
@@ -1781,6 +1789,7 @@  rp.add_argument("-x", "--exclude", help = "Exclude tests from --all run", action
 
 rp.add_argument("--sibling", help = "Restore tests as siblings", action = 'store_true')
 rp.add_argument("--join-ns", help = "Restore tests and join existing namespace", action = 'store_true')
+rp.add_argument("--empty-ns", help = "Restore tests in empty net namespace", action = 'store_true')
 rp.add_argument("--pre", help = "Do some pre-dumps before dump (n[:pause])")
 rp.add_argument("--snaps", help = "Instead of pre-dumps do full dumps", action = 'store_true')
 rp.add_argument("--dedup", help = "Auto-deduplicate images on iterations", action = 'store_true')

Comments

Andrey Vagin Nov. 15, 2016, 5:30 p.m.
On Tue, Nov 15, 2016 at 05:08:20PM +0300, Pavel Emelyanov wrote:
> When C/R-ing a net namespace with --empty-ns net option we should also
> skip netfilter rules too (as per https://github.com/docker/docker/issues/27597).
> 
> However, there's one thing to be handled -- local TCP blocking rules are
> expected to be there on restore by restore_iptables() which is no longer
> the case, so put them back manually.
> 
> Test included, checked on zdtm/static/socket-tcpbuf-local :)
> 
> v2: Full scripts for empty netns setup.
>

Acked-by: Andrei Vagin <avagin@virtuozzo.com>

Thanks!

> Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
> 
> ---
> 
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index e687a4f..e64fe19 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -1733,6 +1733,19 @@ static int restore_root_task(struct pstree_item *init)
>  	if (ret)
>  		goto out_kill;
>  
> +	if (opts.empty_ns & CLONE_NEWNET) {
> +		/*
> +		 * Local TCP connections were locked by network_lock_internal()
> +		 * on dump and normally should have been C/R-ed by respectively
> +		 * dump_iptables() and restore_iptables() in net.c. However in
> +		 * the '--empty-ns net' mode no iptables C/R is done and we
> +		 * need to return these rules by hands.
> +		 */
> +		ret = network_lock_internal();
> +		if (ret)
> +			goto out_kill;
> +	}
> +
>  	timing_start(TIME_FORK);
>  	ret = restore_switch_stage(CR_STATE_RESTORE_SHARED);
>  	if (ret < 0)
> diff --git a/criu/include/net.h b/criu/include/net.h
> index f42a21c..deac65f 100644
> --- a/criu/include/net.h
> +++ b/criu/include/net.h
> @@ -26,6 +26,7 @@ extern int collect_net_namespaces(bool for_dump);
>  
>  extern int network_lock(void);
>  extern void network_unlock(void);
> +extern int network_lock_internal();
>  
>  extern struct ns_desc net_ns_desc;
>  
> diff --git a/criu/net.c b/criu/net.c
> index 15d3783..329c97c 100644
> --- a/criu/net.c
> +++ b/criu/net.c
> @@ -1650,10 +1650,10 @@ int dump_net_ns(int ns_id)
>  			ret = dump_route(fds);
>  		if (!ret)
>  			ret = dump_rule(fds);
> +		if (!ret)
> +			ret = dump_iptables(fds);
>  	}
>  	if (!ret)
> -		ret = dump_iptables(fds);
> -	if (!ret)
>  		ret = dump_nf_ct(fds, CR_FD_NETNF_CT);
>  	if (!ret)
>  		ret = dump_nf_ct(fds, CR_FD_NETNF_EXP);
> @@ -1683,9 +1683,10 @@ int prepare_net_ns(int pid)
>  			ret = restore_route(pid);
>  		if (!ret)
>  			ret = restore_rule(pid);
> +		if (!ret)
> +			ret = restore_iptables(pid);
>  	}
> -	if (!ret)
> -		ret = restore_iptables(pid);
> +
>  	if (!ret)
>  		ret = restore_nf_ct(pid, CR_FD_NETNF_CT);
>  	if (!ret)
> @@ -1757,7 +1758,7 @@ err:
>  	return ret;
>  }
>  
> -static int network_lock_internal()
> +int network_lock_internal()
>  {
>  	char conf[] =	"*filter\n"
>  				":CRIU - [0:0]\n"
> diff --git a/test/empty-netns-prep.sh b/test/empty-netns-prep.sh
> new file mode 100755
> index 0000000..212eb39
> --- /dev/null
> +++ b/test/empty-netns-prep.sh
> @@ -0,0 +1,8 @@
> +#!/bin/sh
> +
> +if [ "$CRTOOLS_SCRIPT_ACTION" == "setup-namespaces" ]; then
> +	echo "Will up lo at $CRTOOLS_INIT_PID netns"
> +	nsenter -t "$CRTOOLS_INIT_PID" --net ip link set up dev lo || exit 1
> +fi
> +
> +exit 0
> diff --git a/test/zdtm.py b/test/zdtm.py
> index f8f409f..a57cd6e 100755
> --- a/test/zdtm.py
> +++ b/test/zdtm.py
> @@ -767,6 +767,7 @@ class criu:
>  		self.__page_server = (opts['page_server'] and True or False)
>  		self.__restore_sibling = (opts['sibling'] and True or False)
>  		self.__join_ns = (opts['join_ns'] and True or False)
> +		self.__empty_ns = (opts['empty_ns'] and True or False)
>  		self.__fault = (opts['fault'])
>  		self.__script = opts['script']
>  		self.__sat = (opts['sat'] and True or False)
> @@ -893,6 +894,8 @@ class criu:
>  
>  		if self.__leave_stopped:
>  			a_opts += ['--leave-stopped']
> +		if self.__empty_ns:
> +			a_opts += ['--empty-ns', 'net']
>  
>  		self.__criu_act(action, opts = a_opts + opts)
>  		if self.__mdedup and self.__iter > 1:
> @@ -914,6 +917,9 @@ class criu:
>  		if self.__join_ns:
>  			r_opts.append("--join-ns")
>  			r_opts.append("net:%s" % join_ns_file)
> +		if self.__empty_ns:
> +			r_opts += ['--empty-ns', 'net']
> +			r_opts += ['--action-script', os.getcwd() + '/empty-netns-prep.sh']
>  
>  		self.__prev_dump_iter = None
>  		criu_dir = os.path.dirname(os.getcwd())
> @@ -1344,7 +1350,7 @@ class launcher:
>  		self.__nr += 1
>  		self.__show_progress()
>  
> -		nd = ('nocr', 'norst', 'pre', 'iters', 'page_server', 'sibling', 'stop',
> +		nd = ('nocr', 'norst', 'pre', 'iters', 'page_server', 'sibling', 'stop', 'empty_ns',
>  				'fault', 'keep_img', 'report', 'snaps', 'sat', 'script', 'rpc',
>  				'join_ns', 'dedup', 'sbs', 'freezecg', 'user', 'dry_run', 'noauto_dedup')
>  		arg = repr((name, desc, flavor, {d: self.__opts[d] for d in nd}))
> @@ -1590,6 +1596,8 @@ def run_tests(opts):
>  			# remove ns and uns flavor in join_ns
>  			if opts['join_ns']:
>  				run_flavs -= set(['ns', 'uns'])
> +			if opts['empty_ns']:
> +				run_flavs -= set(['h'])
>  
>  			if run_flavs:
>  				l.run_test(t, tdesc, run_flavs)
> @@ -1781,6 +1789,7 @@ rp.add_argument("-x", "--exclude", help = "Exclude tests from --all run", action
>  
>  rp.add_argument("--sibling", help = "Restore tests as siblings", action = 'store_true')
>  rp.add_argument("--join-ns", help = "Restore tests and join existing namespace", action = 'store_true')
> +rp.add_argument("--empty-ns", help = "Restore tests in empty net namespace", action = 'store_true')
>  rp.add_argument("--pre", help = "Do some pre-dumps before dump (n[:pause])")
>  rp.add_argument("--snaps", help = "Instead of pre-dumps do full dumps", action = 'store_true')
>  rp.add_argument("--dedup", help = "Auto-deduplicate images on iterations", action = 'store_true')
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu