All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafał Miłecki" <zajec5@gmail.com>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: "Rafał Miłecki" <zajec5@gmail.com>,
	"Brett Rudley" <brudley@broadcom.com>,
	"Arend van Spriel" <arend@broadcom.com>,
	"Franky (Zhenhui) Lin" <frankyl@broadcom.com>,
	"Hante Meuleman" <meuleman@broadcom.com>,
	"Pieter-Paul Giesberts" <pieterpg@broadcom.com>,
	"Franky Lin" <franky.lin@broadcom.com>,
	linux-wireless@vger.kernel.org (open list:BROADCOM BRCM80211
	IEEE802.11n WIRELESS DRIVER),
	brcm80211-dev-list@broadcom.com (open list:BROADCOM BRCM80211
	IEEE802.11n WIRELESS DRIVER),
	netdev@vger.kernel.org (open list:NETWORKING DRIVERS),
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCH] brcmfmac: rework function picking free BSS index
Date: Thu, 26 May 2016 01:44:27 +0200	[thread overview]
Message-ID: <1464219876-13776-1-git-send-email-zajec5@gmail.com> (raw)

The old implementation was overcomplicated and slightly bugged in some
corner cases.

Consider following state of BSS-es (limited to 6 for simplification):
drvr->iflist[0]: { bsscfgidx:0, ndev->name:wlan1, }
drvr->iflist[1]:  (null)
drvr->iflist[2]: { bsscfgidx:2, ndev->name:wlan1-1, }
drvr->iflist[3]: { bsscfgidx:3, ndev->name:wlan1-2, }
drvr->iflist[4]:  (null)
drvr->iflist[5]:  (null)
In such case the next AP interface should bsscfgidx 4 (we don't use 1 as
it's reserved for P2P).

With old code the loop iterations were following:
[ifidx = 0] [bsscfgidx = 2] [highest = 2]
[ifidx = 1] [bsscfgidx = 2] [highest = 2] available = true
[ifidx = 2] [bsscfgidx = 2] [highest = 2] bsscfgidx = highest + 1
[ifidx = 3] [bsscfgidx = 3] [highest = 2] bsscfgidx = highest + 1
[ifidx = 4] [bsscfgidx = 3] [highest = 2] available = true
[ifidx = 5] [bsscfgidx = 3] [highest = 2] available = true
There were 2 obvious problems:
1) Having empty BSS at index 1 was resulting in available being always
   set to true, even if we would run out of BSS-es.
2) Calculated bsscfgidx was invalid (3 instead of 4) resulting in driver
   not being able to create the 4th AP interface.

New code is simpler, placed in file where it's really used, handles
running out of free BSS-es and allows using 4 interfaces at the same
time. It also looks for the first free BSS instead of one after the last
in use. It works well with current driver (which doesn't allow deleting
interfaces) and should be future proof (if we ever allow deleting).

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         | 17 ++++++++++++++-
 .../wireless/broadcom/brcm80211/brcmfmac/core.c    | 24 ----------------------
 .../wireless/broadcom/brcm80211/brcmfmac/core.h    |  1 -
 3 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 3d09d23..d00eef8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -541,6 +541,21 @@ brcmf_cfg80211_update_proto_addr_mode(struct wireless_dev *wdev)
 						ADDR_INDIRECT);
 }
 
+static int brcmf_get_first_free_bsscfgidx(struct brcmf_pub *drvr)
+{
+	int bsscfgidx;
+
+	for (bsscfgidx = 0; bsscfgidx < BRCMF_MAX_IFS; bsscfgidx++) {
+		/* bsscfgidx 1 is reserved for legacy P2P */
+		if (bsscfgidx == 1)
+			continue;
+		if (!drvr->iflist[bsscfgidx])
+			return bsscfgidx;
+	}
+
+	return -ENOMEM;
+}
+
 static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp)
 {
 	struct brcmf_mbss_ssid_le mbss_ssid_le;
@@ -548,7 +563,7 @@ static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp)
 	int err;
 
 	memset(&mbss_ssid_le, 0, sizeof(mbss_ssid_le));
-	bsscfgidx = brcmf_get_next_free_bsscfgidx(ifp->drvr);
+	bsscfgidx = brcmf_get_first_free_bsscfgidx(ifp->drvr);
 	if (bsscfgidx < 0)
 		return bsscfgidx;
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index b590499..6a76480 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -753,30 +753,6 @@ void brcmf_remove_interface(struct brcmf_if *ifp)
 	brcmf_del_if(ifp->drvr, ifp->bsscfgidx);
 }
 
-int brcmf_get_next_free_bsscfgidx(struct brcmf_pub *drvr)
-{
-	int ifidx;
-	int bsscfgidx;
-	bool available;
-	int highest;
-
-	available = false;
-	bsscfgidx = 2;
-	highest = 2;
-	for (ifidx = 0; ifidx < BRCMF_MAX_IFS; ifidx++) {
-		if (drvr->iflist[ifidx]) {
-			if (drvr->iflist[ifidx]->bsscfgidx == bsscfgidx)
-				bsscfgidx = highest + 1;
-			else if (drvr->iflist[ifidx]->bsscfgidx > highest)
-				highest = drvr->iflist[ifidx]->bsscfgidx;
-		} else {
-			available = true;
-		}
-	}
-
-	return available ? bsscfgidx : -ENOMEM;
-}
-
 #ifdef CONFIG_INET
 #define ARPOL_MAX_ENTRIES	8
 static int brcmf_inetaddr_changed(struct notifier_block *nb,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index 647d3cc..2a075c5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -217,7 +217,6 @@ int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked);
 struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
 			      bool is_p2pdev, char *name, u8 *mac_addr);
 void brcmf_remove_interface(struct brcmf_if *ifp);
-int brcmf_get_next_free_bsscfgidx(struct brcmf_pub *drvr);
 void brcmf_txflowblock_if(struct brcmf_if *ifp,
 			  enum brcmf_netif_stop_reason reason, bool state);
 void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success);
-- 
1.8.4.5


             reply	other threads:[~2016-05-26  2:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-25 23:44 Rafał Miłecki [this message]
2016-06-09 16:17 ` brcmfmac: rework function picking free BSS index Kalle Valo
2016-06-09 16:30   ` Kalle Valo
2016-06-09 16:30     ` Kalle Valo
2016-06-09 16:48     ` Rafał Miłecki
2016-06-09 16:48       ` Rafał Miłecki
2016-06-09 19:16 ` [PATCH] " Arend van Spriel
2016-06-09 19:16   ` Arend van Spriel
2016-06-09 19:16   ` Arend van Spriel
2016-06-13 19:30   ` Arend van Spriel
2016-06-13 19:30     ` Arend van Spriel
2016-06-13 19:30     ` Arend van Spriel
2016-06-13 20:25     ` Rafał Miłecki
2016-06-13 20:25       ` Rafał Miłecki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1464219876-13776-1-git-send-email-zajec5@gmail.com \
    --to=zajec5@gmail.com \
    --cc=arend@broadcom.com \
    --cc=brcm80211-dev-list@broadcom.com \
    --cc=brudley@broadcom.com \
    --cc=franky.lin@broadcom.com \
    --cc=frankyl@broadcom.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=meuleman@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=pieterpg@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.