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

Pontus Fuchs pontus.fuchs at gmail.com
Tue Mar 11 05:30:30 EDT 2014


On 2014-03-07 02:44, Jason Mobarak wrote:
> From: Chun-Yeow Yeoh <yeohchunyeow at gmail.com>
>
> The firmware has triggered the QoS NULL frame to monitor the links
> amongst peer mesh STAs. Once one of the peer mesh STAs is leaving or
> out of the range and no ACK received from the peer mesh STA, the
> firmware will automatically delete the peer mesh STA.
>
> After deleting the mesh STA and adding the mesh STA back with the HAL
> command returning no error, the STA still won't work. Unicast data
> frames is not able to transmit out from the mesh STA.
>
> Now, re-insert the inactivite mesh STA with WCN36XX_HAL_CONFIG_STA_REQ
> command with action field set to update seems to solve the problem.
>
> Signed-off-by: Chun-Yeow Yeoh <yeohchunyeow at cozybit.com>
> Signed-off-by: Jason Mobarak <jam at cozybit.com>
> Signed-off-by: Javier Lopez <jlopex at cozybit.com>
> Signed-off-by: Javier Cardona <javier at cozybit.com>
> ---
>   main.c    | 45 +++++++++++++++++++++++++++++++++++++++++----
>   smd.c     | 62 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>   smd.h     |  9 ++++++++-
>   wcn36xx.h |  6 ++++++
>   4 files changed, 103 insertions(+), 19 deletions(-)
>
> diff --git a/main.c b/main.c
> index 2a6a9c4..a1c2a5f 100644
> --- a/main.c
> +++ b/main.c
> @@ -299,6 +299,7 @@ static int wcn36xx_start(struct ieee80211_hw *hw)
>   	wcn36xx_debugfs_init(wcn);
>
>   	INIT_LIST_HEAD(&wcn->vif_list);
> +

Empty line.

