Re: [PATCH net 1/2] ipv6: add missing netconf notif when 'all' is updated

[Nicolas Dichtel]

Le 30/08/2016 à 04:51, YOSHIFUJI Hideaki a écrit :
> 
> 
> Nicolas Dichtel wrote:
[snip]
>> +int old_dftl = net->ipv6.devconf_dflt->forwarding;
> 
> dflt, not dftl?
Good catch!

Re: [PATCH net 1/2] ipv6: add missing netconf notif when 'all' is updated

[YOSHIFUJI Hideaki]

Nicolas Dichtel wrote:
> The 'default' value was not advertised.
> 
> Fixes: f3a1bfb11ccb ("rtnl/ipv6: use netconf msg to advertise forwarding 
> status")
> Signed-off-by: Nicolas Dichtel 
> ---
>  net/ipv6/addrconf.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index f418d2eaeddd..299f0656e87f 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -778,7 +778,14 @@ static int addrconf_fixup_forwarding(struct ctl_table 
> *table, int *p, int newf)
>   }
>  
>   if (p == >ipv6.devconf_all->forwarding) {
> + int old_dftl = net->ipv6.devconf_dflt->forwarding;

dflt, not dftl?

> +
>   net->ipv6.devconf_dflt->forwarding = newf;
> + if ((!newf) ^ (!old_dftl))
> + inet6_netconf_notify_devconf(net, NETCONFA_FORWARDING,
> +  NETCONFA_IFINDEX_DEFAULT,
> +  net->ipv6.devconf_dflt);
> +
>   addrconf_forward_change(net, newf);
>   if ((!newf) ^ (!old))
>   inet6_netconf_notify_devconf(net, NETCONFA_FORWARDING,
> 

-- 
Hideaki Yoshifuji 
Technical Division, MIRACLE LINUX CORPORATION

Re: [PATCH net 1/2] ipv6: add missing netconf notif when 'all' is updated

[Sergei Shtylyov]

On 08/29/2016 05:13 PM, 吉藤英明 wrote:


The 'default' value was not advertised.

Fixes: f3a1bfb11ccb ("rtnl/ipv6: use netconf msg to advertise forwarding
status")
Signed-off-by: Nicolas Dichtel 
---
 net/ipv6/addrconf.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f418d2eaeddd..299f0656e87f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -778,7 +778,14 @@ static int addrconf_fixup_forwarding(struct ctl_table
*table, int *p, int newf)
}

if (p == >ipv6.devconf_all->forwarding) {
+   int old_dftl = net->ipv6.devconf_dflt->forwarding;
+
net->ipv6.devconf_dflt->forwarding = newf;
+   if ((!newf) ^ (!old_dftl))



   IIUC, !'s are not necessary here (and more so the parens around them).
And perhaps ^ can be changed to != for clarity...


No, it will break.


   Well, if those variables are actually bit masks, ! seem to be needed 
indeed. But ^ can be replaced by != anyway.



--yoshfuji


MBR, Sergei

Re: [PATCH net 1/2] ipv6: add missing netconf notif when 'all' is updated

[吉藤英明]

Hi,

2016-08-29 22:00 GMT+09:00 Sergei Shtylyov :
> Hello.
>
> On 8/29/2016 1:05 PM, Nicolas Dichtel wrote:
>
>> The 'default' value was not advertised.
>>
>> Fixes: f3a1bfb11ccb ("rtnl/ipv6: use netconf msg to advertise forwarding
>> status")
>> Signed-off-by: Nicolas Dichtel 
>> ---
>>  net/ipv6/addrconf.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index f418d2eaeddd..299f0656e87f 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -778,7 +778,14 @@ static int addrconf_fixup_forwarding(struct ctl_table
>> *table, int *p, int newf)
>> }
>>
>> if (p == >ipv6.devconf_all->forwarding) {
>> +   int old_dftl = net->ipv6.devconf_dflt->forwarding;
>> +
>> net->ipv6.devconf_dflt->forwarding = newf;
>> +   if ((!newf) ^ (!old_dftl))
>
>
>IIUC, !'s are not necessary here (and more so the parens around them).
> And perhaps ^ can be changed to != for clarity...

No, it will break.

--yoshfuji

Re: [PATCH net 1/2] ipv6: add missing netconf notif when 'all' is updated

[Nicolas Dichtel]

Le 29/08/2016 à 15:00, Sergei Shtylyov a écrit :
[snip]
>>  if (p == >ipv6.devconf_all->forwarding) {
>> +int old_dftl = net->ipv6.devconf_dflt->forwarding;
>> +
>>  net->ipv6.devconf_dflt->forwarding = newf;
>> +if ((!newf) ^ (!old_dftl))
> 
>IIUC, !'s are not necessary here (and more so the parens around them). And
> perhaps ^ can be changed to != for clarity...
Yes, but a lot of places in this code use that. So for consistency I use the 
same.

> 
>> +inet6_netconf_notify_devconf(net, NETCONFA_FORWARDING,
>> + NETCONFA_IFINDEX_DEFAULT,
>> + net->ipv6.devconf_dflt);
>> +
>>  addrconf_forward_change(net, newf);
>>  if ((!newf) ^ (!old))
Here is an example.

Re: [PATCH net 1/2] ipv6: add missing netconf notif when 'all' is updated

[Sergei Shtylyov]

Hello.

On 8/29/2016 1:05 PM, Nicolas Dichtel wrote:


The 'default' value was not advertised.

Fixes: f3a1bfb11ccb ("rtnl/ipv6: use netconf msg to advertise forwarding 
status")
Signed-off-by: Nicolas Dichtel 
---
 net/ipv6/addrconf.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f418d2eaeddd..299f0656e87f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -778,7 +778,14 @@ static int addrconf_fixup_forwarding(struct ctl_table 
*table, int *p, int newf)
}

if (p == >ipv6.devconf_all->forwarding) {
+   int old_dftl = net->ipv6.devconf_dflt->forwarding;
+
net->ipv6.devconf_dflt->forwarding = newf;
+   if ((!newf) ^ (!old_dftl))


   IIUC, !'s are not necessary here (and more so the parens around them). And 
perhaps ^ can be changed to != for clarity...



+   inet6_netconf_notify_devconf(net, NETCONFA_FORWARDING,
+NETCONFA_IFINDEX_DEFAULT,
+net->ipv6.devconf_dflt);
+
addrconf_forward_change(net, newf);
if ((!newf) ^ (!old))
inet6_netconf_notify_devconf(net, NETCONFA_FORWARDING,


MBR, Sergei