Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: Fwd: [PATCH 06/10] mac80211: add HW flag for disabling auto link-PS in AP mode
From: Arik Nemtsov @ 2011-01-17 22:47 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Luciano Coelho
In-Reply-To: <1295256929.3726.2.camel@jlt3.sipsolutions.net>

On Mon, Jan 17, 2011 at 11:35, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Sun, 2011-01-16 at 23:53 +0200, Arik Nemtsov wrote:
>
> > > Also, I'm worried about races between this and RX. All of the RX path
> > > assumes that it is running at the same time. The ap_sta_ps_{start,end}
> > > functions won't be called from the RX path when the flag is set, and
> > > this is dependent on setting the flag, but is there really nothing in
> > > them that could race?
> >
> > They might race if the user is not careful. On a SMP machine a user
> > can call ieee80211_rx() and ap_sta_ps_start() from two different
> > workqueues for example.
> > I can add an explicit mutex, but a documentation warning will suffice here no?
>
> Well, a mutex won't be possible, and we don't do "code path locking"

I meant a spinlock of course (but its not really a good idea)

>
> anyway. I'd appreciate if you could actually see which things would
> potentially race -- I took a brief look into these functions and it
> didn't look like there are actually races except of these two against
> each other maybe?

Upon closer examination, I could find no races in the RX path. A race
of ps_start/stop is possible of course, but I guess we can demand the
calls to be synchronized against each other?

I did notice some concurrency that can happen in the TX path. I think
the same essential race is described in
ieee80211_handle_filtered_frame() between RX and TX paths. Only this
time the PS state is changed manually and not in the RX handler. The
warning in ieee80211_handle_filtered_frame() can be expanded to
include:
"always change PS state of connected stations before TX status events
if ordering can be unknown, for example with different interrupt
status bits."

> > sta_notify is pretty useless when the low level driver calls toggles
> > the PS-mode by itself. How about I simply remove the call to
> > sta_notify in case IEEE80211_HW_AP_LINK_PS is set?
> > If a low level driver needs to do some deferred processing after a
> > PS-mode transition, it can queue a work on its own..
>
> Yes, that's probably a good idea -- but definitely needs to be
> documented in the sta_notify docs. OTOH, mac80211 de-bounces sta_notify
> in a way. Maybe that needs to be done for the function call as well,
> maybe via a return value?
>

I'm afraid I didn't catch your meaning here.

Regards,
Arik

^ permalink raw reply

* Re: [PATCH 2] cfg80211: Extend channel to frequency mapping for 802.11j
From: Bruno Randolf @ 2011-01-18  1:06 UTC (permalink / raw)
  To: Brian Prodoehl; +Cc: johannes, linville, gwingerde, dcbw, linux-wireless
In-Reply-To: <AANLkTinx+wFDFnDrxrBS5T=RqvZgRLxFOeeibrLxorde@mail.gmail.com>

On Tue January 18 2011 05:17:31 Brian Prodoehl wrote:
> Yeah, looks good.  Is it worth noting that none of the drivers
> actually support the 802.11j channels yet?  

I am already using it locally in ath5k. I have it working in 4.9GHz, but i'm 
not exactly sure in which cases i can enable the 4.9GHz channels. I will send 
patches after the mac80211 part got merged.

> Not sure of the format of
> follow-up s-o-b's, so here goes...
> 
> Signed-off-by: Brian Prodoehl <bprodoehl@gmail.com>

Not sure, either, but i hope John will pick it up. Thanks!

bruno

^ permalink raw reply

* Re: [PATCH 2] cfg80211: Extend channel to frequency mapping for 802.11j
From: Brian Prodoehl @ 2011-01-18  2:42 UTC (permalink / raw)
  To: Bruno Randolf; +Cc: johannes, linville, gwingerde, dcbw, linux-wireless
In-Reply-To: <201101181006.17322.br1@einfach.org>

On Mon, Jan 17, 2011 at 8:06 PM, Bruno Randolf <br1@einfach.org> wrote:
> On Tue January 18 2011 05:17:31 Brian Prodoehl wrote:
>> Yeah, looks good.  Is it worth noting that none of the drivers
>> actually support the 802.11j channels yet?
>
> I am already using it locally in ath5k. I have it working in 4.9GHz, but i'm
> not exactly sure in which cases i can enable the 4.9GHz channels. I will send
> patches after the mac80211 part got merged.

I have it working in ath9k, as well.  iw needs the same tweak to
ieee80211_channel_to_frequency.  Where do patches to iw get submitted?

>> Not sure of the format of
>> follow-up s-o-b's, so here goes...
>>
>> Signed-off-by: Brian Prodoehl <bprodoehl@gmail.com>
>
> Not sure, either, but i hope John will pick it up. Thanks!
>
> bruno
>

^ permalink raw reply

* Re: [PATCH 2] cfg80211: Extend channel to frequency mapping for 802.11j
From: Bruno Randolf @ 2011-01-18  2:50 UTC (permalink / raw)
  To: Brian Prodoehl
  Cc: johannes, linville, gwingerde, dcbw, linux-wireless, mcgrof
In-Reply-To: <AANLkTi=SbkiZE0-5bJ+jraMhCMbvyaF=L0HOq5o0LtyJ@mail.gmail.com>

On Tue January 18 2011 11:42:49 Brian Prodoehl wrote:
> On Mon, Jan 17, 2011 at 8:06 PM, Bruno Randolf <br1@einfach.org> wrote:
> > On Tue January 18 2011 05:17:31 Brian Prodoehl wrote:
> >> Yeah, looks good.  Is it worth noting that none of the drivers
> >> actually support the 802.11j channels yet?
> > 
> > I am already using it locally in ath5k. I have it working in 4.9GHz, but
> > i'm not exactly sure in which cases i can enable the 4.9GHz channels. I
> > will send patches after the mac80211 part got merged.
> 
> I have it working in ath9k, as well.  

Good to hear. Do you have any information on when we can enable these 
frequencies? I'm not sure all cards are calibrated for 4.9GHz. Any info about 
this from Atheros, Luis?

> iw needs the same tweak to
> ieee80211_channel_to_frequency.  Where do patches to iw get submitted?

I was waiting with positng the iw part until the mac80211 patch is accepted 
and merged. It should also posted on this list and cc johannes.

bruno

^ permalink raw reply

* RE: [PATCH] mwifiex: remove duplicated NULL checks
From: Bing Zhao @ 2011-01-18  3:07 UTC (permalink / raw)
  To: Julian Calaby
  Cc: linux-wireless@vger.kernel.org, John W. Linville, Johannes Berg,
	Amitkumar Karwar, Kiran Divekar, Frank Huang
In-Reply-To: <AANLkTi=BMTLa36FXW7nNHKu8=PgPCJC517vsmDDNZcYj@mail.gmail.com>

Hi Julian,

> -----Original Message-----
> From: Julian Calaby [mailto:julian.calaby@gmail.com]
> Sent: Friday, January 14, 2011 6:59 PM
> To: Bing Zhao
> Cc: linux-wireless@vger.kernel.org; John W. Linville; Johannes Berg; Amitkumar Karwar; Kiran Divekar;
> Frank Huang
> Subject: Re: [PATCH] mwifiex: remove duplicated NULL checks
> 
> On Sat, Jan 15, 2011 at 06:36, Bing Zhao <bzhao@marvell.com> wrote:
> > From: Amitkumar Karwar <akarwar@marvell.com>
> >
> > Some functions have unnecessory NULL check for an input parameter
> > because the caller to those functions is already checking the parameter
> > for NULL before calling them.
> 
> It may be more readable and efficient to eliminate the NULL checks in
> the callers - and adjust the code to inform the callers of the case
> where the pointer passed is null and make the callers to properly
> handle this case.

