Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ee6c0210-8bec-f924-4f5d-e2f23395a58a@quicinc.com>
Date:   Sat, 21 Jan 2023 14:25:23 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH 1/3] wifi: cfg80211: Extend cfg80211_new_sta() for MLD AP
Content-Language: en-US
To:     Johannes Berg <johannes@sipsolutions.net>
CC:     <linux-wireless@vger.kernel.org>
References: <20221206080226.1702646-1-quic_vjakkam@quicinc.com>
 <20221206080226.1702646-2-quic_vjakkam@quicinc.com>
 <f6a13b05feb05eb648dbd4b6f91932d75b353fbf.camel@sipsolutions.net>
From:   Veerendranath Jakkam <quic_vjakkam@quicinc.com>
In-Reply-To: <f6a13b05feb05eb648dbd4b6f91932d75b353fbf.camel@sipsolutions.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Thanks Johannes for reviewing the patches.


On 1/18/2023 9:11 PM, Johannes Berg wrote:
> Hi,
>
>> + * @mlo_params_valid: Indicates @assoc_link_id and @mld_addr fields are valid.
>> + *	Drivers use this only in cfg80211_new_sta() calls when AP MLD's MLME/SME
>> + *	is offload to driver. Drivers won't fill this information in
>> + *	cfg80211_del_sta_sinfo(), get_station() and dump_station() callbacks.
>>
>> + * @mld_addr: MLD address if the station is an MLD. Otherwise, set to all zeros.
>
>> +	if (sinfo->mlo_params_valid) {
>> +		if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID,
>> +			       sinfo->assoc_link_id))
>> +			goto nla_put_failure;
>> +
>> +		if (!is_zero_ether_addr(sinfo->mld_addr) &&
>> +		    nla_put(msg, NL80211_ATTR_MLD_ADDR, ETH_ALEN,
>> +			    sinfo->mld_addr))
>>
>
> It'd be invalid, but now you could have mlo_params_valid == true &&
> is_zero_ether_addr(sinfo->mld_addr), which would lead to a very strange
> situation for userspace, it would see a link ID but no MLD address.


Only link ID and no MLD address also a valid combination. It indicates 
the connected STA is non-MLD.  User space needs the link ID information 
to determine "STA connected to which affiliated AP of the AP MLD".

>
> With the documented requirement that
> 	mlo_params_valid == (mld_addr is valid)


No, I think there is some misinterpretation of the documentation here. 
Let me submit new patch-set after adding more detailed text in the 
documentation.

Actually what I mean,  cfg80211 can consider parsing both the 
"assoc_link_id" and "mld_addr" fields when "mlo_params_valid" is true. 
"assoc_link_id" will be set to link ID of the affiliated AP on which STA 
(MLD/non-MLD) completed association. "mld_addr" will be set to MLD 
address of the STA if applicable (i.e., MLD STA) . Otherwise, will be 
set to zeros (i.e., non-MLD STA).

--

veeru