Received-SPF: pass (google.com: domain of linux-wireless+bounces-2794-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <55d1aea6-8721-4774-9f09-549245461ba8@quicinc.com>
Date: Tue, 30 Jan 2024 16:13:16 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 3/5] wifi: mac80211: handle set csa/after_csa beacon on
 per link basis
Content-Language: en-US
To: Johannes Berg <johannes@sipsolutions.net>
CC: <linux-wireless@vger.kernel.org>
References: <20240130043225.942202-1-quic_adisi@quicinc.com>
 <20240130043225.942202-4-quic_adisi@quicinc.com>
 <1e73c061653abf8e5f1671ae026e0905b2fb4ded.camel@sipsolutions.net>
From: Aditya Kumar Singh <quic_adisi@quicinc.com>
In-Reply-To: <1e73c061653abf8e5f1671ae026e0905b2fb4ded.camel@sipsolutions.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

On 1/30/24 15:47, Johannes Berg wrote:
> On Tue, 2024-01-30 at 10:02 +0530, Aditya Kumar Singh wrote:
>> In order to support CSA with MLO, there is a need to handle the functions
>> ieee80211_set_csa_beacon() and ieee80211_set_after_csa_beacon() on per
>> link basis.
> 
> nit: "on a per link"

Will address in next version. Thanks.

> 
>> Add changes for the same.
> 
> Is that some cultural thing?
> 
> I always find this phrasing with "for the same" very odd, and would
> rather say something useful such as "Implement this by passing the
> correct link data"... but I see this a lot, hence the question.
> 

No idea :). Even I have seen these quite a few times and thought that 
may be it is fine to use it that way. But I do agree that instead we 
could put something useful instead. Thanks for pointing it out, I will 
address this in next version.


>> @@ -3658,7 +3659,7 @@ static int __ieee80211_csa_finalize(struct ieee80211_link_data *link_data)
>>   
>>   	sdata->vif.bss_conf.csa_active = false;
>>   
>> -	err = ieee80211_set_after_csa_beacon(sdata, &changed);
>> +	err = ieee80211_set_after_csa_beacon(&sdata->deflink, &changed);
> 
> weren't you just saying deflink shouldn't be used?
> 

Correct. This patch's aim is to form the base - basically modify the 
helper function to accept the link data argument. But at this point, CSA 
is not started on per link basis hence in order to keep CSA working 
still as it is before this patch, have used deflink here. Functionality 
wise, this patch is not bringing any change yet.


>> @@ -3928,7 +3930,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
>>   	if (sdata->vif.bss_conf.color_change_active)
>>   		ieee80211_color_change_abort(sdata);
>>   
>> -	err = ieee80211_set_csa_beacon(sdata, params, &changed);
>> +	err = ieee80211_set_csa_beacon(&sdata->deflink, params, &changed);
> 
> dito
> 

Addressed above.