Thank you very much for your comments.

There are still some cases that the NULL check needs to be done at the first place. For example,


/* the caller function */
enum mwifiex_status mwifiex_prepare_cmd(struct mwifiex_private *priv,
                u16 cmd_no, u16 cmd_action, u32 cmd_oid,
		    void *ioctl_buf, void *data_buf)

	......

      cmd_node = mwifiex_get_cmd_node(adapter);

      if (cmd_node == NULL) {
              PRINTM(MERROR, "PREP_CMD: No free cmd node\n");
              ret = MWIFIEX_STATUS_FAILURE;
              goto done;
      }
	/*
	  Here the cmd_node is allocated and the NULL check is preformed
        immediately. If the pointer is NULL then the error code is
	  returned for proper handling.
	*/


	/*
	  Now cmd_node is passed to the callee as a parameter
 	*/
      mwifiex_init_cmd_node(priv, cmd_node, cmd_oid, ioctl_buf,
                              data_buf);

	......
}

/* the callee function */
static void mwifiex_init_cmd_node(struct mwifiex_private *priv,
                      struct cmd_ctrl_node *cmd_node,
                      u32 cmd_oid, void *ioctl_buf, void *data_buf)
{
      ENTER();

-	if (cmd_node == NULL) {
-		LEAVE();
-		return;
-	}
-
	/*
	  Here it is redundant to check it again since the cmd_node
        has already been validated by the caller. 
	*/

      cmd_node->priv = priv;
      cmd_node->cmd_oid = cmd_oid;
      cmd_node->ioctl_buf = ioctl_buf;

	......
}

> 
> This should also make the driver more robust as failures can be passed
> up the stack to places that can deal with them more effectively than
> just printing a message.

I could have misunderstood your point, however. If so, could you please explain a little bit more on how you would make the changes based on above example?

Thanks in advance for your help!

Bing

> 
> Thanks,
> 
> --
> 
> Julian Calaby
> 
> Email: julian.calaby@gmail.com
> Profile: http://www.google.com/profiles/julian.calaby/
> .Plan: http://sites.google.com/site/juliancalaby/

^ permalink raw reply

* Re: [PATCH] mwifiex: remove duplicated NULL checks
From: Julian Calaby @ 2011-01-18  3:26 UTC (permalink / raw)
  To: Bing Zhao
  Cc: linux-wireless@vger.kernel.org, John W. Linville, Johannes Berg,
	Amitkumar Karwar, Kiran Divekar, Frank Huang
In-Reply-To: <477F20668A386D41ADCC57781B1F704307F4E1BA0D@SC-VEXCH1.marvell.com>

On Tue, Jan 18, 2011 at 14:07, Bing Zhao <bzhao@marvell.com> wrote:
> Hi Julian,
>
>> -----Original Message-----
>> From: Julian Calaby [mailto:julian.calaby@gmail.com]
>> Sent: Friday, January 14, 2011 6:59 PM
>> To: Bing Zhao
>> Cc: linux-wireless@vger.kernel.org; John W. Linville; Johannes Berg; Amitkumar Karwar; Kiran Divekar;
>> Frank Huang
>> Subject: Re: [PATCH] mwifiex: remove duplicated NULL checks
>>
>> On Sat, Jan 15, 2011 at 06:36, Bing Zhao <bzhao@marvell.com> wrote:
>> > From: Amitkumar Karwar <akarwar@marvell.com>
>> >
>> > Some functions have unnecessory NULL check for an input parameter
>> > because the caller to those functions is already checking the parameter
>> > for NULL before calling them.
>>
>> It may be more readable and efficient to eliminate the NULL checks in
>> the callers - and adjust the code to inform the callers of the case
>> where the pointer passed is null and make the callers to properly
>> handle this case.
>
> Thank you very much for your comments.
>
> There are still some cases that the NULL check needs to be done at the first place. For example,
>
[snip]

In this case, dropping the null check is the right thing to do, given
that you can guarantee that all callers of mwiflex_init_cmd_node()
will check that the pointer isn't null before calling it.

>> This should also make the driver more robust as failures can be passed
>> up the stack to places that can deal with them more effectively than
>> just printing a message.
>
> I could have misunderstood your point, however. If so, could you please explain a little bit more on how you would make the changes based on above example?

I was referring more to that fact that functions that appear to do
things, e.g. mwifiex_init_cmd_node(), don't return anything - hence if
something fails within them, e.g. being passed a null pointer, they
can't notify their caller, e.g. mwifiex_prepare_cmd() that something
has gone wrong.

That said, I haven't looked at the code except through your patches,
so I can't say if this is a scenario that actually could happen, and
whether this added functionality is worthwhile.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

^ permalink raw reply

* [PATCH] Kernel Panic on Associate with brcm80211.
From: Ilya Kogan @ 2011-01-18  5:15 UTC (permalink / raw)
  To: linux-wireless
In-Reply-To: <4D334D00.4030604@mythicnet.org>

I'm going to admit I'm fairly new to this but after some reading, I 
couldn't quite understand why there is an assertion ensuring that the 
sk_buff being sent is not cloned. I was going to see what exploded if I 
removed it and to my surprise...nothing did. In fact, except for a few

[12014.013409] wl0: wlc_d11hdrs_mac80211: AC_BE txop exceeded phylen 
382/256 dur 3562/1504

things seem to work fine. I'll be doing some more testing with this over 
the next couple of days. What obvious thing does this break horribly?

-- Ilya Kogan

  drivers/staging/brcm80211/sys/wlc_mac80211.c |    1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/brcm80211/sys/wlc_mac80211.c 
b/drivers/staging/brcm80211/sys/wlc_mac80211.c
index 1d5d01a..a130386 100644
--- a/drivers/staging/brcm80211/sys/wlc_mac80211.c
+++ b/drivers/staging/brcm80211/sys/wlc_mac80211.c
@@ -5126,7 +5126,6 @@ wlc_sendpkt_mac80211(struct wlc_info *wlc, struct 
sk_buff *sdu,
         fifo = prio2fifo[prio];

         ASSERT((uint) skb_headroom(sdu) >= TXOFF);
-       ASSERT(!(sdu->cloned));
         ASSERT(!(sdu->next));
         ASSERT(!(sdu->prev));
         ASSERT(fifo < NFIFO);



^ permalink raw reply related

* [PATCHv2] wl12xx: Increase scan channel dwell time for passive scans
From: juuso.oikarinen @ 2011-01-18  6:32 UTC (permalink / raw)
  To: coelho; +Cc: linux-wireless

From: Juuso Oikarinen <juuso.oikarinen@nokia.com>

The passive scan channel dwell time currently used is 30ms-60ms. A typical
beacon interval for AP's is 100ms. This leads to a ~30% worst-case probability
of finding an AP via passive scanning.

For 5GHz bands for DFS frequencies passive scanning is the only scanning
option. Hence for these, the probability of finding an AP is very low.

To fix this, increase the passive channel scan dwell times (also the early
leave value, as 5GHz channels are still typically very silent.) Use a value
of 100ms, because that covers most typical AP configurations.

Based on testing the probability of finding an AP (102.4ms beacon interval) on
a single scan round are as follows (based on 100 iterations):

dwell min/max (ms)   | probability
---------------------+------------
30/60                | 35%
60/60                | 56%
80/80                | 77%
100/100              | 100%

Total scan times now and after the change:

Region | Before (s) | After (s)
-------+------------+----------
00     | 0.77       | 1.48
FI     | 0.95       | 2.01
US     | 0.91       | 1.76

Signed-off-by: Juuso Oikarinen <juuso.oikarinen@nokia.com>
---
v2: Minor comment change "maximum" => "minimum"

 drivers/net/wireless/wl12xx/conf.h |   18 +++++++++---------
 drivers/net/wireless/wl12xx/main.c |    4 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/conf.h b/drivers/net/wireless/wl12xx/conf.h
index f5c048c..7aecbb5 100644
--- a/drivers/net/wireless/wl12xx/conf.h
+++ b/drivers/net/wireless/wl12xx/conf.h
@@ -1084,30 +1084,30 @@ struct conf_scan_settings {
 	/*
 	 * The minimum time to wait on each channel for active scans
 	 *
-	 * Range: 0 - 65536 tu
+	 * Range: u32 tu/1000
 	 */
-	u16 min_dwell_time_active;
+	u32 min_dwell_time_active;
 
 	/*
 	 * The maximum time to wait on each channel for active scans
 	 *
-	 * Range: 0 - 65536 tu
+	 * Range: u32 tu/1000
 	 */
-	u16 max_dwell_time_active;
+	u32 max_dwell_time_active;
 
 	/*
-	 * The maximum time to wait on each channel for passive scans
+	 * The minimum time to wait on each channel for passive scans
 	 *
-	 * Range: 0 - 65536 tu
+	 * Range: u32 tu/1000
 	 */
-	u16 min_dwell_time_passive;
+	u32 min_dwell_time_passive;
 
 	/*
 	 * The maximum time to wait on each channel for passive scans
 	 *
-	 * Range: 0 - 65536 tu
+	 * Range: u32 tu/1000
 	 */
-	u16 max_dwell_time_passive;
+	u32 max_dwell_time_passive;
 
 	/*
 	 * Number of probe requests to transmit on each active scan channel
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 863e660..c1814f2 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -279,8 +279,8 @@ static struct conf_drv_settings default_conf = {
 	.scan = {
 		.min_dwell_time_active        = 7500,
 		.max_dwell_time_active        = 30000,
-		.min_dwell_time_passive       = 30000,
-		.max_dwell_time_passive       = 60000,
+		.min_dwell_time_passive       = 100000,
+		.max_dwell_time_passive       = 100000,
 		.num_probe_reqs               = 2,
 	},
 	.rf = {
-- 
1.7.1


^ permalink raw reply related

* Re: Fwd: [PATCH 06/10] mac80211: add HW flag for disabling auto link-PS in AP mode
From: Johannes Berg @ 2011-01-18  9:05 UTC (permalink / raw)
  To: Arik Nemtsov; +Cc: linux-wireless, Luciano Coelho
In-Reply-To: <AANLkTiky+j6P9oibrWzDzfNugrADtb9p21TVn7WFAY6_@mail.gmail.com>

On Tue, 2011-01-18 at 00:47 +0200, Arik Nemtsov wrote:

> > anyway. I'd appreciate if you could actually see which things would
> > potentially race -- I took a brief look into these functions and it
> > didn't look like there are actually races except of these two against
> > each other maybe?
> 
> Upon closer examination, I could find no races in the RX path. A race
> of ps_start/stop is possible of course, but I guess we can demand the
> calls to be synchronized against each other?

Yeah that seems fair. It'd be a strange driver too anyway, come to think
of it, since this is all from the device so needs to be processed from a
tasklet or whatever anyway.

> I did notice some concurrency that can happen in the TX path. I think
> the same essential race is described in
> ieee80211_handle_filtered_frame() between RX and TX paths. Only this
> time the PS state is changed manually and not in the RX handler. The
> warning in ieee80211_handle_filtered_frame() can be expanded to
> include:
> "always change PS state of connected stations before TX status events
> if ordering can be unknown, for example with different interrupt
> status bits."

Right, that was RX first, then TX status, I guess it can be extended a
bit. But really, this race doesn't apply since the entire PS state
management is done by the device here, and mac80211 just follows it.

> > > sta_notify is pretty useless when the low level driver calls toggles
> > > the PS-mode by itself. How about I simply remove the call to
> > > sta_notify in case IEEE80211_HW_AP_LINK_PS is set?
> > > If a low level driver needs to do some deferred processing after a
> > > PS-mode transition, it can queue a work on its own..
> >
> > Yes, that's probably a good idea -- but definitely needs to be
> > documented in the sta_notify docs. OTOH, mac80211 de-bounces sta_notify
> > in a way. Maybe that needs to be done for the function call as well,
> > maybe via a return value?
> >
> 
> I'm afraid I didn't catch your meaning here.

Well if for instance the station happens to send two frames with the PM
bit set, meaning that it is in PS-poll mode but doesn't want to wake up
even though it's transmitting a frame, mac80211 will only call
sta_notify once for the first frame to tell the driver the station went
to sleep -- the second frame just indicates that it's still asleep.

The same might happen with this, but of course it depends on what the
device does. If the driver will call the new API function twice, then
mac80211 will be very confused. Therefore, I think the new API function
should do some checks about whether the station is already asleep/awake
before invoking ap_sta_ps_start/end so that the counters are correct.

In this case, the driver might want to know if the station was already
in the indicated state -- this might be indicated by the return value.

johannes


^ permalink raw reply

* PATCH v2  0/2] wl12xx: BA Initiator & receiver support
From: Shahar Levi @ 2011-01-18  9:32 UTC (permalink / raw)
  To: linux-wireless; +Cc: Luciano Coelho

That series gather two patches:
[PATCH v6] wl12xx: BA initiator support
[PATCH v4] wl12xx: BA receiver support

Add 80211n BA initiator & receiver session support wl1271 driver.
BA initiator session management included in FW independently.
Include supported FW version auto detection mechanism.

Limitation:
- For now only one BA per direction is supported

Shahar Levi (2):
  wl12xx: BA initiator support
  wl12xx: BA receiver support

 drivers/net/wireless/wl12xx/acx.c    |   85 ++++++++++++++++++++++++++++++++++
 drivers/net/wireless/wl12xx/acx.h    |   62 ++++++++++++++++++++++++-
 drivers/net/wireless/wl12xx/boot.c   |   24 ++++++++-
 drivers/net/wireless/wl12xx/conf.h   |    6 ++
 drivers/net/wireless/wl12xx/init.c   |   41 ++++++++++++++++
 drivers/net/wireless/wl12xx/main.c   |   71 ++++++++++++++++++++++++++--
 drivers/net/wireless/wl12xx/wl12xx.h |   16 ++++++-
 7 files changed, 295 insertions(+), 10 deletions(-)


^ permalink raw reply

* [PATCH v6  1/2] wl12xx: BA initiator support
From: Shahar Levi @ 2011-01-18  9:32 UTC (permalink / raw)
  To: linux-wireless; +Cc: Luciano Coelho
In-Reply-To: <1295343139-25375-1-git-send-email-shahar_levi@ti.com>

Add 80211n BA initiator session support wl1271 driver.
Include BA supported FW version auto detection mechanism.
BA initiator session management included in FW independently.

Signed-off-by: Shahar Levi <shahar_levi@ti.com>
---
Limitation:
- For now only one BA per direction is supported

Changes from v1:
- Remove wl1271_op_ampdu_action()
- set CONF_BA_INACTIVITY_TIMEOUT as configurable value
- Clean to Linux code style.

Changes from v2:
- Two new ACX commands ACX_BA_SESSION_POLICY_CFG & ACX_BA_SESSION_RX_SETUP
- New struct wl1271_acx_ba_session_policy
- Calling wl1271_set_ba_policies() once in init
- Use ieee80211_back_parties
- Rebase to latest

Changes from v3:
- New FW file name due to new FW API

Changes from v4:
- Reverse change of "New FW file name due to new FW API"
- Add supported FW version auto detection mechanism (Parsing fw version once and
 use it to validate BA support)

Changes from v5:
- Improve implantation of FW parsing
- Stylish

 drivers/net/wireless/wl12xx/acx.c    |   51 ++++++++++++++++++++++++++++++++++
 drivers/net/wireless/wl12xx/acx.h    |   41 ++++++++++++++++++++++++++-
 drivers/net/wireless/wl12xx/boot.c   |   24 ++++++++++++++--
 drivers/net/wireless/wl12xx/conf.h   |    6 ++++
 drivers/net/wireless/wl12xx/init.c   |   42 ++++++++++++++++++++++++++++
 drivers/net/wireless/wl12xx/main.c   |   12 +++++--
 drivers/net/wireless/wl12xx/wl12xx.h |   17 ++++++++++-
 7 files changed, 183 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/acx.c b/drivers/net/wireless/wl12xx/acx.c
index 646d278..588056a 100644
--- a/drivers/net/wireless/wl12xx/acx.c
+++ b/drivers/net/wireless/wl12xx/acx.c
@@ -1342,6 +1342,57 @@ out:
 	return ret;
 }
 
+/* Configure BA session initiator/receiver parameters setting in the FW. */
+int wl1271_acx_set_ba_session(struct wl1271 *wl,
+			      enum ieee80211_back_parties direction,
+			      u8 tid_index, u8 policy)
+{
+	struct wl1271_acx_ba_session_policy *acx;
+	int ret;
+
+	wl1271_debug(DEBUG_ACX, "acx ba session setting");
+
+	acx = kzalloc(sizeof(*acx), GFP_KERNEL);
+	if (!acx) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* ANY role */
+	acx->role_id = 0xff;
+	acx->tid = tid_index;
+	acx->enable = policy;
+	acx->ba_direction = direction;
+
+	switch (direction) {
+	case WLAN_BACK_INITIATOR:
+		acx->win_size = wl->conf.ht.tx_ba_win_size;
+		acx->inactivity_timeout = wl->conf.ht.inactivity_timeout;
+		break;
+	case WLAN_BACK_RECIPIENT:
+		acx->win_size = RX_BA_WIN_SIZE;
+		acx->inactivity_timeout = 0;
+		break;
+	default:
+		wl1271_error("Incorrect acx command id=%x\n", direction);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = wl1271_cmd_configure(wl,
+				   ACX_BA_SESSION_POLICY_CFG,
+				   acx,
+				   sizeof(*acx));
+	if (ret < 0) {
+		wl1271_warning("acx ba session setting failed: %d", ret);
+		goto out;
+	}
+
+out:
+	kfree(acx);
+	return ret;
+}
+
 int wl1271_acx_tsf_info(struct wl1271 *wl, u64 *mactime)
 {
 	struct wl1271_acx_fw_tsf_information *tsf_info;
diff --git a/drivers/net/wireless/wl12xx/acx.h b/drivers/net/wireless/wl12xx/acx.h
index 89d3748..c0abaf4 100644
--- a/drivers/net/wireless/wl12xx/acx.h
+++ b/drivers/net/wireless/wl12xx/acx.h
@@ -1061,6 +1061,40 @@ struct wl1271_acx_ht_information {
 	u8 padding[3];
 } __packed;
 
+#define RX_BA_WIN_SIZE 8
+
+struct wl1271_acx_ba_session_policy {
+	struct acx_header header;
+	/*
+	 * Specifies role Id, Range 0-7, 0xFF means ANY role.
+	 * Future use. For now this field is irrelevant
+	 */
+	u8 role_id;
+	/*
+	 * Specifies Link Id, Range 0-31, 0xFF means ANY  Link Id.
+	 * Not applicable if Role Id is set to ANY.
+	 */
+	u8 link_id;
+
+	u8 tid;
+
+	u8 enable;
+
+	/* Windows size in number of packets */
+	u16 win_size;
+
+	/*
+	 * As initiator inactivity timeout in time units(TU) of 1024us.
+	 * As receiver reserved
+	 */
+	u16 inactivity_timeout;
+
+	/* Initiator = 1/Receiver = 0 */
+	u8 ba_direction;
+
+	u8 padding[3];
+} __packed;
+
 struct wl1271_acx_fw_tsf_information {
 	struct acx_header header;
 
@@ -1134,8 +1168,8 @@ enum {
 	ACX_RSSI_SNR_WEIGHTS        = 0x0052,
 	ACX_KEEP_ALIVE_MODE         = 0x0053,
 	ACX_SET_KEEP_ALIVE_CONFIG   = 0x0054,
-	ACX_BA_SESSION_RESPONDER_POLICY = 0x0055,
-	ACX_BA_SESSION_INITIATOR_POLICY = 0x0056,
+	ACX_BA_SESSION_POLICY_CFG   = 0x0055,
+	ACX_BA_SESSION_RX_SETUP     = 0x0056,
 	ACX_PEER_HT_CAP             = 0x0057,
 	ACX_HT_BSS_OPERATION        = 0x0058,
 	ACX_COEX_ACTIVITY           = 0x0059,
@@ -1209,6 +1243,9 @@ int wl1271_acx_set_ht_capabilities(struct wl1271 *wl,
 				    bool allow_ht_operation);
 int wl1271_acx_set_ht_information(struct wl1271 *wl,
 				   u16 ht_operation_mode);
+int wl1271_acx_set_ba_session(struct wl1271 *wl,
+			      enum ieee80211_back_parties direction,
+			      u8 tid_index, u8 policy);
 int wl1271_acx_tsf_info(struct wl1271 *wl, u64 *mactime);
 int wl1271_acx_max_tx_retry(struct wl1271 *wl);
 
diff --git a/drivers/net/wireless/wl12xx/boot.c b/drivers/net/wireless/wl12xx/boot.c
index d7e036f..2e66c0f 100644
--- a/drivers/net/wireless/wl12xx/boot.c
+++ b/drivers/net/wireless/wl12xx/boot.c
@@ -101,6 +101,22 @@ static void wl1271_boot_set_ecpu_ctrl(struct wl1271 *wl, u32 flag)
 	wl1271_write32(wl, ACX_REG_ECPU_CONTROL, cpu_ctrl);
 }
 
+static void wl1271_parse_fw_ver(struct wl1271 *wl)
+{
+	int ret;
+
+	ret = sscanf(wl->chip.fw_ver_str + 4, "%u.%u.%u.%u.%u",
+		     &wl->chip.fw_ver[0], &wl->chip.fw_ver[1],
+		     &wl->chip.fw_ver[2], &wl->chip.fw_ver[3],
+		     &wl->chip.fw_ver[4]);
+
+	if (ret <= 0) {
+		wl1271_warning("fw version incorrect value");
+		memset(wl->chip.fw_ver, 0, sizeof(wl->chip.fw_ver));
+		return;
+	}
+}
+
 static void wl1271_boot_fw_version(struct wl1271 *wl)
 {
 	struct wl1271_static_data static_data;
@@ -108,11 +124,13 @@ static void wl1271_boot_fw_version(struct wl1271 *wl)
 	wl1271_read(wl, wl->cmd_box_addr, &static_data, sizeof(static_data),
 		    false);
 
-	strncpy(wl->chip.fw_ver, static_data.fw_version,
-		sizeof(wl->chip.fw_ver));
+	strncpy(wl->chip.fw_ver_str, static_data.fw_version,
+		sizeof(wl->chip.fw_ver_str));
 
 	/* make sure the string is NULL-terminated */
-	wl->chip.fw_ver[sizeof(wl->chip.fw_ver) - 1] = '\0';
+	wl->chip.fw_ver_str[sizeof(wl->chip.fw_ver_str) - 1] = '\0';
+
+	wl1271_parse_fw_ver(wl);
 }
 
 static int wl1271_boot_upload_firmware_chunk(struct wl1271 *wl, void *buf,
diff --git a/drivers/net/wireless/wl12xx/conf.h b/drivers/net/wireless/wl12xx/conf.h
index f5c048c..135d837 100644
--- a/drivers/net/wireless/wl12xx/conf.h
+++ b/drivers/net/wireless/wl12xx/conf.h
@@ -1138,6 +1138,11 @@ struct conf_rf_settings {
 	u8 tx_per_channel_power_compensation_5[CONF_TX_PWR_COMPENSATION_LEN_5];
 };
 
+struct conf_ht_setting {
+	u16 tx_ba_win_size;
+	u16 inactivity_timeout;
+};
+
 struct conf_drv_settings {
 	struct conf_sg_settings sg;
 	struct conf_rx_settings rx;
@@ -1148,6 +1153,7 @@ struct conf_drv_settings {
 	struct conf_roam_trigger_settings roam_trigger;
 	struct conf_scan_settings scan;
 	struct conf_rf_settings rf;
+	struct conf_ht_setting ht;
 };
 
 #endif
diff --git a/drivers/net/wireless/wl12xx/init.c b/drivers/net/wireless/wl12xx/init.c
index bdb6123..0d939a2 100644
--- a/drivers/net/wireless/wl12xx/init.c
+++ b/drivers/net/wireless/wl12xx/init.c
@@ -455,6 +455,43 @@ static int wl1271_ap_hw_init_post_mem(struct wl1271 *wl)
 	return 0;
 }
 
+static void wl1271_check_ba_support(struct wl1271 *wl)
+{
+	/* validate FW cose ver x.x.x.50-60.x */
+	if ((wl->chip.fw_ver[3] >= WL12XX_BA_SUPPORT_FW_COST_VER2_START) &&
+	    (wl->chip.fw_ver[3] < WL12XX_BA_SUPPORT_FW_COST_VER2_END)) {
+		wl->ba_support = true;
+		return;
+	}
+
+	wl->ba_support = false;
+}
+
+static int wl1271_set_ba_policies(struct wl1271 *wl)
+{
+	u8 tid_index;
+	u8 ret = 0;
+
+	/* Reset the BA RX indicators */
+	wl->ba_allowed = true;
+	wl->ba_rx_bitmap = 0;
+
+	/* validate that FW support BA */
+	wl1271_check_ba_support(wl);
+
+	if (wl->ba_support)
+		/* 802.11n initiator BA session setting */
+		for (tid_index = 0; tid_index < CONF_TX_MAX_TID_COUNT;
+		      ++tid_index) {
+			ret = wl1271_acx_set_ba_session(wl, WLAN_BACK_INITIATOR,
+							tid_index, true);
+			if (ret < 0)
+				break;
+		}
+
+	return ret;
+}
+
 int wl1271_hw_init(struct wl1271 *wl)
 {
 	struct conf_tx_ac_category *conf_ac;
@@ -568,6 +605,11 @@ int wl1271_hw_init(struct wl1271 *wl)
 	if (ret < 0)
 		goto out_free_memmap;
 
+	/* Configure initiator BA sessions policies */
+	ret = wl1271_set_ba_policies(wl);
+	if (ret < 0)
+		goto out_free_memmap;
+
 	return 0;
 
  out_free_memmap:
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 9076555..542b1de 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -274,7 +274,7 @@ static struct conf_drv_settings default_conf = {
 		.avg_weight_rssi_beacon       = 20,
 		.avg_weight_rssi_data         = 10,
 		.avg_weight_snr_beacon        = 20,
-		.avg_weight_snr_data          = 10
+		.avg_weight_snr_data          = 10,
 	},
 	.scan = {
 		.min_dwell_time_active        = 7500,
@@ -293,6 +293,10 @@ static struct conf_drv_settings default_conf = {
 			0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 		},
 	},
+	.ht = {
+		.tx_ba_win_size = 64,
+		.inactivity_timeout = 10000,
+	},
 };
 
 static void __wl1271_op_remove_interface(struct wl1271 *wl);
@@ -890,7 +894,7 @@ int wl1271_plt_start(struct wl1271 *wl)
 
 		wl->state = WL1271_STATE_PLT;
 		wl1271_notice("firmware booted in PLT mode (%s)",
-			      wl->chip.fw_ver);
+			      wl->chip.fw_ver_str);
 		goto out;
 
 irq_disable:
@@ -1131,11 +1135,11 @@ power_off:
 
 	wl->vif = vif;
 	wl->state = WL1271_STATE_ON;
-	wl1271_info("firmware booted (%s)", wl->chip.fw_ver);
+	wl1271_info("firmware booted (%s)", wl->chip.fw_ver_str);
 
 	/* update hw/fw version info in wiphy struct */
 	wiphy->hw_version = wl->chip.id;
-	strncpy(wiphy->fw_version, wl->chip.fw_ver,
+	strncpy(wiphy->fw_version, wl->chip.fw_ver_str,
 		sizeof(wiphy->fw_version));
 
 	/*
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index cf5c1a5..443befc 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -38,6 +38,13 @@
 #define DRIVER_NAME "wl1271"
 #define DRIVER_PREFIX DRIVER_NAME ": "
 
+/* 
+ * FW versions support BA 11n
+ * versions marks x.x.x.50-60.x
+ */
+#define WL12XX_BA_SUPPORT_FW_COST_VER2_START    50
+#define WL12XX_BA_SUPPORT_FW_COST_VER2_END      60
+
 enum {
 	DEBUG_NONE	= 0,
 	DEBUG_IRQ	= BIT(0),
@@ -182,10 +189,13 @@ struct wl1271_partition_set {
 
 struct wl1271;
 
+#define WL12XX_NUM_FW_VER 5
+
 /* FIXME: I'm not sure about this structure name */
 struct wl1271_chip {
 	u32 id;
-	char fw_ver[21];
+	char fw_ver_str[ETHTOOL_BUSINFO_LEN];
+	unsigned int fw_ver[WL12XX_NUM_FW_VER];
 };
 
 struct wl1271_stats {
@@ -460,6 +470,11 @@ struct wl1271 {
 
 	/* bands supported by this instance of wl12xx */
 	struct ieee80211_supported_band bands[IEEE80211_NUM_BANDS];
+
+	/* RX BA constraint value */
+	bool ba_support;
+	u8 ba_allowed;
+	u8 ba_rx_bitmap;
 };
 
 struct wl1271_station {
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH v4  2/2] wl12xx: BA receiver support
From: Shahar Levi @ 2011-01-18  9:32 UTC (permalink / raw)
  To: linux-wireless; +Cc: Luciano Coelho
In-Reply-To: <1295343139-25375-1-git-send-email-shahar_levi@ti.com>

Add new ampdu_action ops to support receiver BA.
The BA initiator session management in FW independently.

Signed-off-by: Shahar Levi <shahar_levi@ti.com>
---
Dependencies:
- BA Initiator patches have a runtime dependency on commit
  "[RFC v6] wl1271: BA Initiator support"

Limitation:
- For now only one BA per direction is supported

Changes from v1:
- Add mutex and (wl->state == WL1271_STATE_OFF) protection
- "BIT(tid)" instead of "BIT(0) << tid"
- Move all event code move to the next patch

Changes from v2:
- Add supported FW version auto detection mechanism

Changes from v3:
- Stylish

 drivers/net/wireless/wl12xx/acx.c    |   34 +++++++++++++++++++
 drivers/net/wireless/wl12xx/acx.h    |   21 ++++++++++++
 drivers/net/wireless/wl12xx/init.c   |    1 -
 drivers/net/wireless/wl12xx/main.c   |   59 ++++++++++++++++++++++++++++++++++
 drivers/net/wireless/wl12xx/wl12xx.h |    1 -
 5 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/acx.c b/drivers/net/wireless/wl12xx/acx.c
index 588056a..bff586a 100644
--- a/drivers/net/wireless/wl12xx/acx.c
+++ b/drivers/net/wireless/wl12xx/acx.c
@@ -1393,6 +1393,40 @@ out:
 	return ret;
 }
 
+/* setup BA session receiver setting in the FW. */
+int wl1271_acx_set_ba_receiver_session(struct wl1271 *wl, u8 tid_index, u16 ssn,
+				       bool enable)
+{
+	struct wl1271_acx_ba_receiver_setup *acx;
+	int ret;
+
+	wl1271_debug(DEBUG_ACX, "acx ba receiver session setting");
+
+	acx = kzalloc(sizeof(*acx), GFP_KERNEL);
+	if (!acx) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* Single link for now */
+	acx->link_id = 1;
+	acx->tid = tid_index;
+	acx->enable = enable;
+	acx->win_size = 0;
+	acx->ssn = ssn;
+
+	ret = wl1271_cmd_configure(wl, ACX_BA_SESSION_RX_SETUP, acx,
+				   sizeof(*acx));
+	if (ret < 0) {
+		wl1271_warning("acx ba receiver session failed: %d", ret);
+		goto out;
+	}
+
+out:
+	kfree(acx);
+	return ret;
+}
+
 int wl1271_acx_tsf_info(struct wl1271 *wl, u64 *mactime)
 {
 	struct wl1271_acx_fw_tsf_information *tsf_info;
diff --git a/drivers/net/wireless/wl12xx/acx.h b/drivers/net/wireless/wl12xx/acx.h
index c0abaf4..c683801 100644
--- a/drivers/net/wireless/wl12xx/acx.h
+++ b/drivers/net/wireless/wl12xx/acx.h
@@ -1095,6 +1095,25 @@ struct wl1271_acx_ba_session_policy {
 	u8 padding[3];
 } __packed;
 
+struct wl1271_acx_ba_receiver_setup {
+	struct acx_header header;
+
+	/* Specifies Link Id, Range 0-31, 0xFF means ANY  Link Id */
+	u8 link_id;
+
+	u8 tid;
+
+	u8 enable;
+
+	u8 padding[1];
+
+	/* Windows size in number of packets */
+	u16 win_size;
+
+	/* BA session starting sequence number.  RANGE 0-FFF */
+	u16 ssn;
+} __packed;
+
 struct wl1271_acx_fw_tsf_information {
 	struct acx_header header;
 
@@ -1246,6 +1265,8 @@ int wl1271_acx_set_ht_information(struct wl1271 *wl,
 int wl1271_acx_set_ba_session(struct wl1271 *wl,
 			      enum ieee80211_back_parties direction,
 			      u8 tid_index, u8 policy);
+int wl1271_acx_set_ba_receiver_session(struct wl1271 *wl, u8 tid_index, u16 ssn,
+				       bool enable);
 int wl1271_acx_tsf_info(struct wl1271 *wl, u64 *mactime);
 int wl1271_acx_max_tx_retry(struct wl1271 *wl);
 
diff --git a/drivers/net/wireless/wl12xx/init.c b/drivers/net/wireless/wl12xx/init.c
index 0d939a2..fe6ea71 100644
--- a/drivers/net/wireless/wl12xx/init.c
+++ b/drivers/net/wireless/wl12xx/init.c
@@ -473,7 +473,6 @@ static int wl1271_set_ba_policies(struct wl1271 *wl)
 	u8 ret = 0;
 
 	/* Reset the BA RX indicators */
-	wl->ba_allowed = true;
 	wl->ba_rx_bitmap = 0;
 
 	/* validate that FW support BA */
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index 542b1de..97a42a7 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -2717,6 +2717,64 @@ out:
 	return ret;
 }
 
+int wl1271_op_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+			   enum ieee80211_ampdu_mlme_action action,
+		   struct ieee80211_sta *sta, u16 tid, u16 *ssn)
+{
+	struct wl1271 *wl = hw->priv;
+	int ret;
+
+	mutex_lock(&wl->mutex);
+
+	if (unlikely(wl->state == WL1271_STATE_OFF)) {
+		ret = -EAGAIN;
+		goto out;
+	}
+
+	ret = wl1271_ps_elp_wakeup(wl, false);
+	if (ret < 0)
+		goto out;
+
+	switch (action) {
+	case IEEE80211_AMPDU_RX_START:
+		if (wl->ba_support) {
+			ret = wl1271_acx_set_ba_receiver_session(wl, tid, *ssn,
+								 true);
+			if (!ret)
+				wl->ba_rx_bitmap |= BIT(tid);
+		} else
+			ret = -ENOTSUPP;
+		break;
+
+	case IEEE80211_AMPDU_RX_STOP:
+		ret = wl1271_acx_set_ba_receiver_session(wl, tid, 0, false);
+		if (!ret)
+			wl->ba_rx_bitmap &= ~BIT(tid);
+		break;
+
+	/*
+	 * The BA initiator session management in FW independently.
+	 * Falling break here on purpose for all TX APDU commands.
+	 */
+	case IEEE80211_AMPDU_TX_START:
+	case IEEE80211_AMPDU_TX_STOP:
+	case IEEE80211_AMPDU_TX_OPERATIONAL:
+		ret = -EINVAL;
+		break;
+
+	default:
+		wl1271_error("Incorrect ampdu action id=%x\n", action);
+		ret = -EINVAL;
+	}
+
+	wl1271_ps_elp_sleep(wl);
+
+out:
+	mutex_unlock(&wl->mutex);
+
+	return ret;
+}
+
 /* can't be const, mac80211 writes to this */
 static struct ieee80211_rate wl1271_rates[] = {
 	{ .bitrate = 10,
@@ -2965,6 +3023,7 @@ static const struct ieee80211_ops wl1271_ops = {
 	.get_survey = wl1271_op_get_survey,
 	.sta_add = wl1271_op_sta_add,
 	.sta_remove = wl1271_op_sta_remove,
+	.ampdu_action = wl1271_op_ampdu_action,
 	CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
 };
 
diff --git a/drivers/net/wireless/wl12xx/wl12xx.h b/drivers/net/wireless/wl12xx/wl12xx.h
index 443befc..a3d1179 100644
--- a/drivers/net/wireless/wl12xx/wl12xx.h
+++ b/drivers/net/wireless/wl12xx/wl12xx.h
@@ -473,7 +473,6 @@ struct wl1271 {
 
 	/* RX BA constraint value */
 	bool ba_support;
-	u8 ba_allowed;
 	u8 ba_rx_bitmap;
 };
 
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH 1/5] mac80211: add DFS related channel flags
From: Johannes Berg @ 2011-01-18 10:40 UTC (permalink / raw)
  To: Bernhard Schmidt
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <20110117161133.66B54207E@mx.techwires.net>

On Mon, 2011-01-17 at 09:31 +0100, Bernhard Schmidt wrote:
> Before a radar channel can be used, a full CAC has be done, the
> IEEE80211_CHAN_RADAR_CLEAR flags indicates that this has been done. If either
> during CAC or in-service monitoring interference is detected, the channel is
> added to NOL and marked with IEEE80211_CHAN_RADAR_INTERFERENCE.

I'm not sure about this -- shouldn't that be global information? If one
device in the system records a radar on the channel, it surely applies
to other devices as well, even if they are hot-plugged after the radar
was found?

johannes


^ permalink raw reply

* Re: [PATCH 1/5] mac80211: add DFS related channel flags
From: Johannes Berg @ 2011-01-18 10:40 UTC (permalink / raw)
  To: Bernhard Schmidt
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <20110117161133.66B54207E@mx.techwires.net>

On Mon, 2011-01-17 at 09:31 +0100, Bernhard Schmidt wrote:
> Before a radar channel can be used, a full CAC has be done, the
> IEEE80211_CHAN_RADAR_CLEAR flags indicates that this has been done. If either
> during CAC or in-service monitoring interference is detected, the channel is
> added to NOL and marked with IEEE80211_CHAN_RADAR_INTERFERENCE.

Also, this is not mac80211, it's cfg80211.

johannes


^ permalink raw reply

* Re: [PATCH 2/5] mac80211: initialize radar params + add driver API
From: Johannes Berg @ 2011-01-18 10:43 UTC (permalink / raw)
  To: Bernhard Schmidt
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <20110117161133.6EEA42080@mx.techwires.net>

On Mon, 2011-01-17 at 10:16 +0100, Bernhard Schmidt wrote:

> @@ -1241,6 +1244,8 @@ static int ieee80211_set_channel(struct wiphy *wiphy,
>  	if (sdata && sdata->vif.type != NL80211_IFTYPE_MONITOR)
>  		ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_HT);
>  
> +	ieee80211_radar_detection_enable(local, chan);
> +

So you're ignoring the return value -- that doesn't seem right at all.
Also, if the driver doesn't implement detection, you should probably
reject using the channel.

johannes


^ permalink raw reply

* Re: [PATCH 3/5] mac80211: add NOL functionality
From: Johannes Berg @ 2011-01-18 10:43 UTC (permalink / raw)
  To: Bernhard Schmidt
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <20110117161133.792EB2082@mx.techwires.net>

On Mon, 2011-01-17 at 11:05 +0100, Bernhard Schmidt wrote:
> NOL is a list of channels on which radar interference has been detected.
> Such channels are not allowed to be used for a certain amount of time.

I don't think this should be implemented in mac80211.

johannes



^ permalink raw reply

* Re: [PATCH 2/5] mac80211: initialize radar params + add driver API
From: Johannes Berg @ 2011-01-18 10:44 UTC (permalink / raw)
  To: Bernhard Schmidt
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <20110117161133.6EEA42080@mx.techwires.net>

On Mon, 2011-01-17 at 10:16 +0100, Bernhard Schmidt wrote:

> +static struct ieee80211_radar_parameters regdomain_params[] = {
> +	{ .cac_period = 60, .nol_period = 1800 },	/* FCC, correct? */
> +	{ .cac_period = 60, .nol_period = 1800 },	/* ETSI */
> +	{ .cac_period = 60, .nol_period = 1800 },	/* JP, correct? */
> +};

These parameters really need to come from cfg80211/userspace.

johannes


^ permalink raw reply

* Re: [PATCH 4/5] mac80211: add primarily CAC support
From: Johannes Berg @ 2011-01-18 10:45 UTC (permalink / raw)
  To: Bernhard Schmidt
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <20110117161133.8150D2084@mx.techwires.net>

On Mon, 2011-01-17 at 11:24 +0100, Bernhard Schmidt wrote:
> ---
>  net/mac80211/radar.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/mac80211/radar.h |    4 +++
>  2 files changed, 69 insertions(+), 0 deletions(-)
> 
> diff --git a/net/mac80211/radar.c b/net/mac80211/radar.c
> index e90b8bf..afb2ae1 100644
> --- a/net/mac80211/radar.c
> +++ b/net/mac80211/radar.c
> @@ -15,6 +15,66 @@
>  #include "driver-ops.h"
>  #include "radar.h"
>  
> +static void cac_timer(unsigned long data)
> +{
> +	struct ieee80211_local *local = (void *) data;
> +	struct ieee80211_radar *radar = &local->radar;
> +	struct ieee80211_channel *chan;
> +
> +	printk(KERN_INFO "CAC done\n");
> +
> +	chan = local->oper_channel;
> +	mutex_lock(&radar->mtx);

Clearly, you haven't even tested this code. I'm not sure why I'm even
reviewing it.

johannes


^ permalink raw reply

* Re: [PATCH 4/5] mac80211: add primarily CAC support
From: Johannes Berg @ 2011-01-18 10:48 UTC (permalink / raw)
  To: Bernhard Schmidt
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <20110117161133.8150D2084@mx.techwires.net>

I don't get this at all. How's the driver supported to report anything?!

johannes


^ permalink raw reply

* Re: intermittent eap authentication failure
From: Johannes Berg @ 2011-01-18 10:52 UTC (permalink / raw)
  To: Chuck Crisler; +Cc: linux-wireless
In-Reply-To: <D323CFEF4DEC4E18AFF56742F78C16B6@ChuckPC>

Chuck,

> Sometime ago I found a conflict between the supplicant and the MAC80211 code 
> dealing with Cisco session timeouts. When we were 'deauthenticated', the MAC 
> code notified the supplicant AND re-associated with the AP.

That doesn't seem right, mac80211 will never re-associate by itself.

> We modified the driver so that if it received a 
> de-auth with reason code 1 (undefined?), it would *NOT* notify the 
> supplicant but would re-associate, then notify the supplicant of the new 
> association. 

What's that driver you're talking about? A mac80211 driver that
reassociates by itself doesn't seem right at all.

johannes


^ permalink raw reply

* Re: [PATCH 1/5] mac80211: add DFS related channel flags
From: Bernhard Schmidt @ 2011-01-18 11:59 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <1295347236.3563.5.camel@jlt3.sipsolutions.net>

On Tuesday, January 18, 2011 11:40:36 Johannes Berg wrote:
> On Mon, 2011-01-17 at 09:31 +0100, Bernhard Schmidt wrote:
> > Before a radar channel can be used, a full CAC has be done, the
> > IEEE80211_CHAN_RADAR_CLEAR flags indicates that this has been done.
> > If either during CAC or in-service monitoring interference is
> > detected, the channel is added to NOL and marked with
> > IEEE80211_CHAN_RADAR_INTERFERENCE.
> 
> I'm not sure about this -- shouldn't that be global information? If
> one device in the system records a radar on the channel, it surely
> applies to other devices as well, even if they are hot-plugged after
> the radar was found?

True, I've based this on our IRC discussions, the consensus there was to 
do it per-wiphy only at first.

-- 
Best regards,

Dipl.-Inf. (FH) Bernhard Schmidt (software development)

saxnet GmbH, Willy-Brandt-Ring 1, 08606 Oelsnitz
Tel. +49 (0) 3741 300 6. 100 - Fax +49 (0) 3741 300 6. 101
managing director: Steffen Dreise - county court Chemnitz - HRB 23017
http://www.saxnet.de

^ permalink raw reply

* Re: [PATCH 2/5] mac80211: initialize radar params + add driver API
From: Bernhard Schmidt @ 2011-01-18 12:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <1295347402.3563.7.camel@jlt3.sipsolutions.net>

On Tuesday, January 18, 2011 11:43:22 Johannes Berg wrote:
> On Mon, 2011-01-17 at 10:16 +0100, Bernhard Schmidt wrote:
> > @@ -1241,6 +1244,8 @@ static int ieee80211_set_channel(struct wiphy
> > *wiphy,
> > 
> >  	if (sdata && sdata->vif.type != NL80211_IFTYPE_MONITOR)
> >  	
> >  		ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_HT);
> > 
> > +	ieee80211_radar_detection_enable(local, chan);
> > +
> 
> So you're ignoring the return value -- that doesn't seem right at
> all. Also, if the driver doesn't implement detection, you should
> probably reject using the channel.

Yeah right, I've missed that. I wasn't sure about how to proper handle 
opmode and then forgot about it.. Point is, this should only fail on an 
opmode which does involve sending beacons (or in other words, opmodes 
which act as a master device) but not for sta. Will think about 
something.

-- 
Best regards,

Dipl.-Inf. (FH) Bernhard Schmidt (software development)

saxnet GmbH, Willy-Brandt-Ring 1, 08606 Oelsnitz
Tel. +49 (0) 3741 300 6. 100 - Fax +49 (0) 3741 300 6. 101
managing director: Steffen Dreise - county court Chemnitz - HRB 23017
http://www.saxnet.de

^ permalink raw reply

* Re: [PATCH 4/5] mac80211: add primarily CAC support
From: Bernhard Schmidt @ 2011-01-18 12:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <1295347692.3563.11.camel@jlt3.sipsolutions.net>

On Tuesday, January 18, 2011 11:48:12 Johannes Berg wrote:
> I don't get this at all. How's the driver supported to report
> anything?!

ieee80211_radar_interference()

-- 
Best regards,

Dipl.-Inf. (FH) Bernhard Schmidt (software development)

saxnet GmbH, Willy-Brandt-Ring 1, 08606 Oelsnitz
Tel. +49 (0) 3741 300 6. 100 - Fax +49 (0) 3741 300 6. 101
managing director: Steffen Dreise - county court Chemnitz - HRB 23017
http://www.saxnet.de

^ permalink raw reply

* Re: [PATCH 3/5] mac80211: add NOL functionality
From: Bernhard Schmidt @ 2011-01-18 12:01 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <1295347426.3563.8.camel@jlt3.sipsolutions.net>

On Tuesday, January 18, 2011 11:43:46 Johannes Berg wrote:
> On Mon, 2011-01-17 at 11:05 +0100, Bernhard Schmidt wrote:
> > NOL is a list of channels on which radar interference has been
> > detected. Such channels are not allowed to be used for a certain
> > amount of time.
> 
> I don't think this should be implemented in mac80211.

In cfg80211 then? I'd really prefer to keep all flag-handling together 
instead of splitting it up between mac80211/userspace.

-- 
Best regards,

Dipl.-Inf. (FH) Bernhard Schmidt (software development)

saxnet GmbH, Willy-Brandt-Ring 1, 08606 Oelsnitz
Tel. +49 (0) 3741 300 6. 100 - Fax +49 (0) 3741 300 6. 101
managing director: Steffen Dreise - county court Chemnitz - HRB 23017
http://www.saxnet.de

^ permalink raw reply

* Re: [PATCH 4/5] mac80211: add primarily CAC support
From: Bernhard Schmidt @ 2011-01-18 12:01 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, lrodriguez, nbd, dubowoj, zefir.kurtisi,
	simon.wunderlich
In-Reply-To: <1295347515.3563.10.camel@jlt3.sipsolutions.net>

On Tuesday, January 18, 2011 11:45:15 Johannes Berg wrote:
> On Mon, 2011-01-17 at 11:24 +0100, Bernhard Schmidt wrote:
> > ---
> > 
> >  net/mac80211/radar.c |   65
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  net/mac80211/radar.h |    4 +++
> >  2 files changed, 69 insertions(+), 0 deletions(-)
> > 
> > diff --git a/net/mac80211/radar.c b/net/mac80211/radar.c
> > index e90b8bf..afb2ae1 100644
> > --- a/net/mac80211/radar.c
> > +++ b/net/mac80211/radar.c
> > @@ -15,6 +15,66 @@
> > 
> >  #include "driver-ops.h"
> >  #include "radar.h"
> > 
> > +static void cac_timer(unsigned long data)
> > +{
> > +	struct ieee80211_local *local = (void *) data;
> > +	struct ieee80211_radar *radar = &local->radar;
> > +	struct ieee80211_channel *chan;
> > +
> > +	printk(KERN_INFO "CAC done\n");
> > +
> > +	chan = local->oper_channel;
> > +	mutex_lock(&radar->mtx);
> 
> Clearly, you haven't even tested this code. I'm not sure why I'm even
> reviewing it.

Granted, I did only some basic tests with only a few predefined 
scenarios, not at all is just wrong. Anyways, I should have mentioned in 
0/13 that preventing channel changes while in CAC should be considered 
and chan should be assigned to local variable then.

-- 
Best regards,

Dipl.-Inf. (FH) Bernhard Schmidt (software development)

saxnet GmbH, Willy-Brandt-Ring 1, 08606 Oelsnitz
Tel. +49 (0) 3741 300 6. 100 - Fax +49 (0) 3741 300 6. 101
managing director: Steffen Dreise - county court Chemnitz - HRB 23017
http://www.saxnet.de

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox