Re: [PATCH net] ipvlan: call dev_change_flags when reset ipvlan mode

[Hangbin Liu <liuhangbin@gmail.com>]

On Wed, Jun 20, 2018 at 10:45:39AM -0700, Cong Wang wrote:
> On Tue, Jun 19, 2018 at 10:31 PM, David Miller <davem@davemloft.net> wrote:
> > From: Hangbin Liu <liuhangbin@gmail.com>
> > Date: Wed, 20 Jun 2018 11:22:54 +0800
> >
> >> The only case dev_change_flags() return an err is when we change IFF_UP flag.
> >> Since we only set/reset IFF_NOARP, do you think we still need to check the
> >> return value?
> >
> > It is bad to try and take shortcuts on error handling using assumptions
> > like that.
> >
> > If dev_change_flags() is adjusted to return error codes in more
> > situations, nobody is going to remember to undo your "optimziation"
> > here.
> >
> > Please check for errors, thank you.
> 
> Yeah. Also since the notifier is triggered in this case:
> 
>         if (dev->flags & IFF_UP &&
>             (changes & ~(IFF_UP | IFF_PROMISC | IFF_ALLMULTI | IFF_VOLATILE))) {
>                 struct netdev_notifier_change_info change_info = {
>                         .info = {
>                                 .dev = dev,
>                         },
>                         .flags_changed = changes,
>                 };
> 
>                 call_netdevice_notifiers_info(NETDEV_CHANGE, &change_info.info);
>         }
> 
> the return value of call_netdevice_notifiers_info() isn't captured
> either, but it should be.

Thanks for the explanation. I will fix it.

Regards
Hangbin