[PATCH 0/5] nl802111/brcmfmac: wowlan netdetect fixes

[PATCH 0/5] nl802111/brcmfmac: wowlan netdetect fixes

[Arend van Spriel]

This series has a fix for nl80211 regarding validation of wowlan
netdetect. It has been discussed with Johannes to take this through
the wireless-drivers-next repository so here it is.

* fix validation of wowlan netdetect parameters in nl80211.
* fix wowlan netdetect support.
* add new cfg80211 callback.

The series is intended for 4.11 and applies to the master branch of the
wireless-drivers-next repository.

Arend van Spriel (5):
  nl80211: fix validation of scheduled scan info for wowlan netdetect
  brcmfmac: provide a value for struct wowlan_support::max_nd_match_sets
  brcmfmac: fix handling firmware results for wowl netdetect
  brcmfmac: allow wowlan support to be per device
  brcmfmac: add .update_connect_params() callback

 .../broadcom/brcm80211/brcmfmac/cfg80211.c         | 52 ++++++++++++++++++----
 net/wireless/nl80211.c                             | 10 +++--
 2 files changed, 50 insertions(+), 12 deletions(-)

--
1.9.1

[PATCH 1/5] nl80211: fix validation of scheduled scan info for wowlan netdetect

[Arend van Spriel]

For wowlan netdetect a separate limit is defined for the number of
matchsets. Currently, this limit is ignored and the regular limit
for scheduled scan matchsets, ie. struct wiphy::max_match_sets, is
used for the net-detect case as well.

Cc: Johannes Berg <johannes@sipsolutions.net>
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 net/wireless/nl80211.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ef5eff93..90833a4 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6850,7 +6850,7 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)

 static struct cfg80211_sched_scan_request *
 nl80211_parse_sched_scan(struct wiphy *wiphy, struct wireless_dev *wdev,
-			 struct nlattr **attrs)
+			 struct nlattr **attrs, int max_match_sets)
 {
 	struct cfg80211_sched_scan_request *request;
 	struct nlattr *attr;
@@ -6915,7 +6915,7 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
 	if (!n_match_sets && default_match_rssi != NL80211_SCAN_RSSI_THOLD_OFF)
 		n_match_sets = 1;

-	if (n_match_sets > wiphy->max_match_sets)
+	if (n_match_sets > max_match_sets)
 		return ERR_PTR(-EINVAL);

 	if (attrs[NL80211_ATTR_IE])
@@ -7189,7 +7189,8 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 		return -EINPROGRESS;

 	sched_scan_req = nl80211_parse_sched_scan(&rdev->wiphy, wdev,
-						  info->attrs);
+						  info->attrs,
+						  rdev->wiphy.max_match_sets);

 	err = PTR_ERR_OR_ZERO(sched_scan_req);
 	if (err)
@@ -9966,7 +9967,8 @@ static int nl80211_parse_wowlan_nd(struct cfg80211_registered_device *rdev,
 	if (err)
 		goto out;

-	trig->nd_config = nl80211_parse_sched_scan(&rdev->wiphy, NULL, tb);
+	trig->nd_config = nl80211_parse_sched_scan(&rdev->wiphy, NULL, tb,
+						   wowlan->max_nd_match_sets);
 	err = PTR_ERR_OR_ZERO(trig->nd_config);
 	if (err)
 		trig->nd_config = NULL;
--
1.9.1

Re: [PATCH 1/5] nl80211: fix validation of scheduled scan info for wowlan netdetect

[Johannes Berg]

On Fri, 2017-01-27 at 12:09 +0000, Arend van Spriel wrote:
> For wowlan netdetect a separate limit is defined for the number of
> matchsets. Currently, this limit is ignored and the regular limit
> for scheduled scan matchsets, ie. struct wiphy::max_match_sets, is
> used for the net-detect case as well.
> 

But .. this patch shouldn't be first in the series, should it? That
leaves brcmfmac broken inbetween.

Also, since you fix the brcmfmac value in patch 2, and it's the same in
both cases, I don't really see a dependency of anything *on* the
nl80211 patch? The brcmfmac changes can go in without it entirely, and
have no effect whatsoever, no?

IOW, I think Kalle can merge 2-5, and I can then pick up this one when
they hit net-next and I've synchronized, no?

johannes

Re: [PATCH 1/5] nl80211: fix validation of scheduled scan info for wowlan netdetect

[Arend Van Spriel]

On 27-1-2017 13:31, Johannes Berg wrote:
> On Fri, 2017-01-27 at 12:09 +0000, Arend van Spriel wrote:
>> For wowlan netdetect a separate limit is defined for the number of
>> matchsets. Currently, this limit is ignored and the regular limit
>> for scheduled scan matchsets, ie. struct wiphy::max_match_sets, is
>> used for the net-detect case as well.
>>
> 
> But .. this patch shouldn't be first in the series, should it? That
> leaves brcmfmac broken inbetween.
> 
> Also, since you fix the brcmfmac value in patch 2, and it's the same in
> both cases, I don't really see a dependency of anything *on* the
> nl80211 patch? The brcmfmac changes can go in without it entirely, and
> have no effect whatsoever, no?

Patch 4 depends on patch 2. Patch 1 and 2 were a single patch, but I
decided to separate them. So yeah, now patch 1 actually depends on patch
2 in order not to break brcmfmac.

> IOW, I think Kalle can merge 2-5, and I can then pick up this one when
> they hit net-next and I've synchronized, no?

Agree.

Regards,
Arend

Re: [PATCH 1/5] nl80211: fix validation of scheduled scan info for wowlan netdetect

[Johannes Berg]

On Fri, 2017-01-27 at 13:38 +0100, Arend Van Spriel wrote:
> 
> > IOW, I think Kalle can merge 2-5, and I can then pick up this one
> > when they hit net-next and I've synchronized, no?
> 
> Agree.

Alright. I see you submitted another version, I'll assign 2-5 to Kalle,
and keep 1 for myself, and not worry about the ordering since I'm the
one to have to remember to actually apply it later :)

johannes

Re: [PATCH 1/5] nl80211: fix validation of scheduled scan info for wowlan netdetect

[Johannes Berg]

On Fri, 2017-01-27 at 12:09 +0000, Arend van Spriel wrote:
> For wowlan netdetect a separate limit is defined for the number of
> matchsets. Currently, this limit is ignored and the regular limit
> for scheduled scan matchsets, ie. struct wiphy::max_match_sets, is
> used for the net-detect case as well.

Applied.

This means that brmcfmac is now broken in mac80211-next for a bit, but
once it hits net-next and I merge back everything will be fine again,
so just a few days (and I assume nobody us using brcmfmac from
mac80211-next anyway ... wireless-testing will be fine)

johannes

Re: [PATCH 1/5] nl80211: fix validation of scheduled scan info for wowlan netdetect

[Arend Van Spriel]

On 8-2-2017 10:10, Johannes Berg wrote:
> On Fri, 2017-01-27 at 12:09 +0000, Arend van Spriel wrote:
>> For wowlan netdetect a separate limit is defined for the number of
>> matchsets. Currently, this limit is ignored and the regular limit
>> for scheduled scan matchsets, ie. struct wiphy::max_match_sets, is
>> used for the net-detect case as well.
> 
> Applied.
> 
> This means that brmcfmac is now broken in mac80211-next for a bit, but
> once it hits net-next and I merge back everything will be fine again,
> so just a few days (and I assume nobody us using brcmfmac from
> mac80211-next anyway ... wireless-testing will be fine)

Well. Almost nobody :-p

Gr. AvS

[PATCH 2/5] brcmfmac: provide a value for struct wowlan_support::max_nd_match_sets

[Arend van Spriel]

The driver advertises support for WOWLAN_NETDETECT but did not specify
maximum amount of netdetect match sets. This was no issue due to a bug
in nl80211. As that has been fixed, brcmfmac also needs fixing.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index ec1171c..302a78d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6358,6 +6358,8 @@ static void brcmf_wiphy_wowl_params(struct wiphy *wiphy, struct brcmf_if *ifp)
 	if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_PNO)) {
 		if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_WOWL_ND)) {
 			brcmf_wowlan_support.flags |= WIPHY_WOWLAN_NET_DETECT;
+			brcmf_wowlan_support.max_nd_match_sets =
+				BRCMF_PNO_MAX_PFN_COUNT;
 			init_waitqueue_head(&cfg->wowl.nd_data_wait);
 		}
 	}
-- 
1.9.1

[PATCH 3/5] brcmfmac: fix handling firmware results for wowl netdetect

[Arend van Spriel]

For wowl netdetect the event data changed for newer chips. This
was recently fixed for scheduled scan, but same change is needed
for wowl netdetect. Removing now pointles += operation from both
result handlers.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 302a78d..c81d78f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3330,7 +3330,6 @@ static int brcmf_start_internal_escan(struct brcmf_if *ifp,
 		goto out_err;
 	}
 
