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;
From:   Jay Vosburgh <jay.vosburgh@canonical.com>
To:     Bo YU <tsu.yubo@gmail.com>
cc:     vfalico@gmail.com, andy@greyhouse.net, davem@davemloft.net,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        yuzibode@126.com
Subject: Re: [PATCH 1/2] net: bonding: fix restricted __be16 degrades to integer
In-reply-to: <2a069375f8393bf05a44669e00281714063a8530.1552023000.git.tsu.yubo@gmail.com>
References: <cover.1552023000.git.tsu.yubo@gmail.com> <2a069375f8393bf05a44669e00281714063a8530.1552023000.git.tsu.yubo@gmail.com>
Comments: In-reply-to Bo YU <tsu.yubo@gmail.com>
   message dated "Fri, 08 Mar 2019 00:33:50 -0500."
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <24215.1552078486.1@famine>
Content-Transfer-Encoding: 8BIT
Date:   Fri, 08 Mar 2019 12:54:46 -0800
Message-ID: <24216.1552078486@famine>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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:2385:26: warning: restricted __be16 degrades to integer
>drivers/net/bonding/bond_main.c:2391:20: warning: restricted __be16 degrades to integer
>...
>drivers/net/bonding/bond_main.c:3241:60: warning: restricted __be16 degrades to integer
>drivers/net/bonding/bond_main.c:3241:60: warning: restricted __be16 degrades to integer
>
>So fix it.
>
>Signed-off-by: Bo YU <tsu.yubo@gmail.com>
>---
> drivers/net/bonding/bond_main.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index b59708c35faf..135fec28daa9 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2382,13 +2382,13 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
> 		return;
> 	}
> 
>-	if (!tags || tags->vlan_proto == VLAN_N_VID)
>+	if (!tags || be16_to_cpu(tags->vlan_proto) == VLAN_N_VID)
> 		goto xmit;
> 
> 	tags++;
> 
> 	/* Go through all the tags backwards and add them to the packet */
>-	while (tags->vlan_proto != VLAN_N_VID) {
>+	while (be16_to_cpu(tags->vlan_proto) != VLAN_N_VID) {

	I believe both of the above are incorrect, as vlan_proto is set
explicitly to VLAN_N_VID (in host byte order) by bond_verify_device_path
as a sentinel value.  Byte swapping the tags->vlan_proto value would
cause the test or loop to miss the sentinel.


> 		if (!tags->vlan_id) {
> 			tags++;
> 			continue;
>@@ -3238,7 +3238,7 @@ static inline u32 bond_eth_hash(struct sk_buff *skb)
> 
> 	ep = skb_header_pointer(skb, 0, sizeof(hdr_tmp), &hdr_tmp);
> 	if (ep)
>-		return ep->h_dest[5] ^ ep->h_source[5] ^ ep->h_proto;
>+		return ep->h_dest[5] ^ ep->h_source[5] ^ be16_to_cpu(ep->h_proto);

	This is probably harmless, other than adding work to the
transmit path.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com