Re: [PATCH wireless-next v4 14/21] wifi: mac80211: add driver_op for SMD substate changes

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2026-11-06 at 13:28 +0530, Pooventhiran G wrote:
> 
> Valid transition order:
>   NONE  -> PREP  (ST preparation completed at the target AP MLD)
>   NONE  -> EXEC  (ST execution in progress at the current AP MLD)
>   EXEC  -> DRAIN (DL drain period started at the current AP MLD)

So ...

> + * @sta_smd_state: Notifies low level driver about IEEE 802.11bn SMD substate
> + *	transition of a station. It must not fail while transitioning down.

What does "down" then mean?

And again also - "SMD state" not really, more like "ST state"? If you
want to take things literally, then the state with the SMD is always 4
anyway.

>  	if (params->smd_params.smd_sta)
>  		sta->sta.smd_params = params->smd_params;
>  
> -	/*
> -	 * SMD BSS Transition sub-states (IEEE P802.11bn/D2.0, Aug 2026,
> -	 * subclause 11.3.1).
> -	 * Only meaningful for SMD-capable stations (WLAN_STA_SMD set).
> -	 */
> -	if (test_sta_flag(sta, WLAN_STA_SMD)) {
> -		ret = sta_apply_smd_state_flags(sta, mask, set);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	/* mark TDLS channel switch support, if the AP allows it */
>  	if (test_sta_flag(sta, WLAN_STA_TDLS_PEER) &&
>  	    !sdata->deflink.u.mgd.tdls_chan_switch_prohibited &&
> @@ -2954,6 +2963,17 @@ static int sta_apply_parameters(struct ieee80211_local *local,
>  			return ret;
>  	}
>  
> +	/*
> +	 * SMD BSS Transition sub-states (IEEE P802.11bn/D2.0, Aug 2026,
> +	 * subclause 11.3.1).
> +	 * Only meaningful for SMD-capable stations (WLAN_STA_SMD set).
> +	 */
> +	if (test_sta_flag(sta, WLAN_STA_SMD)) {
> +		ret = sta_apply_smd_state_flags(sta, mask, set);
> +		if (ret)
> +			return ret;
> +	}

That's crazy, don't add it in the wrong place first?

> @@ -3038,6 +3058,19 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct wireless_dev *wdev,
>  	 */
>  	if (!err && sdata->vif.type == NL80211_IFTYPE_NAN_DATA)
>  		ieee80211_nan_update_ndi_carrier(sta->sdata);
> +
> +	/*
> +	 * SMD sub-state was set in sta_apply_parameters() before the station
> +	 * was inserted so drv_sta_smd_state() was skipped (WLAN_STA_INSERTED
> +	 * was not yet set). Walk the driver step-by-step to the target state.
> +	 */

What does that actually explain that's not in the three lines of code
below it? Did an LLM write it?

> +void sta_info_reset_smd_state(struct sta_info *sta, bool notify)
> +{
> +	if (sta->smd_state == IEEE80211_STA_SMD_NONE)
> +		return;
> +
> +	/* DRAIN -> NONE, EXEC -> NONE or PREP -> NONE */
> +	WARN_ON_ONCE(sta_info_move_smd_state(sta, IEEE80211_STA_SMD_NONE,
> +					     notify));

How is that even useful? If we're talking about a sub-state while in
state 4, then surely moving out of state 4 simply invalidates that, why
bother?

> +	clear_sta_flag(sta, WLAN_STA_SMD_PREP_TARGET);
> +	clear_sta_flag(sta, WLAN_STA_SMD_EXEC_CURRENT);
> +	clear_sta_flag(sta, WLAN_STA_SMD_DL_DRAIN);

Understand that you need to clear the stale flags, but for the driver
that just seems strange?

> +	case IEEE80211_STA_SMD_EXEC:
> +	case IEEE80211_STA_SMD_DRAIN:
> +		err = drv_sta_smd_state(local, sta->sdata, &sta->sta,
> +					IEEE80211_STA_SMD_NONE,
> +					IEEE80211_STA_SMD_EXEC);
> +		if (err)
> +			goto rollback;
> +
> +		if (new_ss == IEEE80211_STA_SMD_DRAIN)
> +			err = drv_sta_smd_state(local, sta->sdata, &sta->sta,
> +						IEEE80211_STA_SMD_EXEC,
> +						IEEE80211_STA_SMD_DRAIN);
> +		break;

you do realize "fallthrough" exists?

> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (!err)
> +		return 0;
> +
> +	/* if EXEC -> DRAIN failed in the driver, notify EXEC -> NONE */
> +	WARN_ON_ONCE(drv_sta_smd_state(local, sta->sdata, &sta->sta,
> +				       IEEE80211_STA_SMD_EXEC,
> +				       IEEE80211_STA_SMD_NONE));

I'm not a fan of sticking whole blocks of code into a WARN, but I'm also
not sure this whole "model what should be a one-way sub-state based on
sta_state" was a good idea in this patch.

If this _fails_ in the driver, how do we even handle it? Should it even
be allowed to fail? What's the point? Worst case we _technically_ break
the spec and continue accepting frames from the STA in the wrong
substate, but otherwise nothing much happens?

> + * @smd_state: Sub-state for STAs in SMD BSS Transition

st_state, or so?

johannes