On 28 February 2015 at 11:18, SenthilKumar Jegadeesan
<sjegadee@xxxxxxxxxxxxxxxx> wrote:
> In HT mode, firmware is not encrypting ADDBA request as PMF
> configuration is not set in peer flags during association.
>
> Set peer flags for MFP enabled station in ath10k driver.
>
> Implemented abstraction layer for peer flags.
>
> This change depends on mac80211 patch "[PATCH] mac80211: send
> station PMF configuration to driver".
>
> Signed-off-by: SenthilKumar Jegadeesan <sjegadee@xxxxxxxxxxxxxxxx>
[...]
> static bool ath10k_mac_sta_has_11g_rates(struct ieee80211_sta *sta)
> @@ -1752,6 +1759,11 @@ static int ath10k_peer_assoc_prepare(struct ath10k *ar,
> ath10k_peer_assoc_h_qos(ar, vif, sta, arg);
> ath10k_peer_assoc_h_phymode(ar, vif, sta, arg);
>
> + if (sta->mfp) {
> + if (ar->wmi.op_version == ATH10K_FW_WMI_OP_VERSION_10_2_4)
> + arg->peer_flags |= ar->wmi.peer_flags->wmi_peer_pmf;
> + }
Why do you check for op_version here? Isn't the peer flag mapping
supposed to deal with the problem already?
This should be just:
if (sta->mfp)
arg->peer_flags |= ar->wmi.peer_flags->wmi_peer_pmf;
Backends which don't support PMF would have the wmi_peer_mpf = 0 so
`|= ` would change nothing.
I also think this should be in a separate patch (i.e. patch1=introduce
peer flag map, patch2=mfp fix).
[...]
> +struct wmi_peer_flags_map {
> + u32 wmi_peer_auth;
> + u32 wmi_peer_qos;
> + u32 wmi_peer_need_ptk_4_way;
> + u32 wmi_peer_need_gtk_2_way;
> + u32 wmi_peer_apsd;
> + u32 wmi_peer_ht;
> + u32 wmi_peer_40MHZ;
> + u32 wmi_peer_stbc;
> + u32 wmi_peer_ldbc;
> + u32 wmi_peer_dyn_mimops;
> + u32 wmi_peer_static_mimops;
> + u32 wmi_peer_spatial_mux;
> + u32 wmi_peer_vht;
> + u32 wmi_peer_80MHZ;
> + u32 wmi_peer_vht_2g;
> + u32 wmi_peer_pmf;
> +};
Does it make sense to have the wmi_peer_ prefix for each entry?
Without it it'd be less wrapping to fit into 80 column limit.
The `MHZ` should probably lower case. It doesn't make much sense to
promote them to uppercase. These will also be a problem if you strip
wmi_peer_ (can't start a symbol with a number) so perhaps bw40/bw80 or
bw_40mhz/bw_80mhz would be fine?
Other than that this looks good. Thanks!
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
