Received-SPF: pass (google.com: domain of linux-wireless+bounces-6735-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <ca17a2f0-eb18-487a-8c5d-811748f2ca00@quicinc.com>
Date: Tue, 23 Apr 2024 11:46:50 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 6/7] wifi: mac80211: add support to call color_change
 and OBSS collision on a link
Content-Language: en-US
To: Aditya Kumar Singh <quic_adisi@quicinc.com>, <johannes@sipsolutions.net>
CC: <linux-wireless@vger.kernel.org>
References: <20240422053412.2024075-1-quic_adisi@quicinc.com>
 <20240422053412.2024075-7-quic_adisi@quicinc.com>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20240422053412.2024075-7-quic_adisi@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 4/21/2024 10:34 PM, Aditya Kumar Singh wrote:
> Currently ieee80211_color_change_finish() function finalizes color change
> by scheduling a finalizing worker using the deflink. Similarly, function
> ieee80211_obss_color_collision_notify() notifies the mac80211 about color
> collision on deflink. With MLO, there is a need to do it on a given link
> basis.
> 
> Pass link ID of the link on which color change needs to be finalized or
> OBSS color collision is detected.
> 
> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
> ---
[...]
>  void
>  ieee80211_obss_color_collision_notify(struct ieee80211_vif *vif,
> -				      u64 color_bitmap)
> +				      u64 color_bitmap, u8 link_id)
>  {
>  	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> -	struct ieee80211_link_data *link = &sdata->deflink;
> +	struct ieee80211_link_data *link;
>  
> -	if (sdata->vif.bss_conf.color_change_active || sdata->vif.bss_conf.csa_active)
> +	if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
>  		return;
>  
> -	if (delayed_work_pending(&link->color_collision_detect_work))
> +	rcu_read_lock();

Johannes, how do you feel about the new cleanup.h functionality? I ask because
if we change this to just be:

	guard(rcu)();

then we can remove all of the explicit rcu_read_unlock() calls --
rcu_read_unlock() will be called automatically when the function goes out of
scope.

> +
> +	link = rcu_dereference(sdata->link[link_id]);
> +	if (WARN_ON(!link)) {
> +		rcu_read_unlock();
>  		return;
> +	}
> +
> +	if (link->conf->color_change_active || link->conf->csa_active) {
> +		rcu_read_unlock();
> +		return;
> +	}
> +
> +	if (delayed_work_pending(&link->color_collision_detect_work)) {
> +		rcu_read_unlock();
> +		return;
> +	}
>  
>  	link->color_bitmap = color_bitmap;
>  	/* queue the color collision detection event every 500 ms in order to
> @@ -4853,6 +4882,8 @@ ieee80211_obss_color_collision_notify(struct ieee80211_vif *vif,
>  	ieee80211_queue_delayed_work(&sdata->local->hw,
>  				     &link->color_collision_detect_work,
>  				     msecs_to_jiffies(500));
> +
> +	rcu_read_unlock();
>  }
>  EXPORT_SYMBOL_GPL(ieee80211_obss_color_collision_notify);