-	data += sizeof(struct brcmf_pno_scanresults_le);
 	netinfo_start = brcmf_get_netinfo_array(pfn_result);
 
 	for (i = 0; i < result_count; i++) {
@@ -3478,8 +3477,7 @@ static s32 brcmf_config_wowl_pattern(struct brcmf_if *ifp, u8 cmd[4],
 		return -EINVAL;
 	}
 
-	data += sizeof(struct brcmf_pno_scanresults_le);
-	netinfo = (struct brcmf_pno_net_info_le *)data;
+	netinfo = brcmf_get_netinfo_array(pfn_result);
 	memcpy(cfg->wowl.nd->ssid.ssid, netinfo->SSID, netinfo->SSID_len);
 	cfg->wowl.nd->ssid.ssid_len = netinfo->SSID_len;
 	cfg->wowl.nd->n_channels = 1;
-- 
1.9.1

[PATCH 4/5] brcmfmac: allow wowlan support to be per device

[Arend van Spriel]

The wowlan support is (partially) determined dynamic by checking the
device/firmware capabilities. So they can differ per device. So it
is not possible to use a static global. Instead use the global as a
template and use kmemdup(). When kmemdup() fails the template is used
unmodified.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         | 26 ++++++++++++++++------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index c81d78f..9d5a561 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6339,7 +6339,7 @@ static void brcmf_wiphy_pno_params(struct wiphy *wiphy)
 }
 
 #ifdef CONFIG_PM
-static struct wiphy_wowlan_support brcmf_wowlan_support = {
+static const struct wiphy_wowlan_support brcmf_wowlan_support = {
 	.flags = WIPHY_WOWLAN_MAGIC_PKT | WIPHY_WOWLAN_DISCONNECT,
 	.n_patterns = BRCMF_WOWL_MAXPATTERNS,
 	.pattern_max_len = BRCMF_WOWL_MAXPATTERNSIZE,
@@ -6352,21 +6352,29 @@ static void brcmf_wiphy_wowl_params(struct wiphy *wiphy, struct brcmf_if *ifp)
 {
 #ifdef CONFIG_PM
 	struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
+	struct wiphy_wowlan_support *wowlan_support;
+
+	wowlan_support = kmemdup(&brcmf_wowlan_support,
+				 sizeof(brcmf_wowlan_support), GFP_KERNEL);
+	if (!wowlan_support) {
+		brcmf_err("only support basic wowlan features\n");
+		wiphy->wowlan = &brcmf_wowlan_support;
+		return;
+	}
 
 	if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_PNO)) {
 		if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_WOWL_ND)) {
-			brcmf_wowlan_support.flags |= WIPHY_WOWLAN_NET_DETECT;
-			brcmf_wowlan_support.max_nd_match_sets =
-				BRCMF_PNO_MAX_PFN_COUNT;
+			wowlan_support->flags |= WIPHY_WOWLAN_NET_DETECT;
+			wowlan_support->max_nd_match_sets = BRCMF_PNO_MAX_PFN_COUNT;
 			init_waitqueue_head(&cfg->wowl.nd_data_wait);
 		}
 	}
 	if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_WOWL_GTK)) {
-		brcmf_wowlan_support.flags |= WIPHY_WOWLAN_SUPPORTS_GTK_REKEY;
-		brcmf_wowlan_support.flags |= WIPHY_WOWLAN_GTK_REKEY_FAILURE;
+		wowlan_support->flags |= WIPHY_WOWLAN_SUPPORTS_GTK_REKEY;
+		wowlan_support->flags |= WIPHY_WOWLAN_GTK_REKEY_FAILURE;
 	}
 
-	wiphy->wowlan = &brcmf_wowlan_support;
+	wiphy->wowlan = wowlan_support;
 #endif
 }
 
@@ -6747,6 +6755,10 @@ static void brcmf_free_wiphy(struct wiphy *wiphy)
 		kfree(wiphy->bands[NL80211_BAND_5GHZ]->channels);
 		kfree(wiphy->bands[NL80211_BAND_5GHZ]);
 	}
+#if IS_ENABLED(CONFIG_PM)
+	if (wiphy->wowlan != &brcmf_wowlan_support)
+		kfree(wiphy->wowlan);
+#endif
 	wiphy_free(wiphy);
 }
 
