On Fri, Mar 8, 2019 at 2:51 PM Jay Vosburgh <jay.vosbu...@canonical.com> wrote:
>
> Bo YU <tsu.y...@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.y...@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.vosbu...@canonical.com
