Re: [PATCH] wifi: p54: unwind failed interface additions

[Christian Lamparter <chunkeey@gmail.com>]

On 10/19/26 3:31 AM, Jiale Yao wrote:
> p54 stores the vif and changes its operating mode before validating the
> interface type and before p54_setup_mac() can fail. Since mac80211 does
> not call remove_interface after a failed add, either path can leave the
> driver believing that an interface remains active.

hmm?

<https://docs.kernel.org/driver-api/80211/mac80211.html>

|stop
|	Called after last netdevice attached to the hardware is disabled.
|	This should turn off the hardware (at least it must turn off frame reception.)
|	>>> May be called right after add_interface if that rejects an interface <<<

so mac80211 should call drv_stop (aka. p54_stop) in ieee80211_do_open() if
p54_add_interface returns with an error code...

and p54_stop does:

priv->mode = NL80211_IFTYPE_UNSPECIFIED;

> Publish the vif only after type validation and restore the monitor state
> when MAC setup fails.
> 
> Fixes: 1ed32e4fc8cf ("mac80211: remove struct ieee80211_if_init_conf")
> Signed-off-by: Jiale Yao <yaojiale02@163.com>

priv->vif and priv->mac_addr remains set. I don't think that having priv->mac_addr
set matters. About priv->vif... Hmm, mac80211 passes it as &sdata->vif. So for the
lifetime of the associated mac80211 device this will always be the same?
(... as p54 only ever has one?)

So, even if beacon_loss traps are injected after a p54_add_interface fails, priv->vif
will be a valid pointer... didn't remember or see any other users...
hmm, it should be the same, but I don't know the mac80211 code well enough.

That said. I don't have a problem with the code. It reduces the "need to know
about mac80211 internals too" load... with that I'm struggeling too.

If you want to respin a v2 with the commit changes, you can have a:

Acked-by: Christian Lamparter <chunkeey@gmail.com>

And if not, I guess it's still fine (but not necessarily 100% correct maybe) as
is either way.

> ---
>   drivers/net/wireless/intersil/p54/main.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c
> index 57a62108cbc3..69d85efb9260 100644
> --- a/drivers/net/wireless/intersil/p54/main.c
> +++ b/drivers/net/wireless/intersil/p54/main.c
> @@ -236,8 +236,6 @@ static int p54_add_interface(struct ieee80211_hw *dev,
>   		return -EOPNOTSUPP;
>   	}
>   
> -	priv->vif = vif;
> -
>   	switch (vif->type) {
>   	case NL80211_IFTYPE_STATION:
>   	case NL80211_IFTYPE_ADHOC:
> @@ -250,8 +248,14 @@ static int p54_add_interface(struct ieee80211_hw *dev,
>   		return -EOPNOTSUPP;
>   	}
>   
> +	priv->vif = vif;
>   	memcpy(priv->mac_addr, vif->addr, ETH_ALEN);
>   	err = p54_setup_mac(priv);
> +	if (err) {
> +		priv->vif = NULL;
> +		priv->mode = NL80211_IFTYPE_MONITOR;
> +		eth_zero_addr(priv->mac_addr);
> +	}
>   	mutex_unlock(&priv->conf_mutex);
>   	return err;
>   }