[CRIU] [PATCH 1/2] net/sysctl: fix ipv4 forwarding

[Pavel Emelyanov]

On 06/10/2016 06:48 PM, Pavel Tikhomirov wrote:
> 
> 
> On 06/10/2016 06:08 PM, Pavel Emelyanov wrote:
>> On 06/06/2016 12:23 PM, Pavel Tikhomirov wrote:
>>> Restore all/accept_redirects and default/forwarding after
>>> all/forwarding as the last can influence the former two.
>>> (see inet_forward_change in kernel)
>>>
>>> # sysctl -w net.ipv4.conf.all.forwarding=1
>>> net.ipv4.conf.all.forwarding = 1
>>> # sysctl -w net.ipv4.conf.default.forwarding=1
>>> net.ipv4.conf.default.forwarding = 1
>>> # sysctl -w net.ipv4.conf.all.forwarding=0
>>> net.ipv4.conf.all.forwarding = 0
>>> # sysctl net.ipv4.conf.default.forwarding
>>> net.ipv4.conf.default.forwarding = 0
>>>
>>> Not to break image backward/forward compatibility just append
>>> another copy of "accept_redirects" to lists
>>>
>>> Trigered with netns-dev test in VZ7CT with vzlinux-6-x86_64 template
>>> https://jira.sw.ru/browse/PSBM-47355
>>>
>>> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>>> ---
>>>  criu/net.c | 10 ++++++----
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/criu/net.c b/criu/net.c
>>> index d57e0aa..4b30a35 100644
>>> --- a/criu/net.c
>>> +++ b/criu/net.c
>>> @@ -108,6 +108,7 @@ static char *devconfs4[] = {
>>>  	"ignore_routes_with_linkdown",
>>>  	"drop_gratuitous_arp",
>>>  	"drop_unicast_in_l2_multicast",
>>> +	"accept_redirects",
>>>  };
>>>
>>>  char *devconfs6[] = {
>>> @@ -153,6 +154,7 @@ char *devconfs6[] = {
>>>  	"use_oif_addrs_only",
>>>  	"use_optimistic",
>>>  	"use_tempaddr",
>>> +	"accept_redirects",
>>>  };
>>>
>>>  #define CONF_OPT_PATH "net/%s/conf/%s/%s"
>>> @@ -1327,18 +1329,18 @@ static int restore_netns_conf(int pid, NetnsEntry **netns)
>>>  	}
>>>
>>>  	if ((*netns)->def_conf4) {
>>> -		ret = ipv4_conf_op("default", (*netns)->def_conf4, (*netns)->n_def_conf4, CTL_WRITE, NULL);
>>> +		ret = ipv4_conf_op("all", (*netns)->all_conf4, (*netns)->n_all_conf4, CTL_WRITE, NULL);
>>>  		if (ret)
>>>  			goto out;
>>> -		ret = ipv4_conf_op("all", (*netns)->all_conf4, (*netns)->n_all_conf4, CTL_WRITE, NULL);
>>> +		ret = ipv4_conf_op("default", (*netns)->def_conf4, (*netns)->n_def_conf4, CTL_WRITE, NULL);
>>
>> But you also change the current all/default restore logic for everything.
>> Let's better add quirks for these sysctls as you did for mtu.
> 
> I think it is better to change here for all sysctls, as: 1) it will be 
> same order as in ipv6, 2) If someone makes something similar to 
> "forwarding" in kernel we will be ready 3)quirk for mtu just not 
> optimizes mtu restore, but here we will need to add second restore of 
> default forwarding...

OK, I don't mind flipping all/default restore, but keeping 2 accept_redirects
in list doesn't look great. Better quirk it in the code.

>>
>>>  		if (ret)
>>>  			goto out;
>>>  	} else if ((*netns)->def_conf) {
>>>  		/* Backward compatibility */
>>> -		ret = ipv4_conf_op_old("default", (*netns)->def_conf, (*netns)->n_def_conf, CTL_WRITE, NULL);
>>> +		ret = ipv4_conf_op_old("all", (*netns)->all_conf, (*netns)->n_all_conf, CTL_WRITE, NULL);
>>>  		if (ret)
>>>  			goto out;
>>> -		ret = ipv4_conf_op_old("all", (*netns)->all_conf, (*netns)->n_all_conf, CTL_WRITE, NULL);
>>> +		ret = ipv4_conf_op_old("default", (*netns)->def_conf, (*netns)->n_def_conf, CTL_WRITE, NULL);
>>>  		if (ret)
>>>  			goto out;
>>>  	}
>>>
>>
>