From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jay Vosburgh Subject: Re: [PATCH net-next 2/4] bonding: use common mac addr checks Date: Fri, 11 May 2018 13:53:58 -0700 Message-ID: <4921.1526072038@famine> References: <20180511192548.8119-1-dbanerje@akamai.com> <20180511192548.8119-3-dbanerje@akamai.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Cc: "David S . Miller" , netdev@vger.kernel.org, Veaceslav Falico , Andy Gospodarek To: Debabrata Banerjee Return-path: Received: from youngberry.canonical.com ([91.189.89.112]:49841 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750876AbeEKUyF (ORCPT ); Fri, 11 May 2018 16:54:05 -0400 In-reply-to: <20180511192548.8119-3-dbanerje@akamai.com> Content-ID: <4920.1526072038.1@famine> Sender: netdev-owner@vger.kernel.org List-ID: Debabrata Banerjee wrote: >Replace homegrown mac addr checks with faster defs from etherdevice.h > >Signed-off-by: Debabrata Banerjee >--- > drivers/net/bonding/bond_alb.c | 28 +++++++++------------------- > 1 file changed, 9 insertions(+), 19 deletions(-) > >diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c >index c2f6c58e4e6a..180e50f7806f 100644 >--- a/drivers/net/bonding/bond_alb.c >+++ b/drivers/net/bonding/bond_alb.c >@@ -40,11 +40,6 @@ > #include > #include > >- >- >-static const u8 mac_bcast[ETH_ALEN + 2] __long_aligned = { >- 0xff, 0xff, 0xff, 0xff, 0xff, 0xff >-}; > static const u8 mac_v6_allmcast[ETH_ALEN + 2] __long_aligned = { > 0x33, 0x33, 0x00, 0x00, 0x00, 0x01 > }; >@@ -420,9 +415,7 @@ static void rlb_clear_slave(struct bonding *bond, struct slave *slave) > > if (assigned_slave) { > rx_hash_table[index].slave = assigned_slave; >- if (!ether_addr_equal_64bits(rx_hash_table[index].mac_dst, >- mac_bcast) && >- !is_zero_ether_addr(rx_hash_table[index].mac_dst)) { >+ if (is_valid_ether_addr(rx_hash_table[index].mac_dst)) { This change and the similar ones below will now fail non-broadcast multicast Ethernet addresses, where the prior code would not. Is this an intentional change? -J > bond_info->rx_hashtbl[index].ntt = 1; > bond_info->rx_ntt = 1; > /* A slave has been removed from the >@@ -525,8 +518,7 @@ static void rlb_req_update_slave_clients(struct bonding *bond, struct slave *sla > client_info = &(bond_info->rx_hashtbl[hash_index]); > > if ((client_info->slave == slave) && >- !ether_addr_equal_64bits(client_info->mac_dst, mac_bcast) && >- !is_zero_ether_addr(client_info->mac_dst)) { >+ is_valid_ether_addr(client_info->mac_dst)) { > client_info->ntt = 1; > ntt = 1; > } >@@ -567,8 +559,7 @@ static void rlb_req_update_subnet_clients(struct bonding *bond, __be32 src_ip) > if ((client_info->ip_src == src_ip) && > !ether_addr_equal_64bits(client_info->slave->dev->dev_addr, > bond->dev->dev_addr) && >- !ether_addr_equal_64bits(client_info->mac_dst, mac_bcast) && >- !is_zero_ether_addr(client_info->mac_dst)) { >+ is_valid_ether_addr(client_info->mac_dst)) { > client_info->ntt = 1; > bond_info->rx_ntt = 1; > } >@@ -596,7 +587,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon > if ((client_info->ip_src == arp->ip_src) && > (client_info->ip_dst == arp->ip_dst)) { > /* the entry is already assigned to this client */ >- if (!ether_addr_equal_64bits(arp->mac_dst, mac_bcast)) { >+ if (!is_broadcast_ether_addr(arp->mac_dst)) { > /* update mac address from arp */ > ether_addr_copy(client_info->mac_dst, arp->mac_dst); > } >@@ -644,8 +635,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon > ether_addr_copy(client_info->mac_src, arp->mac_src); > client_info->slave = assigned_slave; > >- if (!ether_addr_equal_64bits(client_info->mac_dst, mac_bcast) && >- !is_zero_ether_addr(client_info->mac_dst)) { >+ if (is_valid_ether_addr(client_info->mac_dst)) { > client_info->ntt = 1; > bond->alb_info.rx_ntt = 1; > } else { >@@ -1418,9 +1408,9 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev) > case ETH_P_IP: { > const struct iphdr *iph = ip_hdr(skb); > >- if (ether_addr_equal_64bits(eth_data->h_dest, mac_bcast) || >- (iph->daddr == ip_bcast) || >- (iph->protocol == IPPROTO_IGMP)) { >+ if (is_broadcast_ether_addr(eth_data->h_dest) || >+ iph->daddr == ip_bcast || >+ iph->protocol == IPPROTO_IGMP) { > do_tx_balance = false; > break; > } >@@ -1432,7 +1422,7 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev) > /* IPv6 doesn't really use broadcast mac address, but leave > * that here just in case. > */ >- if (ether_addr_equal_64bits(eth_data->h_dest, mac_bcast)) { >+ if (is_broadcast_ether_addr(eth_data->h_dest)) { > do_tx_balance = false; > break; > } >-- >2.17.0 >