[CRIU] [PATCH v1 05/12] gc: implement unlocking of tcp connections

[Eugene Batalov]

Hi Tycho,

2016-08-04 0:23 GMT+03:00 Tycho Andersen <tycho.andersen at canonical.com>:

> Hi Eugene,
>
> Sorry for the delay, and thanks for looking into this,
>
> On Thu, Jul 28, 2016 at 08:14:41AM +0300, Eugene Batalov wrote:
> > The logic is the same as on criu restore.
> >
> > First we read ps tree dump to get information about all its
> > tcp connections. If ps tree shares net ns with host then
> > we simply run iptables to unlock all ps tree tcp connections.
> > If ps tree has unshared net ns then we do nothing because we know
> > that this net ns is already destroyed with its ps tree and all
> > incoming ps tree TCP packets are just dropped (no destination tcp stack
> > exists).
> >
> > Signed-off-by: Eugene Batalov <eabatalov89 at gmail.com>
> > ---
> >  criu/cr-gc.c           | 10 ++++++++++
> >  criu/include/net.h     |  1 +
> >  criu/include/sk-inet.h |  2 +-
> >  criu/net.c             | 12 ++++++++++++
> >  criu/sk-tcp.c          |  9 ++++++---
> >  5 files changed, 30 insertions(+), 4 deletions(-)
> >
> > diff --git a/criu/cr-gc.c b/criu/cr-gc.c
> > index 2879365..4c8bea9 100644
> > --- a/criu/cr-gc.c
> > +++ b/criu/cr-gc.c
> > @@ -10,6 +10,8 @@
> >  #include "cr_options.h"
> >  #include "namespaces.h"
> >  #include "util.h"
> > +#include "sockets.h"
> > +#include "net.h"
> >
> >  static int gc_validate_opts(void)
> >  {
> > @@ -80,6 +82,9 @@ static int gc_do(void)
> >       if (gc_link_remaps() < 0)
> >               return -1;
> >
> > +     if (gc_network_unlock() < 0)
> > +             return -1;
> > +
> >       return 0;
> >  }
> >
> > @@ -122,6 +127,11 @@ int cr_gc(void)
> >               goto exit;
> >       }
> >
> > +     if (collect_inet_sockets()) {
> > +             ret = -1;
> > +             goto exit;
> > +     }
> > +
> >       if (opts.show)
> >               ret = gc_show();
> >       else
> > diff --git a/criu/include/net.h b/criu/include/net.h
> > index ede380f..5e6260a 100644
> > --- a/criu/include/net.h
> > +++ b/criu/include/net.h
> > @@ -19,6 +19,7 @@ extern int collect_net_namespaces(bool for_dump);
> >
> >  extern int network_lock(void);
> >  extern void network_unlock(void);
> > +extern int gc_network_unlock(void);
> >
> >  extern struct ns_desc net_ns_desc;
> >
> > diff --git a/criu/include/sk-inet.h b/criu/include/sk-inet.h
> > index 9d2bda6..c82ffd4 100644
> > --- a/criu/include/sk-inet.h
> > +++ b/criu/include/sk-inet.h
> > @@ -65,7 +65,7 @@ static inline void tcp_repair_off(int fd)
> >  }
> >
> >  extern void tcp_locked_conn_add(struct inet_sk_info *);
> > -extern void rst_unlock_tcp_connections(void);
> > +extern int rst_unlock_tcp_connections(void);
> >  extern void cpt_unlock_tcp_connections(void);
> >
> >  extern int dump_one_tcp(int sk, struct inet_sk_desc *sd);
> > diff --git a/criu/net.c b/criu/net.c
> > index a03c168..ef203a8 100644
> > --- a/criu/net.c
> > +++ b/criu/net.c
> > @@ -1618,6 +1618,18 @@ void network_unlock(void)
> >       }
> >  }
> >
> > +int gc_network_unlock(void)
> > +{
> > +     /*
> > +      * Unshared ps tree net ns is destroyed after successful dump.
> > +      * No need to call network_unlock_internal.
> > +      * Also don't call ACT_NET_UNLOCK script because we don't
> > +      * resume/restore ps tree - this call would break
> > +      * ACT_NET_UNLOCK semantics.
> > +      */
> > +     return rst_unlock_tcp_connections();
>
> What about cpt_unlock_tcp_connections()? IIUC this list is not
> persisted, and so if we leave around the network lock stuff, we would
> never turn of TCP repair mode.
>
> Tycho
>
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.
That's why we don't need to turn any tcp repair mode off - it is per socket
thing.
Also cpt_unlock_tcp_connections called from network_unlock() that is called
during criu restore
does nothing because cpt_tcp_repair_sockets list is empty. So for me it
looks like criu gc works
the same as criu restore but without calling of network unlock action
script.

>
> > +}
> > +
> >  int veth_pair_add(char *in, char *out)
> >  {
> >       char *aux;
> > diff --git a/criu/sk-tcp.c b/criu/sk-tcp.c
> > index 13d175a..c4aa58f 100644
> > --- a/criu/sk-tcp.c
> > +++ b/criu/sk-tcp.c
> > @@ -713,16 +713,19 @@ void tcp_locked_conn_add(struct inet_sk_info *ii)
> >       ii->sk_fd = -1;
> >  }
> >
> > -void rst_unlock_tcp_connections(void)
> > +int rst_unlock_tcp_connections(void)
> >  {
> >       struct inet_sk_info *ii;
> >
> >       /* Network will be unlocked by network-unlock scripts */
> >       if (root_ns_mask & CLONE_NEWNET)
> > -             return;
> > +             return 0;
> >
> >       list_for_each_entry(ii, &rst_tcp_repair_sockets, rlist)
> > -             nf_unlock_connection_info(ii);
> > +             if (nf_unlock_connection_info(ii))
> > +                     return -1;
> > +
> > +     return 0;
> >  }
> >
> >  int check_tcp(void)
> > --
> > 1.9.1
> >
> > _______________________________________________
> > CRIU mailing list
> > CRIU at openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu
>



-- 
Best regards,
Eugene Batalov.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20160804/2443fb90/attachment.html>