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

On Wed, Jun 20, 2018 at 10:45:39AM -0700, Cong Wang wrote:

On Tue, Jun 19, 2018 at 10:31 PM, David Miller [off-list ref] wrote:

quoted

From: Hangbin Liu <redacted>
Date: Wed, 20 Jun 2018 11:22:54 +0800

quoted

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