[CRIU] [PATCH criu v5 7/7] optimization: do not restore configs for devices if value is default

[Pavel Emelyanov]

On 03/31/2015 10:51 AM, Pavel Tikhomirov wrote:
> because namespace default options are set before devices creation,
> devices will gain default options(except lo).

OK, modulo two comments in this patch I like the set :)
Plz, fix the comments and I'll apply one.

> Signed-off-by: Pavel Tikhomirov <ptikhomirov at parallels.com>
> ---
>  net.c | 51 +++++++++++++++++++++++++++++++++------------------
>  1 file changed, 33 insertions(+), 18 deletions(-)
> 
> diff --git a/net.c b/net.c
> index e38cf78..0a0043c 100644
> --- a/net.c
> +++ b/net.c
> @@ -85,20 +85,29 @@ char *devconfs[] = {
>  #define NET_CONF_PATH "net/ipv4/conf"
>  #define MAX_CONF_OPT_PATH IFNAMSIZ+50
>  
> -static int ipv4_conf_op(char *tgt, int *conf, int op)
> +static int ipv4_conf_op(char *tgt, int *conf, int op, NetnsEntry **netns)
>  {
> -	int i;
> +	int i, shift = 0;
>  	int ret;
>  	struct sysctl_req req[ARRAY_SIZE(devconfs) + 1];
>  	char path[ARRAY_SIZE(devconfs)][MAX_CONF_OPT_PATH];
>  
>  	for (i = 0; devconfs[i]; i++) {
> +		/*
> +		 * If dev conf value is the same as default skip restoring it
> +		 */
> +		if (netns && conf[i] == (*netns)->def_conf[i]) {
> +			shift++;
> +			pr_debug("DEBUG Skip %s/%s, val =%d\n", tgt, devconfs[i], conf[i]);
> +			continue;
> +		}
> +
>  		sprintf(path[i], "%s/%s/%s", NET_CONF_PATH, tgt, devconfs[i]);
> -		req[i].name = path[i];
> -		req[i].arg = &conf[i];
> -		req[i].type = CTL_32;
> +		req[i - shift].name = path[i];

This i - shift is hard to parse (by eyes). Better introduce the "int ri" variable,
use it as index in req[] array and increment one once you put new entry in.

> +		req[i - shift].arg = &conf[i];
> +		req[i - shift].type = CTL_32;
>  	}
> -	req[i].name = NULL;
> +	req[i - shift].name = NULL;
>  
>  	ret = sysctl_op(req, op);
>  	if (ret < 0) {

> @@ -470,7 +479,12 @@ static int restore_links(int pid)
>  		}
>  
>  		if (nde->conf) {
> -			ret = ipv4_conf_op(nde->name, nde->conf, CTL_WRITE);
> +			/*
> +			 * lo is created with namespace and before default is set
> +			 * so we cant optimize its restore

I don't get what the optimization is. If the ret == 0 (it's lo) then we
pass NULL and make all devconfs to get restored.

> +			 */
> +			ret = strcmp(nde->name, "lo");

Loopback should be detected by nde->type == ND_TYPE__LOOPBACK, not by name.

> +			ret = ipv4_conf_op(nde->name, nde->conf, CTL_WRITE, ret == 0 ? NULL : netns);
>  			if (!ret)
>  				return ret;
>  		}