* [PATCH] staging: brcm80211: Remove redundant unlikely()
From: Tobias Klauser @ 2010-12-09 15:01 UTC (permalink / raw)
To: Brett Rudley, Henry Ptasinski, Nohee Ko, Greg Kroah-Hartman
Cc: linux-wireless, devel, kernel-janitors
IS_ERR() already implies unlikely(), so it can be omitted here.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c
index 0d7aa4a..edf300d 100644
--- a/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c
@@ -3196,7 +3196,7 @@ s32 wl_cfg80211_attach(struct net_device *ndev, void *data)
}
WL_DBG(("func %p\n", wl_cfg80211_get_sdio_func()));
wdev = wl_alloc_wdev(sizeof(struct wl_iface), &wl_cfg80211_get_sdio_func()->dev);
- if (unlikely(IS_ERR(wdev)))
+ if (IS_ERR(wdev))
return -ENOMEM;
wdev->iftype = wl_mode_to_nl80211_iftype(WL_MODE_BSS);
--
1.7.0.4
^ permalink raw reply related
* [PATCH] wl1271: fixed problem with WPS IEs in probe requests
From: Guy Eilam @ 2010-12-09 14:54 UTC (permalink / raw)
To: linux-wireless; +Cc: luciano.coelho
Inclusion of a WPS IE in probe requests caused a problem
in the driver due to the maximum size of the probe request
template and the max_scan_ie_len values at initialization.
Increased the size of probe request template
to the maximum size allowed by the firmware.
Struct wl12xx_probe_req_template, which was only used
for calculating the max size of the probe request template,
is no longer used and needed.
max_scan_ie_len is used for validating the size of
additional IEs in scan requests.
Initialized the max_scan_ie_len field to the maximum size
of the probe request template minus the ieee80211 header size.
Signed-off-by: Guy Eilam <guy@wizery.com>
---
drivers/net/wireless/wl12xx/init.c | 6 ++----
drivers/net/wireless/wl12xx/main.c | 7 +++++++
drivers/net/wireless/wl12xx/wl12xx_80211.h | 8 --------
3 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/net/wireless/wl12xx/init.c b/drivers/net/wireless/wl12xx/init.c
index 8a4e80d..68db4da 100644
--- a/drivers/net/wireless/wl12xx/init.c
+++ b/drivers/net/wireless/wl12xx/init.c
@@ -53,18 +53,16 @@ static int wl1271_init_hwenc_config(struct wl1271 *wl)
int wl1271_init_templates_config(struct wl1271 *wl)
{
int ret, i;
- size_t size;
/* send empty templates for fw memory reservation */
ret = wl1271_cmd_template_set(wl, CMD_TEMPL_CFG_PROBE_REQ_2_4, NULL,
- sizeof(struct wl12xx_probe_req_template),
+ WL1271_CMD_TEMPL_MAX_SIZE,
0, WL1271_RATE_AUTOMATIC);
if (ret < 0)
return ret;
- size = sizeof(struct wl12xx_probe_req_template);
ret = wl1271_cmd_template_set(wl, CMD_TEMPL_CFG_PROBE_REQ_5,
- NULL, size, 0,
+ NULL, WL1271_CMD_TEMPL_MAX_SIZE, 0,
WL1271_RATE_AUTOMATIC);
if (ret < 0)
return ret;
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 7fecefe..81c8d66 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2654,6 +2654,13 @@ int wl1271_init_ieee80211(struct wl1271 *wl)
wl->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
BIT(NL80211_IFTYPE_ADHOC);
wl->hw->wiphy->max_scan_ssids = 1;
+ /*
+ * Maximum length of elements in scanning probe request templates
+ * should be the maximum length possible for a template, without
+ * the IEEE80211 header of the template
+ */
+ wl->hw->wiphy->max_scan_ie_len = WL1271_CMD_TEMPL_MAX_SIZE -
+ sizeof(struct ieee80211_header);
wl->hw->wiphy->bands[IEEE80211_BAND_2GHZ] = &wl1271_band_2ghz;
wl->hw->wiphy->bands[IEEE80211_BAND_5GHZ] = &wl1271_band_5ghz;
diff --git a/drivers/net/wireless/wl12xx/wl12xx_80211.h b/drivers/net/wireless/wl12xx/wl12xx_80211.h
index 1846280..00c9627 100644
--- a/drivers/net/wireless/wl12xx/wl12xx_80211.h
+++ b/drivers/net/wireless/wl12xx/wl12xx_80211.h
@@ -133,14 +133,6 @@ struct wl12xx_qos_null_data_template {
__le16 qos_ctl;
} __packed;
-struct wl12xx_probe_req_template {
- struct ieee80211_header header;
- struct wl12xx_ie_ssid ssid;
- struct wl12xx_ie_rates rates;
- struct wl12xx_ie_rates ext_rates;
-} __packed;
-
-
struct wl12xx_probe_resp_template {
struct ieee80211_header header;
__le32 time_stamp[2];
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] mac80211: Fix deadlock in ieee80211_do_stop.
From: Tejun Heo @ 2010-12-09 14:46 UTC (permalink / raw)
To: Johannes Berg; +Cc: Ben Greear, Luis R. Rodriguez, linux-wireless
In-Reply-To: <1291905750.3540.14.camel@jlt3.sipsolutions.net>
Hello, Johannes.
On 12/09/2010 03:42 PM, Johannes Berg wrote:
> On Thu, 2010-12-09 at 15:34 +0100, Tejun Heo wrote:
>
>> [<78447ce1>] flush_work+0x23/0x27
>> [<f91a2646>] ieee80211_do_stop+0x25c/0x403 [mac80211]
>
>> [<787001fe>] rtnetlink_rcv+0x1b/0x22 <- rtnl lock
>
> Right, so we're flushing here under RTNL ... I believe this is the one
> that Ben hacked up to not flush or so?
He made it to cancel instead of flush.
>> [<7878cdab>] _cond_resched+0x2b/0x44
>> [<7878d84f>] mutex_lock_nested+0x22/0x3b
>> [<f919fddc>] ieee80211_sta_rx_queued_mgmt+0x2d/0x3a6 [mac80211]
>> [<f91a2f53>] ieee80211_iface_work+0x1ff/0x282 [mac80211]
>
>> But, sdata->work is busy running ieee80211_iface_work(). I _suspect_
>> for some reason iee80211_iface_work() isn't finishing.
>
> It's trying to acquire a mutex here, which must be &ifmgd->mtx or
> &local->mtx, but neither of them ever nest around the RTNL.
Yeah, but the task state is 'R' not 'D' and no one else is holding the
lock. It seems more like ieee80211_iface_work() is looping
constantly.
>> That, or, the new flush_work() implementation is broken and it's
>> failing to flush when a work is being executed back to back. I'll
>> prep a debug patch to determine what's going on.
>
> Thanks.
>
> I wonder if Ben can attempt to reproduce this using compat-wireless
> against a kernel that doesn't have the workqueue changes, was the last
> one without those 2.6.34? 2.6.35?
As I think we're now pretty close to where the problem is, I'd like to
try a few things before going that path.
>> The rest of the system going down the toilet after this is mostly
>> caused by the held rtnl_lock above.
>
> Indeed, the rtnl is pretty important :-)
Heh, yeah, it's one of the most widely used mutex. It's scary. :-)
--
tejun
^ permalink raw reply
* Re: [PATCH] ath: fix build break with ATH_DBG_WARN_ON_ONCE
From: John W. Linville @ 2010-12-09 14:42 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless, Hauke Mehrtens, Luis R. Rodriguez, Ben Greear
In-Reply-To: <1291904272.3540.10.camel@jlt3.sipsolutions.net>
On Thu, Dec 09, 2010 at 03:17:52PM +0100, Johannes Berg wrote:
> On Thu, 2010-12-09 at 09:12 -0500, John W. Linville wrote:
>
> > diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
> > index 3eb95e2..45d4d91 100644
> > --- a/drivers/net/wireless/ath/ath.h
> > +++ b/drivers/net/wireless/ath/ath.h
> > @@ -272,7 +272,7 @@ ath_dbg(struct ath_common *common, enum ATH_DEBUG dbg_mask,
> > return 0;
> > }
> > #define ATH_DBG_WARN(foo, arg...) do {} while (0)
> > -#define ATH_DBG_WARN_ON_ONCE(foo) do {} while (0)
> > +#define ATH_DBG_WARN_ON_ONCE(foo) ({ 0; })
>
> That'll fix the build error, but wouldn't you need (foo) for correctness
> instead?
Well, true -- that was too simple. Unfortunately (foo) gives warnings
when the return code is not checked.
John
---
>From b7613370db5ba66ad81e41cd3a5417fde4d5e03c Mon Sep 17 00:00:00 2001
From: John W. Linville <linville@tuxdriver.com>
Date: Thu, 9 Dec 2010 09:08:47 -0500
Subject: [PATCH] ath: fix build break with ATH_DBG_WARN_ON_ONCE
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Description by Hauke:
"If CONFIG_ATH_DEBUG=y is set ATH_DBG_WARN_ON_ONCE uses WARN_ON_ONCE and
returns something, but if CONFIG_ATH_DEBUG is not set it does not return
anything. Now ATH_DBG_WARN_ON_ONCE is used in the boolean expression in
an if case and is not returning anything and causes a compile error.
CC [M] /drivers/net/wireless/ath/ath9k/main.o
/drivers/net/wireless/ath/ath9k/main.c: In function ‘ath_isr’:
/drivers/net/wireless/ath/ath9k/main.c:769: error: expected expression
before ‘do’
make[5]: *** [/drivers/net/wireless/ath/ath9k/main.o] Error 1
make[4]: *** [/drivers/net/wireless/ath/ath9k] Error 2"
Reported-by: Hauke Mehrtens <hauke@hauke-m.de>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
drivers/net/wireless/ath/ath.h | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 3eb95e2..e43210c 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -272,7 +272,10 @@ ath_dbg(struct ath_common *common, enum ATH_DEBUG dbg_mask,
return 0;
}
#define ATH_DBG_WARN(foo, arg...) do {} while (0)
-#define ATH_DBG_WARN_ON_ONCE(foo) do {} while (0)
+#define ATH_DBG_WARN_ON_ONCE(foo) ({ \
+ int __ret_warn_once = !!(foo); \
+ unlikely(__ret_warn_once); \
+})
#endif /* CONFIG_ATH_DEBUG */
--
1.7.3.2
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply related
* Re: [PATCH] mac80211: Fix deadlock in ieee80211_do_stop.
From: Johannes Berg @ 2010-12-09 14:42 UTC (permalink / raw)
To: Tejun Heo; +Cc: Ben Greear, Luis R. Rodriguez, linux-wireless
In-Reply-To: <4D00E8E2.1030201@kernel.org>
On Thu, 2010-12-09 at 15:34 +0100, Tejun Heo wrote:
> [<78447ce1>] flush_work+0x23/0x27
> [<f91a2646>] ieee80211_do_stop+0x25c/0x403 [mac80211]
> [<787001fe>] rtnetlink_rcv+0x1b/0x22 <- rtnl lock
Right, so we're flushing here under RTNL ... I believe this is the one
that Ben hacked up to not flush or so?
> [<7878cdab>] _cond_resched+0x2b/0x44
> [<7878d84f>] mutex_lock_nested+0x22/0x3b
> [<f919fddc>] ieee80211_sta_rx_queued_mgmt+0x2d/0x3a6 [mac80211]
> [<f91a2f53>] ieee80211_iface_work+0x1ff/0x282 [mac80211]
> But, sdata->work is busy running ieee80211_iface_work(). I _suspect_
> for some reason iee80211_iface_work() isn't finishing.
It's trying to acquire a mutex here, which must be &ifmgd->mtx or
&local->mtx, but neither of them ever nest around the RTNL.
> That, or, the
> new flush_work() implementation is broken and it's failing to flush
> when a work is being executed back to back. I'll prep a debug patch
> to determine what's going on.
Thanks.
I wonder if Ben can attempt to reproduce this using compat-wireless
against a kernel that doesn't have the workqueue changes, was the last
one without those 2.6.34? 2.6.35?
> The rest of the system going down the toilet after this is mostly
> caused by the held rtnl_lock above.
Indeed, the rtnl is pretty important :-)
johannes
^ permalink raw reply
* Re: [PATCH 2/4] b43: set TMS to work with current band width for N-PHY
From: Michael Büsch @ 2010-12-09 14:41 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
In-Reply-To: <AANLkTinJUsKzNO6vO8F9KupxF0uc1h97wffhAgBas1Pe@mail.gmail.com>
On Thu, 2010-12-09 at 15:23 +0100, Rafał Miłecki wrote:
> W dniu 9 grudnia 2010 15:19 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
> > On Thu, 2010-12-09 at 15:13 +0100, Rafał Miłecki wrote:
> >> W dniu 9 grudnia 2010 15:12 użytkownik Rafał Miłecki <zajec5@gmail.com> napisał:
> >> > W dniu 7 grudnia 2010 22:08 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
> >> >> On Tue, 2010-12-07 at 21:55 +0100, Rafał Miłecki wrote:
> >> >>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> >> >>> ---
> >> >>> drivers/net/wireless/b43/main.c | 6 ++++++
> >> >>> drivers/net/wireless/b43/phy_common.c | 7 +++++++
> >> >>> drivers/net/wireless/b43/phy_common.h | 2 ++
> >> >>> drivers/net/wireless/b43/phy_n.c | 7 -------
> >> >>> 4 files changed, 15 insertions(+), 7 deletions(-)
> >> >>>
> >> >>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
> >> >>> index fa48803..670fd7b 100644
> >> >>> --- a/drivers/net/wireless/b43/main.c
> >> >>> +++ b/drivers/net/wireless/b43/main.c
> >> >>> @@ -1150,6 +1150,12 @@ void b43_wireless_core_reset(struct b43_wldev *dev, u32 flags)
> >> >>>
> >> >>> flags |= B43_TMSLOW_PHYCLKEN;
> >> >>> flags |= B43_TMSLOW_PHYRESET;
> >> >>> + if (dev->phy.type == B43_PHYTYPE_N) {
> >> >>> + if (b43_channel_type_is_40mhz(dev->phy.channel_type))
> >> >>
> >> >> Is channel_type already set at this time?
The more I think about this, the wronger it looks to me.
Why do we care about the channel type at reset time anyway?
I guess the channel type is a parameter that can change any time on an
operating card. So just unconditionally set B43_TMSLOW_PHYCLKSPEED_80MHZ
at reset time and set the correct TMSLOW bits later when channel_type is
actually changed. That seems to be the only sane thing to do.
I guess we don't want to fully reset the device when channel_type
changes, do we?
--
Greetings Michael.
^ permalink raw reply
* Re: ath5k: Weird Retransmission Behaviour
From: Jonathan Guerin @ 2010-12-09 14:34 UTC (permalink / raw)
To: Bob Copeland
Cc: Helmut Schaa, Bruno Randolf, linux-wireless, ath5k-devel, nbd
In-Reply-To: <20101209123841.GA31566@hash.localnet>
On Thu, Dec 9, 2010 at 10:38 PM, Bob Copeland <me@bobcopeland.com> wrote:
> On Thu, Dec 09, 2010 at 10:21:34AM +0100, Helmut Schaa wrote:
>> On Wed, Dec 8, 2010 at 10:53 PM, Jonathan Guerin <jonathan@guerin.id.au> wrote:
>> > I only seem to have Minstrel as the only available Rate Control
>> > algorithm in my kernel config?
>>
>> PID is only selectable on embedded platforms:
>>
>> config MAC80211_RC_PID
>> bool "PID controller based rate control algorithm" if EMBEDDED
>>
>> Just remove the "if EMBEDDED" from net/mac8011/Kconfig and retry.
>
> For what it's worth, I tested pid and minstrel a while ago with a
> modified mac80211_hwsim, and found minstrel to be quite a bit better
> at rate adaptation. So while it may be worth testing out for this
> particular use case, minstrel is probably the way to go in general.
So, while I say this with no idea how to do it, might it be worth
fixing Minstrel so that it adheres to mac80211's maximum retry values?
Cheers,
Jonathan
>
> --
> Bob Copeland %% www.bobcopeland.com
>
>
^ permalink raw reply
* Re: [PATCH] mac80211: Fix deadlock in ieee80211_do_stop.
From: Tejun Heo @ 2010-12-09 14:34 UTC (permalink / raw)
To: Ben Greear; +Cc: Luis R. Rodriguez, Johannes Berg, linux-wireless
In-Reply-To: <4CFFCE47.8040305@candelatech.com>
Hello,
Sorry about the delay.
On 12/08/2010 07:28 PM, Ben Greear wrote:
>> And here's a log with lockdep enabled:
>>
>> http://www.candelatech.com/~greearb/minicom_ath9k_log2.txt
>>
>> The sysrq output starts at line 1346 in this file.
>>
>> Seems I have a decent environment for reproducing this today,
>> in case you have any debug you'd like me to add.
ip is trying to flush sdata->work while holding rtnl_lock.
ip D 0000001c 0 14687 14600 0x00000080
ddcd99c0 00000046 7845a1f1 0000001c 00000000 ddcd9a84 8bc77ee2 0000001c
78a04380 f4223ea0 78a04380 f422411c f4224118 f4224118 78a04380 78a04380
005aba36 00000000 f3fd4c80 0000001c f4223ea0 f4223e02 0043e2ba 00000000
Call Trace:
[<7878cfaa>] schedule_timeout+0x16/0x9f
[<7878ce7f>] wait_for_common+0xbb/0x101
[<7878cf48>] wait_for_completion+0x12/0x14
[<78447ce1>] flush_work+0x23/0x27
[<f91a2646>] ieee80211_do_stop+0x25c/0x403 [mac80211]
[<f91a27ff>] ieee80211_stop+0x12/0x16 [mac80211]
[<786f6199>] __dev_close+0x73/0x88
[<786f3e96>] __dev_change_flags+0xa5/0x11a
[<786f6044>] dev_change_flags+0x13/0x3f
[<78700827>] do_setlink+0x23a/0x525
[<78700e55>] rtnl_newlink+0x283/0x45a
[<7870038d>] rtnetlink_rcv_msg+0x188/0x19e
[<7870e614>] netlink_rcv_skb+0x30/0x77
[<787001fe>] rtnetlink_rcv+0x1b/0x22 <- rtnl lock
[<7870e433>] netlink_unicast+0xbe/0x11a
[<7870f004>] netlink_sendmsg+0x23e/0x255
[<786e6304>] __sock_sendmsg+0x54/0x5b
[<786e67ce>] sock_sendmsg+0x95/0xac
[<786e6bf7>] sys_sendmsg+0x14d/0x19a
[<786e7f76>] sys_socketcall+0x227/0x289
[<784030dc>] sysenter_do_call+0x12/0x38
kworker/u:3 R running 0 43 2 0x00000000
f3ad9e8c 00000046 f8b4e008 00000000 78b6dbec f3ad9e1c 31e6ae69 00000024
78a04380 f39e3430 78a04380 f39e36ac f39e36a8 f39e36ac 78a04380 78a04380
000f5552 00000000 df4ce780 00000024 f39e3430 00000046 00000000 78bcc5fc
Call Trace:
[<7878cdab>] _cond_resched+0x2b/0x44
[<7878d84f>] mutex_lock_nested+0x22/0x3b
[<f919fddc>] ieee80211_sta_rx_queued_mgmt+0x2d/0x3a6 [mac80211]
[<f91a2f53>] ieee80211_iface_work+0x1ff/0x282 [mac80211]
[<78446fd4>] process_one_work+0x1af/0x2bf
[<78448722>] worker_thread+0xf9/0x1bf
[<7844b252>] kthread+0x62/0x67
[<784036c6>] kernel_thread_helper+0x6/0x1a
But, sdata->work is busy running ieee80211_iface_work(). I _suspect_
for some reason iee80211_iface_work() isn't finishing. That, or, the
new flush_work() implementation is broken and it's failing to flush
when a work is being executed back to back. I'll prep a debug patch
to determine what's going on.
The rest of the system going down the toilet after this is mostly
caused by the held rtnl_lock above.
> And one more thing: It seems it doesn't always block forever.
> The system in that last trace actually recovered after a
> minute or two, though it periodically enters the blocked
> state again.
And as this is not a deadlock but more of a livelock, yeah, it's quite
possible that it resolves itself in time.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 2/4] b43: set TMS to work with current band width for N-PHY
From: Michael Büsch @ 2010-12-09 14:28 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
In-Reply-To: <AANLkTinJUsKzNO6vO8F9KupxF0uc1h97wffhAgBas1Pe@mail.gmail.com>
On Thu, 2010-12-09 at 15:23 +0100, Rafał Miłecki wrote:
> W dniu 9 grudnia 2010 15:19 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
> > On Thu, 2010-12-09 at 15:13 +0100, Rafał Miłecki wrote:
> >> W dniu 9 grudnia 2010 15:12 użytkownik Rafał Miłecki <zajec5@gmail.com> napisał:
> >> > W dniu 7 grudnia 2010 22:08 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
> >> >> On Tue, 2010-12-07 at 21:55 +0100, Rafał Miłecki wrote:
> >> >>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> >> >>> ---
> >> >>> drivers/net/wireless/b43/main.c | 6 ++++++
> >> >>> drivers/net/wireless/b43/phy_common.c | 7 +++++++
> >> >>> drivers/net/wireless/b43/phy_common.h | 2 ++
> >> >>> drivers/net/wireless/b43/phy_n.c | 7 -------
> >> >>> 4 files changed, 15 insertions(+), 7 deletions(-)
> >> >>>
> >> >>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
> >> >>> index fa48803..670fd7b 100644
> >> >>> --- a/drivers/net/wireless/b43/main.c
> >> >>> +++ b/drivers/net/wireless/b43/main.c
> >> >>> @@ -1150,6 +1150,12 @@ void b43_wireless_core_reset(struct b43_wldev *dev, u32 flags)
> >> >>>
> >> >>> flags |= B43_TMSLOW_PHYCLKEN;
> >> >>> flags |= B43_TMSLOW_PHYRESET;
> >> >>> + if (dev->phy.type == B43_PHYTYPE_N) {
> >> >>> + if (b43_channel_type_is_40mhz(dev->phy.channel_type))
> >> >>
> >> >> Is channel_type already set at this time?
> >> >
> >> > Yeah, that core switching seems to be a little tricky. I didn't figure
> >> > it out completely yet. We may need to fix that, or fix calls to this
> >> > function.
> >> >
> >> > Thanks for reviewing!
> >>
> >> In other words: it will work for now, but we need to fix that in
> >> future if we want to get 40 MHz channels working.
> >
> > That's exactly what I was saying. ;)
> > To answer my original question: No, channel_type is not set, but the
> > code will work for non-40mhz.
> >
> > This seems a bit tricky to fix, though. Maybe we should reset the
> > wireless core twice. Once initially and once again after the
> > configuration was set. The first reset would just be to be able
> > to access the PHY and wireless core at all to probe it.
>
> Even more tricky is that we already reset code twice... At least in
> b43_wireless_core_attach. Will have to redesign that a little.
I don't think so.
We will have to live with at least two resets.
The first is to make the device accessible, so that we can fetch
the device type and revision numbers (phy type, rev, radio type,....)
After that is done, the device is shut down completely and reinitialized
when the interface comes up with the correct parameters.
It is important that no device setup is done whatsoever at attach stage.
Doing so would also break suspend/resume.
I do not think a redesign of the init code is required.
--
Greetings Michael.
^ permalink raw reply
* Re: [PATCH 2/4] b43: set TMS to work with current band width for N-PHY
From: Rafał Miłecki @ 2010-12-09 14:23 UTC (permalink / raw)
To: Michael Büsch; +Cc: linux-wireless, John W. Linville, b43-dev
In-Reply-To: <1291904384.2905.2.camel@maggie>
W dniu 9 grudnia 2010 15:19 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
> On Thu, 2010-12-09 at 15:13 +0100, Rafał Miłecki wrote:
>> W dniu 9 grudnia 2010 15:12 użytkownik Rafał Miłecki <zajec5@gmail.com> napisał:
>> > W dniu 7 grudnia 2010 22:08 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
>> >> On Tue, 2010-12-07 at 21:55 +0100, Rafał Miłecki wrote:
>> >>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> >>> ---
>> >>> drivers/net/wireless/b43/main.c | 6 ++++++
>> >>> drivers/net/wireless/b43/phy_common.c | 7 +++++++
>> >>> drivers/net/wireless/b43/phy_common.h | 2 ++
>> >>> drivers/net/wireless/b43/phy_n.c | 7 -------
>> >>> 4 files changed, 15 insertions(+), 7 deletions(-)
>> >>>
>> >>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
>> >>> index fa48803..670fd7b 100644
>> >>> --- a/drivers/net/wireless/b43/main.c
>> >>> +++ b/drivers/net/wireless/b43/main.c
>> >>> @@ -1150,6 +1150,12 @@ void b43_wireless_core_reset(struct b43_wldev *dev, u32 flags)
>> >>>
>> >>> flags |= B43_TMSLOW_PHYCLKEN;
>> >>> flags |= B43_TMSLOW_PHYRESET;
>> >>> + if (dev->phy.type == B43_PHYTYPE_N) {
>> >>> + if (b43_channel_type_is_40mhz(dev->phy.channel_type))
>> >>
>> >> Is channel_type already set at this time?
>> >
>> > Yeah, that core switching seems to be a little tricky. I didn't figure
>> > it out completely yet. We may need to fix that, or fix calls to this
>> > function.
>> >
>> > Thanks for reviewing!
>>
>> In other words: it will work for now, but we need to fix that in
>> future if we want to get 40 MHz channels working.
>
> That's exactly what I was saying. ;)
> To answer my original question: No, channel_type is not set, but the
> code will work for non-40mhz.
>
> This seems a bit tricky to fix, though. Maybe we should reset the
> wireless core twice. Once initially and once again after the
> configuration was set. The first reset would just be to be able
> to access the PHY and wireless core at all to probe it.
Even more tricky is that we already reset code twice... At least in
b43_wireless_core_attach. Will have to redesign that a little.
--
Rafał
^ permalink raw reply
* Re: [PATCH 2/4] b43: set TMS to work with current band width for N-PHY
From: Michael Büsch @ 2010-12-09 14:19 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
In-Reply-To: <AANLkTin14p=3=SA_4oVj24z+8NuS_y6hWqQmEEiXaSqr@mail.gmail.com>
On Thu, 2010-12-09 at 15:13 +0100, Rafał Miłecki wrote:
> W dniu 9 grudnia 2010 15:12 użytkownik Rafał Miłecki <zajec5@gmail.com> napisał:
> > W dniu 7 grudnia 2010 22:08 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
> >> On Tue, 2010-12-07 at 21:55 +0100, Rafał Miłecki wrote:
> >>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> >>> ---
> >>> drivers/net/wireless/b43/main.c | 6 ++++++
> >>> drivers/net/wireless/b43/phy_common.c | 7 +++++++
> >>> drivers/net/wireless/b43/phy_common.h | 2 ++
> >>> drivers/net/wireless/b43/phy_n.c | 7 -------
> >>> 4 files changed, 15 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
> >>> index fa48803..670fd7b 100644
> >>> --- a/drivers/net/wireless/b43/main.c
> >>> +++ b/drivers/net/wireless/b43/main.c
> >>> @@ -1150,6 +1150,12 @@ void b43_wireless_core_reset(struct b43_wldev *dev, u32 flags)
> >>>
> >>> flags |= B43_TMSLOW_PHYCLKEN;
> >>> flags |= B43_TMSLOW_PHYRESET;
> >>> + if (dev->phy.type == B43_PHYTYPE_N) {
> >>> + if (b43_channel_type_is_40mhz(dev->phy.channel_type))
> >>
> >> Is channel_type already set at this time?
> >
> > Yeah, that core switching seems to be a little tricky. I didn't figure
> > it out completely yet. We may need to fix that, or fix calls to this
> > function.
> >
> > Thanks for reviewing!
>
> In other words: it will work for now, but we need to fix that in
> future if we want to get 40 MHz channels working.
That's exactly what I was saying. ;)
To answer my original question: No, channel_type is not set, but the
code will work for non-40mhz.
This seems a bit tricky to fix, though. Maybe we should reset the
wireless core twice. Once initially and once again after the
configuration was set. The first reset would just be to be able
to access the PHY and wireless core at all to probe it.
--
Greetings Michael.
^ permalink raw reply
* Re: [PATCH] ath: fix build break with ATH_DBG_WARN_ON_ONCE
From: Johannes Berg @ 2010-12-09 14:17 UTC (permalink / raw)
To: John W. Linville
Cc: linux-wireless, Hauke Mehrtens, Luis R. Rodriguez, Ben Greear
In-Reply-To: <1291903927-5406-1-git-send-email-linville@tuxdriver.com>
On Thu, 2010-12-09 at 09:12 -0500, John W. Linville wrote:
> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
> index 3eb95e2..45d4d91 100644
> --- a/drivers/net/wireless/ath/ath.h
> +++ b/drivers/net/wireless/ath/ath.h
> @@ -272,7 +272,7 @@ ath_dbg(struct ath_common *common, enum ATH_DEBUG dbg_mask,
> return 0;
> }
> #define ATH_DBG_WARN(foo, arg...) do {} while (0)
> -#define ATH_DBG_WARN_ON_ONCE(foo) do {} while (0)
> +#define ATH_DBG_WARN_ON_ONCE(foo) ({ 0; })
That'll fix the build error, but wouldn't you need (foo) for correctness
instead?
johannes
^ permalink raw reply
* [PATCH] ath: fix build break with ATH_DBG_WARN_ON_ONCE
From: John W. Linville @ 2010-12-09 14:12 UTC (permalink / raw)
To: linux-wireless
Cc: Hauke Mehrtens, Luis R. Rodriguez, Ben Greear, John W. Linville
In-Reply-To: <4D00DDEA.7040907@hauke-m.de>
Description by Hauke:
"If CONFIG_ATH_DEBUG=y is set ATH_DBG_WARN_ON_ONCE uses WARN_ON_ONCE and
returns something, but if CONFIG_ATH_DEBUG is not set it does not return
anything. Now ATH_DBG_WARN_ON_ONCE is used in the boolean expression in
an if case and is not returning anything and causes a compile error.
CC [M] /drivers/net/wireless/ath/ath9k/main.o
/drivers/net/wireless/ath/ath9k/main.c: In function ‘ath_isr’:
/drivers/net/wireless/ath/ath9k/main.c:769: error: expected expression
before ‘do’
make[5]: *** [/drivers/net/wireless/ath/ath9k/main.o] Error 1
make[4]: *** [/drivers/net/wireless/ath/ath9k] Error 2"
Reported-by: Hauke Mehrtens <hauke@hauke-m.de>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
drivers/net/wireless/ath/ath.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 3eb95e2..45d4d91 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -272,7 +272,7 @@ ath_dbg(struct ath_common *common, enum ATH_DEBUG dbg_mask,
return 0;
}
#define ATH_DBG_WARN(foo, arg...) do {} while (0)
-#define ATH_DBG_WARN_ON_ONCE(foo) do {} while (0)
+#define ATH_DBG_WARN_ON_ONCE(foo) ({ 0; })
#endif /* CONFIG_ATH_DEBUG */
--
1.7.3.2
^ permalink raw reply related
* Re: [PATCH 2/4] b43: set TMS to work with current band width for N-PHY
From: Rafał Miłecki @ 2010-12-09 14:13 UTC (permalink / raw)
To: Michael Büsch; +Cc: linux-wireless, John W. Linville, b43-dev
In-Reply-To: <AANLkTinN_12RwWiugzxn5dB1he3b6h_y7K3C=8vFPyXO@mail.gmail.com>
W dniu 9 grudnia 2010 15:12 użytkownik Rafał Miłecki <zajec5@gmail.com> napisał:
> W dniu 7 grudnia 2010 22:08 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
>> On Tue, 2010-12-07 at 21:55 +0100, Rafał Miłecki wrote:
>>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>>> ---
>>> drivers/net/wireless/b43/main.c | 6 ++++++
>>> drivers/net/wireless/b43/phy_common.c | 7 +++++++
>>> drivers/net/wireless/b43/phy_common.h | 2 ++
>>> drivers/net/wireless/b43/phy_n.c | 7 -------
>>> 4 files changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
>>> index fa48803..670fd7b 100644
>>> --- a/drivers/net/wireless/b43/main.c
>>> +++ b/drivers/net/wireless/b43/main.c
>>> @@ -1150,6 +1150,12 @@ void b43_wireless_core_reset(struct b43_wldev *dev, u32 flags)
>>>
>>> flags |= B43_TMSLOW_PHYCLKEN;
>>> flags |= B43_TMSLOW_PHYRESET;
>>> + if (dev->phy.type == B43_PHYTYPE_N) {
>>> + if (b43_channel_type_is_40mhz(dev->phy.channel_type))
>>
>> Is channel_type already set at this time?
>
> Yeah, that core switching seems to be a little tricky. I didn't figure
> it out completely yet. We may need to fix that, or fix calls to this
> function.
>
> Thanks for reviewing!
In other words: it will work for now, but we need to fix that in
future if we want to get 40 MHz channels working.
--
Rafał
^ permalink raw reply
* Re: [PATCH 2/4] b43: set TMS to work with current band width for N-PHY
From: Rafał Miłecki @ 2010-12-09 14:12 UTC (permalink / raw)
To: Michael Büsch; +Cc: linux-wireless, John W. Linville, b43-dev
In-Reply-To: <1291756117.27497.101.camel@maggie>
W dniu 7 grudnia 2010 22:08 użytkownik Michael Büsch <mb@bu3sch.de> napisał:
> On Tue, 2010-12-07 at 21:55 +0100, Rafał Miłecki wrote:
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>> drivers/net/wireless/b43/main.c | 6 ++++++
>> drivers/net/wireless/b43/phy_common.c | 7 +++++++
>> drivers/net/wireless/b43/phy_common.h | 2 ++
>> drivers/net/wireless/b43/phy_n.c | 7 -------
>> 4 files changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
>> index fa48803..670fd7b 100644
>> --- a/drivers/net/wireless/b43/main.c
>> +++ b/drivers/net/wireless/b43/main.c
>> @@ -1150,6 +1150,12 @@ void b43_wireless_core_reset(struct b43_wldev *dev, u32 flags)
>>
>> flags |= B43_TMSLOW_PHYCLKEN;
>> flags |= B43_TMSLOW_PHYRESET;
>> + if (dev->phy.type == B43_PHYTYPE_N) {
>> + if (b43_channel_type_is_40mhz(dev->phy.channel_type))
>
> Is channel_type already set at this time?
Yeah, that core switching seems to be a little tricky. I didn't figure
it out completely yet. We may need to fix that, or fix calls to this
function.
Thanks for reviewing!
--
Rafał
^ permalink raw reply
* Re: [PATCH 0/4] Make N-PHY support experimental
From: Rafał Miłecki @ 2010-12-09 14:11 UTC (permalink / raw)
To: Larry Finger
Cc: Gábor Stefanik, linux-wireless, John W. Linville, b43-dev
In-Reply-To: <4CFEFD8D.1080808@lwfinger.net>
2010/12/8 Larry Finger <Larry.Finger@lwfinger.net>:
> I have a bit more info regarding the problem with WPA2 encryption on an N PHY.
> As shown in the NetworkManager log attached, Authentication is successful, but
> DHCP fails. Either the transmit data is improperly encrypted, or the decryption
> fails. I will use wireshark to sort that out. Note: Using software encryption
> makes no difference.
Hm, I expected software encryption to work, that's weird. I don't
really have nice access to my development machine, so some tests would
be great.
--
Rafał
^ permalink raw reply
* Re: [PATCH 0/4] Make N-PHY support experimental
From: Rafał Miłecki @ 2010-12-09 14:09 UTC (permalink / raw)
To: Gábor Stefanik; +Cc: linux-wireless, John W. Linville, b43-dev
In-Reply-To: <AANLkTikp9ErFopLZVmXMjN3tXF2L62SwEEcujKeOBLYC@mail.gmail.com>
W dniu 7 grudnia 2010 23:57 użytkownik Gábor Stefanik
<netrolller.3d@gmail.com> napisał:
> 2010/12/7 Rafał Miłecki <zajec5@gmail.com>:
>> Finally, thanks for Larry and Francesco I got b43 working with BCM4328 chipset!
>
> Do you mean BCM4321 (14e4:4328)? BCM4328 is AFAIK handled by brcmfmac.
Yeah, I removed following hack some time ago:
- } else if (bus->chip_id == 0x4321) {
- /* the BCM4328 has a chipid == 0x4321 and a rev 4 SPROM */
and got lost what is my chipset. You're right.
--
Rafał
^ permalink raw reply
* Re: [PATCH] cfg80211: update information elements in cached BSS struct
From: Johannes Berg @ 2010-12-09 14:07 UTC (permalink / raw)
To: Sven Neumann; +Cc: linux-wireless
In-Reply-To: <1291903524-8000-1-git-send-email-s.neumann@raumfeld.com>
On Thu, 2010-12-09 at 15:05 +0100, Sven Neumann wrote:
> When a cached BSS struct is updated because a new beacon was received,
> the code replaces the cached information elements by the IEs from the
> new beacon. However it did not update the pub.information_elements
> and pub.len_information_elements fields leaving them either pointing
> to the old beacon IEs or in an inconsistent state where the data is
> replaced by the new beacon IEs but len_information_elements still has
> its value from the first beacon.
>
> Fix this by updating the information elements fields if they are
> pointing to beacon IEs.
>
> Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
Reviewed-by: Johannes Berg <johannes@sipsolutions.net>
> ---
> net/wireless/scan.c | 11 +++++++++++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/net/wireless/scan.c b/net/wireless/scan.c
> index 503ebb8..ea427f4 100644
> --- a/net/wireless/scan.c
> +++ b/net/wireless/scan.c
> @@ -464,6 +464,9 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
> if (res->pub.beacon_ies) {
> size_t used = dev->wiphy.bss_priv_size + sizeof(*res);
> size_t ielen = res->pub.len_beacon_ies;
> + bool information_elements_is_beacon_ies =
> + (found->pub.information_elements ==
> + found->pub.beacon_ies);
>
> if (found->pub.beacon_ies &&
> !found->beacon_ies_allocated &&
> @@ -487,6 +490,14 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
> found->pub.len_beacon_ies = ielen;
> }
> }
> +
> + /* Override IEs if they were from a beacon before */
> + if (information_elements_is_beacon_ies) {
> + found->pub.information_elements =
> + found->pub.beacon_ies;
> + found->pub.len_information_elements =
> + found->pub.len_beacon_ies;
> + }
> }
>
> kref_put(&res->ref, bss_release);
^ permalink raw reply
* Re: [PATCH 1/4] b43: N-PHY: update init tables
From: Rafał Miłecki @ 2010-12-09 14:06 UTC (permalink / raw)
To: ikorot; +Cc: Larry Finger, Hauke Mehrtens, linux-wireless, b43-dev
In-Reply-To: <1127342.1291773334294.JavaMail.root@elwamui-rubis.atl.sa.earthlink.net>
2010/12/8 <ikorot@earthlink.net>:
> Larry,
>
>
> -----Original Message-----
>>From: Larry Finger <Larry.Finger@lwfinger.net>
>>Sent: Dec 7, 2010 12:00 PM
>>To: ikorot@earthlink.net
>>Cc: Hauke Mehrtens <hauke@hauke-m.de>, linux-wireless@vger.kernel.org, b43-dev@lists.infradead.org
>>Subject: Re: [PATCH 1/4] b43: N-PHY: update init tables
>>
>>On 12/07/2010 01:49 PM, ikorot@earthlink.net wrote:
>>>> One reason is that these tables changed from non-zero to zero values between
>>>> Broadcom driver 4.174.64.19 and 5.10.56.46. As they might change again, I think
>>>> we should retain the full version.
>>>
>>> Is there a way to check for version of the driver?
>>> This way whoever uses old one won't be screwed...
>>
>>That will not be a problem as no older version of b43 works at all with b43. To
>>help you understand this change, when the 4.174.64.19 Broadcom driver was
>>reverse-engineered, the tables had non-zero values. Rafel later compared the
>>trace dumps of b43 with the latest version of wl and found that the values are
>>now zero. When I rechecked driver 5.10.56.46, I found them to be zero there as
>>well and changed the specs.
>
> Well, it maybe feasible to keep the old values and check
> the version of the driver, since you are saying yourself
> that the value might be changing in the future.
> Maybe for couple of releases only?
Sorry, but I don't understand that at all. What do you mean by version
of the driver? What driver?
b43 comes with tables built in, there is no way b43 could use tables
from other version of b43...
--
Rafał
^ permalink raw reply
* [PATCH] cfg80211: update information elements in cached BSS struct
From: Sven Neumann @ 2010-12-09 14:05 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg, Sven Neumann
In-Reply-To: <1291902830.3540.8.camel@jlt3.sipsolutions.net>
When a cached BSS struct is updated because a new beacon was received,
the code replaces the cached information elements by the IEs from the
new beacon. However it did not update the pub.information_elements
and pub.len_information_elements fields leaving them either pointing
to the old beacon IEs or in an inconsistent state where the data is
replaced by the new beacon IEs but len_information_elements still has
its value from the first beacon.
Fix this by updating the information elements fields if they are
pointing to beacon IEs.
Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
---
net/wireless/scan.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 503ebb8..ea427f4 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -464,6 +464,9 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
if (res->pub.beacon_ies) {
size_t used = dev->wiphy.bss_priv_size + sizeof(*res);
size_t ielen = res->pub.len_beacon_ies;
+ bool information_elements_is_beacon_ies =
+ (found->pub.information_elements ==
+ found->pub.beacon_ies);
if (found->pub.beacon_ies &&
!found->beacon_ies_allocated &&
@@ -487,6 +490,14 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
found->pub.len_beacon_ies = ielen;
}
}
+
+ /* Override IEs if they were from a beacon before */
+ if (information_elements_is_beacon_ies) {
+ found->pub.information_elements =
+ found->pub.beacon_ies;
+ found->pub.len_information_elements =
+ found->pub.len_beacon_ies;
+ }
}
kref_put(&res->ref, bss_release);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] cfg80211: update information elements in cached BSS struct
From: Johannes Berg @ 2010-12-09 13:53 UTC (permalink / raw)
To: Sven Neumann; +Cc: linux-wireless
In-Reply-To: <1291900640.1924.33.camel@sven>
On Thu, 2010-12-09 at 14:17 +0100, Sven Neumann wrote:
[snip
> Please review this patch again. I am pretty sure that there's something
> broken. Perhaps my fix is not totally correct, but it fixes a
> reproducable association problem for us.
You're right, the if catches that.
Can you please make the variable a bool, remove the blank line before
it? And your line length was way above 80. A comment would also be good
about why this is done, maybe a one-liner like the probe resp one, just
saying "Override IEs if they were from a beacon before".
Thanks!
johannes
^ permalink raw reply
* Compile problem in ath9k with ATH_DBG_WARN_ON_ONCE
From: Hauke Mehrtens @ 2010-12-09 13:47 UTC (permalink / raw)
To: Luis R. Rodriguez, Ben Greear; +Cc: linux-wireless@vger.kernel.org
Hi,
I got an compile problem in drivers/net/wireless/ath/ath9k/main.c
This is caused by commit ff9f0b639f33c92a89b0799263ab625444be6ee1
ath9k: skip ATH9K_INT_TIM_TIMER when we are idle
and d7fd1b50a51be3fe6554fbab8953fa8a3ff4009b
ath9k: Make DMA warning in ath_stoprecv WARN_ON_ONCE.
If CONFIG_ATH_DEBUG=y is set ATH_DBG_WARN_ON_ONCE uses WARN_ON_ONCE and
returns something, but if CONFIG_ATH_DEBUG is not set it does not return
anything. Now ATH_DBG_WARN_ON_ONCE is used in the boolean expression in
an if case and is not returning anything and causes a compile error.
CC [M] /drivers/net/wireless/ath/ath9k/main.o
/drivers/net/wireless/ath/ath9k/main.c: In function ‘ath_isr’:
/drivers/net/wireless/ath/ath9k/main.c:769: error: expected expression
before ‘do’
make[5]: *** [/drivers/net/wireless/ath/ath9k/main.o] Error 1
make[4]: *** [/drivers/net/wireless/ath/ath9k] Error 2
Hauke
^ permalink raw reply
* Re: [PATCH] cfg80211: update information elements in cached BSS struct
From: Sven Neumann @ 2010-12-09 13:17 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <1291898876.3540.4.camel@jlt3.sipsolutions.net>
Hi,
On Thu, 2010-12-09 at 13:47 +0100, Johannes Berg wrote:
> On Thu, 2010-12-09 at 11:48 +0100, Sven Neumann wrote:
> > When a cached BSS struct is updated because a new beacon was received,
> > the code replaces the cached information elements by the IEs from the
> > new beacon. However it did not update the pub.information_elements
> > and pub.len_information_elements fields leaving them either pointing
> > to the old beacon IEs or in an inconsistent state where the data is
> > replaced by the new beacon IEs but len_information_elements still has
> > its value from the first beacon.
> >
> > Fix this by updating the information elements fields if they are
> > pointing to beacon IEs.
>
> Err, no, this is intentional -- we want data from probe responses to
> "win" and be displayed preferentially. If you look at the code that
> dumps them out, that's what will happen.
As far as I can see my patch does not break this behavior. The
information elements fields are only updated if they are currently
pointing to beacon IEs. If there's ever a probe response frame, then it
will win and the information elements fields won't be changed for
beacons arriving later.
> If you need the most up-to-date beacon IEs, use (the equivalent of) "iw
> wlan0 scan dump -b".
What my patch is fixing are the following situations (and yes, this
happened in real life, it is reproducable and it is reproducably fixed
by the patch that I've submitted):
AP sends a beacon without RSN IE. This is the first beacon received
and the station with IEs is added to the BSS cache.
pub.information_elements now points to pub.beacon_ies and
pub.len_information_elements has the value of pub.len_beacon_ies.
AP sends another beacon, this time it includes RSN IE. As there's
not enough place for the larger IEs, memory is allocated for it
and the pub.beacon_ies pointer is changed to point to this newly
allocated memory. Now pub.information_elements still points to
the old value.
User-space asks for scan results and will receive the IEs from the
first beacon because even though the data from subsequent beacons
has been added to the cache, pub.information_elements still points
to the old data.
As a result user-space looking for a WPA enabled access-point will
skip the AP and never associate with it. This situation will only
ever fix itself if you are lucky enough that so few beacons are
arriving that the cached RSS expires at some point.
And there are even worse scenarios:
If the second beacon is smaller than the first or only slightly larger
(so that it fits into the allocated memory which has some padding),
then no new memory will be allocated for the IEs. Instead the data
in pub.beacon_ies will be changed and pub.len_beacon_ies will be
updated. Now pub.information_elements points to the new data
but pub.len_information_elements still holds the old value. Data
and length have become inconsistent. As a result invalid IEs are
returned to user-space when it asks for scan results.
As you can see this is all without any probe response frames. Our driver
(libertas) doesn't do any active scanning and thus there are no probe
response frames received. I still tried to write the patch in a way that
should not break things if probe response frames are received.
Please review this patch again. I am pretty sure that there's something
broken. Perhaps my fix is not totally correct, but it fixes a
reproducable association problem for us.
Regards,
Sven
--
Sven Neumann
Head of Software Development
RAUMFELD GmbH | Reichenberger Str. 124 | 10999 Berlin | Germany
Tel: +49.30.340.60.98.0 | Fax: +49.30.340.60.98.99 | s.neumann@raumfeld.com
^ permalink raw reply
* Re: ath5k: Weird Retransmission Behaviour
From: Bob Copeland @ 2010-12-09 12:38 UTC (permalink / raw)
To: Helmut Schaa
Cc: Jonathan Guerin, Bruno Randolf, linux-wireless, ath5k-devel, nbd
In-Reply-To: <AANLkTim5hESSHAMYNryUwak=D4tQetZJuDEK41Yq3A+L@mail.gmail.com>
On Thu, Dec 09, 2010 at 10:21:34AM +0100, Helmut Schaa wrote:
> On Wed, Dec 8, 2010 at 10:53 PM, Jonathan Guerin <jonathan@guerin.id.au> wrote:
> > I only seem to have Minstrel as the only available Rate Control
> > algorithm in my kernel config?
>
> PID is only selectable on embedded platforms:
>
> config MAC80211_RC_PID
> bool "PID controller based rate control algorithm" if EMBEDDED
>
> Just remove the "if EMBEDDED" from net/mac8011/Kconfig and retry.
For what it's worth, I tested pid and minstrel a while ago with a
modified mac80211_hwsim, and found minstrel to be quite a bit better
at rate adaptation. So while it may be worth testing out for this
particular use case, minstrel is probably the way to go in general.
--
Bob Copeland %% www.bobcopeland.com
^ permalink raw reply
* Re: [PATCH] cfg80211: update information elements in cached BSS struct
From: Johannes Berg @ 2010-12-09 12:47 UTC (permalink / raw)
To: Sven Neumann; +Cc: linux-wireless
In-Reply-To: <1291891690-26118-1-git-send-email-s.neumann@raumfeld.com>
On Thu, 2010-12-09 at 11:48 +0100, Sven Neumann wrote:
> When a cached BSS struct is updated because a new beacon was received,
> the code replaces the cached information elements by the IEs from the
> new beacon. However it did not update the pub.information_elements
> and pub.len_information_elements fields leaving them either pointing
> to the old beacon IEs or in an inconsistent state where the data is
> replaced by the new beacon IEs but len_information_elements still has
> its value from the first beacon.
>
> Fix this by updating the information elements fields if they are
> pointing to beacon IEs.
Err, no, this is intentional -- we want data from probe responses to
"win" and be displayed preferentially. If you look at the code that
dumps them out, that's what will happen.
If you need the most up-to-date beacon IEs, use (the equivalent of) "iw
wlan0 scan dump -b".
johannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox