Re: [PATCH] etherdevice: introduce broadcast_ether_addr

From: Johannes Berg <redacted>
Date: Tue, 03 Jul 2012 12:16:27 +0200

From: Johannes Berg <redacted>

A lot of code has either the memset or an
inefficient copy from a static array that
contains the all-ones broadcast address.
Introduce broadcast_ether_addr() to fill
an address with all ones, making the code
clearer and allowing us to get rid of the
various constant arrays.

Signed-off-by: Johannes Berg <redacted>

I would prefer if this were named "eth_something()", thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Johannes Berg <redacted>

A lot of code has either the memset or an inefficient copy
from a static array that contains the all-ones broadcast
address. Introduce eth_broadcast_addr() to fill an address
with all ones, making the code clearer and allowing us to
get rid of some constant arrays.

Signed-off-by: Johannes Berg <redacted>
---
 include/linux/etherdevice.h |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 3d406e0..98a27cc 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h

@@ -138,6 +138,17 @@ static inline void random_ether_addr(u8 *addr)
 }
 
 /**
+ * eth_broadcast_addr - Assign broadcast address
+ * @addr: Pointer to a six-byte array containing the Ethernet address
+ *
+ * Assign the broadcast address to the given address array.
+ */
+static inline void eth_broadcast_addr(u8 *addr)
+{
+	memset(addr, 0xff, ETH_ALEN);
+}
+
+/**
  * eth_hw_addr_random - Generate software assigned random Ethernet and
  * set device flag
  * @dev: pointer to net_device structure

-- 
1.7.10.4

On Tue, Jul 10, 2012 at 12:18 PM, Johannes Berg
[off-list ref] wrote:

From: Johannes Berg <redacted>

A lot of code has either the memset or an inefficient copy
from a static array that contains the all-ones broadcast

Shouldn't we see all that "lot of code" here in this same
commit, now using this new shortcut?  If we apply this, we
have a new function, but with no users.  If you have done
the audit, and found the inefficient cases, why isn't it here?

I would think it better to just fix those people who have a
pointless static array of all-ones to use the memset.  If it was a
multi line thing to achieve the eth_broadcast_addr() then it
might make sense to exist.  But as a one line alias, it does
seem somewhat pointless to me.

Paul.
--

From: Paul Gortmaker <redacted>
Date: Tue, 10 Jul 2012 20:09:44 -0400

On Tue, Jul 10, 2012 at 12:18 PM, Johannes Berg
[off-list ref] wrote:

quoted

From: Johannes Berg <redacted>

A lot of code has either the memset or an inefficient copy
from a static array that contains the all-ones broadcast

Shouldn't we see all that "lot of code" here in this same
commit, now using this new shortcut?

I disagree and I intend to apply Johannes's patch as-is to net-next.

From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 10 Jul 2012 18:18:44 +0200

From: Johannes Berg <redacted>

A lot of code has either the memset or an inefficient copy
from a static array that contains the all-ones broadcast
address. Introduce eth_broadcast_addr() to fill an address
with all ones, making the code clearer and allowing us to
get rid of some constant arrays.

Signed-off-by: Johannes Berg <redacted>

Applied, thanks.