<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Hi Tycho,</div><div class="gmail_quote"><br></div><div class="gmail_quote">2016-08-04 0:23 GMT+03:00 Tycho Andersen <span dir="ltr"><<a href="mailto:tycho.andersen@canonical.com" target="_blank">tycho.andersen@canonical.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Hi Eugene,<br>
<br>
Sorry for the delay, and thanks for looking into this,<br>
<div><div class="h5"><br>
On Thu, Jul 28, 2016 at 08:14:41AM +0300, Eugene Batalov wrote:<br>
> The logic is the same as on criu restore.<br>
><br>
> First we read ps tree dump to get information about all its<br>
> tcp connections. If ps tree shares net ns with host then<br>
> we simply run iptables to unlock all ps tree tcp connections.<br>
> If ps tree has unshared net ns then we do nothing because we know<br>
> that this net ns is already destroyed with its ps tree and all<br>
> incoming ps tree TCP packets are just dropped (no destination tcp stack<br>
> exists).<br>
><br>
> Signed-off-by: Eugene Batalov <<a href="mailto:eabatalov89@gmail.com">eabatalov89@gmail.com</a>><br>
> ---<br>
> criu/cr-gc.c | 10 ++++++++++<br>
> criu/include/net.h | 1 +<br>
> criu/include/sk-inet.h | 2 +-<br>
> criu/net.c | 12 ++++++++++++<br>
> criu/sk-tcp.c | 9 ++++++---<br>
> 5 files changed, 30 insertions(+), 4 deletions(-)<br>
><br>
> diff --git a/criu/cr-gc.c b/criu/cr-gc.c<br>
> index 2879365..4c8bea9 100644<br>
> --- a/criu/cr-gc.c<br>
> +++ b/criu/cr-gc.c<br>
> @@ -10,6 +10,8 @@<br>
> #include "cr_options.h"<br>
> #include "namespaces.h"<br>
> #include "util.h"<br>
> +#include "sockets.h"<br>
> +#include "net.h"<br>
><br>
> static int gc_validate_opts(void)<br>
> {<br>
> @@ -80,6 +82,9 @@ static int gc_do(void)<br>
> if (gc_link_remaps() < 0)<br>
> return -1;<br>
><br>
> + if (gc_network_unlock() < 0)<br>
> + return -1;<br>
> +<br>
> return 0;<br>
> }<br>
><br>
> @@ -122,6 +127,11 @@ int cr_gc(void)<br>
> goto exit;<br>
> }<br>
><br>
> + if (collect_inet_sockets()) {<br>
> + ret = -1;<br>
> + goto exit;<br>
> + }<br>
> +<br>
> if (opts.show)<br>
> ret = gc_show();<br>
> else<br>
> diff --git a/criu/include/net.h b/criu/include/net.h<br>
> index ede380f..5e6260a 100644<br>
> --- a/criu/include/net.h<br>
> +++ b/criu/include/net.h<br>
> @@ -19,6 +19,7 @@ extern int collect_net_namespaces(bool for_dump);<br>
><br>
> extern int network_lock(void);<br>
> extern void network_unlock(void);<br>
> +extern int gc_network_unlock(void);<br>
><br>
> extern struct ns_desc net_ns_desc;<br>
><br>
> diff --git a/criu/include/sk-inet.h b/criu/include/sk-inet.h<br>
> index 9d2bda6..c82ffd4 100644<br>
> --- a/criu/include/sk-inet.h<br>
> +++ b/criu/include/sk-inet.h<br>
> @@ -65,7 +65,7 @@ static inline void tcp_repair_off(int fd)<br>
> }<br>
><br>
> extern void tcp_locked_conn_add(struct inet_sk_info *);<br>
> -extern void rst_unlock_tcp_connections(<wbr>void);<br>
> +extern int rst_unlock_tcp_connections(<wbr>void);<br>
> extern void cpt_unlock_tcp_connections(<wbr>void);<br>
><br>
> extern int dump_one_tcp(int sk, struct inet_sk_desc *sd);<br>
> diff --git a/criu/net.c b/criu/net.c<br>
> index a03c168..ef203a8 100644<br>
> --- a/criu/net.c<br>
> +++ b/criu/net.c<br>
> @@ -1618,6 +1618,18 @@ void network_unlock(void)<br>
> }<br>
> }<br>
><br>
> +int gc_network_unlock(void)<br>
> +{<br>
> + /*<br>
> + * Unshared ps tree net ns is destroyed after successful dump.<br>
> + * No need to call network_unlock_internal.<br>
> + * Also don't call ACT_NET_UNLOCK script because we don't<br>
> + * resume/restore ps tree - this call would break<br>
> + * ACT_NET_UNLOCK semantics.<br>
> + */<br>
> + return rst_unlock_tcp_connections();<br>
<br>
</div></div>What about cpt_unlock_tcp_connections()? IIUC this list is not<br>
persisted, and so if we leave around the network lock stuff, we would<br>
never turn of TCP repair mode.<br>
<br>
Tycho<br></blockquote><div>Let's consider the moment when we start criu gc or criu restore. At this moment dumpee ps tree doesn't exist and ps tree sockets don't exist.<br></div><div>That's why we don't need to turn any tcp repair mode off - it is per socket thing.</div><div>Also cpt_unlock_tcp_connections called from network_unlock() that is called during criu restore</div><div>does nothing because cpt_tcp_repair_sockets list is empty. So for me it looks like criu gc works</div><div>the same as criu restore but without calling of network unlock action script.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<div><div class="h5"><br>
> +}<br>
> +<br>
> int veth_pair_add(char *in, char *out)<br>
> {<br>
> char *aux;<br>
> diff --git a/criu/sk-tcp.c b/criu/sk-tcp.c<br>
> index 13d175a..c4aa58f 100644<br>
> --- a/criu/sk-tcp.c<br>
> +++ b/criu/sk-tcp.c<br>
> @@ -713,16 +713,19 @@ void tcp_locked_conn_add(struct inet_sk_info *ii)<br>
> ii->sk_fd = -1;<br>
> }<br>
><br>
> -void rst_unlock_tcp_connections(<wbr>void)<br>
> +int rst_unlock_tcp_connections(<wbr>void)<br>
> {<br>
> struct inet_sk_info *ii;<br>
><br>
> /* Network will be unlocked by network-unlock scripts */<br>
> if (root_ns_mask & CLONE_NEWNET)<br>
> - return;<br>
> + return 0;<br>
><br>
> list_for_each_entry(ii, &rst_tcp_repair_sockets, rlist)<br>
> - nf_unlock_connection_info(ii);<br>
> + if (nf_unlock_connection_info(ii)<wbr>)<br>
> + return -1;<br>
> +<br>
> + return 0;<br>
> }<br>
><br>
> int check_tcp(void)<br>
> --<br>
> 1.9.1<br>
><br>
</div></div>> ______________________________<wbr>_________________<br>
> CRIU mailing list<br>
> <a href="mailto:CRIU@openvz.org">CRIU@openvz.org</a><br>
> <a href="https://lists.openvz.org/mailman/listinfo/criu" rel="noreferrer" target="_blank">https://lists.openvz.org/<wbr>mailman/listinfo/criu</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Best regards,<br>Eugene Batalov.</div>
</div></div>