>   	return 0;
>
>   out_smd_stop:
> @@ -673,7 +674,7 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,
>   			 * config_sta must be called from  because this is the
>   			 * place where AID is available.
>   			 */
> -			wcn36xx_smd_config_sta(wcn, vif, sta);
> +			wcn36xx_smd_config_sta(wcn, vif, sta, 0);
>   			rcu_read_unlock();
>   		} else {
>   			wcn36xx_dbg(WCN36XX_DBG_MAC,
> @@ -777,6 +778,9 @@ static int wcn36xx_add_interface(struct ieee80211_hw *hw,
>   		return -EOPNOTSUPP;
>   	}
>
> +	INIT_LIST_HEAD(&vif_priv->sta_list);
> +	spin_lock_init(&vif_priv->sta_list_spinlock);
> +
>   	list_add(&vif_priv->list, &wcn->vif_list);
>   	wcn36xx_smd_add_sta_self(wcn, vif);
>
> @@ -789,10 +793,22 @@ static int wcn36xx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>   	struct wcn36xx *wcn = hw->priv;
>   	struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
>   	struct wcn36xx_sta *sta_priv = wcn36xx_sta_to_priv(sta);
> +	struct wcn36xx_sta *sta_priv_existing = NULL;
>   	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac sta add vif %p sta %pM\n",
>   		    vif, sta->addr);
>
>   	sta_priv->vif = vif_priv;
> +
> +	spin_lock(&vif_priv->sta_list_spinlock);
> +	sta_priv_existing = wcn36xx_find_sta(vif_priv, sta->addr);
> +
> +	if (sta_priv_existing)
> +		sta_priv = sta_priv_existing;
> +	else
> +		list_add(&sta_priv->list, &vif_priv->sta_list);
> +
> +	spin_unlock(&vif_priv->sta_list_spinlock);
> +
>   	/*
>   	 * For STA mode HW will be configured on BSS_CHANGED_ASSOC because
>   	 * at this stage AID is not available yet.
> @@ -800,8 +816,12 @@ static int wcn36xx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>   	if (NL80211_IFTYPE_STATION != vif->type) {
>   		wcn36xx_update_allowed_rates(sta, WCN36XX_BAND(wcn));
>   		sta_priv->aid = sta->aid;
> -		wcn36xx_smd_config_sta(wcn, vif, sta);
> +		if (sta_priv_existing)
> +			wcn36xx_smd_config_sta(wcn, vif, sta, 1);
> +		else
> +			wcn36xx_smd_config_sta(wcn, vif, sta, 0);

wcn36xx_smd_config_sta(wcn, vif, sta, !!sta_priv_existing);

>   	}
> +

Empty line;

>   	return 0;
>   }
>
> @@ -811,12 +831,29 @@ static int wcn36xx_sta_remove(struct ieee80211_hw *hw,
>   {
>   	struct wcn36xx *wcn = hw->priv;
>   	struct wcn36xx_sta *sta_priv = wcn36xx_sta_to_priv(sta);
> +	struct wcn36xx_vif *vif_priv = (struct wcn36xx_vif *)vif->drv_priv;

Use vif_to_priv

> +	struct wcn36xx_sta *sta_priv_existing = NULL;
>
>   	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac sta remove vif %p sta %pM index %d\n",
>   		    vif, sta->addr, sta_priv->sta_index);
>
> -	wcn36xx_smd_delete_sta(wcn, sta_priv->sta_index);
> -	sta_priv->vif = NULL;
> +	spin_lock(&vif_priv->sta_list_spinlock);
> +	sta_priv_existing = wcn36xx_find_sta(vif_priv, sta->addr);
> +
> +	if (sta_priv_existing)
> +		list_del(&sta_priv_existing->list);
> +
> +	spin_unlock(&vif_priv->sta_list_spinlock);
> +
> +	if (sta_priv_existing) {
> +		sta_priv_existing->vif = NULL;
> +		wcn36xx_smd_delete_sta(wcn, sta_priv_existing->sta_index);
> +	}
> +	else {
> +		sta_priv->vif = NULL;
> +		wcn36xx_smd_delete_sta(wcn, sta_priv_existing->sta_index);
> +	}
> +
>   	return 0;
>   }
>
> diff --git a/smd.c b/smd.c
> index e01933a..55bcab0 100644
> --- a/smd.c
> +++ b/smd.c
> @@ -860,7 +860,8 @@ out:
>
>   static void wcn36xx_smd_convert_sta_to_v1(struct wcn36xx *wcn,
>   			const struct wcn36xx_hal_config_sta_params *orig,
> -			struct wcn36xx_hal_config_sta_params_v1 *v1)
> +			struct wcn36xx_hal_config_sta_params_v1 *v1,
> +			u8 action)

No need to add action here. Use the action field in the source.

>   {
>   	/* convert orig to v1 format */
>   	memcpy(&v1->bssid, orig->bssid, ETH_ALEN);
> @@ -869,6 +870,7 @@ static void wcn36xx_smd_convert_sta_to_v1(struct wcn36xx *wcn,
>   	v1->type = orig->type;
>   	v1->listen_interval = orig->listen_interval;
>   	v1->ht_capable = orig->ht_capable;
> +	v1->action = action;

In later versions action gets copied from the source.

>   	v1->max_ampdu_size = orig->max_ampdu_size;
>   	v1->max_ampdu_density = orig->max_ampdu_density;
> @@ -905,6 +907,8 @@ static int wcn36xx_smd_config_sta_rsp(struct wcn36xx *wcn,
>   	sta_priv->dpu_desc_index = params->dpu_index;
>   	sta_priv->ucast_dpu_sign = params->uc_ucast_sig;
>
> +	memcpy(sta_priv->mac_addr, sta->addr, ETH_ALEN);
> +
>   	wcn36xx_dbg(WCN36XX_DBG_HAL,
>   		    "hal config sta rsp status %d sta_index %d bssid_index %d uc_ucast_sig %d p2p %d\n",
>   		    params->status, params->sta_index, params->bssid_index,
> @@ -914,7 +918,8 @@ static int wcn36xx_smd_config_sta_rsp(struct wcn36xx *wcn,
>   }
>
>   static int wcn36xx_smd_config_sta_v1(struct wcn36xx *wcn,
> -		     const struct wcn36xx_hal_config_sta_req_msg *orig)
> +		     const struct wcn36xx_hal_config_sta_req_msg *orig,
> +		     u8 action)
>   {
>   	struct wcn36xx_hal_config_sta_req_msg_v1 msg_body;
>   	struct wcn36xx_hal_config_sta_params_v1 *sta = &msg_body.sta_params;
> @@ -922,7 +927,7 @@ static int wcn36xx_smd_config_sta_v1(struct wcn36xx *wcn,
>   	INIT_HAL_MSG(msg_body, WCN36XX_HAL_CONFIG_STA_REQ);
>
>   	wcn36xx_smd_convert_sta_to_v1(wcn, &orig->sta_params,
> -				      &msg_body.sta_params);
> +				      &msg_body.sta_params, action);
>
>   	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
>
> @@ -935,7 +940,7 @@ static int wcn36xx_smd_config_sta_v1(struct wcn36xx *wcn,
>   }
>
>   int wcn36xx_smd_config_sta(struct wcn36xx *wcn, struct ieee80211_vif *vif,
> -			   struct ieee80211_sta *sta)
> +			   struct ieee80211_sta *sta, u8 action)
>   {
>   	struct wcn36xx_hal_config_sta_req_msg msg;
>   	struct wcn36xx_hal_config_sta_params *sta_params;
> @@ -949,7 +954,7 @@ int wcn36xx_smd_config_sta(struct wcn36xx *wcn, struct ieee80211_vif *vif,
>   	wcn36xx_smd_set_sta_params(wcn, vif, sta, sta_params);
>
>   	if (!wcn36xx_is_fw_version(wcn, 1, 2, 2, 24)) {
> -		ret = wcn36xx_smd_config_sta_v1(wcn, &msg);
> +		ret = wcn36xx_smd_config_sta_v1(wcn, &msg, action);
>   	} else {
>   		PREPARE_HAL_BUF(wcn->hal_buf, msg);
>
> @@ -1068,7 +1073,7 @@ static int wcn36xx_smd_config_bss_v1(struct wcn36xx *wcn,
>   	msg_body.bss_params.max_tx_power = orig->bss_params.max_tx_power;
>
>   	wcn36xx_smd_convert_sta_to_v1(wcn, &orig->bss_params.sta,
> -				      &msg_body.bss_params.sta);
> +				      &msg_body.bss_params.sta, 0);
>
>   	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
>
> @@ -1956,7 +1961,9 @@ static int wcn36xx_smd_delete_sta_context_ind(struct wcn36xx *wcn,
>   {
>   	struct wcn36xx_hal_delete_sta_context_ind_msg *rsp = buf;
>   	struct wcn36xx_vif *tmp;
> -	struct ieee80211_sta *sta;
> +	struct ieee80211_sta *sta = NULL;
> +	struct wcn36xx_sta *sta_priv = NULL;
> +	struct ieee80211_vif *vif = NULL;
>
>   	if (len != sizeof(*rsp)) {
>   		wcn36xx_warn("Corrupted delete sta indication\n");
> @@ -1967,18 +1974,26 @@ static int wcn36xx_smd_delete_sta_context_ind(struct wcn36xx *wcn,
>   		    rsp->addr2, rsp->sta_id);
>
>   	list_for_each_entry(tmp, &wcn->vif_list, list) {
> -		rcu_read_lock();
> -		sta = ieee80211_find_sta(wcn36xx_priv_to_vif(tmp), rsp->addr2);
> -		if (sta)
> -			ieee80211_report_low_ack(sta, 0);
> -		rcu_read_unlock();
> -		if (sta)
> -			return 0;
> +        wcn36xx_warn("STA %pM index %d is leaving\n", rsp->addr2, rsp->sta_id);
> +
> +        sta_priv = wcn36xx_find_sta(tmp, rsp->addr2);
> +        if(!sta_priv)
> +            continue;
> +
> +        sta = ieee80211_find_sta(wcn36xx_priv_to_vif(tmp), rsp->addr2);
> +        vif = wcn36xx_priv_to_vif(tmp);

First priv_to_vif can be avoided if you swap these.

> +
> +        if (vif->type == NL80211_IFTYPE_MESH_POINT) {
> +            wcn36xx_smd_config_sta(wcn, vif, sta, 1);
> +        }
> +
> +        return 0;
>   	}
>
>   	wcn36xx_warn("STA with addr %pM and index %d not found\n",
>   		     rsp->addr2,
>   		     rsp->sta_id);
> +
>   	return -ENOENT;
>   }
>
> @@ -2164,3 +2179,22 @@ void wcn36xx_smd_close(struct wcn36xx *wcn)
>   	destroy_workqueue(wcn->hal_ind_wq);
>   	mutex_destroy(&wcn->hal_ind_mutex);
>   }
> +
> +/**
> + * Assumes lock wcn36xx_vif->sta_list_spinlock is already held.
> + */
> +struct wcn36xx_sta* wcn36xx_find_sta(struct wcn36xx_vif *priv, u8 *mac_addr)

struct wcn36xx_sta *wcn...

> +{
> +    struct wcn36xx_sta* entry = NULL;
> +
> +    if (!mac_addr)
> +        return NULL;
> +
> +    list_for_each_entry(entry, &priv->sta_list, list) {
> +        if (!memcmp(entry->mac_addr, mac_addr, ETH_ALEN))
> +            return entry;
> +    }
> +
> +    return NULL;
> +}
> +
> diff --git a/smd.h b/smd.h
> index 751b62b..6a12874 100644
> --- a/smd.h
> +++ b/smd.h
> @@ -77,7 +77,7 @@ int wcn36xx_smd_config_bss(struct wcn36xx *wcn, struct ieee80211_vif *vif,
>   			   bool update);
>   int wcn36xx_smd_delete_bss(struct wcn36xx *wcn, struct ieee80211_vif *vif);
>   int wcn36xx_smd_config_sta(struct wcn36xx *wcn, struct ieee80211_vif *vif,
> -			   struct ieee80211_sta *sta);
> +			   struct ieee80211_sta *sta, u8 action);
>   int wcn36xx_smd_send_beacon(struct wcn36xx *wcn, struct ieee80211_vif *vif,
>   			    struct sk_buff *skb_beacon, u16 tim_off,
>   			    u16 p2p_off);
> @@ -128,4 +128,11 @@ int wcn36xx_smd_del_ba(struct wcn36xx *wcn, u16 tid, u8 sta_index);
>   int wcn36xx_smd_trigger_ba(struct wcn36xx *wcn, u8 sta_index);
>
>   int wcn36xx_smd_update_cfg(struct wcn36xx *wcn, u32 cfg_id, u32 value);
> +int wcn36xx_smd_get_stats(struct wcn36xx *wcn, u32 sta_index,
> +			  enum wcn36xx_hal_stats_mask stats_mask,
> +			  void *stats);
> +
> +struct wcn36xx_vif;
> +struct wcn36xx_sta* wcn36xx_find_sta(struct wcn36xx_vif *priv, u8 *mac_addr);
> +
>   #endif	/* _SMD_H_ */
> diff --git a/wcn36xx.h b/wcn36xx.h
> index 82d773c..8be2788 100644
> --- a/wcn36xx.h
> +++ b/wcn36xx.h
> @@ -121,6 +121,10 @@ struct wcn36xx_vif {
>   	bool is_joining;
>   	struct wcn36xx_hal_mac_ssid ssid;
>
> +	struct list_head sta_list;
> +	/* spin lock for associated station list */
> +	spinlock_t sta_list_spinlock;
> +

Can you document why this lock is needed (synchronization with the 
indication wq context).

Cheers,

Pontus



More information about the wcn36xx mailing list