Re: [PATCH v4 1/3] mac80211_hwsim: add permanent mac address option for new radios

[Jouni Malinen <j@w1.fi>]

On Mon, Jan 22, 2018 at 06:04:35PM +0100, Benjamin Beichler wrote:
> If simulation needs predictable permanent mac addresses of hwsim wireless
> phy, this patch add the ability to create a new radio with a user defined
> permanent mac address. Allowed mac addresses needs to be locally
> administrated mac addresses (as also the former fixed 42:* and 02:* were).
> 
> To do not break the operation with legacy software using hwsim, the new
> address is set twice. The problem here is, the netlink call backs use
> wiphy->addresses[1] as identification of a radio and not the proposed
> permanent address (wiphy->addresses[0]). This design decision is not
> documented in the kernel repo, therefore this patch simply reproduces this,
> but with the same address.

But this patch does break operation with legacy software.. Namely, some
mac80211_hwsim test cases in hostap.git. However, that seems to be due
to a strange swapping of operations here:

> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
> @@ -2571,15 +2573,25 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,

> -	eth_zero_addr(addr);
> -	addr[0] = 0x02;
> -	addr[3] = idx >> 8;
> -	addr[4] = idx;
> -	memcpy(data->addresses[0].addr, addr, ETH_ALEN);
> -	memcpy(data->addresses[1].addr, addr, ETH_ALEN);
> -	data->addresses[1].addr[0] |= 0x40;
> -	hw->wiphy->n_addresses = 2;
> -	hw->wiphy->addresses = data->addresses;
> +	if (!param->perm_addr) {
> +		eth_zero_addr(addr);
> +		addr[0] = 0x02;
> +		addr[3] = idx >> 8;
> +		addr[4] = idx;
> +		memcpy(data->addresses[0].addr, addr, ETH_ALEN);
> +		/* Why need here second address ? */
> +		data->addresses[1].addr[0] |= 0x40;
> +		memcpy(data->addresses[1].addr, addr, ETH_ALEN);
> +		hw->wiphy->n_addresses = 2;
> +		hw->wiphy->addresses = data->addresses;

Why did that ORing of 0x40 got moved to be before the memcpy()?
Effectively, that removes the OR operation completely since the
following memcpy() overrides the first octets of
data->addresses[1].addr. Restoring the order of those two lines seems to
fix the behavior.

-- 
Jouni Malinen                                            PGP id EFC895FA