[PATCH 1/3] cfg80211: remove unnecessary check

[PATCH 1/3] cfg80211: remove unnecessary check

[Zhao, Gang]

RCU pointer bss->pub.beacon_ies is checked before in previous
statement:

if (rcu_access_pointer(bss->pub.beacon_ies))
	continue;

There is no need to check it twice(and in the wrong way :) ).

Signed-off-by: Zhao, Gang <gamerh2o@gmail.com>
---
 net/wireless/scan.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index d1ed4ae..7297d18 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -659,9 +659,6 @@ static bool cfg80211_combine_bsses(struct cfg80211_registered_device *dev,
 			continue;
 		if (ssidlen && ie[1] != ssidlen)
 			continue;
-		/* that would be odd ... */
-		if (bss->pub.beacon_ies)
-			continue;
 		if (WARN_ON_ONCE(bss->pub.hidden_beacon_bss))
 			continue;
 		if (WARN_ON_ONCE(!list_empty(&bss->hidden_list)))
-- 
1.8.5.3

[PATCH 2/3] cfg80211: remove macro ASSERT_RDEV_LOCK(rdev)

[Zhao, Gang]

Macro ASSERT_RDEV_LOCK(rdev) is equal to ASSERT_RTNL(), so replace it
with ASSERT_RTNL() and remove it.

Signed-off-by: Zhao, Gang <gamerh2o@gmail.com>
---
 net/wireless/core.h     | 1 -
 net/wireless/sme.c      | 1 -
 net/wireless/util.c     | 3 +--
 net/wireless/wext-sme.c | 2 +-
 4 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/wireless/core.h b/net/wireless/core.h
index 766aabc..6ef60d3 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -166,7 +166,6 @@ static inline void wdev_unlock(struct wireless_dev *wdev)
 	mutex_unlock(&wdev->mtx);
 }
 
-#define ASSERT_RDEV_LOCK(rdev) ASSERT_RTNL()
 #define ASSERT_WDEV_LOCK(wdev) lockdep_assert_held(&(wdev)->mtx)
 
 static inline bool cfg80211_has_monitors_only(struct cfg80211_registered_device *rdev)
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index f04d4c3..acdcb4a8 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -64,7 +64,6 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
 	int n_channels, err;
 
 	ASSERT_RTNL();
-	ASSERT_RDEV_LOCK(rdev);
 	ASSERT_WDEV_LOCK(wdev);
 
 	if (rdev->scan_req || rdev->scan_msg)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 57b3ce7..114bbdf 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -838,7 +838,6 @@ void cfg80211_process_rdev_events(struct cfg80211_registered_device *rdev)
 	struct wireless_dev *wdev;
 
 	ASSERT_RTNL();
-	ASSERT_RDEV_LOCK(rdev);
 
 	list_for_each_entry(wdev, &rdev->wdev_list, list)
 		cfg80211_process_wdev_events(wdev);
@@ -851,7 +850,7 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
 	int err;
 	enum nl80211_iftype otype = dev->ieee80211_ptr->iftype;
 
-	ASSERT_RDEV_LOCK(rdev);
+	ASSERT_RTNL();
 
 	/* don't support changing VLANs, you just re-create them */
 	if (otype == NL80211_IFTYPE_AP_VLAN)
