Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9bf3dc09-73df-8914-aa13-68ed9d48f350@gmail.com>
Date:   Fri, 8 Jul 2022 20:27:57 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH 2/2] brcmfmac: Replace default (not configured) MAC with a
 random MAC
Content-Language: en-US
To:     Hans de Goede <hdegoede@redhat.com>,
        Franky Lin <franky.lin@broadcom.com>,
        Hante Meuleman <hante.meuleman@broadcom.com>,
        Kalle Valo <kvalo@kernel.org>
Cc:     linux-wireless@vger.kernel.org,
        brcm80211-dev-list.pdl@broadcom.com,
        SHA-cyfmac-dev-list@infineon.com
References: <20220708133712.102179-1-hdegoede@redhat.com>
 <20220708133712.102179-2-hdegoede@redhat.com>
From:   Arend Van Spriel <aspriel@gmail.com>
In-Reply-To: <20220708133712.102179-2-hdegoede@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/8/2022 3:37 PM, Hans de Goede wrote:
> 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).

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   .../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

[...]

> @@ -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)) {

Is it necessary to use the unaligned variant?

Regards,
Arend