[PATCH 2/2] wcn36xx: fix the mesh STA for rejoin
Bob Copeland
me at bobcopeland.com
Fri Mar 21 10:09:11 EDT 2014
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
More information about the wcn36xx
mailing list