diff --git a/net/wireless/wext-sme.c b/net/wireless/wext-sme.c
index 14c9a25..86c331a 100644
--- a/net/wireless/wext-sme.c
+++ b/net/wireless/wext-sme.c
@@ -21,7 +21,7 @@ int cfg80211_mgd_wext_connect(struct cfg80211_registered_device *rdev,
 	const u8 *prev_bssid = NULL;
 	int err, i;
 
-	ASSERT_RDEV_LOCK(rdev);
+	ASSERT_RTNL();
 	ASSERT_WDEV_LOCK(wdev);
 
 	if (!netif_running(wdev->netdev))
-- 
1.8.5.3

[PATCH 3/3] cfg80211: slightly clean up of cfg80211_sme_connect()

[Zhao, Gang]

Wdev->ssid has already been set in cfg80211_connect() and is equal to
connect->ssid, so the assignment here doesn't hurt. Use connect->ssid
instead of wdev->ssid so it will be consistent with following ssid_len
assignment statement.

If bss is found in cfg80211_get_conn_bss(), wdev->conn->state is set
to CFG80211_CONN_AUTHENTICATE_NEXT in there. So it's not needed to set
it manually to CFG80211_CONN_AUTHENTICATE_NEXT if bss is found in that
function.

Signed-off-by: Zhao, Gang <gamerh2o@gmail.com>
---
 net/wireless/sme.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index acdcb4a8..c259aa6 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -467,7 +467,7 @@ static int cfg80211_sme_connect(struct wireless_dev *wdev,
 		wdev->conn->auto_auth = false;
 	}
 
-	wdev->conn->params.ssid = wdev->ssid;
+	wdev->conn->params.ssid = connect->ssid;
 	wdev->conn->params.ssid_len = connect->ssid_len;
 
 	/* see if we have the bss already */
@@ -480,7 +480,6 @@ static int cfg80211_sme_connect(struct wireless_dev *wdev,
 
 	/* we're good if we have a matching bss struct */
 	if (bss) {
-		wdev->conn->state = CFG80211_CONN_AUTHENTICATE_NEXT;
 		err = cfg80211_conn_do_work(wdev);
 		cfg80211_put_bss(wdev->wiphy, bss);
 	} else {
-- 
1.8.5.3

Re: [PATCH 1/3] cfg80211: remove unnecessary check

[Johannes Berg]

On Wed, 2014-03-19 at 17:04 +0800, Zhao, Gang wrote:
> RCU pointer bss->pub.beacon_ies is checked before in previous
> statement:
> 
> if (rcu_access_pointer(bss->pub.beacon_ies))
> 	continue;
> 
> There is no need to check it twice(and in the wrong way :) ).

Applied.

johannes

Re: [PATCH 2/3] cfg80211: remove macro ASSERT_RDEV_LOCK(rdev)

[Johannes Berg]

On Wed, 2014-03-19 at 17:04 +0800, Zhao, Gang wrote:
> Macro ASSERT_RDEV_LOCK(rdev) is equal to ASSERT_RTNL(), so replace it
> with ASSERT_RTNL() and remove it.

Applied.

johannes

Re: [PATCH 3/3] cfg80211: slightly clean up of cfg80211_sme_connect()

[Johannes Berg]

On Wed, 2014-03-19 at 17:04 +0800, Zhao, Gang wrote:

> -	wdev->conn->params.ssid = wdev->ssid;
> +	wdev->conn->params.ssid = connect->ssid;

I'm pretty sure this is wrong since one is a pointer and the other an
array.

johannes

[PATCH 3/3 v2] cfg80211: slightly clean up of cfg80211_sme_connect()

[Zhao, Gang]

Wdev->ssid_len has already been set in cfg80211_connect() and is equal
to connect->ssid_len. Use wdev->ssid_len instead of connect->ssid_len
so it will be consistent with previous ssid assignment statement.

If bss is found in cfg80211_get_conn_bss(), wdev->conn->state is set
to CFG80211_CONN_AUTHENTICATE_NEXT in there. So it's not needed to set
it manually to CFG80211_CONN_AUTHENTICATE_NEXT if bss is found in that
function.

Signed-off-by: Zhao, Gang <gamerh2o@gmail.com>
---

v2: change the assignment of ssid_len, instead of ssid before.

 net/wireless/sme.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index acdcb4a8..998674f 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -468,7 +468,7 @@ static int cfg80211_sme_connect(struct wireless_dev *wdev,
 	}
 
 	wdev->conn->params.ssid = wdev->ssid;
-	wdev->conn->params.ssid_len = connect->ssid_len;
+	wdev->conn->params.ssid_len = wdev->ssid_len;
 
 	/* see if we have the bss already */
 	bss = cfg80211_get_conn_bss(wdev);
@@ -480,7 +480,6 @@ static int cfg80211_sme_connect(struct wireless_dev *wdev,
 
 	/* we're good if we have a matching bss struct */
 	if (bss) {
-		wdev->conn->state = CFG80211_CONN_AUTHENTICATE_NEXT;
 		err = cfg80211_conn_do_work(wdev);
 		cfg80211_put_bss(wdev->wiphy, bss);
 	} else {
-- 
1.8.5.3

Re: [PATCH 3/3 v2] cfg80211: slightly clean up of cfg80211_sme_connect()

[Johannes Berg]

On Wed, 2014-03-19 at 23:02 +0800, Zhao, Gang wrote:
> Wdev->ssid_len has already been set in cfg80211_connect() and is equal
> to connect->ssid_len. Use wdev->ssid_len instead of connect->ssid_len
> so it will be consistent with previous ssid assignment statement.
> 
> If bss is found in cfg80211_get_conn_bss(), wdev->conn->state is set
> to CFG80211_CONN_AUTHENTICATE_NEXT in there. So it's not needed to set
> it manually to CFG80211_CONN_AUTHENTICATE_NEXT if bss is found in that
> function.

Applied.

johannes