2022-07-08 13:34:18

by Hans de Goede

[permalink] [raw]
Subject: [PATCH 1/2] brcmfmac: Add brcmf_c_set_cur_etheraddr() helper

Add a little helper to send "cur_etheraddr" commands to the interface
and to handle the error reporting of it in a single place.

Signed-off-by: Hans de Goede <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/common.c | 18 +++++++++++++-----
.../broadcom/brcm80211/brcmfmac/common.h | 1 +
.../broadcom/brcm80211/brcmfmac/core.c | 8 ++------
3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index fe01da9e620d..dccd8f4ca1d0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -190,6 +190,17 @@ static int brcmf_c_process_clm_blob(struct brcmf_if *ifp)
return err;
}

+int brcmf_c_set_cur_etheraddr(struct brcmf_if *ifp, const u8 *addr)
+{
+ s32 err;
+
+ err = brcmf_fil_iovar_data_set(ifp, "cur_etheraddr", addr, ETH_ALEN);
+ if (err < 0)
+ bphy_err(ifp->drvr, "Setting cur_etheraddr failed, %d\n", err);
+
+ return err;
+}
+
int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
{
struct brcmf_pub *drvr = ifp->drvr;
@@ -204,12 +215,9 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)

if (is_valid_ether_addr(ifp->mac_addr)) {
/* set mac address */
- err = brcmf_fil_iovar_data_set(ifp, "cur_etheraddr", ifp->mac_addr,
- ETH_ALEN);
- if (err < 0) {
- bphy_err(ifp->drvr, "Setting cur_etheraddr failed, %d\n", err);
+ err = brcmf_c_set_cur_etheraddr(ifp, ifp->mac_addr);
+ if (err < 0)
goto done;
- }
} else {
/* retrieve mac address */
err = brcmf_fil_iovar_data_get(ifp, "cur_etheraddr", ifp->mac_addr,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index 15accc88d5c0..7329eb751945 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -65,6 +65,7 @@ void brcmf_release_module_param(struct brcmf_mp_device *module_param);

/* Sets dongle media info (drv_version, mac address). */
int brcmf_c_preinit_dcmds(struct brcmf_if *ifp);
+int brcmf_c_set_cur_etheraddr(struct brcmf_if *ifp, const u8 *addr);

#ifdef CONFIG_DMI
void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 87aef211b35f..bd164a0821f9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -233,16 +233,12 @@ static int brcmf_netdev_set_mac_address(struct net_device *ndev, void *addr)
{
struct brcmf_if *ifp = netdev_priv(ndev);
struct sockaddr *sa = (struct sockaddr *)addr;
- struct brcmf_pub *drvr = ifp->drvr;
int err;

brcmf_dbg(TRACE, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);

- err = brcmf_fil_iovar_data_set(ifp, "cur_etheraddr", sa->sa_data,
- ETH_ALEN);
- if (err < 0) {
- bphy_err(drvr, "Setting cur_etheraddr failed, %d\n", err);
- } else {
+ err = brcmf_c_set_cur_etheraddr(ifp, sa->sa_data);
+ if (err >= 0) {
brcmf_dbg(TRACE, "updated to %pM\n", sa->sa_data);
memcpy(ifp->mac_addr, sa->sa_data, ETH_ALEN);
eth_hw_addr_set(ifp->ndev, ifp->mac_addr);
--
2.36.0


2022-07-08 13:35:03

by Hans de Goede

[permalink] [raw]
Subject: [PATCH 2/2] brcmfmac: Replace default (not configured) MAC with a random MAC

On some boards there is no eeprom to hold the nvram, in this case instead
a board specific nvram is loaded from /lib/firmware. On most boards the
macaddr=... setting in the /lib/firmware nvram file is ignored because
the wifi/bt chip has a unique MAC programmed into the chip itself.

But in some cases the actual MAC from the /lib/firmware nvram file gets
used, leading to MAC conflicts.

The MAC addresses in the troublesome nvram files seem to all come from
the same nvram file template, so we can detect this by checking for
the template nvram file MAC.

Detect that the default MAC address is being used and replace it
with a random MAC address to avoid MAC address conflicts.

Note that udev will detect this is a random MAC based on
/sys/class/net/wlan0/addr_assign_type and then replace this with
a MAC based on hashing the netdev-name + the machine-id. So that
the MAC address is both guaranteed to be unique per machine while
it is still the same/persistent at each boot (assuming the
default Link.MACAddressPolicy=persistent udev setting).

Signed-off-by: Hans de Goede <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/common.c | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index dccd8f4ca1d0..7485e784be2a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -201,6 +201,20 @@ int brcmf_c_set_cur_etheraddr(struct brcmf_if *ifp, const u8 *addr)
return err;
}

+/* On some boards there is no eeprom to hold the nvram, in this case instead
+ * a board specific nvram is loaded from /lib/firmware. On most boards the
+ * macaddr setting in the /lib/firmware nvram file is ignored because the
+ * wifibt chip has a unique MAC programmed into the chip itself.
+ * But in some cases the actual MAC from the /lib/firmware nvram file gets
+ * used, leading to MAC conflicts.
+ * The MAC addresses in the troublesome nvram files seem to all come from
+ * the same nvram file template, so we only need to check for 1 known
+ * address to detect this.
+ */
+static const u8 brcmf_default_mac_address[ETH_ALEN] = {
+ 0x00, 0x90, 0x4c, 0xc5, 0x12, 0x38
+};
+
int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
{
struct brcmf_pub *drvr = ifp->drvr;
@@ -226,6 +240,15 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
bphy_err(drvr, "Retrieving cur_etheraddr failed, %d\n", err);
goto done;
}
+
+ if (ether_addr_equal_unaligned(ifp->mac_addr, brcmf_default_mac_address)) {
+ bphy_err(drvr, "Default MAC is used, replacing with random MAC to avoid conflicts\n");
+ eth_random_addr(ifp->mac_addr);
+ ifp->ndev->addr_assign_type = NET_ADDR_RANDOM;
+ err = brcmf_c_set_cur_etheraddr(ifp, ifp->mac_addr);
+ if (err < 0)
+ goto done;
+ }
}

memcpy(ifp->drvr->mac, ifp->mac_addr, sizeof(ifp->drvr->mac));
--
2.36.0

2022-07-08 13:36:28

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 1/2] brcmfmac: Add brcmf_c_set_cur_etheraddr() helper

Hi,

On 7/8/22 15:32, Hans de Goede wrote:
> Add a little helper to send "cur_etheraddr" commands to the interface
> and to handle the error reporting of it in a single place.
>
> Signed-off-by: Hans de Goede <[email protected]>

Ugh, I accidentally used Kalle's old address. I'll resend this with
the right address, sorry for the duplicate emails.

Regards,

Hans


> ---
> .../broadcom/brcm80211/brcmfmac/common.c | 18 +++++++++++++-----
> .../broadcom/brcm80211/brcmfmac/common.h | 1 +
> .../broadcom/brcm80211/brcmfmac/core.c | 8 ++------
> 3 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> index fe01da9e620d..dccd8f4ca1d0 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> @@ -190,6 +190,17 @@ static int brcmf_c_process_clm_blob(struct brcmf_if *ifp)
> return err;
> }
>
> +int brcmf_c_set_cur_etheraddr(struct brcmf_if *ifp, const u8 *addr)
> +{
> + s32 err;
> +
> + err = brcmf_fil_iovar_data_set(ifp, "cur_etheraddr", addr, ETH_ALEN);
> + if (err < 0)
> + bphy_err(ifp->drvr, "Setting cur_etheraddr failed, %d\n", err);
> +
> + return err;
> +}
> +
> int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
> {
> struct brcmf_pub *drvr = ifp->drvr;
> @@ -204,12 +215,9 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
>
> if (is_valid_ether_addr(ifp->mac_addr)) {
> /* set mac address */
> - err = brcmf_fil_iovar_data_set(ifp, "cur_etheraddr", ifp->mac_addr,
> - ETH_ALEN);
> - if (err < 0) {
> - bphy_err(ifp->drvr, "Setting cur_etheraddr failed, %d\n", err);
> + err = brcmf_c_set_cur_etheraddr(ifp, ifp->mac_addr);
> + if (err < 0)
> goto done;
> - }
> } else {
> /* retrieve mac address */
> err = brcmf_fil_iovar_data_get(ifp, "cur_etheraddr", ifp->mac_addr,
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
> index 15accc88d5c0..7329eb751945 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
> @@ -65,6 +65,7 @@ void brcmf_release_module_param(struct brcmf_mp_device *module_param);
>
> /* Sets dongle media info (drv_version, mac address). */
> int brcmf_c_preinit_dcmds(struct brcmf_if *ifp);
> +int brcmf_c_set_cur_etheraddr(struct brcmf_if *ifp, const u8 *addr);
>
> #ifdef CONFIG_DMI
> void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev);
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index 87aef211b35f..bd164a0821f9 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> @@ -233,16 +233,12 @@ static int brcmf_netdev_set_mac_address(struct net_device *ndev, void *addr)
> {
> struct brcmf_if *ifp = netdev_priv(ndev);
> struct sockaddr *sa = (struct sockaddr *)addr;
> - struct brcmf_pub *drvr = ifp->drvr;
> int err;
>
> brcmf_dbg(TRACE, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
>
> - err = brcmf_fil_iovar_data_set(ifp, "cur_etheraddr", sa->sa_data,
> - ETH_ALEN);
> - if (err < 0) {
> - bphy_err(drvr, "Setting cur_etheraddr failed, %d\n", err);
> - } else {
> + err = brcmf_c_set_cur_etheraddr(ifp, sa->sa_data);
> + if (err >= 0) {
> brcmf_dbg(TRACE, "updated to %pM\n", sa->sa_data);
> memcpy(ifp->mac_addr, sa->sa_data, ETH_ALEN);
> eth_hw_addr_set(ifp->ndev, ifp->mac_addr);

2022-07-26 13:10:57

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] brcmfmac: Add brcmf_c_set_cur_etheraddr() helper

Hans de Goede <[email protected]> writes:

> Hi,
>
> On 7/8/22 15:32, Hans de Goede wrote:
>> Add a little helper to send "cur_etheraddr" commands to the interface
>> and to handle the error reporting of it in a single place.
>>
>> Signed-off-by: Hans de Goede <[email protected]>
>
> Ugh, I accidentally used Kalle's old address. I'll resend this with
> the right address, sorry for the duplicate emails.

No worries. And it actually doesn't even matter as I take the patches
directly from patchwork :)

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches