Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <cover.1552023000.git.tsu.yubo@gmail.com> <2fefb2a6962f668f76db79d2f9a2b866b077e42f.1552023000.git.tsu.yubo@gmail.com>
 <6018.1552027866@famine>
In-Reply-To: <6018.1552027866@famine>
From:   Bo YU <tsu.yubo@gmail.com>
Date:   Fri, 8 Mar 2019 16:28:44 +0800
Message-ID: <CAKq8=3KenqZO_O7H-K8ASzMxKUcZL4HMkS7JiU92ytOs=x4aug@mail.gmail.com>
Subject: Re: [PATCH 2/2] net: bonding: fix incorrect type in assignment
To:     Jay Vosburgh <jay.vosburgh@canonical.com>
Cc:     vfalico@gmail.com, Andy Gospodarek <andy@greyhouse.net>,
        David Miller <davem@davemloft.net>,
        Netdev <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        =?UTF-8?B?5LqO5rOi?= <yuzibode@126.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Mar 8, 2019 at 2:51 PM Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>
> Bo YU <tsu.yubo@gmail.com> wrote:
>
> >There are some warning when:
> >
> >sudo make C=1 CF=-D__CHECK_ENDIAN__ drivers/net/bonding/
> >
> >drivers/net/bonding/bond_main.c:2438:40: warning: incorrect type in assignment (different base types)
> >drivers/net/bonding/bond_main.c:2438:40:    expected restricted __be16 [usertype] vlan_proto
> >drivers/net/bonding/bond_main.c:2438:40:
> >...
> >rivers/net/bonding/bond_options.c:1089:24: warning: incorrect type in assignment (different base types)
> >drivers/net/bonding/bond_options.c:1089:24:    expected restricted __be32 [addressable] [usertype] target
> >drivers/net/bonding/bond_options.c:1089:24:    got unsigned long long const [usertype] value
> >
> >So fix it
> >
> >Signed-off-by: Bo YU <tsu.yubo@gmail.com>
> >---
> > drivers/net/bonding/bond_main.c    | 2 +-
> > drivers/net/bonding/bond_options.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> >index 135fec28daa9..07e52d863e91 100644
> >--- a/drivers/net/bonding/bond_main.c
> >+++ b/drivers/net/bonding/bond_main.c
> >@@ -2435,7 +2435,7 @@ struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
> >               tags = kcalloc(level + 1, sizeof(*tags), GFP_ATOMIC);
> >               if (!tags)
> >                       return ERR_PTR(-ENOMEM);
> >-              tags[level].vlan_proto = VLAN_N_VID;
> >+              tags[level].vlan_proto = cpu_to_be16(VLAN_N_VID);
> >               return tags;
> >       }
> >
> >diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> >index da1fc17295d9..3a196999bd1b 100644
> >--- a/drivers/net/bonding/bond_options.c
> >+++ b/drivers/net/bonding/bond_options.c
> >@@ -1086,7 +1086,7 @@ static int bond_option_arp_ip_targets_set(struct bonding *bond,
> >               else
> >                       netdev_err(bond->dev, "no command found in arp_ip_targets file - use +<addr> or -<addr>\n");
> >       } else {
> >-              target = newval->value;
> >+              target = cpu_to_be32(newval->value);
> >               ret = bond_option_arp_ip_target_add(bond, target);
>
>         I'm not sure this is correct; if I'm reading the call path
> correctly, bond_changelink will
>
>         if (data[IFLA_BOND_ARP_IP_TARGET]) {
> [...]
>                         __be32 target;
> [...]
>                         target = nla_get_be32(attr);
>
>                         bond_opt_initval(&newval, (__force u64)target);
>                         err = __bond_opt_set(bond, BOND_OPT_ARP_TARGETS,
>                                              &newval);
>
>         thus, newval.value is initially be32, but stored in a u64.
> __bond_opt_set will call bond_opt_parse, which in turn will call
> bond_option_arp_ip_targets_set (via .set), and the change above would
> swap the newval.value back to host order (on little endian architectures
> for which cpu_to_be32 is not a no-op).
>
>         Am I misunderstanding?  Did you test this change on an x86 or
> other little endian system?
Hi,
After above patch:
I have enable BONDING config in .config and dmesg:

[    0.247417] Ethernet Channel Bonding Driver: v3.7.1 (April 27, 2011)

I don't have the HW, so just add printfk in  bond_option_arp_ip_targets_set
However, i don't get what i want to print.
I do the test in qemu-system-x86_64.
I do not know how to hit the driver maybe.
So, not confirmed :(
>
>         -J
>
> ---
>         -Jay Vosburgh, jay.vosburgh@canonical.com