From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hangbin Liu <liuhangbin@gmail.com>
Subject: Re: [PATCH net] ipvlan: call dev_change_flags when reset ipvlan mode
Date: Thu, 21 Jun 2018 09:18:43 +0800
Message-ID: <20180621011843.GX8958@leo.usersys.redhat.com>
References: <1529330677-15328-1-git-send-email-liuhangbin@gmail.com>
 <CAM_iQpXeuU=Cxons5=centGNJzm0OaNU3Jj5hE91hvJH0o2-Eg@mail.gmail.com>
 <20180620032254.GW8958@leo.usersys.redhat.com>
 <20180620.143147.2291173423483856091.davem@davemloft.net>
 <CAM_iQpVdCJ2wjujco0+xZ_Jjeyu-qZ_g+8cVHv9LmUARhJmMoQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: David Miller <davem@davemloft.net>,
        Linux Kernel Network Developers <netdev@vger.kernel.org>,
        Stefano Brivio <sbrivio@redhat.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Mahesh Bandewar <maheshb@google.com>
To: Cong Wang <xiyou.wangcong@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pl0-f67.google.com ([209.85.160.67]:46359 "EHLO
        mail-pl0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1754061AbeFUBSz (ORCPT
        <rfc822;netdev@vger.kernel.org>); Wed, 20 Jun 2018 21:18:55 -0400
Received: by mail-pl0-f67.google.com with SMTP id 30-v6so700635pld.13
        for <netdev@vger.kernel.org>; Wed, 20 Jun 2018 18:18:54 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <CAM_iQpVdCJ2wjujco0+xZ_Jjeyu-qZ_g+8cVHv9LmUARhJmMoQ@mail.gmail.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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