-- 
1.9.1

Re: [PATCH 4/5] brcmfmac: allow wowlan support to be per device

[Arend Van Spriel]

On 27-1-2017 13:09, Arend van Spriel wrote:
> The wowlan support is (partially) determined dynamic by checking the
> device/firmware capabilities. So they can differ per device. So it
> is not possible to use a static global. Instead use the global as a
> template and use kmemdup(). When kmemdup() fails the template is used
> unmodified.
> 
> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> ---
>  .../broadcom/brcm80211/brcmfmac/cfg80211.c         | 26 ++++++++++++++++------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index c81d78f..9d5a561 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -6339,7 +6339,7 @@ static void brcmf_wiphy_pno_params(struct wiphy *wiphy)
>  }
>  
>  #ifdef CONFIG_PM
> -static struct wiphy_wowlan_support brcmf_wowlan_support = {
> +static const struct wiphy_wowlan_support brcmf_wowlan_support = {
>  	.flags = WIPHY_WOWLAN_MAGIC_PKT | WIPHY_WOWLAN_DISCONNECT,
>  	.n_patterns = BRCMF_WOWL_MAXPATTERNS,
>  	.pattern_max_len = BRCMF_WOWL_MAXPATTERNSIZE,
> @@ -6352,21 +6352,29 @@ static void brcmf_wiphy_wowl_params(struct wiphy *wiphy, struct brcmf_if *ifp)
>  {
>  #ifdef CONFIG_PM
>  	struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
> +	struct wiphy_wowlan_support *wowlan_support;
> +
> +	wowlan_support = kmemdup(&brcmf_wowlan_support,
> +				 sizeof(brcmf_wowlan_support), GFP_KERNEL);
> +	if (!wowlan_support) {
> +		brcmf_err("only support basic wowlan features\n");
> +		wiphy->wowlan = &brcmf_wowlan_support;
> +		return;
> +	}
>  
>  	if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_PNO)) {
>  		if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_WOWL_ND)) {
> -			brcmf_wowlan_support.flags |= WIPHY_WOWLAN_NET_DETECT;
> -			brcmf_wowlan_support.max_nd_match_sets =
> -				BRCMF_PNO_MAX_PFN_COUNT;
> +			wowlan_support->flags |= WIPHY_WOWLAN_NET_DETECT;
> +			wowlan_support->max_nd_match_sets = BRCMF_PNO_MAX_PFN_COUNT;

Did not checkpatch these before sending. This is over 80 chars so please
drop this series and I will send a V2.

Regards,
Arend

[PATCH 5/5] brcmfmac: add .update_connect_params() callback

[Arend van Spriel]

Add support for the .update_connect_params() callback for roaming
or subsequent (re)association.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 9d5a561..6741477 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5067,6 +5067,29 @@ static int brcmf_cfg80211_tdls_oper(struct wiphy *wiphy,
 	return ret;
 }
 
+static int
+brcmf_cfg80211_update_conn_params(struct wiphy *wiphy,
+				  struct net_device *ndev,
+				  struct cfg80211_connect_params *sme,
+				  u32 changed)
+{
+	struct brcmf_if *ifp;
+	int err;
+
+	if (!(changed & UPDATE_ASSOC_IES))
+		return 0;
+
+	ifp = netdev_priv(ndev);
+	err = brcmf_vif_set_mgmt_ie(ifp->vif, BRCMF_VNDR_IE_ASSOCREQ_FLAG,
+				    sme->ie, sme->ie_len);
+	if (err)
+		brcmf_err("Set Assoc REQ IE Failed\n");
+	else
+		brcmf_dbg(TRACE, "Applied Vndr IEs for Assoc request\n");
+
+	return err;
+}
+
 #ifdef CONFIG_PM
 static int
 brcmf_cfg80211_set_rekey_data(struct wiphy *wiphy, struct net_device *ndev,
@@ -5134,6 +5157,7 @@ static int brcmf_cfg80211_tdls_oper(struct wiphy *wiphy,
 	.crit_proto_start = brcmf_cfg80211_crit_proto_start,
 	.crit_proto_stop = brcmf_cfg80211_crit_proto_stop,
 	.tdls_oper = brcmf_cfg80211_tdls_oper,
+	.update_connect_params = brcmf_cfg80211_update_conn_params,
 };
 
 struct brcmf_cfg80211_vif *brcmf_alloc_vif(struct brcmf_cfg80211_info *cfg,
-- 
1.9.1