public inbox for b43-dev@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals
@ 2018-03-31  7:05 Joe Perches
  2018-03-31  7:05 ` [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address Joe Perches
  2018-04-05 13:27 ` [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals Felix Fietkau
  0 siblings, 2 replies; 9+ messages in thread
From: Joe Perches @ 2018-03-31  7:05 UTC (permalink / raw)
  To: netdev, linux-wireless, b43-dev, bridge, netfilter-devel,
	coreteam
  Cc: linux-kernel, brcm80211-dev-list.pdl, brcm80211-dev-list

There are many local static and non-static arrays that are used for
Ethernet broadcast address output or comparison.

Centralize the array into a single separate file and remove the local
arrays.

Joe Perches (12):
  ethernet: Add generic ether_<foo>_addr addresses
  treewide/net: Rename eth_stp_addr to ether_stp_addr
  net: mac80211: Use global ether_broadcast_addr
  bridge: netfilter: Use the new global ether_<foo>_addr arrays
  net: atm: Use ether_broadcast_addr
  wireless: Convert simple uses of a static const Ethernet broadcast address
  brcmfmac: Convert ALLFFMAC to ether_broadcast_addr
  iwlegacy: Remove EXPORT_SYMBOL(il_bcast_addr) and use ether_broadcast_addr
  iwlwifi: Remove local iwl_bcast_addr and use ether_broadcast_addr
  mvpp2: Use ether_broadcast_addr instead of a local array
  qlogic: Convert local bcast_addr to global ether_broadcast_addr
  ethernet: Use ether_zero_addr instead of local statics

 drivers/net/dsa/lan9303-core.c                        |  4 ++--
 drivers/net/ethernet/broadcom/b44.c                   |  5 ++---
 drivers/net/ethernet/freescale/gianfar.c              |  3 +--
 drivers/net/ethernet/marvell/mvpp2.c                  |  4 +---
 drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c    |  5 +----
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c        |  6 ++----
 .../net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c  |  6 ++----
 drivers/net/wireless/admtek/adm8211.c                 |  3 +--
 drivers/net/wireless/ath/carl9170/mac.c               |  4 +---
 drivers/net/wireless/broadcom/b43/main.c              |  3 +--
 .../net/wireless/broadcom/brcm80211/brcmfmac/common.c |  2 --
 .../net/wireless/broadcom/brcm80211/brcmfmac/common.h |  2 --
 .../wireless/broadcom/brcm80211/brcmfmac/flowring.c   |  8 ++++----
 drivers/net/wireless/intel/iwlegacy/3945-mac.c        |  2 +-
 drivers/net/wireless/intel/iwlegacy/4965-mac.c        |  2 +-
 drivers/net/wireless/intel/iwlegacy/common.c          |  3 ---
 drivers/net/wireless/intel/iwlegacy/common.h          |  1 -
 drivers/net/wireless/intel/iwlwifi/dvm/dev.h          |  1 -
 drivers/net/wireless/intel/iwlwifi/dvm/scan.c         |  2 +-
 drivers/net/wireless/intel/iwlwifi/dvm/sta.c          |  4 +---
 drivers/net/wireless/marvell/mwifiex/cfg80211.c       |  3 +--
 drivers/net/wireless/realtek/rtlwifi/core.c           |  5 ++---
 drivers/net/wireless/rndis_wlan.c                     |  6 +-----
 drivers/net/wireless/ti/wl1251/main.c                 |  5 +----
 drivers/net/wireless/ti/wlcore/main.c                 |  5 +----
 include/linux/etherdevice.h                           | 13 +++++++++----
 net/atm/lec.c                                         | 12 +++++-------
 net/bridge/br_device.c                                |  4 ++--
 net/bridge/netfilter/ebt_stp.c                        |  6 ++----
 net/ethernet/Makefile                                 |  2 +-
 net/ethernet/ether_addrs.c                            | 19 +++++++++++++++++++
 net/mac80211/iface.c                                  |  5 +----
 net/mac80211/key.c                                    |  6 ++----
 net/mac80211/mesh_hwmp.c                              | 19 ++++++++++---------
 net/mac80211/mesh_pathtbl.c                           |  8 ++++----
 35 files changed, 83 insertions(+), 105 deletions(-)
 create mode 100644 net/ethernet/ether_addrs.c

-- 
2.15.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address
  2018-03-31  7:05 [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals Joe Perches
@ 2018-03-31  7:05 ` Joe Perches
  2018-03-31 14:01   ` Pkshih
  2018-04-05 12:39   ` Kalle Valo
  2018-04-05 13:27 ` [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals Felix Fietkau
  1 sibling, 2 replies; 9+ messages in thread
From: Joe Perches @ 2018-03-31  7:05 UTC (permalink / raw)
  To: Christian Lamparter, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, Ping-Ke Shih, Jussi Kivilinna,
	Kalle Valo
  Cc: linux-wireless, netdev, linux-kernel, b43-dev

Use the new ether_broadcast_addr global instead to save some object code.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/wireless/admtek/adm8211.c           | 3 +--
 drivers/net/wireless/ath/carl9170/mac.c         | 4 +---
 drivers/net/wireless/broadcom/b43/main.c        | 3 +--
 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 3 +--
 drivers/net/wireless/realtek/rtlwifi/core.c     | 5 ++---
 drivers/net/wireless/rndis_wlan.c               | 6 +-----
 drivers/net/wireless/ti/wl1251/main.c           | 5 +----
 drivers/net/wireless/ti/wlcore/main.c           | 5 +----
 8 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/admtek/adm8211.c b/drivers/net/wireless/admtek/adm8211.c
index 3b0802fc5bf5..0aa2d114d5b3 100644
--- a/drivers/net/wireless/admtek/adm8211.c
+++ b/drivers/net/wireless/admtek/adm8211.c
@@ -1354,7 +1354,6 @@ static void adm8211_configure_filter(struct ieee80211_hw *dev,
 				     unsigned int *total_flags,
 				     u64 multicast)
 {
-	static const u8 bcast[ETH_ALEN] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
 	struct adm8211_priv *priv = dev->priv;
 	unsigned int new_flags;
 	u32 mc_filter[2];
@@ -1385,7 +1384,7 @@ static void adm8211_configure_filter(struct ieee80211_hw *dev,
 		__clear_bit(IEEE80211_HW_RX_INCLUDES_FCS, dev->flags);
 
 	if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
-		adm8211_set_bssid(dev, bcast);
+		adm8211_set_bssid(dev, ether_broadcast_addr);
 	else
 		adm8211_set_bssid(dev, priv->bssid);
 
diff --git a/drivers/net/wireless/ath/carl9170/mac.c b/drivers/net/wireless/ath/carl9170/mac.c
index 7d4a72dc98db..b1c608db047c 100644
--- a/drivers/net/wireless/ath/carl9170/mac.c
+++ b/drivers/net/wireless/ath/carl9170/mac.c
@@ -476,10 +476,8 @@ int carl9170_upload_key(struct ar9170 *ar, const u8 id, const u8 *mac,
 			const int keylen)
 {
 	struct carl9170_set_key_cmd key = { };
-	static const u8 bcast[ETH_ALEN] = {
-		0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
-	mac = mac ? : bcast;
+	mac = mac ? : ether_broadcast_addr;
 
 	key.user = cpu_to_le16(id);
 	key.keyId = cpu_to_le16(keyidx);
diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c
index b37e7391f55d..f23d60064691 100644
--- a/drivers/net/wireless/broadcom/b43/main.c
+++ b/drivers/net/wireless/broadcom/b43/main.c
@@ -4185,7 +4185,6 @@ static int b43_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	u8 algorithm;
 	u8 index;
 	int err;
-	static const u8 bcast_addr[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
 	if (modparam_nohwcrypt)
 		return -ENOSPC; /* User disabled HW-crypto */
@@ -4293,7 +4292,7 @@ static int b43_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		b43dbg(wl, "%s hardware based encryption for keyidx: %d, "
 		       "mac: %pM\n",
 		       cmd == SET_KEY ? "Using" : "Disabling", key->keyidx,
-		       sta ? sta->addr : bcast_addr);
+		       sta ? sta->addr : ether_broadcast_addr);
 		b43_dump_keymemory(dev);
 	}
 	mutex_unlock(&wl->mutex);
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 7f7e9de2db1c..400a4a67d545 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -174,7 +174,6 @@ mwifiex_cfg80211_del_key(struct wiphy *wiphy, struct net_device *netdev,
 static int
 mwifiex_form_mgmt_frame(struct sk_buff *skb, const u8 *buf, size_t len)
 {
-	u8 addr[ETH_ALEN] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
 	u16 pkt_len;
 	u32 tx_control = 0, pkt_type = PKT_TYPE_MGMT;
 
@@ -191,7 +190,7 @@ mwifiex_form_mgmt_frame(struct sk_buff *skb, const u8 *buf, size_t len)
 
 	/* Add packet data and address4 */
 	skb_put_data(skb, buf, sizeof(struct ieee80211_hdr_3addr));
-	skb_put_data(skb, addr, ETH_ALEN);
+	skb_put_data(skb, ether_broadcast_addr, ETH_ALEN);
 	skb_put_data(skb, buf + sizeof(struct ieee80211_hdr_3addr),
 		     len - sizeof(struct ieee80211_hdr_3addr));
 
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c
index cfea57efa7f4..8c534a93dad5 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -1527,7 +1527,6 @@ static int rtl_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	bool wep_only = false;
 	int err = 0;
 	u8 mac_addr[ETH_ALEN];
-	u8 bcast_addr[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
 	rtlpriv->btcoexist.btc_info.in_4way = false;
 
@@ -1544,7 +1543,7 @@ static int rtl_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	RT_TRACE(rtlpriv, COMP_SEC, DBG_DMESG,
 		 "%s hardware based encryption for keyidx: %d, mac: %pM\n",
 		  cmd == SET_KEY ? "Using" : "Disabling", key->keyidx,
-		  sta ? sta->addr : bcast_addr);
+		  sta ? sta->addr : ether_broadcast_addr);
 	rtlpriv->sec.being_setkey = true;
 	rtl_ips_nic_on(hw);
 	mutex_lock(&rtlpriv->locks.conf_mutex);
@@ -1649,7 +1648,7 @@ static int rtl_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 			memcpy(rtlpriv->sec.key_buf[key_idx],
 			       key->key, key->keylen);
 			rtlpriv->sec.key_len[key_idx] = key->keylen;
-			memcpy(mac_addr, bcast_addr, ETH_ALEN);
+			memcpy(mac_addr, ether_broadcast_addr, ETH_ALEN);
 		} else {	/* pairwise key */
 			RT_TRACE(rtlpriv, COMP_SEC, DBG_DMESG,
 				 "set pairwise key\n");
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 9935bd09db1f..da0c4f790dd0 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1021,11 +1021,7 @@ static int set_bssid(struct usbnet *usbdev, const u8 *bssid)
 
 static int clear_bssid(struct usbnet *usbdev)
 {
-	static const u8 broadcast_mac[ETH_ALEN] = {
-		0xff, 0xff, 0xff, 0xff, 0xff, 0xff
-	};
-
-	return set_bssid(usbdev, broadcast_mac);
+	return set_bssid(usbdev, ether_broadcast_addr);
 }
 
 static int get_bssid(struct usbnet *usbdev, u8 bssid[ETH_ALEN])
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
index bd8641ad953b..442a557ad2e2 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -887,9 +887,6 @@ static int wl1251_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	const u8 *addr;
 	int ret;
 
-	static const u8 bcast_addr[ETH_ALEN] =
-		{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
-
 	wl1251_debug(DEBUG_MAC80211, "mac80211 set key");
 
 	wl_cmd = kzalloc(sizeof(*wl_cmd), GFP_KERNEL);
@@ -898,7 +895,7 @@ static int wl1251_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		goto out;
 	}
 
-	addr = sta ? sta->addr : bcast_addr;
+	addr = sta ? sta->addr : ether_broadcast_addr;
 
 	wl1251_debug(DEBUG_CRYPT, "CMD: 0x%x", cmd);
 	wl1251_dump(DEBUG_CRYPT, "ADDR: ", addr, ETH_ALEN);
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 3a51ab116e79..a4a408f48eeb 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3392,11 +3392,8 @@ static int wl1271_set_key(struct wl1271 *wl, struct wl12xx_vif *wlvif,
 			return ret;
 	} else {
 		const u8 *addr;
-		static const u8 bcast_addr[ETH_ALEN] = {
-			0xff, 0xff, 0xff, 0xff, 0xff, 0xff
-		};
 
-		addr = sta ? sta->addr : bcast_addr;
+		addr = sta ? sta->addr : ether_broadcast_addr;
 
 		if (is_zero_ether_addr(addr)) {
 			/* We dont support TX only encryption */
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address
  2018-03-31  7:05 ` [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address Joe Perches
@ 2018-03-31 14:01   ` Pkshih
  2018-03-31 14:33     ` Joe Perches
  2018-04-05 12:39   ` Kalle Valo
  1 sibling, 1 reply; 9+ messages in thread
From: Pkshih @ 2018-03-31 14:01 UTC (permalink / raw)
  To: huxm@marvell.com, gbhat@marvell.com, jussi.kivilinna@iki.fi,
	nishants@marvell.com, kvalo@codeaurora.org, joe@perches.com,
	amitkarwar@gmail.com, chunkeey@googlemail.com
  Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, b43-dev@lists.infradead.org

On Sat, 2018-03-31 at 00:05 -0700, Joe Perches wrote:
> Use the new ether_broadcast_addr global instead to save some object code.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> ?drivers/net/wireless/admtek/adm8211.c???????????| 3 +--
> ?drivers/net/wireless/ath/carl9170/mac.c?????????| 4 +---
> ?drivers/net/wireless/broadcom/b43/main.c????????| 3 +--
> ?drivers/net/wireless/marvell/mwifiex/cfg80211.c | 3 +--
> ?drivers/net/wireless/realtek/rtlwifi/core.c?????| 5 ++---
> ?drivers/net/wireless/rndis_wlan.c???????????????| 6 +-----
> ?drivers/net/wireless/ti/wl1251/main.c???????????| 5 +----
> ?drivers/net/wireless/ti/wlcore/main.c???????????| 5 +----
> ?8 files changed, 9 insertions(+), 25 deletions(-)
> 
> ?
> diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c
> b/drivers/net/wireless/realtek/rtlwifi/core.c
> index cfea57efa7f4..8c534a93dad5 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/core.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/core.c
> @@ -1527,7 +1527,6 @@ static int rtl_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> ?	bool wep_only = false;
> ?	int err = 0;
> ?	u8 mac_addr[ETH_ALEN];
> -	u8 bcast_addr[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
> ?
> ?	rtlpriv->btcoexist.btc_info.in_4way = false;
> ?
> @@ -1544,7 +1543,7 @@ static int rtl_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> ?	RT_TRACE(rtlpriv, COMP_SEC, DBG_DMESG,
> ?		?"%s hardware based encryption for keyidx: %d, mac: %pM\n",
> ?		??cmd == SET_KEY ? "Using" : "Disabling", key->keyidx,
> -		??sta ? sta->addr : bcast_addr);
> +		??sta ? sta->addr : ether_broadcast_addr);
> ?	rtlpriv->sec.being_setkey = true;
> ?	rtl_ips_nic_on(hw);
> ?	mutex_lock(&rtlpriv->locks.conf_mutex);
> @@ -1649,7 +1648,7 @@ static int rtl_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> ?			memcpy(rtlpriv->sec.key_buf[key_idx],
> ?			???????key->key, key->keylen);
> ?			rtlpriv->sec.key_len[key_idx] = key->keylen;
> -			memcpy(mac_addr, bcast_addr, ETH_ALEN);
> +			memcpy(mac_addr, ether_broadcast_addr, ETH_ALEN);

Use?ether_addr_copy(mac_addr, ether_broadcast_addr) ?


> ?		} else {	/* pairwise key */
> ?			RT_TRACE(rtlpriv, COMP_SEC, DBG_DMESG,
> ?				?"set pairwise key\n");

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address
  2018-03-31 14:01   ` Pkshih
@ 2018-03-31 14:33     ` Joe Perches
  0 siblings, 0 replies; 9+ messages in thread
From: Joe Perches @ 2018-03-31 14:33 UTC (permalink / raw)
  To: Pkshih, huxm@marvell.com, gbhat@marvell.com,
	jussi.kivilinna@iki.fi, nishants@marvell.com,
	kvalo@codeaurora.org, amitkarwar@gmail.com,
	chunkeey@googlemail.com
  Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, b43-dev@lists.infradead.org

On Sat, 2018-03-31 at 14:01 +0000, Pkshih wrote:
> On Sat, 2018-03-31 at 00:05 -0700, Joe Perches wrote:
> > Use the new ether_broadcast_addr global instead to save some object code.
[]
> > diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c
[]
> > @@ -1649,7 +1648,7 @@ static int rtl_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >  			memcpy(rtlpriv->sec.key_buf[key_idx],
> >  			       key->key, key->keylen);
> >  			rtlpriv->sec.key_len[key_idx] = key->keylen;
> > -			memcpy(mac_addr, bcast_addr, ETH_ALEN);
> > +			memcpy(mac_addr, ether_broadcast_addr, ETH_ALEN);
> 
> Use ether_addr_copy(mac_addr, ether_broadcast_addr) ?

This should use eth_broadcast_addr(mac_addr) instead.

There are similar conversion still possible in the tree.
One day.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address
  2018-03-31  7:05 ` [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address Joe Perches
  2018-03-31 14:01   ` Pkshih
@ 2018-04-05 12:39   ` Kalle Valo
  2018-04-05 12:48     ` Kalle Valo
  1 sibling, 1 reply; 9+ messages in thread
From: Kalle Valo @ 2018-04-05 12:39 UTC (permalink / raw)
  To: Joe Perches
  Cc: Christian Lamparter, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, Ping-Ke Shih, Jussi Kivilinna,
	linux-wireless, netdev, linux-kernel, b43-dev

Joe Perches <joe@perches.com> writes:

> Use the new ether_broadcast_addr global instead to save some object code.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/net/wireless/admtek/adm8211.c           | 3 +--
>  drivers/net/wireless/ath/carl9170/mac.c         | 4 +---
>  drivers/net/wireless/broadcom/b43/main.c        | 3 +--
>  drivers/net/wireless/marvell/mwifiex/cfg80211.c | 3 +--
>  drivers/net/wireless/realtek/rtlwifi/core.c     | 5 ++---
>  drivers/net/wireless/rndis_wlan.c               | 6 +-----
>  drivers/net/wireless/ti/wl1251/main.c           | 5 +----
>  drivers/net/wireless/ti/wlcore/main.c           | 5 +----
>  8 files changed, 9 insertions(+), 25 deletions(-)

It would be really helpful if you could CLEARLY document in the patches
(or in the cover letter but then you need to cc all parties) to what
tree the patches are meant for. Otherwise I, and other maintainers as
well, need to waste time trying to guess what's your plan.

I will now drop the four wireless patches from my queue. So if you want
to me to take them please resubmit.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address
  2018-04-05 12:39   ` Kalle Valo
@ 2018-04-05 12:48     ` Kalle Valo
  0 siblings, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2018-04-05 12:48 UTC (permalink / raw)
  To: Joe Perches
  Cc: Christian Lamparter, Amitkumar Karwar, Nishant Sarmukadam,
	Ganapathi Bhat, Xinming Hu, Ping-Ke Shih, Jussi Kivilinna,
	linux-wireless, netdev, linux-kernel, b43-dev

Kalle Valo <kvalo@codeaurora.org> writes:

> Joe Perches <joe@perches.com> writes:
>
>> Use the new ether_broadcast_addr global instead to save some object code.
>>
>> Signed-off-by: Joe Perches <joe@perches.com>
>> ---
>>  drivers/net/wireless/admtek/adm8211.c           | 3 +--
>>  drivers/net/wireless/ath/carl9170/mac.c         | 4 +---
>>  drivers/net/wireless/broadcom/b43/main.c        | 3 +--
>>  drivers/net/wireless/marvell/mwifiex/cfg80211.c | 3 +--
>>  drivers/net/wireless/realtek/rtlwifi/core.c     | 5 ++---
>>  drivers/net/wireless/rndis_wlan.c               | 6 +-----
>>  drivers/net/wireless/ti/wl1251/main.c           | 5 +----
>>  drivers/net/wireless/ti/wlcore/main.c           | 5 +----
>>  8 files changed, 9 insertions(+), 25 deletions(-)
>
> It would be really helpful if you could CLEARLY document in the patches
> (or in the cover letter but then you need to cc all parties) to what
> tree the patches are meant for. Otherwise I, and other maintainers as
> well, need to waste time trying to guess what's your plan.

Forgot to mention that for me the best approach is to have the tree name
in subject, just like Dave and Linus both recommend:

[PATCH 06/12 wireless-drivers-next] wireless: Convert simple uses of a static const Ethernet broadcast address

This way I can immeaditely see from patchwork to which tree the patch
should go which helps tremendously. And if the tree name is too long you
can always shorten it to w-d and w-d-next.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals
  2018-03-31  7:05 [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals Joe Perches
  2018-03-31  7:05 ` [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address Joe Perches
@ 2018-04-05 13:27 ` Felix Fietkau
  2018-04-05 13:51   ` Joe Perches
  1 sibling, 1 reply; 9+ messages in thread
From: Felix Fietkau @ 2018-04-05 13:27 UTC (permalink / raw)
  To: Joe Perches, netdev, linux-wireless, b43-dev, bridge,
	netfilter-devel, coreteam
  Cc: linux-kernel, brcm80211-dev-list.pdl, brcm80211-dev-list

On 2018-03-31 09:05, Joe Perches wrote:
> There are many local static and non-static arrays that are used for
> Ethernet broadcast address output or comparison.
> 
> Centralize the array into a single separate file and remove the local
> arrays.
I suspect that for many targets and configurations, the local arrays
might actually be smaller than exporting a global. You have to factor in
not just the .text size, but the fact that referencing an exported
symbol needs a .reloc entry as well, which also eats up some space (at
least when the code is being built as module).

In my opinion, your series probably causes more bloat in common
configurations instead of reducing it.

You're also touching several places that could easily use
eth_broadcast_addr and eth_zero_addr. I think making those changes would
be more productive than what you did in this series.

- Felix

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals
  2018-04-05 13:27 ` [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals Felix Fietkau
@ 2018-04-05 13:51   ` Joe Perches
  2018-04-05 14:05     ` Felix Fietkau
  0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2018-04-05 13:51 UTC (permalink / raw)
  To: Felix Fietkau, netdev, linux-wireless, b43-dev, bridge,
	netfilter-devel, coreteam
  Cc: linux-kernel, brcm80211-dev-list.pdl, brcm80211-dev-list

On Thu, 2018-04-05 at 15:27 +0200, Felix Fietkau wrote:
> On 2018-03-31 09:05, Joe Perches wrote:
> > There are many local static and non-static arrays that are used for
> > Ethernet broadcast address output or comparison.
> > 
> > Centralize the array into a single separate file and remove the local
> > arrays.
> 
> I suspect that for many targets and configurations, the local arrays
> might actually be smaller than exporting a global.

I tried x86-64 allnoconfig and defconfig.
Those both did not increase vmlinux size.

The defconfig actually got smaller, but that might have been
some object alignment oddity.

>  You have to factor in
> not just the .text size, but the fact that referencing an exported
> symbol needs a .reloc entry as well, which also eats up some space (at
> least when the code is being built as module).

Thanks, the modules I built got smaller.

> In my opinion, your series probably causes more bloat in common
> configurations instead of reducing it.
> 
> You're also touching several places that could easily use
> eth_broadcast_addr and eth_zero_addr. I think making those changes would
> be more productive than what you did in this series.

Doubtful. AFAIK: possible unaligned addresses.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals
  2018-04-05 13:51   ` Joe Perches
@ 2018-04-05 14:05     ` Felix Fietkau
  0 siblings, 0 replies; 9+ messages in thread
From: Felix Fietkau @ 2018-04-05 14:05 UTC (permalink / raw)
  To: Joe Perches, netdev, linux-wireless, b43-dev, bridge,
	netfilter-devel, coreteam
  Cc: linux-kernel, brcm80211-dev-list.pdl, brcm80211-dev-list

On 2018-04-05 15:51, Joe Perches wrote:
>>  You have to factor in
>> not just the .text size, but the fact that referencing an exported
>> symbol needs a .reloc entry as well, which also eats up some space (at
>> least when the code is being built as module).
> 
> Thanks, the modules I built got smaller.
Please post some numbers to show this. By the way, on other
architectures the numbers will probably be different, especially on
ARM/MIPS.
>> In my opinion, your series probably causes more bloat in common
>> configurations instead of reducing it.
>> 
>> You're also touching several places that could easily use
>> eth_broadcast_addr and eth_zero_addr. I think making those changes would
>> be more productive than what you did in this series.
> 
> Doubtful. AFAIK: possible unaligned addresses.
Those two are just memset calls, alignment does not matter.

- Felix

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-04-05 14:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-31  7:05 [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals Joe Perches
2018-03-31  7:05 ` [PATCH 06/12] wireless: Convert simple uses of a static const Ethernet broadcast address Joe Perches
2018-03-31 14:01   ` Pkshih
2018-03-31 14:33     ` Joe Perches
2018-04-05 12:39   ` Kalle Valo
2018-04-05 12:48     ` Kalle Valo
2018-04-05 13:27 ` [PATCH 00/12] Ethernet: Add and use ether_<type>_addr globals Felix Fietkau
2018-04-05 13:51   ` Joe Perches
2018-04-05 14:05     ` Felix Fietkau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox