[wireless-next,v6,5/5] wifi: mac80211: set tx power per radio in a wiphy

From: Rameshkumar Sundaram <quic_ramess@quicinc.com>

If set tx power is being done without a valid wdev/sdata
then the configuration applies to the radio but currently
it is being done at wiphy level(i.e. to all radios of wiphy)
since radio identifier is not available.

Use the radio_id argument of ieee80211_set_tx_power() to identify
to which radio of the wiphy the configuration should be applied.
If the wiphy is a multi-radio wiphy(wiphy.n_radios > 0), validate the
radio index of link's channel context against the radio id provided
and apply the configuration.

radio id value of NL80211_WIPHY_RADIO_ID_DEFAULT(-1) indicates that radio
index is not mentioned and the configuration applies to all radio(s) of
the wiphy.

Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
Signed-off-by: Roopni Devanathan <quic_rdevanat@quicinc.com>
---
 net/mac80211/cfg.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

> +++ b/net/mac80211/cfg.c
> @@ -3080,6 +3080,7 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
>  	struct ieee80211_local *local = wiphy_priv(wiphy);
>  	struct ieee80211_sub_if_data *sdata;
>  	enum nl80211_tx_power_setting txp_type = type;
> +	struct ieee80211_chanctx_conf *conf;
>  	bool update_txp_type = false;
>  	bool has_monitor = false;
>  	int user_power_level;
> @@ -3155,6 +3156,12 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
>  			if (!link)
>  				continue;
>  
> +			if (radio_id >= 0 && radio_id < wiphy->n_radio) {
> +				conf = wiphy_dereference(wiphy, link->conf->chanctx_conf);
> +				if (!conf || conf->radio_idx != radio_id)
> +					continue;
> +			}
> +
>  			link->user_power_level = local->user_power_level;
>  			if (txp_type != link->conf->txpower_type)
>  				update_txp_type = true;
> @@ -3175,6 +3182,12 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
>  			if (!link)
>  				continue;
>  
> +			if (radio_id >= 0 && radio_id < wiphy->n_radio) {
> +				conf = wiphy_dereference(wiphy, link->conf->chanctx_conf);
> +				if (!conf || conf->radio_idx != radio_id)
> +					continue;
> +			}
> +
> 


Hmm. Is this really enough? What if the link gets disabled and re-
enabled on a whole different chanctx on a different radio? Or other
things like that?

Seems like we may need to change how the TX power is stored in mac80211,
rather than just paper over it like this?

johannes

On 4/23/2025 9:11 PM, Johannes Berg wrote:
> 
>> +++ b/net/mac80211/cfg.c
>> @@ -3080,6 +3080,7 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
>>  	struct ieee80211_local *local = wiphy_priv(wiphy);
>>  	struct ieee80211_sub_if_data *sdata;
>>  	enum nl80211_tx_power_setting txp_type = type;
>> +	struct ieee80211_chanctx_conf *conf;
>>  	bool update_txp_type = false;
>>  	bool has_monitor = false;
>>  	int user_power_level;
>> @@ -3155,6 +3156,12 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
>>  			if (!link)
>>  				continue;
>>  
>> +			if (radio_id >= 0 && radio_id < wiphy->n_radio) {
>> +				conf = wiphy_dereference(wiphy, link->conf->chanctx_conf);
>> +				if (!conf || conf->radio_idx != radio_id)
>> +					continue;
>> +			}
>> +
>>  			link->user_power_level = local->user_power_level;
>>  			if (txp_type != link->conf->txpower_type)
>>  				update_txp_type = true;
>> @@ -3175,6 +3182,12 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
>>  			if (!link)
>>  				continue;
>>  
>> +			if (radio_id >= 0 && radio_id < wiphy->n_radio) {
>> +				conf = wiphy_dereference(wiphy, link->conf->chanctx_conf);
>> +				if (!conf || conf->radio_idx != radio_id)
>> +					continue;
>> +			}
>> +
>>
> 
> 
> Hmm. Is this really enough? What if the link gets disabled and re-
> enabled on a whole different chanctx on a different radio? Or other
> things like that?
> 
> Seems like we may need to change how the TX power is stored in mac80211,
> rather than just paper over it like this?
> 
This means that get_tx_power handling should also be changed. So I might have to work
on resigning this. I'll handle this parallelly. Meanwhile, can I send out the first three
patches handling get/set RTS threshold in a separate patch series? I will re-design
tx-power patches and send separately.

> johannes

On Thu, 2025-04-24 at 18:57 +0530, Roopni Devanathan wrote:
> > 
> > Hmm. Is this really enough? What if the link gets disabled and re-
> > enabled on a whole different chanctx on a different radio? Or other
> > things like that?
> > 
> > Seems like we may need to change how the TX power is stored in mac80211,
> > rather than just paper over it like this?
> > 
> This means that get_tx_power handling should also be changed. So I might have to work
> on resigning this. 
> 

Maybe, I'm not really sure? Was just thinking about it.

> I'll handle this parallelly. Meanwhile, can I send out the first three
> patches handling get/set RTS threshold in a separate patch series?

Sure.

johannes

On 4/24/2025 6:59 PM, Johannes Berg wrote:
> On Thu, 2025-04-24 at 18:57 +0530, Roopni Devanathan wrote:
>>>
>>> Hmm. Is this really enough? What if the link gets disabled and re-
>>> enabled on a whole different chanctx on a different radio? Or other
>>> things like that?
>>>
>>> Seems like we may need to change how the TX power is stored in mac80211,
>>> rather than just paper over it like this?
>>>
>> This means that get_tx_power handling should also be changed. So I might have to work
>> on resigning this. 
>>
> 
> Maybe, I'm not really sure? Was just thinking about it.
> 
>> I'll handle this parallelly. Meanwhile, can I send out the first three
>> patches handling get/set RTS threshold in a separate patch series?
> 
> Sure.
> 
Will do, then. Thanks.

> johannes