[PATCH 2/2] wcn36xx: fix the mesh STA for rejoin

[Bob Copeland]

On Wed, Mar 19, 2014 at 11:56:43AM -0700, Jason Mobarak wrote:
> From: Chun-Yeow Yeoh <yeohchunyeow at gmail.com>
> To work around this, we re-insert the inactivite mesh STA with the HAL

"inactive"

> command WCN36XX_HAL_CONFIG_STA_REQ and the action field set to 'update',
> which seems to solve the problem.


> +static bool wcn36xx_sta_preexists(struct ieee80211_vif *vif,
> +		struct ieee80211_sta *sta)
> +{
> +	struct ieee80211_sta *sta_preexist;
> +	rcu_read_lock();
> +	sta_preexist = ieee80211_find_sta(vif, sta->addr);
> +	rcu_read_unlock();
> +	return !!sta_preexist;

Ok, matter of taste, but I would personally prefer explicit
"!= NULL" here.

On a conceptual level, I don't quite understand this - can
you add printk to this function to see if this ever returns
true?  Seems to me it would always be false, or else mac80211
would not have called drv_sta_add().  So then you might as
well just pass 0 from sta_add.

> -		wcn36xx_smd_config_sta(wcn, vif, sta);
> +		wcn36xx_smd_config_sta(wcn, vif, sta, wcn36xx_sta_preexists(vif, sta));

Line over 80 cols (checkpatch.pl warns about it)

>  	wcn36xx_smd_delete_sta(wcn, sta_priv->sta_index);
>  	sta_priv->vif = NULL;
> +
>  	return 0;

Random empty line added.

>  		    rsp->addr2, rsp->sta_id);
>  
>  	list_for_each_entry(tmp, &wcn->vif_list, list) {
> +		wcn36xx_warn("STA %pM index %d is leaving\n", rsp->addr2, rsp->sta_id);
>  		rcu_read_lock();
> -		sta = ieee80211_find_sta(wcn36xx_priv_to_vif(tmp), rsp->addr2);
> +		vif = wcn36xx_priv_to_vif(tmp);
> +		sta = ieee80211_find_sta(vif, rsp->addr2);
>  		if (sta)
>  			ieee80211_report_low_ack(sta, 0);
> +		if (sta && vif->type == NL80211_IFTYPE_MESH_POINT)
> +			wcn36xx_smd_config_sta(wcn, vif, sta, 1);

Ok, this seems like the change that is giving positive
results: re-add the sta in the mesh case.

>  		rcu_read_unlock();

But this is a bit of a problem: the rcu_read_lock() is needed to
protect the sta, but wcn36xx_smd_config_sta() takes a mutex.
You aren't allowed to sleep inside rcu read critical sections.

Actually, this crops up several other places in the driver,
not just in this patch.

I feel like making the firmware connection monitor not do its
thing in mesh mode, if possible, would be a better way forward
than re-adding the STAs like this.

-- 
Bob Copeland %% www.bobcopeland.com