[CRIU] [PATCH] restore: Defer new net-namespace creation until forked

[Andrew Vagin]

On Mon, Oct 27, 2014 at 11:24:10PM +0300, Cyrill Gorcunov wrote:
> On Mon, Oct 27, 2014 at 11:02:04PM +0300, Andrew Vagin wrote:
> > On Mon, Oct 27, 2014 at 07:19:03PM +0300, Cyrill Gorcunov wrote:
> > > Some kernel modules such as pktgen runs kthred upon
> > > new-net creation taking last_pid we were requested.
> > > Lets workaround this problem using clone + unshare
> > > bundle.
> > > 
> > > Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> > > ---
> > >  cr-restore.c | 18 +++++++++++++++++-
> > >  1 file changed, 17 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/cr-restore.c b/cr-restore.c
> > > index d046f4bd0921..d3811cb7b5d7 100644
> > > --- a/cr-restore.c
> > > +++ b/cr-restore.c
> > > @@ -1086,8 +1086,16 @@ static inline int fork_with_pid(struct pstree_item *item)
> > >  		if (netns_pre_create())
> > >  			goto err_unlock;
> > >  
> > > +	/*
> > > +	 * Some kernel modules, such as netwrok packet generator
> > > +	 * run kernel thread upon net-namespace creattion taking
> > > +	 * the @pid we've been requeting via LAST_PID_PATH interface
> > > +	 * so that we can't restore a take with pid needed.
> > > +	 *
> > > +	 * Here is an idea -- unhare net namespace in callee instead.
> > > +	 */
> > >  	ret = clone(restore_task_with_children, ca.stack_ptr,
> > > -			ca.clone_flags | SIGCHLD, &ca);
> > > +		    (ca.clone_flags & ~CLONE_NEWNET) | SIGCHLD, &ca);
> > >  
> > >  	if (ret < 0) {
> > >  		pr_perror("Can't fork for %d", pid);
> > > @@ -1395,6 +1403,14 @@ static int restore_task_with_children(void *_arg)
> > >  	if ( !(ca->clone_flags & CLONE_FILES))
> > >  		close_safe(&ca->fd);
> > >  
> > > +	if (ca->clone_flags & CLONE_NEWNET) {
> > > +		ret = unshare(CLONE_NEWNET);
> > > +		if (ret) {
> > > +			pr_perror("Can't unshare net-namespace");
> > > +			goto err;
> > > +		}
> > > +	}
> > 
> > I would prefer to move this code after:
> >         if (current->pid.virt != pid) {
> >                 pr_err("Pid %d do not match expected %d\n", pid, current->pid.virt);
> >                 goto err;
> >         }
> > 
> > And now we doesn't support nested net namespaces, so we can have it in
> > the "Restore root task" blob.
> 
> But we're supposed to support them in future, right? So lets this hunk
> after pid check.

> From f1588f840dd0b5049504dd33ba8937d36e5bd77e Mon Sep 17 00:00:00 2001
> From: Cyrill Gorcunov <gorcunov at openvz.org>
> Date: Mon, 27 Oct 2014 18:31:11 +0300
> Subject: [PATCH] restore: Defer new net-namespace creation until forked
> 
> Some kernel modules such as pktgen runs kthred upon
> new-net creation taking last_pid we were requested.
> Lets workaround this problem using clone + unshare
> bundle.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
Acked-by: Andrew Vagin <avagin at parallels.com>
> ---
>  cr-restore.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index d046f4bd0921..05296f885764 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -1086,8 +1086,16 @@ static inline int fork_with_pid(struct pstree_item *item)
>  		if (netns_pre_create())
>  			goto err_unlock;
>  
> +	/*
> +	 * Some kernel modules, such as netwrok packet generator
> +	 * run kernel thread upon net-namespace creattion taking
> +	 * the @pid we've been requeting via LAST_PID_PATH interface
> +	 * so that we can't restore a take with pid needed.
> +	 *
> +	 * Here is an idea -- unhare net namespace in callee instead.
> +	 */
>  	ret = clone(restore_task_with_children, ca.stack_ptr,
> -			ca.clone_flags | SIGCHLD, &ca);
> +		    (ca.clone_flags & ~CLONE_NEWNET) | SIGCHLD, &ca);
>  
>  	if (ret < 0) {
>  		pr_perror("Can't fork for %d", pid);
> @@ -1411,6 +1419,14 @@ static int restore_task_with_children(void *_arg)
>  	if (ret < 0)
>  		goto err;
>  
> +	if (ca->clone_flags & CLONE_NEWNET) {
> +		ret = unshare(CLONE_NEWNET);
> +		if (ret) {
> +			pr_perror("Can't unshare net-namespace");
> +			goto err;
> +		}
> +	}
> +
>  	/* Restore root task */
>  	if (current->parent == NULL) {
>  		if (restore_finish_stage(CR_STATE_RESTORE_NS) < 0)
> -- 
> 1.9.3
>