Re: [PATCH v4 1/3] wifi: nl80211: add link id of transmitted profile for MLO MBSSID

[Aloka Dixit <aloka.dixit@oss.qualcomm.com>]

On 3/12/2025 1:55 AM, Johannes Berg wrote:
> Hi Aloka, all,
> 
>> + * @NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID: Link ID of the transmitted profile.
>> + *	This parameter is mandatory if the transmitted profile is part of an MLD
>> + *	and the interface getting configured is a non-transmitted profile. For all
>> + *	other cases it will be ignored.
> 
> So I guess it's a question of what "the interface getting configured"
> means, but I guess you could set up the transmitting interface?
> 
Right, if the interface getting configured is the transmitted profile 
the link_id is ignored.

>> @@ -5561,6 +5563,18 @@ static int nl80211_parse_mbssid_config(struct wiphy *wiphy,
>>   			}
>>   
>>   			config->tx_wdev = tx_netdev->ieee80211_ptr;
>> +
>> +			if (config->tx_wdev->valid_links) {
>> +				if (!tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID])
>> +					return -ENOLINK;
>> +
>> +				config->tx_link_id =
>> +					nla_get_u8(tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID]);
>> +
>> +				if (!(config->tx_wdev->valid_links &
>> +				      BIT(config->tx_link_id)))
>> +					return -ENOLINK;
>> +			}
>>   		} else {
>>   			config->tx_wdev = dev->ieee80211_ptr;
>>   		}
> 
> So shouldn't that be one layer out, so the link ID can also apply if no
> interface index was given, i.e. we took the else branch? Seems like that
> should be applicable, or is there a specific reason not to apply in that
> case?

The 'else' case in the highlighted snippet above is the same scenario as 
the 'else' case you referred to, which is one layer out: "else if 
(!config->index) {".

Both are executed when the interface getting configured is the 
transmitting interface. The difference between these two 'else's is that 
in one case userspace explicitly provides 
"NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX" and in other it hasn't.

Similarly, for MLO, mac80211 does not need the link_id of the tx 
interface because it already has the link_conf for it as part of basic 
AP configuration parameters, hence link_id is ignored in both 'else's.

Thanks.