2012-07-03 10:16:30

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] etherdevice: introduce broadcast_ether_addr

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
include/linux/etherdevice.h | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 3d406e0..6da05bb 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -138,6 +138,17 @@ static inline void random_ether_addr(u8 *addr)
}

/**
+ * broadcast_ether_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 broadcast_ether_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





2012-07-11 00:41:43

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] etherdevice: introduce eth_broadcast_addr

From: Paul Gortmaker <[email protected]>
Date: Tue, 10 Jul 2012 20:09:44 -0400

> On Tue, Jul 10, 2012 at 12:18 PM, Johannes Berg
> <[email protected]> wrote:
>> From: Johannes Berg <[email protected]>
>>
>> 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.


2012-07-16 10:17:36

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH net-next 0/8] etherdevice: Rename random_ether_addr to eth_random_addr

On Thu, Jul 12, 2012 at 10:33:04PM -0700, Joe Perches wrote:
> net-next commit ad7eee98be ("etherdevice: introduce eth_broadcast_addr")
> added a new style API. Rename random_ether_addr to eth_random_addr to
> create some API symmetry.
>
> Joe Perches (8):
> etherdevice: Rename random_ether_addr to eth_random_addr

if you're really renaming the function, then this patch alone will break
all of the below users. That should all be a single patch, I'm afraid.

> ethernet: Use eth_random_addr
> net: usb: Use eth_random_addr
> wireless: Use eth_random_addr
> drivers/net: Use eth_random_addr
> s390: Use eth_random_addr
> usb: Use eth_random_addr
> arch: Use eth_random_addr
>
> arch/blackfin/mach-bf537/boards/stamp.c | 2 +-
> arch/c6x/kernel/soc.c | 2 +-
> arch/mips/ar7/platform.c | 4 ++--
> arch/mips/powertv/powertv_setup.c | 6 +++---
> arch/um/drivers/net_kern.c | 2 +-
> drivers/net/ethernet/atheros/atl1c/atl1c_hw.c | 2 +-
> drivers/net/ethernet/atheros/atlx/atl1.c | 2 +-
> drivers/net/ethernet/atheros/atlx/atl2.c | 2 +-
> drivers/net/ethernet/ethoc.c | 2 +-
> drivers/net/ethernet/intel/igb/igb_main.c | 4 ++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 2 +-
> drivers/net/ethernet/lantiq_etop.c | 2 +-
> drivers/net/ethernet/micrel/ks8851.c | 2 +-
> drivers/net/ethernet/micrel/ks8851_mll.c | 2 +-
> drivers/net/ethernet/smsc/smsc911x.c | 2 +-
> drivers/net/ethernet/ti/cpsw.c | 2 +-
> drivers/net/ethernet/tile/tilegx.c | 2 +-
> drivers/net/ethernet/wiznet/w5100.c | 2 +-
> drivers/net/ethernet/wiznet/w5300.c | 2 +-
> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 +-
> drivers/net/tun.c | 2 +-
> drivers/net/usb/smsc75xx.c | 2 +-
> drivers/net/usb/smsc95xx.c | 2 +-
> drivers/net/usb/usbnet.c | 2 +-
> drivers/net/wimax/i2400m/driver.c | 2 +-
> drivers/net/wireless/adm8211.c | 2 +-
> drivers/net/wireless/p54/eeprom.c | 2 +-
> drivers/net/wireless/rt2x00/rt2400pci.c | 2 +-
> drivers/net/wireless/rt2x00/rt2500pci.c | 2 +-
> drivers/net/wireless/rt2x00/rt2500usb.c | 2 +-
> drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
> drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
> drivers/net/wireless/rt2x00/rt73usb.c | 2 +-
> drivers/net/wireless/rtl818x/rtl8180/dev.c | 2 +-
> drivers/net/wireless/rtl818x/rtl8187/dev.c | 2 +-
> drivers/s390/net/qeth_l2_main.c | 2 +-
> drivers/s390/net/qeth_l3_main.c | 2 +-
> drivers/usb/atm/xusbatm.c | 4 ++--
> drivers/usb/gadget/u_ether.c | 2 +-
> include/linux/etherdevice.h | 14 ++++++++------
> 40 files changed, 52 insertions(+), 50 deletions(-)
>
> --
> 1.7.8.111.gad25c.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
balbi


Attachments:
(No filename) (3.51 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-07-03 15:13:23

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] etherdevice: introduce broadcast_ether_addr

On Tue, 2012-07-03 at 12:16 +0200, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> 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.
[]
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
[]
> +static inline void broadcast_ether_addr(u8 *addr)
> +{
> + memset(addr, 0xff, ETH_ALEN);
> +}

I think this sort of patch should come as the first
patch in a series with some example conversions.

It might be too easy to confuse is_broadcast_ether_addr
with this function name too. Maybe set_broadcast_ether_addr
might be better.

I really don't see an issue with using memset though.
Everyone already knows what that does.



2012-07-16 11:17:28

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next 0/8] etherdevice: Rename random_ether_addr to eth_random_addr

From: Felipe Balbi <[email protected]>
Date: Mon, 16 Jul 2012 14:12:19 +0300

> Acked-by: Felipe Balbi <[email protected]>

You need to provide this in a reply to the patch you actually want
to ACK, so that the patch tracking system attaches your ACK to
the proper patch.

Thank you.

2012-07-16 10:29:04

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next 0/8] etherdevice: Rename random_ether_addr to eth_random_addr

From: Felipe Balbi <[email protected]>
Date: Mon, 16 Jul 2012 13:14:38 +0300

> if you're really renaming the function, then this patch alone will break
> all of the below users. That should all be a single patch, I'm afraid.

It would help if you actually read his patches before saying what they
might or might not do.

He provides a macro in the first patch that provides the old name,
and this will get removed at the end.

2012-07-11 01:07:14

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] etherdevice: introduce eth_broadcast_addr

From: Johannes Berg <[email protected]>
Date: Tue, 10 Jul 2012 18:18:44 +0200

> From: Johannes Berg <[email protected]>
>
> 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 <[email protected]>

Applied, thanks.

2012-07-13 07:15:09

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH net-next 4/8] wireless: Use eth_random_addr

On Fri, Jul 13, 2012 at 7:33 AM, Joe Perches <[email protected]> wrote:
> Convert the existing uses of random_ether_addr to
> the new eth_random_addr.
>
> Signed-off-by: Joe Perches <[email protected]>

For the rt2x00 parts:

Acked-by: Gertjan van Wingerde <[email protected]>

> ---
> drivers/net/wireless/adm8211.c | 2 +-
> drivers/net/wireless/p54/eeprom.c | 2 +-
> drivers/net/wireless/rt2x00/rt2400pci.c | 2 +-
> drivers/net/wireless/rt2x00/rt2500pci.c | 2 +-
> drivers/net/wireless/rt2x00/rt2500usb.c | 2 +-
> drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
> drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
> drivers/net/wireless/rt2x00/rt73usb.c | 2 +-
> drivers/net/wireless/rtl818x/rtl8180/dev.c | 2 +-
> drivers/net/wireless/rtl818x/rtl8187/dev.c | 2 +-
> 10 files changed, 10 insertions(+), 10 deletions(-)
>

---
Gertjan

2012-07-10 16:18:48

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] etherdevice: introduce eth_broadcast_addr

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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




2012-07-11 07:27:43

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] etherdevice: introduce eth_broadcast_addr

On Tue, 2012-07-10 at 20:09 -0400, Paul Gortmaker wrote:
> On Tue, Jul 10, 2012 at 12:18 PM, Johannes Berg
> <[email protected]> wrote:
> > From: Johannes Berg <[email protected]>
> >
> > 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'm planning to fix the wireless uses (at least the ones I'm responsible
for), but I'm just going to stick them into my mac80211-next tree after
this patch percolates down there, I don't see a need to send around a
ton of patches for it.

> 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.

At least in my code I'm going to prefer this over the memset for
documentation purposes. I'll agree that memset(..., 0xff, ETH_ALEN) is
pretty obvious already, but eth_broadcast_addr(...) is even easier to
read IMHO.

johannes


2012-07-13 05:33:39

by Joe Perches

[permalink] [raw]
Subject: [PATCH net-next 0/8] etherdevice: Rename random_ether_addr to eth_random_addr

net-next commit ad7eee98be ("etherdevice: introduce eth_broadcast_addr")
added a new style API. Rename random_ether_addr to eth_random_addr to
create some API symmetry.

Joe Perches (8):
etherdevice: Rename random_ether_addr to eth_random_addr
ethernet: Use eth_random_addr
net: usb: Use eth_random_addr
wireless: Use eth_random_addr
drivers/net: Use eth_random_addr
s390: Use eth_random_addr
usb: Use eth_random_addr
arch: Use eth_random_addr

arch/blackfin/mach-bf537/boards/stamp.c | 2 +-
arch/c6x/kernel/soc.c | 2 +-
arch/mips/ar7/platform.c | 4 ++--
arch/mips/powertv/powertv_setup.c | 6 +++---
arch/um/drivers/net_kern.c | 2 +-
drivers/net/ethernet/atheros/atl1c/atl1c_hw.c | 2 +-
drivers/net/ethernet/atheros/atlx/atl1.c | 2 +-
drivers/net/ethernet/atheros/atlx/atl2.c | 2 +-
drivers/net/ethernet/ethoc.c | 2 +-
drivers/net/ethernet/intel/igb/igb_main.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 2 +-
drivers/net/ethernet/lantiq_etop.c | 2 +-
drivers/net/ethernet/micrel/ks8851.c | 2 +-
drivers/net/ethernet/micrel/ks8851_mll.c | 2 +-
drivers/net/ethernet/smsc/smsc911x.c | 2 +-
drivers/net/ethernet/ti/cpsw.c | 2 +-
drivers/net/ethernet/tile/tilegx.c | 2 +-
drivers/net/ethernet/wiznet/w5100.c | 2 +-
drivers/net/ethernet/wiznet/w5300.c | 2 +-
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 +-
drivers/net/tun.c | 2 +-
drivers/net/usb/smsc75xx.c | 2 +-
drivers/net/usb/smsc95xx.c | 2 +-
drivers/net/usb/usbnet.c | 2 +-
drivers/net/wimax/i2400m/driver.c | 2 +-
drivers/net/wireless/adm8211.c | 2 +-
drivers/net/wireless/p54/eeprom.c | 2 +-
drivers/net/wireless/rt2x00/rt2400pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2500pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2500usb.c | 2 +-
drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
drivers/net/wireless/rt2x00/rt73usb.c | 2 +-
drivers/net/wireless/rtl818x/rtl8180/dev.c | 2 +-
drivers/net/wireless/rtl818x/rtl8187/dev.c | 2 +-
drivers/s390/net/qeth_l2_main.c | 2 +-
drivers/s390/net/qeth_l3_main.c | 2 +-
drivers/usb/atm/xusbatm.c | 4 ++--
drivers/usb/gadget/u_ether.c | 2 +-
include/linux/etherdevice.h | 14 ++++++++------
40 files changed, 52 insertions(+), 50 deletions(-)

--
1.7.8.111.gad25c.dirty


2012-07-17 05:39:48

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next 0/8] etherdevice: Rename random_ether_addr to eth_random_addr

From: Joe Perches <[email protected]>
Date: Thu, 12 Jul 2012 22:33:04 -0700

> net-next commit ad7eee98be ("etherdevice: introduce eth_broadcast_addr")
> added a new style API. Rename random_ether_addr to eth_random_addr to
> create some API symmetry.

Series applied, thanks Joe.

2012-07-09 06:58:09

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] etherdevice: introduce broadcast_ether_addr

From: Johannes Berg <[email protected]>
Date: Tue, 03 Jul 2012 12:16:27 +0200

> From: Johannes Berg <[email protected]>
>
> 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 <[email protected]>

I would prefer if this were named "eth_something()", thanks.

2012-07-03 15:16:56

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] etherdevice: introduce broadcast_ether_addr

On Tue, 2012-07-03 at 08:13 -0700, Joe Perches wrote:
> On Tue, 2012-07-03 at 12:16 +0200, Johannes Berg wrote:
> > From: Johannes Berg <[email protected]>
> >
> > 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.
> []
> > diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> []
> > +static inline void broadcast_ether_addr(u8 *addr)
> > +{
> > + memset(addr, 0xff, ETH_ALEN);
> > +}
>
> I think this sort of patch should come as the first
> patch in a series with some example conversions.
>
> It might be too easy to confuse is_broadcast_ether_addr
> with this function name too. Maybe set_broadcast_ether_addr
> might be better.

Well, it's void so that'd be a compiler error :-)
Also, it's more like random_ether_addr()

johannes


2012-07-11 01:09:28

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] etherdevice: introduce eth_broadcast_addr

On Tue, 2012-07-10 at 17:41 -0700, David Miller wrote:
> From: Paul Gortmaker <[email protected]>
> Date: Tue, 10 Jul 2012 20:09:44 -0400
>
> > On Tue, Jul 10, 2012 at 12:18 PM, Johannes Berg
> > <[email protected]> wrote:
> >> From: Johannes Berg <[email protected]>
> >>
> >> 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 I grepped properly, there are 42 instances of static arrays for
for broadcast ethernet addresses in drivers/net and drivers/staging
so it'd save some smallish amount of code by using a combination of
is_broadcast_ether_addr and this new func.

I think there are 53 instances of the memset(foo, 0xff, 6|ETH_ALEN).

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

Sounds fine to me.

For some additional style symmetry, how about a conversion of
random_ether_address to eth_random_addr too via

o Rename random_ether_addr to eth_random_addr and add a
#define random_ether_addr eth_random_addr
o sed 's/\brandom_ether_addr\b/eth_random_addr/g' files_that_use_REA
o remove the #define after awhile



2012-07-13 05:54:18

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: [PATCH net-next 4/8] wireless: Use eth_random_addr

--- On Fri, 13/7/12, Joe Perches <[email protected]> wrote:

> From: Joe Perches <[email protected]>
> Subject: [PATCH net-next 4/8] wireless: Use eth_random_addr
> To: "David Miller" <[email protected]>, "John W. Linville" <[email protected]>, "Christian Lamparter" <[email protected]>, "Ivo van Doorn" <[email protected]>, "Gertjan van Wingerde" <[email protected]>, "Helmut Schaa" <[email protected]>, "Herton Ronaldo Krzesinski" <[email protected]>, "Hin-Tak Leung" <[email protected]>, "Larry Finger" <[email protected]>
> Cc: "Johannes Berg" <[email protected]>, [email protected], [email protected], [email protected], [email protected]
> Date: Friday, 13 July, 2012, 6:33
> Convert the existing uses of
> random_ether_addr to
> the new eth_random_addr.
>
> Signed-off-by: Joe Perches <[email protected]>

Acked-by: Hin-Tak Leung <[email protected]>

Would it make sense to have a "check & set" macro?

> ---
> drivers/net/wireless/adm8211.c? ? ? ?
> ? ???|? ? 2 +-
> drivers/net/wireless/p54/eeprom.c? ? ?
> ? ? |? ? 2 +-
> drivers/net/wireless/rt2x00/rt2400pci.c? ?
> |? ? 2 +-
> drivers/net/wireless/rt2x00/rt2500pci.c? ?
> |? ? 2 +-
> drivers/net/wireless/rt2x00/rt2500usb.c? ?
> |? ? 2 +-
> drivers/net/wireless/rt2x00/rt2800lib.c? ?
> |? ? 2 +-
> drivers/net/wireless/rt2x00/rt61pci.c? ? ?
> |? ? 2 +-
> drivers/net/wireless/rt2x00/rt73usb.c? ? ?
> |? ? 2 +-
> drivers/net/wireless/rtl818x/rtl8180/dev.c |? ? 2
> +-
> drivers/net/wireless/rtl818x/rtl8187/dev.c |? ? 2
> +-
> 10 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/adm8211.c
> b/drivers/net/wireless/adm8211.c
> index 97afcec..689a71c 100644
> --- a/drivers/net/wireless/adm8211.c
> +++ b/drivers/net/wireless/adm8211.c
> @@ -1854,7 +1854,7 @@ static int __devinit
> adm8211_probe(struct pci_dev *pdev,
> ??? if (!is_valid_ether_addr(perm_addr)) {
> ??? ??? printk(KERN_WARNING
> "%s (adm8211): Invalid hwaddr in EEPROM!\n",
> ??? ??? ? ?
> ???pci_name(pdev));
> -??? ???
> random_ether_addr(perm_addr);
> +??? ???
> eth_random_addr(perm_addr);
> ??? }
> ??? SET_IEEE80211_PERM_ADDR(dev,
> perm_addr);
>
> diff --git a/drivers/net/wireless/p54/eeprom.c
> b/drivers/net/wireless/p54/eeprom.c
> index 636daf2..1403709 100644
> --- a/drivers/net/wireless/p54/eeprom.c
> +++ b/drivers/net/wireless/p54/eeprom.c
> @@ -857,7 +857,7 @@ good_eeprom:
>
> ??? ???
> wiphy_warn(dev->wiphy,
> ??? ??? ???
> ???"Invalid hwaddr! Using randomly generated
> MAC addr\n");
> -??? ???
> random_ether_addr(perm_addr);
> +??? ???
> eth_random_addr(perm_addr);
> ??? ???
> SET_IEEE80211_PERM_ADDR(dev, perm_addr);
> ??? }
>
> diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c
> b/drivers/net/wireless/rt2x00/rt2400pci.c
> index 5e6b501..8b9dbd7 100644
> --- a/drivers/net/wireless/rt2x00/rt2400pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2400pci.c
> @@ -1455,7 +1455,7 @@ static int
> rt2400pci_validate_eeprom(struct rt2x00_dev *rt2x00dev)
> ?????*/
> ??? mac = rt2x00_eeprom_addr(rt2x00dev,
> EEPROM_MAC_ADDR_0);
> ??? if (!is_valid_ether_addr(mac)) {
> -??? ???
> random_ether_addr(mac);
> +??? ???
> eth_random_addr(mac);
> ??? ??? EEPROM(rt2x00dev,
> "MAC: %pM\n", mac);
> ??? }
>
> diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c
> b/drivers/net/wireless/rt2x00/rt2500pci.c
> index 136b849..d2cf8a4 100644
> --- a/drivers/net/wireless/rt2x00/rt2500pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2500pci.c
> @@ -1585,7 +1585,7 @@ static int
> rt2500pci_validate_eeprom(struct rt2x00_dev *rt2x00dev)
> ?????*/
> ??? mac = rt2x00_eeprom_addr(rt2x00dev,
> EEPROM_MAC_ADDR_0);
> ??? if (!is_valid_ether_addr(mac)) {
> -??? ???
> random_ether_addr(mac);
> +??? ???
> eth_random_addr(mac);
> ??? ??? EEPROM(rt2x00dev,
> "MAC: %pM\n", mac);
> ??? }
>
> diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c
> b/drivers/net/wireless/rt2x00/rt2500usb.c
> index 669aecd..3aae36b 100644
> --- a/drivers/net/wireless/rt2x00/rt2500usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2500usb.c
> @@ -1352,7 +1352,7 @@ static int
> rt2500usb_validate_eeprom(struct rt2x00_dev *rt2x00dev)
> ?????*/
> ??? mac = rt2x00_eeprom_addr(rt2x00dev,
> EEPROM_MAC_ADDR_0);
> ??? if (!is_valid_ether_addr(mac)) {
> -??? ???
> random_ether_addr(mac);
> +??? ???
> eth_random_addr(mac);
> ??? ??? EEPROM(rt2x00dev,
> "MAC: %pM\n", mac);
> ??? }
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c
> b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 068276e..d857d55 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -4340,7 +4340,7 @@ int rt2800_validate_eeprom(struct
> rt2x00_dev *rt2x00dev)
> ?????*/
> ??? mac = rt2x00_eeprom_addr(rt2x00dev,
> EEPROM_MAC_ADDR_0);
> ??? if (!is_valid_ether_addr(mac)) {
> -??? ???
> random_ether_addr(mac);
> +??? ???
> eth_random_addr(mac);
> ??? ??? EEPROM(rt2x00dev,
> "MAC: %pM\n", mac);
> ??? }
>
> diff --git a/drivers/net/wireless/rt2x00/rt61pci.c
> b/drivers/net/wireless/rt2x00/rt61pci.c
> index ee22bd7..f322596 100644
> --- a/drivers/net/wireless/rt2x00/rt61pci.c
> +++ b/drivers/net/wireless/rt2x00/rt61pci.c
> @@ -2415,7 +2415,7 @@ static int
> rt61pci_validate_eeprom(struct rt2x00_dev *rt2x00dev)
> ?????*/
> ??? mac = rt2x00_eeprom_addr(rt2x00dev,
> EEPROM_MAC_ADDR_0);
> ??? if (!is_valid_ether_addr(mac)) {
> -??? ???
> random_ether_addr(mac);
> +??? ???
> eth_random_addr(mac);
> ??? ??? EEPROM(rt2x00dev,
> "MAC: %pM\n", mac);
> ??? }
>
> diff --git a/drivers/net/wireless/rt2x00/rt73usb.c
> b/drivers/net/wireless/rt2x00/rt73usb.c
> index 77ccbbc..ba6e434 100644
> --- a/drivers/net/wireless/rt2x00/rt73usb.c
> +++ b/drivers/net/wireless/rt2x00/rt73usb.c
> @@ -1770,7 +1770,7 @@ static int
> rt73usb_validate_eeprom(struct rt2x00_dev *rt2x00dev)
> ?????*/
> ??? mac = rt2x00_eeprom_addr(rt2x00dev,
> EEPROM_MAC_ADDR_0);
> ??? if (!is_valid_ether_addr(mac)) {
> -??? ???
> random_ether_addr(mac);
> +??? ???
> eth_random_addr(mac);
> ??? ??? EEPROM(rt2x00dev,
> "MAC: %pM\n", mac);
> ??? }
>
> diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c
> b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> index 3b50539..aceaf68 100644
> --- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
> +++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> @@ -1078,7 +1078,7 @@ static int __devinit
> rtl8180_probe(struct pci_dev *pdev,
> ??? if (!is_valid_ether_addr(mac_addr)) {
> ??? ??? printk(KERN_WARNING
> "%s (rtl8180): Invalid hwaddr! Using"
> ??? ??? ? ?
> ???" randomly generated MAC addr\n",
> pci_name(pdev));
> -??? ???
> random_ether_addr(mac_addr);
> +??? ???
> eth_random_addr(mac_addr);
> ??? }
> ??? SET_IEEE80211_PERM_ADDR(dev, mac_addr);
>
> diff --git a/drivers/net/wireless/rtl818x/rtl8187/dev.c
> b/drivers/net/wireless/rtl818x/rtl8187/dev.c
> index 4fb1ca1..71a30b0 100644
> --- a/drivers/net/wireless/rtl818x/rtl8187/dev.c
> +++ b/drivers/net/wireless/rtl818x/rtl8187/dev.c
> @@ -1486,7 +1486,7 @@ static int __devinit
> rtl8187_probe(struct usb_interface *intf,
> ??? if (!is_valid_ether_addr(mac_addr)) {
> ??? ??? printk(KERN_WARNING
> "rtl8187: Invalid hwaddr! Using randomly "
> ??? ??? ? ?
> ???"generated MAC address\n");
> -??? ???
> random_ether_addr(mac_addr);
> +??? ???
> eth_random_addr(mac_addr);
> ??? }
> ??? SET_IEEE80211_PERM_ADDR(dev, mac_addr);
>
> --
> 1.7.8.111.gad25c.dirty
>
>

2012-07-13 05:34:31

by Joe Perches

[permalink] [raw]
Subject: [PATCH net-next 4/8] wireless: Use eth_random_addr

Convert the existing uses of random_ether_addr to
the new eth_random_addr.

Signed-off-by: Joe Perches <[email protected]>
---
drivers/net/wireless/adm8211.c | 2 +-
drivers/net/wireless/p54/eeprom.c | 2 +-
drivers/net/wireless/rt2x00/rt2400pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2500pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2500usb.c | 2 +-
drivers/net/wireless/rt2x00/rt2800lib.c | 2 +-
drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
drivers/net/wireless/rt2x00/rt73usb.c | 2 +-
drivers/net/wireless/rtl818x/rtl8180/dev.c | 2 +-
drivers/net/wireless/rtl818x/rtl8187/dev.c | 2 +-
10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/adm8211.c b/drivers/net/wireless/adm8211.c
index 97afcec..689a71c 100644
--- a/drivers/net/wireless/adm8211.c
+++ b/drivers/net/wireless/adm8211.c
@@ -1854,7 +1854,7 @@ static int __devinit adm8211_probe(struct pci_dev *pdev,
if (!is_valid_ether_addr(perm_addr)) {
printk(KERN_WARNING "%s (adm8211): Invalid hwaddr in EEPROM!\n",
pci_name(pdev));
- random_ether_addr(perm_addr);
+ eth_random_addr(perm_addr);
}
SET_IEEE80211_PERM_ADDR(dev, perm_addr);

diff --git a/drivers/net/wireless/p54/eeprom.c b/drivers/net/wireless/p54/eeprom.c
index 636daf2..1403709 100644
--- a/drivers/net/wireless/p54/eeprom.c
+++ b/drivers/net/wireless/p54/eeprom.c
@@ -857,7 +857,7 @@ good_eeprom:

wiphy_warn(dev->wiphy,
"Invalid hwaddr! Using randomly generated MAC addr\n");
- random_ether_addr(perm_addr);
+ eth_random_addr(perm_addr);
SET_IEEE80211_PERM_ADDR(dev, perm_addr);
}

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 5e6b501..8b9dbd7 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -1455,7 +1455,7 @@ static int rt2400pci_validate_eeprom(struct rt2x00_dev *rt2x00dev)
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
if (!is_valid_ether_addr(mac)) {
- random_ether_addr(mac);
+ eth_random_addr(mac);
EEPROM(rt2x00dev, "MAC: %pM\n", mac);
}

diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 136b849..d2cf8a4 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -1585,7 +1585,7 @@ static int rt2500pci_validate_eeprom(struct rt2x00_dev *rt2x00dev)
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
if (!is_valid_ether_addr(mac)) {
- random_ether_addr(mac);
+ eth_random_addr(mac);
EEPROM(rt2x00dev, "MAC: %pM\n", mac);
}

diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index 669aecd..3aae36b 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -1352,7 +1352,7 @@ static int rt2500usb_validate_eeprom(struct rt2x00_dev *rt2x00dev)
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
if (!is_valid_ether_addr(mac)) {
- random_ether_addr(mac);
+ eth_random_addr(mac);
EEPROM(rt2x00dev, "MAC: %pM\n", mac);
}

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 068276e..d857d55 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -4340,7 +4340,7 @@ int rt2800_validate_eeprom(struct rt2x00_dev *rt2x00dev)
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
if (!is_valid_ether_addr(mac)) {
- random_ether_addr(mac);
+ eth_random_addr(mac);
EEPROM(rt2x00dev, "MAC: %pM\n", mac);
}

diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index ee22bd7..f322596 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -2415,7 +2415,7 @@ static int rt61pci_validate_eeprom(struct rt2x00_dev *rt2x00dev)
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
if (!is_valid_ether_addr(mac)) {
- random_ether_addr(mac);
+ eth_random_addr(mac);
EEPROM(rt2x00dev, "MAC: %pM\n", mac);
}

diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index 77ccbbc..ba6e434 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -1770,7 +1770,7 @@ static int rt73usb_validate_eeprom(struct rt2x00_dev *rt2x00dev)
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
if (!is_valid_ether_addr(mac)) {
- random_ether_addr(mac);
+ eth_random_addr(mac);
EEPROM(rt2x00dev, "MAC: %pM\n", mac);
}

diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
index 3b50539..aceaf68 100644
--- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
@@ -1078,7 +1078,7 @@ static int __devinit rtl8180_probe(struct pci_dev *pdev,
if (!is_valid_ether_addr(mac_addr)) {
printk(KERN_WARNING "%s (rtl8180): Invalid hwaddr! Using"
" randomly generated MAC addr\n", pci_name(pdev));
- random_ether_addr(mac_addr);
+ eth_random_addr(mac_addr);
}
SET_IEEE80211_PERM_ADDR(dev, mac_addr);

diff --git a/drivers/net/wireless/rtl818x/rtl8187/dev.c b/drivers/net/wireless/rtl818x/rtl8187/dev.c
index 4fb1ca1..71a30b0 100644
--- a/drivers/net/wireless/rtl818x/rtl8187/dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8187/dev.c
@@ -1486,7 +1486,7 @@ static int __devinit rtl8187_probe(struct usb_interface *intf,
if (!is_valid_ether_addr(mac_addr)) {
printk(KERN_WARNING "rtl8187: Invalid hwaddr! Using randomly "
"generated MAC address\n");
- random_ether_addr(mac_addr);
+ eth_random_addr(mac_addr);
}
SET_IEEE80211_PERM_ADDR(dev, mac_addr);

--
1.7.8.111.gad25c.dirty


2012-07-11 00:10:17

by Paul Gortmaker

[permalink] [raw]
Subject: Re: [PATCH] etherdevice: introduce eth_broadcast_addr

On Tue, Jul 10, 2012 at 12:18 PM, Johannes Berg
<[email protected]> wrote:
> From: Johannes Berg <[email protected]>
>
> 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.
--

> 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 <[email protected]>
> ---
> 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
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2012-07-16 11:15:18

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH net-next 0/8] etherdevice: Rename random_ether_addr to eth_random_addr

Hi,

On Mon, Jul 16, 2012 at 03:29:01AM -0700, David Miller wrote:
> From: Felipe Balbi <[email protected]>
> Date: Mon, 16 Jul 2012 13:14:38 +0300
>
> > if you're really renaming the function, then this patch alone will break
> > all of the below users. That should all be a single patch, I'm afraid.
>
> It would help if you actually read his patches before saying what they
> might or might not do.
>
> He provides a macro in the first patch that provides the old name,
> and this will get removed at the end.

that's why I put an "if" there. The subject was misleading and I really
couldn't bother going search for the patch on the mail archives.

Anyway, if nothing will be broken then for drivers/usb/gadget/:

Acked-by: Felipe Balbi <[email protected]>

--
balbi


Attachments:
(No filename) (764.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments