[PATCH 0/4] ath10k: ibss crypto fixes

[PATCH 0/4] ath10k: ibss crypto fixes

[Michal Kazior]

I've mainly tested this on kvalo/pending branch.
Currently master branch contains 0f611d28fc2e
("mac80211: count interfaces correctly for
combination checks") which breaks IBSS in
wpa_supplicant due to P2P Device and interface
combinations. This has been pointed out in the
following thread:

 [1]: http://thread.gmane.org/gmane.linux.kernel.wireless.general/135570/focus=136643

If someone's interested in running ath10k IBSS
make sure to use kernel prior to 4.0, revert the
mentioned mac80211 patch or apply wpa_supplicant
workaround patch from [1].


Janusz Dziedzic (1):
  ath10k: enable ibss-rsn

Michal Kazior (3):
  ath10k: don't use reassoc flag
  ath10k: fix multiple key static wep with ibss
  ath10k: set def key idx for ibss

 drivers/net/wireless/ath/ath10k/mac.c | 44 +++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 7 deletions(-)

-- 
2.1.4

[PATCH 1/4] ath10k: don't use reassoc flag

[Michal Kazior]

Firmware actually re-creates peer entry when
reassoc flag is set. This is undesired and could
cause trouble with IBSS crypto-wise. This is also
important for upcomming bitrate mask improvement.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index a7a84f52b8b4..6664bcc9ba88 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2593,7 +2593,6 @@ static int ath10k_station_assoc(struct ath10k *ar,
 		return ret;
 	}
 
-	peer_arg.peer_reassoc = reassoc;
 	ret = ath10k_wmi_peer_assoc(ar, &peer_arg);
 	if (ret) {
 		ath10k_warn(ar, "failed to run peer assoc for STA %pM vdev %i: %d\n",
-- 
2.1.4

[PATCH 2/4] ath10k: fix multiple key static wep with ibss

[Michal Kazior]

Apparently firmware requires both pairwise and
groupwise keys to be installed per-peer for static
WEP in IBSS. This wasn't necessary for AP mode
(and installing both doesn't seem to break AP
mode thus there's no special handling).

Also there seems to be some kind of issue with
mapping tx/rx keys in firmware properly which
resulted in wrong keys being used and broken
communication between devices.

It can be argued the vdev param part is more of a
workaround than a real fix. However I couldn't
figure out how to fix this differently. It works
and isn't super ugly.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 6664bcc9ba88..93f753cb839a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -230,9 +230,13 @@ static int ath10k_install_peer_wep_keys(struct ath10k_vif *arvif,
 		flags = 0;
 		flags |= WMI_KEY_PAIRWISE;
 
-		/* set TX_USAGE flag for default key id */
-		if (arvif->def_wep_key_idx == i)
-			flags |= WMI_KEY_TX_USAGE;
+		ret = ath10k_install_key(arvif, arvif->wep_keys[i], SET_KEY,
+					 addr, flags);
+		if (ret)
+			return ret;
+
+		flags = 0;
+		flags |= WMI_KEY_GROUP;
 
 		ret = ath10k_install_key(arvif, arvif->wep_keys[i], SET_KEY,
 					 addr, flags);
@@ -244,6 +248,27 @@ static int ath10k_install_peer_wep_keys(struct ath10k_vif *arvif,
 		spin_unlock_bh(&ar->data_lock);
 	}
 
+	/* In some cases (notably with static WEP IBSS with multiple keys)
+	 * multicast Tx becomes broken. Both pairwise and groupwise keys are
+	 * installed already. Using WMI_KEY_TX_USAGE in different combinations
+	 * didn't seem help. Using def_keyid vdev parameter seems to be
+	 * effective so use that.
+	 *
+	 * FIXME: Revisit. Perhaps this can be done in a less hacky way.
+	 */
+	if (arvif->def_wep_key_idx == -1)
+		return 0;
+
+	ret = ath10k_wmi_vdev_set_param(arvif->ar,
+					arvif->vdev_id,
+					arvif->ar->wmi.vdev_param->def_keyid,
+					arvif->def_wep_key_idx);
+	if (ret) {
+		ath10k_warn(ar, "failed to re-set def wpa key idxon vdev %i: %d\n",
+			    arvif->vdev_id, ret);
+		return ret;
+	}
+
 	return 0;
 }
 
-- 
2.1.4

Re: [PATCH 2/4] ath10k: fix multiple key static wep with ibss

[Ben Greear]

On 04/10/2015 06:23 AM, Michal Kazior wrote:
> Apparently firmware requires both pairwise and
> groupwise keys to be installed per-peer for static
> WEP in IBSS. This wasn't necessary for AP mode
> (and installing both doesn't seem to break AP
> mode thus there's no special handling).
> 
> Also there seems to be some kind of issue with
> mapping tx/rx keys in firmware properly which
> resulted in wrong keys being used and broken
> communication between devices.
> 
> It can be argued the vdev param part is more of a
> workaround than a real fix. However I couldn't
> figure out how to fix this differently. It works
> and isn't super ugly.
> 
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/mac.c | 31 ++++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 6664bcc9ba88..93f753cb839a 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -230,9 +230,13 @@ static int ath10k_install_peer_wep_keys(struct ath10k_vif *arvif,
>  		flags = 0;
>  		flags |= WMI_KEY_PAIRWISE;
>  
> -		/* set TX_USAGE flag for default key id */
> -		if (arvif->def_wep_key_idx == i)
> -			flags |= WMI_KEY_TX_USAGE;
> +		ret = ath10k_install_key(arvif, arvif->wep_keys[i], SET_KEY,
> +					 addr, flags);
> +		if (ret)
> +			return ret;
> +
> +		flags = 0;
> +		flags |= WMI_KEY_GROUP;

That could be a single assigment to WMI_KEY_GROUP?

Also, this does not merge cleanly with my 4.0 tree,
but I'm not using WEP anyway, so no big deal...

Thanks,
Ben



-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

[PATCH 3/4] ath10k: set def key idx for ibss

[Michal Kazior]

Some time ago there was a weird issue with AP
using wrong multicast keys and generating
corrupted traffic on 10.1 firmware.

Apparently a very similar problem applies for
IBSS-RSN on 999.999.0.636.

ath10k doesn't have IBSS-RSN enabled yet. This
patch is a prerequisite to support it.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 93f753cb839a..9428d75d0d76 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4691,10 +4691,14 @@ static void ath10k_set_key_h_def_keyidx(struct ath10k *ar,
 	 * frames with multi-vif APs. This is not required for main firmware
 	 * branch (e.g. 636).
 	 *
-	 * FIXME: This has been tested only in AP. It remains unknown if this
-	 * is required for multi-vif STA interfaces on 10.1 */
+	 * This is also needed for 636 fw for IBSS-RSN to work more reliably.
+	 *
+	 * FIXME: It remains unknown if this is required for multi-vif STA
+	 * interfaces on 10.1.
+	 */
 
-	if (arvif->vdev_type != WMI_VDEV_TYPE_AP)
+	if (arvif->vdev_type != WMI_VDEV_TYPE_AP &&
+	    arvif->vdev_type != WMI_VDEV_TYPE_IBSS)
 		return;
 
 	if (key->cipher == WLAN_CIPHER_SUITE_WEP40)
-- 
2.1.4

[PATCH 4/4] ath10k: enable ibss-rsn

[Michal Kazior]

From: Janusz Dziedzic <janusz.dziedzic@tieto.com>

With latest additions to the driver it seems
viable to enable support for IBSS-RSN.

It seems to work on QCA988X and 999.999.0.636 but
is a bit slow to exchange RSN keys for some
reason. This may be a firmware quirk or ath10k is
missing something. Nevertheless it makes sense to
finally enable IBSS-RSN in ath10k even if somewhat
handicapped.

QCA6174 firmware doesn't seem to be able to Tx
EAPOL frames at all now (they get stuck in hw
queues for some reason) so it never gets to set
the keys in driver. It's fairly safe to assume that
once this is fixed IBSS-RSN will work with QCA6174
firmware without any additional changes. Hence no
special handling for advertising
IEEE80211_HW_SUPPORTS_PER_STA_GTK and
WIPHY_FLAG_IBSS_RSN is done now.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 9428d75d0d76..7485d5ccee3d 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -6720,11 +6720,13 @@ int ath10k_mac_register(struct ath10k *ar)
 			IEEE80211_HW_SPECTRUM_MGMT |
 			IEEE80211_HW_SW_CRYPTO_CONTROL |
 			IEEE80211_HW_CONNECTION_MONITOR |
+			IEEE80211_HW_SUPPORTS_PER_STA_GTK |
 			IEEE80211_HW_WANT_MONITOR_VIF |
 			IEEE80211_HW_CHANCTX_STA_CSA |
 			IEEE80211_HW_QUEUE_CONTROL;
 
 	ar->hw->wiphy->features |= NL80211_FEATURE_STATIC_SMPS;
+	ar->hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
 
 	if (ar->ht_cap_info & WMI_HT_CAP_DYNAMIC_SMPS)
 		ar->hw->wiphy->features |= NL80211_FEATURE_DYNAMIC_SMPS;
-- 
2.1.4

Re: [PATCH 4/4] ath10k: enable ibss-rsn

[Ben Greear]

On 04/10/2015 06:23 AM, Michal Kazior wrote:
> From: Janusz Dziedzic <janusz.dziedzic@tieto.com>
> 
> With latest additions to the driver it seems
> viable to enable support for IBSS-RSN.
> 
> It seems to work on QCA988X and 999.999.0.636 but
> is a bit slow to exchange RSN keys for some
> reason. This may be a firmware quirk or ath10k is
> missing something. Nevertheless it makes sense to
> finally enable IBSS-RSN in ath10k even if somewhat
> handicapped.
> 
> QCA6174 firmware doesn't seem to be able to Tx
> EAPOL frames at all now (they get stuck in hw
> queues for some reason) so it never gets to set
> the keys in driver. It's fairly safe to assume that
> once this is fixed IBSS-RSN will work with QCA6174
> firmware without any additional changes. Hence no
> special handling for advertising
> IEEE80211_HW_SUPPORTS_PER_STA_GTK and
> WIPHY_FLAG_IBSS_RSN is done now.

With your patches (though not certain they are all needed on my firmware
now), and this patch below, my 10.1 system seems to be doing IBSS + RSN properly.

In case you are seeing any issues with small packets on your IBSS systems, I would
be curious to know your results if you gave this patch a try.

Without this, I see runt frames received on the peer (ARP responses in this particular case).

I was also seeing similarly corrupted EAPOL frames from what I can tell.

I don't really know why this is required, or if it is required for more than just
IBSS + RSN at this point.

[greearb@ben-dt2 ath10k]$ git diff
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index d3add01..853bf55 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2631,6 +2631,19 @@ static void ath10k_tx(struct ieee80211_hw *hw,
        struct ieee80211_vif *vif = info->control.vif;
        struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;

+
+       if (vif && vif->type == NL80211_IFTYPE_ADHOC) {
+               static int my_zlen = 78;
+               /* This nasty little hack fixes IBSS + RSN with small
+                * frames frames (ARP, etc) on 10.1 (CT) firmware.
+                * Shouldn't hurt any firmware, and may help other as well. --Ben
+                */
+               if (skb->len < my_zlen) {
+                       if (skb_put_padto(skb, my_zlen))
+                               return; /* skb was consumed by skb_padto on error */
+               }
+       }
+
        /* We should disable CCK RATE due to P2P */
        if (info->flags & IEEE80211_TX_CTL_NO_CCK_RATE)
                ath10k_dbg(ar, ATH10K_DBG_MAC, "IEEE80211_TX_CTL_NO_CCK_RATE\n");


Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 4/4] ath10k: enable ibss-rsn

[Michal Kazior]

On 10 April 2015 at 15:23, Michal Kazior <michal.kazior@tieto.com> wrote:
> From: Janusz Dziedzic <janusz.dziedzic@tieto.com>
>
> With latest additions to the driver it seems
> viable to enable support for IBSS-RSN.
>
> It seems to work on QCA988X and 999.999.0.636 but
> is a bit slow to exchange RSN keys for some
> reason. This may be a firmware quirk or ath10k is
> missing something. Nevertheless it makes sense to
> finally enable IBSS-RSN in ath10k even if somewhat
> handicapped.
>
> QCA6174 firmware doesn't seem to be able to Tx
> EAPOL frames at all now (they get stuck in hw
> queues for some reason) so it never gets to set
> the keys in driver. It's fairly safe to assume that
> once this is fixed IBSS-RSN will work with QCA6174
> firmware without any additional changes. Hence no
> special handling for advertising
> IEEE80211_HW_SUPPORTS_PER_STA_GTK and
> WIPHY_FLAG_IBSS_RSN is done now.

As a matter of fact QCA6174 firmware does seem to work with IBSS as
well. I just found out (by accident, after leaving a failed test setup
for a while) that after approx. 300 seconds firmware discards all
stuck EAPOL frames and starts sending subsequent ones properly. Keys
gets exchanged, installed and traffic seems to work fine with this
patchset. I suppose this warrants an updated commit log.


Michał

Re: [PATCH 0/4] ath10k: ibss crypto fixes

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> I've mainly tested this on kvalo/pending branch.
> Currently master branch contains 0f611d28fc2e
> ("mac80211: count interfaces correctly for
> combination checks") which breaks IBSS in
> wpa_supplicant due to P2P Device and interface
> combinations. This has been pointed out in the
> following thread:
>
>  [1]: http://thread.gmane.org/gmane.linux.kernel.wireless.general/135570/focus=136643
>
> If someone's interested in running ath10k IBSS
> make sure to use kernel prior to 4.0, revert the
> mentioned mac80211 patch or apply wpa_supplicant
> workaround patch from [1].

Is this IBSS regression fixed now or what's the status?

-- 
Kalle Valo

Re: [PATCH 0/4] ath10k: ibss crypto fixes

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> I've mainly tested this on kvalo/pending branch.
> Currently master branch contains 0f611d28fc2e
> ("mac80211: count interfaces correctly for
> combination checks") which breaks IBSS in
> wpa_supplicant due to P2P Device and interface
> combinations. This has been pointed out in the
> following thread:
>
>  [1]: http://thread.gmane.org/gmane.linux.kernel.wireless.general/135570/focus=136643
>
> If someone's interested in running ath10k IBSS
> make sure to use kernel prior to 4.0, revert the
> mentioned mac80211 patch or apply wpa_supplicant
> workaround patch from [1].
>
>
> Janusz Dziedzic (1):
>   ath10k: enable ibss-rsn
>
> Michal Kazior (3):
>   ath10k: don't use reassoc flag
>   ath10k: fix multiple key static wep with ibss
>   ath10k: set def key idx for ibss

Thanks, all four patches applied.

-- 
Kalle Valo