[CRIU] [PATCH v1 05/12] gc: implement unlocking of tcp connections
Eugene Batalov
eabatalov89 at gmail.com
Thu Aug 4 09:49:32 PDT 2016
Pavel, it turned out that criu gc doesn't solve Tycho's use case because he
wants to perform continuation of live ps tree after criu dump
--leave-stopped. Implementation of this feature is very different comparing
to implementation of criu gc that uses dumps as source of information about
dead ps tree garbage.
So do we still need new criu gc command? If yes then I'll send v2 patch set
with extended help message for the command.
2016-08-04 19:26 GMT+03:00 Tycho Andersen <tycho.andersen at canonical.com>:
> Hi Eugene,
>
> On Thu, Aug 04, 2016 at 07:01:36PM +0300, Eugene Batalov wrote:
> > 2016-08-04 18:53 GMT+03:00 Tycho Andersen <tycho.andersen at canonical.com>
> :
> >
> > > On Thu, Aug 04, 2016 at 06:42:06PM +0300, Eugene Batalov wrote:
> > > > 2016-08-04 18:08 GMT+03:00 Tycho Andersen <
> tycho.andersen at canonical.com>
> > > :
> > > >
> > > > > 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.
> > > > >
> > > > Looks like this is the use case you need to support. Let's look at
> > > current
> > > > implementation. Does it satisfies your needs?
> > >
> > > I worked around my issue in another way, so I think we can ignore my
> > > needs for the purpose of `criu gc`. If we wanted to clean up after a
> > > dump that left the network locked, I don't think this set would do it
> > > completely, but it sounds like we may not care about that.
> > >
> > Could you propose an example when this patch set doesn't unlock the
> network?
>
> Sorry, I was speaking of the case I already mentioned, about trying to
> use criu gc to clean up after a criu dump --leave-stopped. I think
> this patchset doesn't handle that case because of the missing
> cpt_unlock_tcp_connections(); but it sounds like we don't care about
> that, so never mind :)
>
> Tycho
>
> >
> > >
> > > > We can kill whole ps tree with any standard signal based logic so it
> > > looks
> > > > like there is no problem here.
> > > > But is it possible for you to have dump of this killed ps tree. criu
> gc
> > > > needs this dump in order to do its job.
> > > > Also criu gc requires only subset of the dump (memory is not needed).
> > > >
> > > > For now I prefer current criu gc implementation that works with dumps
> > > > rather then with live ps trees.
> > > > In general If we want to c/r we always create dump from live ps tree
> but
> > > we
> > > > don't always restore ps tree from dump.
> > > > So dump is more basic thing that criu gc relies on. The only
> question is
> > > > about availability of such a dump for gc.
> > >
> > > Ok, but I still don't understand, what's the use case for something
> this?
> > >
> > I have some thoughts like "if we don't restore ps tree we leave some
> > garbage in the system and we should be able to remove this garbage".
> > Maybe Pavel can tell us about more important use case.
> >
> >
> > >
> > > > >
> > > > > > 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 :)
> > > > >
> > > > Yep. I'll add more info into criu help message in the next patch set
> > > > version. Thank you for suggestion.
> > >
> > > OK :)
> > >
> > > Tycho
> > >
> > > > >
> > > > > 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.
> > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > > Eugene Batalov.
> > >
> >
> >
> >
> > --
> > Best regards,
> > Eugene Batalov.
>
--
Best regards,
Eugene Batalov.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20160804/7f5d62ca/attachment.html>
More information about the CRIU
mailing list