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

[Tycho Andersen]

Hi Eugene,

On Thu, Aug 04, 2016 at 02:49:23PM +0300, Eugene Batalov wrote:
> Hi Tycho,
> 
> > > +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.

Hmm. Perhaps I misunderstood then. I thought the point was to be able
to use --leave-stopped (so the ps tree would still exist), and then we
could at a later time run `criu gc` to unlock things again and clean
this up.

> 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.

I understand; it may be worth noting somewhere in the help output or
patch notes what exactly the use case is. Right now I don't undersand
what it is :)

Tycho

> >
> > > +}
> > > +
> > >  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.