2010-03-21 23:22:49

by Larry Finger

[permalink] [raw]
Subject: [PATCH V2] ssb: Implement virtual SPROM on disk

Some recent BCM43XX devices lack an on-board SPROM. The pertinent data
from the SPROM could be included in the kernel; however, this presents
a problem in the generation of a unique, reproducible MAC address. The
solution has been to create a utility that generates a virtual SPROM
image with a random MAC address. This image is stored in the firmware
area, and loaded using the asyncronous firmware load facility.

Signed-off-by: Larry Finger <[email protected]>
---

Michael,

I think this patch eliminates the need for the fallback sprom code; however,
I have not touched that facility yet.

Larry
---
V2 - make virstual SPROM loading asynchronous.

Index: wireless-testing/drivers/ssb/pci.c
===================================================================
--- wireless-testing.orig/drivers/ssb/pci.c
+++ wireless-testing/drivers/ssb/pci.c
@@ -19,6 +19,7 @@
#include <linux/ssb/ssb_regs.h>
#include <linux/pci.h>
#include <linux/delay.h>
+#include <linux/firmware.h>

#include "ssb_private.h"

@@ -613,6 +614,53 @@ static int sprom_extract(struct ssb_bus
return 0;
}

+static void ssb_vsprom_load_failed(void)
+{
+ printk(KERN_ERR "ssb: The BCM43XX device does not have an "
+ "SPROM built in, and the virtual SPROM file is not"
+ " available.\n");
+ printk(KERN_ERR "ssb: Please go to "
+ "http://wireless.kernel.org/en/users/Drivers/b43 "
+ "and download the utility to create the file.\n");
+}
+
+static void ssb_get_vsprom(const struct firmware *fw, void *context)
+{
+ /* Second part of asynchronous firmware load */
+ int i;
+ u16 *buf;
+ size_t size = SSB_SPROMSIZE_WORDS_R4;
+ struct ssb_bus *bus = context;
+
+ if (!fw) {
+ ssb_vsprom_load_failed();
+ return;
+ }
+ buf = kcalloc(size, sizeof(u16), GFP_KERNEL);
+ if (!buf) {
+ printk(KERN_ERR "ssb: no memory for virtual sprom\n");
+ goto out;
+ }
+ if (fw->size != size * 2) {
+ printk(KERN_ERR "ssb: The virtual SPROM file has the wrong"
+ " size\n");
+ goto out_free;
+ }
+ for (i = 0; i < size; i++)
+ buf[i] = fw->data[2 * i + 1] << 8 | fw->data[2 * i];
+ if(sprom_check_crc(buf, size)) {
+ printk(KERN_ERR "ssb: Invalid CRC for virtual SPROM\n");
+ goto out_free;
+ }
+ sprom_extract(bus, &bus->sprom, buf, size);
+ printk(KERN_DEBUG "ssb: revision %d\n", bus->sprom.revision);
+
+out_free:
+ kfree(buf);
+out:
+ release_firmware(fw);
+}
+
static int ssb_pci_sprom_get(struct ssb_bus *bus,
struct ssb_sprom *sprom)
{
@@ -620,8 +668,17 @@ static int ssb_pci_sprom_get(struct ssb_
int err = -ENOMEM;
u16 *buf;

- if (!ssb_is_sprom_available(bus))
- return -ENODEV;
+ if (!ssb_is_sprom_available(bus)) {
+ /* This device has no SPROM. Try for a virtual version */
+ err = request_firmware_nowait(THIS_MODULE,
+ FW_ACTION_HOTPLUG, "ssb/vsprom_image",
+ &bus->host_pci->dev, GFP_KERNEL, bus, ssb_get_vsprom);
+ if (err) {
+ ssb_vsprom_load_failed();
+ return err;
+ }
+ return 0;
+ }

buf = kcalloc(SSB_SPROMSIZE_WORDS_R123, sizeof(u16), GFP_KERNEL);
if (!buf)
Index: wireless-testing/drivers/net/wireless/b43/main.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/main.c
+++ wireless-testing/drivers/net/wireless/b43/main.c
@@ -4858,7 +4858,17 @@ static int b43_one_core_attach(struct ss
static void b43_sprom_fixup(struct ssb_bus *bus)
{
struct pci_dev *pdev;
+ int i;

+ /* The sprom contents may be loaded asynchronously. Check that the
+ * revision is set before proceeding */
+ for (i = 0; i < 1000; i++) {
+ if (bus->sprom.revision)
+ break;
+ msleep(1);
+ }
+ if (i == 1000)
+ printk(KERN_INFO "b43: No sprom image loaded\n");
/* boardflags workarounds */
if (bus->boardinfo.vendor == SSB_BOARDVENDOR_DELL &&
bus->chip_id == 0x4301 && bus->boardinfo.rev == 0x74)


2010-03-23 22:02:50

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Tuesday 23 March 2010 21:58:22 Calvin Walton wrote:
> On Tue, 2010-03-23 at 09:25 -0500, Larry Finger wrote:
> > Will someone please write me udev rule(s) that do the following:
> >
> > 1. Detect a MAC address of FF:FF:FF:FF:FF:FF
> > 2. If this is the first time for this bus address, then generate a random MAC
> > address with the bus address encoded in it.
> > 3. Preserve the address for future reloads
> > 4. Load the saved address into the device.
> > 5. Do the above with only standard external commands - no new programs
> >
> > My skills with udev are not up to the task.
>
> I will warn you that the following is rather untested, as I don't have
> any of the affected hardware (or any b43 devices at all, actually), but
> something along these lines should work. There's no syntax errors, at
> least :)
>
> --- /lib/udev/rules.d/65-persistent-b43-mac-generator.rules
>
> ACTION!="add" GOTO="persistent_b43_mac_generator_end"
>
> SUBSYSTEM=="net", DRIVERS=="b43", ATTR{address}=="ff:ff:ff:ff:ff:ff", IMPORT{program}="write_persistent_b43_mac"
>
> SUBSYSTEM=="net", ENV{MACADDRESS_NEW}=="?*", RUN+="ifconfig $env{INTERFACE} hw ether $env{MACADDRESS_NEW}"
>
> LABEL="persistent_b43_mac_generator_end"
>
> --- /lib/udev/write_persistent_b43_mac (chmod +x)
>
> #!/bin/bash
>
> # This mac address generation function could be replaced with something better
> MACADDRESS=$(dd if=/dev/urandom bs=1 count=6 2>/dev/null | od -tx1 | head -1 | cut -d' ' -f2- | awk '{ print $1":"$2":"$3":"$4":"$5":"$6 }')
>
> RULES_FILE='/etc/udev/rules.d/60-persistent-b43-mac.rules'
>
> . /lib64/udev/rule_generator.functions
>
> lock_rules_file
>
> choose_rules_file
>
> echo "DEVPATH==\"$DEVPATH\", DRIVERS==\"b43\", ATTR{address}==\"ff:ff:ff:ff:ff:ff\", RUN+=\"ifconfig $INTERFACE hw ether $MACADDRESS\"" >> $RULES_FILE
>
> echo "MACADDRESS_NEW=$MACADDRESS"
>
> unlock_rules_file
>
> ---
>
> A new file "/etc/udev/rules.d/60-persistent-b43-mac.rules" will be
> created, which will contain the the saved mac address and bypass the
> generating script on future boots.
>
> This should probably be run by the udev maintainers, but is a start,
> anyways.
>
>

Thanks a lot for this proof of concept. This looks very nice.

--
Greetings, Michael.

2010-03-28 18:25:25

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On 03/28/2010 12:14 PM, Nicolas de Peslo?an wrote:
> Larry Finger wrote:
>> On 03/24/2010 02:21 PM, Michael Buesch wrote:
>>> On Wednesday 24 March 2010 15:16:03 Larry Finger wrote:
>>>> I have modified ssb to supply a MAC address of 80:80:80:80:80:80,
>>>> rather than
>>> What about also setting the local-assignment bit for this temporary
>>> address?
>>>
>>>> The one remaining problem is that the interface has already been
>>>> renamed before
>>>> 60-persistent-b43-mac.rules is processed. In my case, the interface
>>>> is wlan13,
>>>> not wlan0. After I manually modified 60-..., then the new address is
>>>> applied.
>>>> I'm still working on this problem.
>>> Well, udev scripts are processed in alphabetical order. Can't you
>>> simply run
>>> the persistent mac rules before the persistent ifname rules?
>>
>> I finally figured out the problem I was having. The address attribute
>> was not
>> being changed by the "ifconfig" call that changed the hardware
>> address. The fix
>> is to create a new environment when the hardware address and lock out
>> the rule
>> generation process when that value is detected. The new code for
>> /lib/udev/rules.d/65-persistent-b43-mac-generator.rules is as follows
>> (Note:
>> These files are line-wrapped here.):
>>
>> #=======================================
>> #
>> # Rules file to assign a unique, permanent address to BCM43XX devices
>> without
>> # an SPROM.
>> #
>> # Copyright (c) 2010 by Calvin Walton <[email protected]>
>> # Copyright (c) 2010 by Larry Finger <[email protected]>
>>
>> # skip this code if action is not add, i.e. change or remove
>>
>> ACTION!="add", GOTO="persistent_b43_mac_generator_end"
>>
>> # Use the value of the MAC_CHANGED environment variable to see if the
>> address
>> # has already been changed.
>>
>> ENV{MAC_CHANGED}=="yes", GOTO="persistent_b43_mac_generator_end"
>>
>> # Call script to get a random address - if this device previously
>> encountered,
>> # the address will already have been changed.
>>
>> SUBSYSTEM=="net", ATTR{address}=="82:82:82:82:82:82",
>> IMPORT{program}="write_persistent_b43_mac"
>>
>> # Apply the new hardware address returned by the script
>>
>> SUBSYSTEM=="net", ATTR{address}=="82:82:82:82:82:82",
>> RUN+="/sbin/ifconfig
>> $env{INTERFACE} hw ether $env{MACADDRESS_NEW}"
>
> Why do you use ifconfig hw ether instead of ip link set address ?
>
>> LABEL="persistent_b43_mac_generator_end"
>> #=======================================
>>
>> The code for /lib/udev/write_persistent_b43_mac is as follows:
>>
>> #=======================================
>> #!/bin/bash
>>
>> # Script to Generate a random MAC address for a BCM43XX device without
>> # an SPROM.
>> #
>> # Copyright (c) 2010 by Calvin Walton <[email protected]>
>> # Copyright (c) 2010 by Larry Finger <[email protected]>
>>
>> # Use /dev/urandom to generate the last 5 bytes of the address.
>> # Make the first byte 2 to avoid generating a multicast address and to
>> set
>> # the locally administered address bit.
>>
>> MACADDRESS=$(/bin/dd if=/dev/urandom bs=1 count=5 2>/dev/null |
>> /usr/bin/od -tx1
>> | /usr/bin/head -1 | \
>> /usr/bin/cut -d' ' -f2- | /usr/bin/awk '{ print
>> "02:"$1":"$2":"$3":"$4":"$5 }')
>
> A suggest the following :
>
> - 6 bytes of randomness and force lower half of first byte to 2 if the
> value does not have bit #2 set.
> - sed, instead of head|cut|awk
>
> MACADDRESS=$(/bin/dd if=/dev/random bs=1 count=6 2>/dev/null |
> /usr/bin/od -tx1 |
> sed -ne '1{;s/0000000 //;s/^\(.\)[014589cd]/\12/;y/ /:/;p}'

It also needs to be even as an odd value would be a broadcast address. Using
only sed instead of head|cut|awk does have merit and the randomness is increased
by 6 bits. It will take me a while to understand the sed here. Regular
expressions are not my thing.

>> # Define the output rules file
>> RULES_FILE='/etc/udev/rules.d/60-persistent-b43-mac.rules'
>>
>> . /lib/udev/rule_generator.functions
>>
>> # Prevent concurrent processes from modifying the file at the same time.
>> lock_rules_file
>>
>> # Check if the rules file is writeable.
>> choose_rules_file
>>
>> # The rule should apply for all wlan devices -s some other wireless
>> driver might
>> # be loaded first - change wlanNN to wlan*
>> GEN_PATH=$(echo $DEVPATH | /usr/bin/sed s/wlan[0-9]*/wlan*/)
>
> sed should be quoted here : /usr/bin/sed -e 's/wlan[0-9]*/wlan*/'
> Else, it might be fun if you happen to have a file called s/wlan7/wlan15
> in current directory.

OK - I see your point. As the current directory is /lib/udev, the presence of
such a file is unlikely, but better to protect against the problem.

>> # Output new rule
>> echo "SUBSYSTEM==\"net\", DEVPATH==\"$GEN_PATH\",
>> ATTR{address}==\"82:82:82:82:82:82\", ENV{MAC_CHANGED}=\"yes\",
>> RUN+=\"/sbin/ifconfig \$env{INTERFACE} hw ether $MACADDRESS\"" >>
>> $RULES_FILE
>
> If DEVPATH is "generic" (wlan*), how would you distinguish between two
> broadcom NIC present in the system, both without an SPROM ?

That is covered by the /devices/pci0000:00/0000:00:0d.0/0000:04:00.0/... part of
DEVPATH. A second device on the same bridge would have ...04:01.0... and a
device on a different bridge would change some other part of the string. The
change to wlan* handles the case where the BCM43XX device is discovered first
with some configuration, and second when another device is plugged in at
discovery time.

Larry

2010-03-22 08:38:03

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Monday 22 March 2010 07:28:23 Calvin Walton wrote:
> Hi,
>
> On Sat, 2010-03-20 at 19:14 -0500, Larry Finger wrote:
> Some recent BCM43XX devices lack an on-board SPROM. The pertinent data
> > from the SPROM could be included in the kernel; however, this presents
> > a problem in the generation of a unique, reproducible MAC address. The
> > solution has been to create a utility that generates a virtual SPROM
> > image with a random MAC address. This image is stored in the firmware
> > area, and loaded using the asyncronous firmware load facility.
>
> I'm curious, how would this firmware-loading scheme deal with having
> multiple cards of this type installed? This seems like an unusual
> situation, but it looks like this patch will cause all of the cards to
> start up with the same MAC address due to the fixed filename.
>
> Instead of using a firmware file to load in the MAC address, might it be
> possible to move the persistent MAC setting to a simple udev rule which
> generates a persistent MAC address, saves it, then sets it each boot
> using a command like "ip link set wlan0 address XX:XX:XX:XX:XX:XX" ?
>
> This would remove the need to have this "fake" firmware file available
> at boot, provided that the driver can handle leaving the address
> unconfigured until userspace gets around to setting it. As well, it
> could be written to work with multiple cards easily, saving a different
> MAC for each.
>
> Some thoughts for your consideration,
>

I think this actually is a very good idea.
This way we could live without a user-supplied sprom image, which I would _really_
prefer.

--
Greetings, Michael.

2010-03-29 00:33:57

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On 03/28/2010 02:04 PM, Nicolas de Peslo?an wrote:
> Larry Finger wrote:
>> On 03/28/2010 12:14 PM, Nicolas de Peslo?an wrote:
>>> Larry Finger wrote:
>>> MACADDRESS=$(/bin/dd if=/dev/random bs=1 count=6 2>/dev/null |
>>> /usr/bin/od -tx1 |
>>> sed -ne '1{;s/0000000 //;s/^\(.\)[014589cd]/\12/;y/ /:/;p}'
>>
>> It also needs to be even as an odd value would be a broadcast address.
>> Using
>> only sed instead of head|cut|awk does have merit and the randomness is
>> increased
>> by 6 bits. It will take me a while to understand the sed here. Regular
>> expressions are not my thing.
>
> Yes, you are right, so I need to change a few things, in order to keep
> the highest possible level of randomness while ensuring the lower half
> of first byte is 2, 6, a or e.
>
> dd if=/dev/random bs=1 count=6 2>/dev/null |
> od -tx1 |
> sed -ne '1{;s/0000000 //;
> s/^\(.\)[013]/\12/;s/^\(.\)[457]/\16/;
> s/^\(.\)[89b]/\1a/;s/^\(.\)[cdf]/\1e/;
> y/ /:/;p}'
>
> Translation to humain form :
>
> # for the first line only,
> # If second char is 0, 1 or 3, replace it with 2.
> # If second char is 4, 5 or 7, replace it with 6.
> # If second char is 8, 9 or b, replace it with a.
> # If second char is c, d or f, replace it with e.
> # Replace all spaces with ':'.
> # print
> # Print nothing for other lines, thanks to -n option.

That is a lot of work for 2 extra bits, but thanks. I had changed the [014589cd]
of your original with [01345789bcdf], which resulted in X2 as the first byte for
most generated addresses.

>>> If DEVPATH is "generic" (wlan*), how would you distinguish between two
>>> broadcom NIC present in the system, both without an SPROM ?
>>
>> That is covered by the
>> /devices/pci0000:00/0000:00:0d.0/0000:04:00.0/... part of
>
> Ok, sounds good for me. Did you had the opportunity to test with two
> such devices ?

Yes. On a machine where I have two PCI versions of BCM43XX devices, one is
...00:08.0/... and the other is ...00:0a.0/... The generated rules are

SUBSYSTEM=="net", DEVPATH=="/devices/pci0000:00/0000:00:08.0/ssb0:0/net/wlan*",
ATTR{address}=="82:82:82:82:82:82", ENV{MAC_CHANGED}="yes", RUN+="/sbin/ifconfig
$env{INTERFACE} hw ether c2:e7:d3:d8:73:a8"
SUBSYSTEM=="net", DEVPATH=="/devices/pci0000:00/0000:00:0a.0/ssb1:0/net/wlan*",
ATTR{address}=="82:82:82:82:82:82", ENV{MAC_CHANGED}="yes", RUN+="/sbin/ifconfig
$env{INTERFACE} hw ether 02:56:31:dd:7f:85"

Incidentally, I tried ip rather than ifconfig, and found that the MAC address
was never changed.

Larry

2010-03-22 22:20:00

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On 03/22/2010 04:55 PM, Michael Buesch wrote:
>
> I don't see a problem for udev to distinguish the cards. It can do it merely on
> the bus-ID. That's unique. Yeah, it might change if you change the hardware.
> But do we care? I say no, because you cannot actually change the hardware in real life
> for any of these devices. And even if you could reorder the devices on the bus or whatever.
> What would happen? The card would get a new MAC address. That's all. That's acceptable.
>
> The kernel would (for example) just set the mac address to all-ones. Udev would
> notice this (invalid) mac address and reassign a new persistent one to the device. It then
> stores the address on the harddisk.

What ensures that this persistent name would be unique?

> In fact, if we implement a mechanism in the kernel, we have _exactly_ the same problem.
> However, currently Larry's patches just ignore that problem and assume that there's only
> one card in the system anyway.

As I said in a posting a few minutes ago, that problem is solved.

Larry

2010-03-28 19:04:37

by Nicolas de Pesloüan

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

Larry Finger wrote:
> On 03/28/2010 12:14 PM, Nicolas de Peslo?an wrote:
>> Larry Finger wrote:
>>> On 03/24/2010 02:21 PM, Michael Buesch wrote:
>>>> On Wednesday 24 March 2010 15:16:03 Larry Finger wrote:
>>>>> I have modified ssb to supply a MAC address of 80:80:80:80:80:80,
>>>>> rather than
>>>> What about also setting the local-assignment bit for this temporary
>>>> address?
>>>>
>>>>> The one remaining problem is that the interface has already been
>>>>> renamed before
>>>>> 60-persistent-b43-mac.rules is processed. In my case, the interface
>>>>> is wlan13,
>>>>> not wlan0. After I manually modified 60-..., then the new address is
>>>>> applied.
>>>>> I'm still working on this problem.
>>>> Well, udev scripts are processed in alphabetical order. Can't you
>>>> simply run
>>>> the persistent mac rules before the persistent ifname rules?
>>> I finally figured out the problem I was having. The address attribute
>>> was not
>>> being changed by the "ifconfig" call that changed the hardware
>>> address. The fix
>>> is to create a new environment when the hardware address and lock out
>>> the rule
>>> generation process when that value is detected. The new code for
>>> /lib/udev/rules.d/65-persistent-b43-mac-generator.rules is as follows
>>> (Note:
>>> These files are line-wrapped here.):
>>>
>>> #=======================================
>>> #
>>> # Rules file to assign a unique, permanent address to BCM43XX devices
>>> without
>>> # an SPROM.
>>> #
>>> # Copyright (c) 2010 by Calvin Walton <[email protected]>
>>> # Copyright (c) 2010 by Larry Finger <[email protected]>
>>>
>>> # skip this code if action is not add, i.e. change or remove
>>>
>>> ACTION!="add", GOTO="persistent_b43_mac_generator_end"
>>>
>>> # Use the value of the MAC_CHANGED environment variable to see if the
>>> address
>>> # has already been changed.
>>>
>>> ENV{MAC_CHANGED}=="yes", GOTO="persistent_b43_mac_generator_end"
>>>
>>> # Call script to get a random address - if this device previously
>>> encountered,
>>> # the address will already have been changed.
>>>
>>> SUBSYSTEM=="net", ATTR{address}=="82:82:82:82:82:82",
>>> IMPORT{program}="write_persistent_b43_mac"
>>>
>>> # Apply the new hardware address returned by the script
>>>
>>> SUBSYSTEM=="net", ATTR{address}=="82:82:82:82:82:82",
>>> RUN+="/sbin/ifconfig
>>> $env{INTERFACE} hw ether $env{MACADDRESS_NEW}"
>> Why do you use ifconfig hw ether instead of ip link set address ?
>>
>>> LABEL="persistent_b43_mac_generator_end"
>>> #=======================================
>>>
>>> The code for /lib/udev/write_persistent_b43_mac is as follows:
>>>
>>> #=======================================
>>> #!/bin/bash
>>>
>>> # Script to Generate a random MAC address for a BCM43XX device without
>>> # an SPROM.
>>> #
>>> # Copyright (c) 2010 by Calvin Walton <[email protected]>
>>> # Copyright (c) 2010 by Larry Finger <[email protected]>
>>>
>>> # Use /dev/urandom to generate the last 5 bytes of the address.
>>> # Make the first byte 2 to avoid generating a multicast address and to
>>> set
>>> # the locally administered address bit.
>>>
>>> MACADDRESS=$(/bin/dd if=/dev/urandom bs=1 count=5 2>/dev/null |
>>> /usr/bin/od -tx1
>>> | /usr/bin/head -1 | \
>>> /usr/bin/cut -d' ' -f2- | /usr/bin/awk '{ print
>>> "02:"$1":"$2":"$3":"$4":"$5 }')
>> A suggest the following :
>>
>> - 6 bytes of randomness and force lower half of first byte to 2 if the
>> value does not have bit #2 set.
>> - sed, instead of head|cut|awk
>>
>> MACADDRESS=$(/bin/dd if=/dev/random bs=1 count=6 2>/dev/null |
>> /usr/bin/od -tx1 |
>> sed -ne '1{;s/0000000 //;s/^\(.\)[014589cd]/\12/;y/ /:/;p}'
>
> It also needs to be even as an odd value would be a broadcast address. Using
> only sed instead of head|cut|awk does have merit and the randomness is increased
> by 6 bits. It will take me a while to understand the sed here. Regular
> expressions are not my thing.

Yes, you are right, so I need to change a few things, in order to keep the highest possible level of
randomness while ensuring the lower half of first byte is 2, 6, a or e.

dd if=/dev/random bs=1 count=6 2>/dev/null |
od -tx1 |
sed -ne '1{;s/0000000 //;
s/^\(.\)[013]/\12/;s/^\(.\)[457]/\16/;
s/^\(.\)[89b]/\1a/;s/^\(.\)[cdf]/\1e/;
y/ /:/;p}'

Translation to humain form :

# for the first line only,
# If second char is 0, 1 or 3, replace it with 2.
# If second char is 4, 5 or 7, replace it with 6.
# If second char is 8, 9 or b, replace it with a.
# If second char is c, d or f, replace it with e.
# Replace all spaces with ':'.
# print
# Print nothing for other lines, thanks to -n option.

>
>>> # Define the output rules file
>>> RULES_FILE='/etc/udev/rules.d/60-persistent-b43-mac.rules'
>>>
>>> . /lib/udev/rule_generator.functions
>>>
>>> # Prevent concurrent processes from modifying the file at the same time.
>>> lock_rules_file
>>>
>>> # Check if the rules file is writeable.
>>> choose_rules_file
>>>
>>> # The rule should apply for all wlan devices -s some other wireless
>>> driver might
>>> # be loaded first - change wlanNN to wlan*
>>> GEN_PATH=$(echo $DEVPATH | /usr/bin/sed s/wlan[0-9]*/wlan*/)
>> sed should be quoted here : /usr/bin/sed -e 's/wlan[0-9]*/wlan*/'
>> Else, it might be fun if you happen to have a file called s/wlan7/wlan15
>> in current directory.
>
> OK - I see your point. As the current directory is /lib/udev, the presence of
> such a file is unlikely, but better to protect against the problem.
>
>>> # Output new rule
>>> echo "SUBSYSTEM==\"net\", DEVPATH==\"$GEN_PATH\",
>>> ATTR{address}==\"82:82:82:82:82:82\", ENV{MAC_CHANGED}=\"yes\",
>>> RUN+=\"/sbin/ifconfig \$env{INTERFACE} hw ether $MACADDRESS\"" >>
>>> $RULES_FILE
>> If DEVPATH is "generic" (wlan*), how would you distinguish between two
>> broadcom NIC present in the system, both without an SPROM ?
>
> That is covered by the /devices/pci0000:00/0000:00:0d.0/0000:04:00.0/... part of
> DEVPATH. A second device on the same bridge would have ...04:01.0... and a
> device on a different bridge would change some other part of the string. The
> change to wlan* handles the case where the BCM43XX device is discovered first
> with some configuration, and second when another device is plugged in at
> discovery time.

Ok, sounds good for me. Did you had the opportunity to test with two such devices ?
>
> Larry
>

2010-03-23 20:58:30

by Calvin Walton

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Tue, 2010-03-23 at 09:25 -0500, Larry Finger wrote:
> Will someone please write me udev rule(s) that do the following:
>
> 1. Detect a MAC address of FF:FF:FF:FF:FF:FF
> 2. If this is the first time for this bus address, then generate a random MAC
> address with the bus address encoded in it.
> 3. Preserve the address for future reloads
> 4. Load the saved address into the device.
> 5. Do the above with only standard external commands - no new programs
>
> My skills with udev are not up to the task.

I will warn you that the following is rather untested, as I don't have
any of the affected hardware (or any b43 devices at all, actually), but
something along these lines should work. There's no syntax errors, at
least :)

--- /lib/udev/rules.d/65-persistent-b43-mac-generator.rules

ACTION!="add" GOTO="persistent_b43_mac_generator_end"

SUBSYSTEM=="net", DRIVERS=="b43", ATTR{address}=="ff:ff:ff:ff:ff:ff", IMPORT{program}="write_persistent_b43_mac"

SUBSYSTEM=="net", ENV{MACADDRESS_NEW}=="?*", RUN+="ifconfig $env{INTERFACE} hw ether $env{MACADDRESS_NEW}"

LABEL="persistent_b43_mac_generator_end"

--- /lib/udev/write_persistent_b43_mac (chmod +x)

#!/bin/bash

# This mac address generation function could be replaced with something better
MACADDRESS=$(dd if=/dev/urandom bs=1 count=6 2>/dev/null | od -tx1 | head -1 | cut -d' ' -f2- | awk '{ print $1":"$2":"$3":"$4":"$5":"$6 }')

RULES_FILE='/etc/udev/rules.d/60-persistent-b43-mac.rules'

. /lib64/udev/rule_generator.functions

lock_rules_file

choose_rules_file

echo "DEVPATH==\"$DEVPATH\", DRIVERS==\"b43\", ATTR{address}==\"ff:ff:ff:ff:ff:ff\", RUN+=\"ifconfig $INTERFACE hw ether $MACADDRESS\"" >> $RULES_FILE

echo "MACADDRESS_NEW=$MACADDRESS"

unlock_rules_file

---

A new file "/etc/udev/rules.d/60-persistent-b43-mac.rules" will be
created, which will contain the the saved mac address and bypass the
generating script on future boots.

This should probably be run by the udev maintainers, but is a start,
anyways.

--
Calvin Walton <[email protected]>


2010-03-22 15:06:39

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On 03/22/2010 03:37 AM, Michael Buesch wrote:
> On Monday 22 March 2010 07:28:23 Calvin Walton wrote:
>> Hi,
>>
>> On Sat, 2010-03-20 at 19:14 -0500, Larry Finger wrote:
>> Some recent BCM43XX devices lack an on-board SPROM. The pertinent data
>>> from the SPROM could be included in the kernel; however, this presents
>>> a problem in the generation of a unique, reproducible MAC address. The
>>> solution has been to create a utility that generates a virtual SPROM
>>> image with a random MAC address. This image is stored in the firmware
>>> area, and loaded using the asyncronous firmware load facility.
>>
>> I'm curious, how would this firmware-loading scheme deal with having
>> multiple cards of this type installed? This seems like an unusual
>> situation, but it looks like this patch will cause all of the cards to
>> start up with the same MAC address due to the fixed filename.
>>
>> Instead of using a firmware file to load in the MAC address, might it be
>> possible to move the persistent MAC setting to a simple udev rule which
>> generates a persistent MAC address, saves it, then sets it each boot
>> using a command like "ip link set wlan0 address XX:XX:XX:XX:XX:XX" ?
>>
>> This would remove the need to have this "fake" firmware file available
>> at boot, provided that the driver can handle leaving the address
>> unconfigured until userspace gets around to setting it. As well, it
>> could be written to work with multiple cards easily, saving a different
>> MAC for each.
>>
>> Some thoughts for your consideration,
>>
>
> I think this actually is a very good idea.
> This way we could live without a user-supplied sprom image, which I would _really_
> prefer.

Avoiding the use of a new user-space program would be desirable, but I cannot
think of any way that a udev rule could distinguish one card from another. If we
had any unique features such as a serial number, then we wouldn't need user
space at all. Any suggestions?

Larry



2010-03-22 22:28:42

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Monday 22 March 2010 23:19:54 Larry Finger wrote:
> On 03/22/2010 04:55 PM, Michael Buesch wrote:
> >
> > I don't see a problem for udev to distinguish the cards. It can do it merely on
> > the bus-ID. That's unique. Yeah, it might change if you change the hardware.
> > But do we care? I say no, because you cannot actually change the hardware in real life
> > for any of these devices. And even if you could reorder the devices on the bus or whatever.
> > What would happen? The card would get a new MAC address. That's all. That's acceptable.
> >
> > The kernel would (for example) just set the mac address to all-ones. Udev would
> > notice this (invalid) mac address and reassign a new persistent one to the device. It then
> > stores the address on the harddisk.
>
> What ensures that this persistent name would be unique?

The same mechanism that ensures that an UUID is unique: A good random number generator.

> > In fact, if we implement a mechanism in the kernel, we have _exactly_ the same problem.
> > However, currently Larry's patches just ignore that problem and assume that there's only
> > one card in the system anyway.
>
> As I said in a posting a few minutes ago, that problem is solved.

Well. You distinguish by bus-ID, right? That's exactly the thing that udev does.

--
Greetings, Michael.

2010-03-23 11:43:18

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Tuesday 23 March 2010 09:55:39 Florian Fainelli wrote:
> Why not use random_ether_addr and only use the digits that you are interested
> in?

Because it's not constant across reboots.

--
Greetings, Michael.

2010-03-22 21:55:09

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Monday 22 March 2010 16:06:34 Larry Finger wrote:
> Avoiding the use of a new user-space program would be desirable, but I cannot
> think of any way that a udev rule could distinguish one card from another. If we
> had any unique features such as a serial number, then we wouldn't need user
> space at all. Any suggestions?

I don't see a problem for udev to distinguish the cards. It can do it merely on
the bus-ID. That's unique. Yeah, it might change if you change the hardware.
But do we care? I say no, because you cannot actually change the hardware in real life
for any of these devices. And even if you could reorder the devices on the bus or whatever.
What would happen? The card would get a new MAC address. That's all. That's acceptable.

The kernel would (for example) just set the mac address to all-ones. Udev would
notice this (invalid) mac address and reassign a new persistent one to the device. It then
stores the address on the harddisk.

In fact, if we implement a mechanism in the kernel, we have _exactly_ the same problem.
However, currently Larry's patches just ignore that problem and assume that there's only
one card in the system anyway.

--
Greetings, Michael.

2010-03-26 03:47:07

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On 03/24/2010 02:21 PM, Michael Buesch wrote:
> On Wednesday 24 March 2010 15:16:03 Larry Finger wrote:
>> I have modified ssb to supply a MAC address of 80:80:80:80:80:80, rather than
>
> What about also setting the local-assignment bit for this temporary address?
>
>> The one remaining problem is that the interface has already been renamed before
>> 60-persistent-b43-mac.rules is processed. In my case, the interface is wlan13,
>> not wlan0. After I manually modified 60-..., then the new address is applied.
>> I'm still working on this problem.
>
> Well, udev scripts are processed in alphabetical order. Can't you simply run
> the persistent mac rules before the persistent ifname rules?

I finally figured out the problem I was having. The address attribute was not
being changed by the "ifconfig" call that changed the hardware address. The fix
is to create a new environment when the hardware address and lock out the rule
generation process when that value is detected. The new code for
/lib/udev/rules.d/65-persistent-b43-mac-generator.rules is as follows (Note:
These files are line-wrapped here.):

#=======================================
#
# Rules file to assign a unique, permanent address to BCM43XX devices without
# an SPROM.
#
# Copyright (c) 2010 by Calvin Walton <[email protected]>
# Copyright (c) 2010 by Larry Finger <[email protected]>

# skip this code if action is not add, i.e. change or remove

ACTION!="add", GOTO="persistent_b43_mac_generator_end"

# Use the value of the MAC_CHANGED environment variable to see if the address
# has already been changed.

ENV{MAC_CHANGED}=="yes", GOTO="persistent_b43_mac_generator_end"

# Call script to get a random address - if this device previously encountered,
# the address will already have been changed.

SUBSYSTEM=="net", ATTR{address}=="82:82:82:82:82:82",
IMPORT{program}="write_persistent_b43_mac"

# Apply the new hardware address returned by the script

SUBSYSTEM=="net", ATTR{address}=="82:82:82:82:82:82", RUN+="/sbin/ifconfig
$env{INTERFACE} hw ether $env{MACADDRESS_NEW}"

LABEL="persistent_b43_mac_generator_end"
#=======================================

The code for /lib/udev/write_persistent_b43_mac is as follows:

#=======================================
#!/bin/bash

# Script to Generate a random MAC address for a BCM43XX device without
# an SPROM.
#
# Copyright (c) 2010 by Calvin Walton <[email protected]>
# Copyright (c) 2010 by Larry Finger <[email protected]>

# Use /dev/urandom to generate the last 5 bytes of the address.
# Make the first byte 2 to avoid generating a multicast address and to set
# the locally administered address bit.

MACADDRESS=$(/bin/dd if=/dev/urandom bs=1 count=5 2>/dev/null | /usr/bin/od -tx1
| /usr/bin/head -1 | \
/usr/bin/cut -d' ' -f2- | /usr/bin/awk '{ print "02:"$1":"$2":"$3":"$4":"$5 }')

# Define the output rules file
RULES_FILE='/etc/udev/rules.d/60-persistent-b43-mac.rules'

. /lib/udev/rule_generator.functions

# Prevent concurrent processes from modifying the file at the same time.
lock_rules_file

# Check if the rules file is writeable.
choose_rules_file

# The rule should apply for all wlan devices -s some other wireless driver might
# be loaded first - change wlanNN to wlan*
GEN_PATH=$(echo $DEVPATH | /usr/bin/sed s/wlan[0-9]*/wlan*/)

# Output new rule
echo "SUBSYSTEM==\"net\", DEVPATH==\"$GEN_PATH\",
ATTR{address}==\"82:82:82:82:82:82\", ENV{MAC_CHANGED}=\"yes\",
RUN+=\"/sbin/ifconfig \$env{INTERFACE} hw ether $MACADDRESS\"" >> $RULES_FILE

# Report the new address back to the caller who will set the address for this
new interface
echo "MACADDRESS_NEW=$MACADDRESS"

unlock_rules_file

exit 0
#=======================================

Is there a location to put a tar file containing the script and rules files?

Larry

2010-03-23 14:25:52

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On 03/23/2010 03:52 AM, Michael Buesch wrote:
> On Tuesday 23 March 2010 00:45:28 Larry Finger wrote:
>> (2) In drivers/ssb/pci.c, the firmware loading process reads the file, then
>> modifies it using the bus address.
>
> No. You don't need firmware loading at all.
> udev can just set the mac address through the standard ioctl calls.
> Just like how ifconfig and ip can do.

Will someone please write me udev rule(s) that do the following:

1. Detect a MAC address of FF:FF:FF:FF:FF:FF
2. If this is the first time for this bus address, then generate a random MAC
address with the bus address encoded in it.
3. Preserve the address for future reloads
4. Load the saved address into the device.
5. Do the above with only standard external commands - no new programs

My skills with udev are not up to the task.

I now have code running that uses the firmware load facility to read a
file created by /bin/dbus-uuidgen to create a MAC address that will satisfy the
requirements. If the file is needed but not available, then the user is prompted
with the commands needed to create it.

Larry

2010-03-24 14:16:08

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On 03/23/2010 03:58 PM, Calvin Walton wrote:
>
> I will warn you that the following is rather untested, as I don't have
> any of the affected hardware (or any b43 devices at all, actually), but
> something along these lines should work. There's no syntax errors, at
> least :)
>
> --- /lib/udev/rules.d/65-persistent-b43-mac-generator.rules
>
> ACTION!="add" GOTO="persistent_b43_mac_generator_end"
>
> SUBSYSTEM=="net", DRIVERS=="b43", ATTR{address}=="ff:ff:ff:ff:ff:ff", IMPORT{program}="write_persistent_b43_mac"
>
> SUBSYSTEM=="net", ENV{MACADDRESS_NEW}=="?*", RUN+="ifconfig $env{INTERFACE} hw ether $env{MACADDRESS_NEW}"
>
> LABEL="persistent_b43_mac_generator_end"
>
> --- /lib/udev/write_persistent_b43_mac (chmod +x)
>
> #!/bin/bash
>
> # This mac address generation function could be replaced with something better
> MACADDRESS=$(dd if=/dev/urandom bs=1 count=6 2>/dev/null | od -tx1 | head -1 | cut -d' ' -f2- | awk '{ print $1":"$2":"$3":"$4":"$5":"$6 }')
>
> RULES_FILE='/etc/udev/rules.d/60-persistent-b43-mac.rules'
>
> . /lib64/udev/rule_generator.functions
>
> lock_rules_file
>
> choose_rules_file
>
> echo "DEVPATH==\"$DEVPATH\", DRIVERS==\"b43\", ATTR{address}==\"ff:ff:ff:ff:ff:ff\", RUN+=\"ifconfig $INTERFACE hw ether $MACADDRESS\"" >> $RULES_FILE
>
> echo "MACADDRESS_NEW=$MACADDRESS"
>
> unlock_rules_file
>
> ---
>
> A new file "/etc/udev/rules.d/60-persistent-b43-mac.rules" will be
> created, which will contain the the saved mac address and bypass the
> generating script on future boots.
>
> This should probably be run by the udev maintainers, but is a start,
> anyways.

Thanks for the really good start.

The above routines have a minor problem in that using a random value for the
first byte of the MAC address runs the risk of having the multicast bit set. By
fixing the first one to a multiple of 4, that problem is avoided.

A second minor problem is that the full path must be given for all standard
utilities, otherwise they are assumed to be in /lib/udev. Thus all references to
"ifconfig" were replaced by "/sbin/ifconfig". I hope all distros put ifconfig in
/sbin.

I have modified ssb to supply a MAC address of 80:80:80:80:80:80, rather than
FF:FF:FF:FF:FF:FF. This way the interface can come up even though the new udev
routines are not available. When b43 detects that address, it will log a message
describing why the udev routines are needed, and how to obtain them.

The one remaining problem is that the interface has already been renamed before
60-persistent-b43-mac.rules is processed. In my case, the interface is wlan13,
not wlan0. After I manually modified 60-..., then the new address is applied.
I'm still working on this problem.

Thanks again for the help.

Larry

2010-03-22 06:28:29

by Calvin Walton

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

Hi,

On Sat, 2010-03-20 at 19:14 -0500, Larry Finger wrote:
Some recent BCM43XX devices lack an on-board SPROM. The pertinent data
> from the SPROM could be included in the kernel; however, this presents
> a problem in the generation of a unique, reproducible MAC address. The
> solution has been to create a utility that generates a virtual SPROM
> image with a random MAC address. This image is stored in the firmware
> area, and loaded using the asyncronous firmware load facility.

I'm curious, how would this firmware-loading scheme deal with having
multiple cards of this type installed? This seems like an unusual
situation, but it looks like this patch will cause all of the cards to
start up with the same MAC address due to the fixed filename.

Instead of using a firmware file to load in the MAC address, might it be
possible to move the persistent MAC setting to a simple udev rule which
generates a persistent MAC address, saves it, then sets it each boot
using a command like "ip link set wlan0 address XX:XX:XX:XX:XX:XX" ?

This would remove the need to have this "fake" firmware file available
at boot, provided that the driver can handle leaving the address
unconfigured until userspace gets around to setting it. As well, it
could be written to work with multiple cards easily, saving a different
MAC for each.

Some thoughts for your consideration,
--
Calvin Walton <[email protected]>


2010-03-28 17:14:20

by Nicolas de Pesloüan

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

Larry Finger wrote:
> On 03/24/2010 02:21 PM, Michael Buesch wrote:
>> On Wednesday 24 March 2010 15:16:03 Larry Finger wrote:
>>> I have modified ssb to supply a MAC address of 80:80:80:80:80:80, rather than
>> What about also setting the local-assignment bit for this temporary address?
>>
>>> The one remaining problem is that the interface has already been renamed before
>>> 60-persistent-b43-mac.rules is processed. In my case, the interface is wlan13,
>>> not wlan0. After I manually modified 60-..., then the new address is applied.
>>> I'm still working on this problem.
>> Well, udev scripts are processed in alphabetical order. Can't you simply run
>> the persistent mac rules before the persistent ifname rules?
>
> I finally figured out the problem I was having. The address attribute was not
> being changed by the "ifconfig" call that changed the hardware address. The fix
> is to create a new environment when the hardware address and lock out the rule
> generation process when that value is detected. The new code for
> /lib/udev/rules.d/65-persistent-b43-mac-generator.rules is as follows (Note:
> These files are line-wrapped here.):
>
> #=======================================
> #
> # Rules file to assign a unique, permanent address to BCM43XX devices without
> # an SPROM.
> #
> # Copyright (c) 2010 by Calvin Walton <[email protected]>
> # Copyright (c) 2010 by Larry Finger <[email protected]>
>
> # skip this code if action is not add, i.e. change or remove
>
> ACTION!="add", GOTO="persistent_b43_mac_generator_end"
>
> # Use the value of the MAC_CHANGED environment variable to see if the address
> # has already been changed.
>
> ENV{MAC_CHANGED}=="yes", GOTO="persistent_b43_mac_generator_end"
>
> # Call script to get a random address - if this device previously encountered,
> # the address will already have been changed.
>
> SUBSYSTEM=="net", ATTR{address}=="82:82:82:82:82:82",
> IMPORT{program}="write_persistent_b43_mac"
>
> # Apply the new hardware address returned by the script
>
> SUBSYSTEM=="net", ATTR{address}=="82:82:82:82:82:82", RUN+="/sbin/ifconfig
> $env{INTERFACE} hw ether $env{MACADDRESS_NEW}"

Why do you use ifconfig hw ether instead of ip link set address ?

> LABEL="persistent_b43_mac_generator_end"
> #=======================================
>
> The code for /lib/udev/write_persistent_b43_mac is as follows:
>
> #=======================================
> #!/bin/bash
>
> # Script to Generate a random MAC address for a BCM43XX device without
> # an SPROM.
> #
> # Copyright (c) 2010 by Calvin Walton <[email protected]>
> # Copyright (c) 2010 by Larry Finger <[email protected]>
>
> # Use /dev/urandom to generate the last 5 bytes of the address.
> # Make the first byte 2 to avoid generating a multicast address and to set
> # the locally administered address bit.
>
> MACADDRESS=$(/bin/dd if=/dev/urandom bs=1 count=5 2>/dev/null | /usr/bin/od -tx1
> | /usr/bin/head -1 | \
> /usr/bin/cut -d' ' -f2- | /usr/bin/awk '{ print "02:"$1":"$2":"$3":"$4":"$5 }')

A suggest the following :

- 6 bytes of randomness and force lower half of first byte to 2 if the value does not have bit #2 set.
- sed, instead of head|cut|awk

MACADDRESS=$(/bin/dd if=/dev/random bs=1 count=6 2>/dev/null | /usr/bin/od -tx1 |
sed -ne '1{;s/0000000 //;s/^\(.\)[014589cd]/\12/;y/ /:/;p}'

> # Define the output rules file
> RULES_FILE='/etc/udev/rules.d/60-persistent-b43-mac.rules'
>
> . /lib/udev/rule_generator.functions
>
> # Prevent concurrent processes from modifying the file at the same time.
> lock_rules_file
>
> # Check if the rules file is writeable.
> choose_rules_file
>
> # The rule should apply for all wlan devices -s some other wireless driver might
> # be loaded first - change wlanNN to wlan*
> GEN_PATH=$(echo $DEVPATH | /usr/bin/sed s/wlan[0-9]*/wlan*/)

sed should be quoted here : /usr/bin/sed -e 's/wlan[0-9]*/wlan*/'
Else, it might be fun if you happen to have a file called s/wlan7/wlan15 in current directory.

> # Output new rule
> echo "SUBSYSTEM==\"net\", DEVPATH==\"$GEN_PATH\",
> ATTR{address}==\"82:82:82:82:82:82\", ENV{MAC_CHANGED}=\"yes\",
> RUN+=\"/sbin/ifconfig \$env{INTERFACE} hw ether $MACADDRESS\"" >> $RULES_FILE

If DEVPATH is "generic" (wlan*), how would you distinguish between two broadcom NIC present in the
system, both without an SPROM ?

Nicolas.

>
> # Report the new address back to the caller who will set the address for this
> new interface
> echo "MACADDRESS_NEW=$MACADDRESS"
>
> unlock_rules_file
>
> exit 0
> #=======================================
>
> Is there a location to put a tar file containing the script and rules files?
>
> Larry
> _______________________________________________
> Bcm43xx-dev mailing list
> [email protected]
> https://lists.berlios.de/mailman/listinfo/bcm43xx-dev
>


2010-03-24 19:21:43

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Wednesday 24 March 2010 15:16:03 Larry Finger wrote:
> I have modified ssb to supply a MAC address of 80:80:80:80:80:80, rather than

What about also setting the local-assignment bit for this temporary address?

> The one remaining problem is that the interface has already been renamed before
> 60-persistent-b43-mac.rules is processed. In my case, the interface is wlan13,
> not wlan0. After I manually modified 60-..., then the new address is applied.
> I'm still working on this problem.

Well, udev scripts are processed in alphabetical order. Can't you simply run
the persistent mac rules before the persistent ifname rules?

--
Greetings, Michael.

2010-03-22 22:25:38

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Monday 22 March 2010 22:56:44 Larry Finger wrote:
> Does anyone have any suggestions on what characteristic could be used to
> generate a unique MAC address for a box in a udev rule?

/dev/urandom

Yeah, there's the chance of clashes. In practice there won't be any clashes,
however. If you think there's a real risk, you should start playing
the lottery tomorrow. You'll immediately win a million dollars so you don't have
to worry about those questions anymore. ;)

In fact, I think the risk for mac clashes is not really reduced by generating the mac
address from serial numbers, whatever, etc...

--
Greetings, Michael.

2010-03-22 23:53:20

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

Michael,

Would the following be sufficient?

(1) The utility /bin/dbus-uuidgen is used to create a file
/lib/firmware/ssb/mac_address. The command to create this file if it does not
exist could be added to the scripts that use fwcutter.

(2) In drivers/ssb/pci.c, the firmware loading process reads the file, then
modifies it using the bus address.

I think this process would create a persistent MAC address that is unique within
the box, with a small probability of being duplicated on another machine.

Larry


2010-03-22 21:56:49

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On 03/22/2010 01:28 AM, Calvin Walton wrote:
>
> I'm curious, how would this firmware-loading scheme deal with having
> multiple cards of this type installed? This seems like an unusual
> situation, but it looks like this patch will cause all of the cards to
> start up with the same MAC address due to the fixed filename.

I have a workaround for this problem based on the bus address of the device.

> Instead of using a firmware file to load in the MAC address, might it be
> possible to move the persistent MAC setting to a simple udev rule which
> generates a persistent MAC address, saves it, then sets it each boot
> using a command like "ip link set wlan0 address XX:XX:XX:XX:XX:XX" ?
>
> This would remove the need to have this "fake" firmware file available
> at boot, provided that the driver can handle leaving the address
> unconfigured until userspace gets around to setting it. As well, it
> could be written to work with multiple cards easily, saving a different
> MAC for each.

Does anyone have any suggestions on what characteristic could be used to
generate a unique MAC address for a box in a udev rule? Unless someone proposes
a workable solution, I will push V3 of my patch with the bus address change
noted above.

Larry

2010-03-24 02:26:32

by Ehud Gavron

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk



Michael Buesch wrote:
> On Monday 22 March 2010 22:56:44 Larry Finger wrote:
>
>> Does anyone have any suggestions on what characteristic could be used to
>> generate a unique MAC address for a box in a udev rule?
>>
>
> /dev/urandom
>
> Yeah, there's the chance of clashes. In practice there won't be any clashes,
> however. If you think there's a real risk, you should start playing
> the lottery tomorrow. You'll immediately win a million dollars so you don't have
> to worry about those questions anymore. ;)
>
> In fact, I think the risk for mac clashes is not really reduced by generating the mac
> address from serial numbers, whatever, etc...
>
>
DEC used the L3 address to encode a new MAC at the time the [L3] address was
set (DECnet v4). The advantage was they didn't need to use the equivalent
of ARP. Of course this is a violation of strict layer separation.

Octet1-Octet3 - Broadcom assigned MAC IDs. I found the following:
00-05-B5
00-10-18
00-1B-E9
18-C0-86

Octet4-octet6 - Lowest three octets of the unixtime.


Advantages: for the local area network all TZ settings should be the same,
so the MAC addresses *will* be different. Beyond the first router that
won't
matter. Also for the same machine different interfaces are GUARANTEED
to have different MAC addresses.

For two machines to have the same MAC they would have to be booted at
(same time x processing factor) such that the B43 initialization will
result
in the same MAC address. I'd like to think those odds are even lower than
your lottery.

A million dollars?
http://www.active-domain.com/resources/million-dollar-domains.htm
Yeah got the t-shirt.


E


Attachments:
smime.p7s (5.38 kB)
S/MIME Cryptographic Signature

2010-03-23 08:52:59

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Tuesday 23 March 2010 00:45:28 Larry Finger wrote:
> (2) In drivers/ssb/pci.c, the firmware loading process reads the file, then
> modifies it using the bus address.

No. You don't need firmware loading at all.
udev can just set the mac address through the standard ioctl calls.
Just like how ifconfig and ip can do.

--
Greetings, Michael.

2010-03-23 08:55:51

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH V2] ssb: Implement virtual SPROM on disk

On Monday 22 March 2010 22:56:44 Larry Finger wrote:
> On 03/22/2010 01:28 AM, Calvin Walton wrote:
> > I'm curious, how would this firmware-loading scheme deal with having
> > multiple cards of this type installed? This seems like an unusual
> > situation, but it looks like this patch will cause all of the cards to
> > start up with the same MAC address due to the fixed filename.
>
> I have a workaround for this problem based on the bus address of the
> device.
>
> > Instead of using a firmware file to load in the MAC address, might it be
> > possible to move the persistent MAC setting to a simple udev rule which
> > generates a persistent MAC address, saves it, then sets it each boot
> > using a command like "ip link set wlan0 address XX:XX:XX:XX:XX:XX" ?
> >
> > This would remove the need to have this "fake" firmware file available
> > at boot, provided that the driver can handle leaving the address
> > unconfigured until userspace gets around to setting it. As well, it
> > could be written to work with multiple cards easily, saving a different
> > MAC for each.
>
> Does anyone have any suggestions on what characteristic could be used to
> generate a unique MAC address for a box in a udev rule? Unless someone
> proposes a workable solution, I will push V3 of my patch with the bus
> address change noted above.

Why not use random_ether_addr and only use the digits that you are interested
in?
--
Florian