[PATCH] Skip down interfaces (v2)

[PATCH] Skip down interfaces (v2)

[Dan Smith]

This makes the netns checkpoint code skip interfaces that are not up.
Later, we want to make it possible to checkpoint down interfaces with
a flag, but for now this helps prevent people from getting stuck on
stock kernels with various unsupported and persistent virtual interfaces,
such as tunnel devices.

Changes in v2:
 - Fix logic to only skip down interfaces if they're not supported

Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 net/checkpoint_dev.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/net/checkpoint_dev.c b/net/checkpoint_dev.c
index bc0415d..e8d574c 100644
--- a/net/checkpoint_dev.c
+++ b/net/checkpoint_dev.c
@@ -268,7 +268,16 @@ int checkpoint_netns(struct ckpt_ctx *ctx, void *ptr)
 		goto out;
 
 	for_each_netdev(net, dev) {
-		ret = checkpoint_obj(ctx, dev, CKPT_OBJ_NETDEV);
+		if (dev->netdev_ops->ndo_checkpoint)
+			ret = checkpoint_obj(ctx, dev, CKPT_OBJ_NETDEV);
+		else if (dev->flags & IFF_UP)
+			ret = -ENOSYS;
+		else
+			/* TODO: There should be a flag to attempt a
+			 * checkpoint of downed interfaces, regardless
+			 * of whether they support checkpoint or not.
+			 */
+			ret = 0;
 		if (ret < 0)
 			break;
 	}
-- 
1.6.2.5

Re: [PATCH] Skip down interfaces (v2)

[Serge E. Hallyn]

Quoting Dan Smith (danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org):
> This makes the netns checkpoint code skip interfaces that are not up.
> Later, we want to make it possible to checkpoint down interfaces with
> a flag, but for now this helps prevent people from getting stuck on
> stock kernels with various unsupported and persistent virtual interfaces,
> such as tunnel devices.
> 
> Changes in v2:
>  - Fix logic to only skip down interfaces if they're not supported
> 
> Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Looks good, thanks.

Tested-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

-serge

> ---
>  net/checkpoint_dev.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/net/checkpoint_dev.c b/net/checkpoint_dev.c
> index bc0415d..e8d574c 100644
> --- a/net/checkpoint_dev.c
> +++ b/net/checkpoint_dev.c
> @@ -268,7 +268,16 @@ int checkpoint_netns(struct ckpt_ctx *ctx, void *ptr)
>  		goto out;
> 
>  	for_each_netdev(net, dev) {
> -		ret = checkpoint_obj(ctx, dev, CKPT_OBJ_NETDEV);
> +		if (dev->netdev_ops->ndo_checkpoint)
> +			ret = checkpoint_obj(ctx, dev, CKPT_OBJ_NETDEV);
> +		else if (dev->flags & IFF_UP)
> +			ret = -ENOSYS;
> +		else
> +			/* TODO: There should be a flag to attempt a
> +			 * checkpoint of downed interfaces, regardless
> +			 * of whether they support checkpoint or not.
> +			 */
> +			ret = 0;
>  		if (ret < 0)
>  			break;
>  	}
> -- 
> 1.6.2.5
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [PATCH] Skip down interfaces (v2)

[Oren Laadan]

Acked and pulled for v21-rc2, thanks.

Dan Smith wrote:
> This makes the netns checkpoint code skip interfaces that are not up.
> Later, we want to make it possible to checkpoint down interfaces with
> a flag, but for now this helps prevent people from getting stuck on
> stock kernels with various unsupported and persistent virtual interfaces,
> such as tunnel devices.
> 
> Changes in v2:
>  - Fix logic to only skip down interfaces if they're not supported
> 
> Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>  net/checkpoint_dev.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/net/checkpoint_dev.c b/net/checkpoint_dev.c
> index bc0415d..e8d574c 100644
> --- a/net/checkpoint_dev.c
> +++ b/net/checkpoint_dev.c
> @@ -268,7 +268,16 @@ int checkpoint_netns(struct ckpt_ctx *ctx, void *ptr)
>  		goto out;
>  
>  	for_each_netdev(net, dev) {
> -		ret = checkpoint_obj(ctx, dev, CKPT_OBJ_NETDEV);
> +		if (dev->netdev_ops->ndo_checkpoint)
> +			ret = checkpoint_obj(ctx, dev, CKPT_OBJ_NETDEV);
> +		else if (dev->flags & IFF_UP)
> +			ret = -ENOSYS;
> +		else
> +			/* TODO: There should be a flag to attempt a
> +			 * checkpoint of downed interfaces, regardless
> +			 * of whether they support checkpoint or not.
> +			 */
> +			ret = 0;
>  		if (ret < 0)
>  			break;
>  	}

Re: [PATCH] Skip down interfaces (v2)

[Oren Laadan]

Acked and pulled for v21-rc2, thanks.

Dan Smith wrote:
> This makes the netns checkpoint code skip interfaces that are not up.
> Later, we want to make it possible to checkpoint down interfaces with
> a flag, but for now this helps prevent people from getting stuck on
> stock kernels with various unsupported and persistent virtual interfaces,
> such as tunnel devices.
> 
> Changes in v2:
>  - Fix logic to only skip down interfaces if they're not supported
> 
> Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>  net/checkpoint_dev.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/net/checkpoint_dev.c b/net/checkpoint_dev.c
> index bc0415d..e8d574c 100644
> --- a/net/checkpoint_dev.c
> +++ b/net/checkpoint_dev.c
> @@ -268,7 +268,16 @@ int checkpoint_netns(struct ckpt_ctx *ctx, void *ptr)
>  		goto out;
>  
>  	for_each_netdev(net, dev) {
> -		ret = checkpoint_obj(ctx, dev, CKPT_OBJ_NETDEV);
> +		if (dev->netdev_ops->ndo_checkpoint)
> +			ret = checkpoint_obj(ctx, dev, CKPT_OBJ_NETDEV);
> +		else if (dev->flags & IFF_UP)
> +			ret = -ENOSYS;
> +		else
> +			/* TODO: There should be a flag to attempt a
> +			 * checkpoint of downed interfaces, regardless
> +			 * of whether they support checkpoint or not.
> +			 */
> +			ret = 0;
>  		if (ret < 0)
>  			break;
>  	}

Re: [PATCH] Skip down interfaces (v2)

[Brian Haley]

Dan Smith wrote:
> This makes the netns checkpoint code skip interfaces that are not up.
> Later, we want to make it possible to checkpoint down interfaces with
> a flag, but for now this helps prevent people from getting stuck on
> stock kernels with various unsupported and persistent virtual interfaces,
> such as tunnel devices.

But devices that are not IFF_UP can still have addresses associated
with them, wouldn't this cause those addresses to not be
checkpointed?

-Brian

Re: [PATCH] Skip down interfaces (v2)

[Serge E. Hallyn]

Quoting Brian Haley (brian.haley-VXdhtT5mjnY@public.gmane.org):
> Dan Smith wrote:
> > This makes the netns checkpoint code skip interfaces that are not up.
> > Later, we want to make it possible to checkpoint down interfaces with
> > a flag, but for now this helps prevent people from getting stuck on
> > stock kernels with various unsupported and persistent virtual interfaces,
> > such as tunnel devices.
> 
> But devices that are not IFF_UP can still have addresses associated
> with them, wouldn't this cause those addresses to not be
> checkpointed?

Only if it's a device that wouldn't have supported checkpoint anyway.
So if veth0 is down, it'll be checkpointed.  If tunl0 is up, checkpoint
will be refused.  But if tunl0 is down, then it'll just be ignored.
The container didn't want that jerk around anyway, it just couldn't
get rid of it.

-serge

Re: [PATCH] Skip down interfaces (v2)

[Dan Smith]

BH> But devices that are not IFF_UP can still have addresses
BH> associated with them, wouldn't this cause those addresses to not
BH> be checkpointed?

Yes.  This was discussed on IRC a bit as being a reasonable
compromise.  Right now, there are several pernet devices that could
show up in a new network namespace that will prevent a checkpoint from
completing.  Since you can't easily, and at runtime, get rid of these
devices without recompiling the kernel, you're kinda stuck.  So, we
discussed this as something that will let you out of that situation
for interfaces that are down.

We could, I suppose, inspect the interfaces a bit to see if they have
any addresses configured before we agree to skip them.  However, that
could falsely trigger a failure because of autoconf addresses, I would
think.

The plan is to have a "strict mode" flag that will attempt to
checkpoint and fail on down but unsupported interfaces to make sure we
don't save an inaccurate representation of the netns that the
application is in.

-- 
Dan Smith
IBM Linux Technology Center
email: danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org