[Patch] ath9k: fix erased ieee80211_rx_status.mactime

[Patch] ath9k: fix erased ieee80211_rx_status.mactime

[Jan Friedrich]

This patch addresses the erasure of ieee80211_rx_status.mactime.

After the mactime is set in "ath_rx_tasklet" it is immediately nulled in
"ath9k_rx_skb_preprocess".

Signed-off-by: Jan Friedrich <jft@dev2day.de>
---
 drivers/net/wireless/ath/ath9k/recv.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c
b/drivers/net/wireless/ath/ath9k/recv.c
index da0cfe9..ae4bd30 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1140,6 +1140,11 @@ int ath_rx_tasklet(struct ath_softc *sc, int
flush, bool hp)
 		if (flush)
 			goto requeue;

+		retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
+						 rxs, &decrypt_error);
+		if (retval)
+			goto requeue;
+			
 		rxs->mactime = (tsf & ~0xffffffffULL) | rs.rs_tstamp;
 		if (rs.rs_tstamp > tsf_lower &&
 		    unlikely(rs.rs_tstamp - tsf_lower > 0x10000000))
@@ -1149,11 +1154,6 @@ int ath_rx_tasklet(struct ath_softc *sc, int
flush, bool hp)
 		    unlikely(tsf_lower - rs.rs_tstamp > 0x10000000))
 			rxs->mactime += 0x100000000ULL;

-		retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
-						 rxs, &decrypt_error);
-		if (retval)
-			goto requeue;
-
 		/* Ensure we always have an skb to requeue once we are done
 		 * processing the current buffer's skb */
 		requeue_skb = ath_rxbuf_alloc(common, common->rx_bufsize, GFP_ATOMIC);

Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime

[Luis R. Rodriguez]

On Mon, Aug 02, 2010 at 01:16:14PM -0700, Jan Friedrich wrote:
> This patch addresses the erasure of ieee80211_rx_status.mactime.
> 
> After the mactime is set in "ath_rx_tasklet" it is immediately nulled in
> "ath9k_rx_skb_preprocess".

What effect does this have? What issue does this fix? Is this a
fix which also needs to be propagated to the stable kernels, if so
why? etc.

Your patch describes what it does, it doesn't say why its a good
fix or what was affected before. Please clarify all this as best
as you can so we can help ACK/NACK/propagate the patch to stable
if it is appropriate.

  Luis
>  		requeue_skb = ath_rxbuf_alloc(common, common->rx_bufsize, GFP_ATOMIC);

Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime

[Jan Friedrich]

> What effect does this have? What issue does this fix? Is this a
> fix which also needs to be propagated to the stable kernels, if so
> why? etc.

The mactime is always set to 0. I encountered the problem in IBSS mode.
In ieee80211_rx_bss_info (net/mac80211/ibss.c) the mactime is used to
trigger an IBSS merge. At the moment the merge is always triggered.

I got the "beacon TSF higher than local TSF - IBSS merge with BSSID" log
message with every beacon and so recognized this odd behaviour.

I think this has to be fixed and at least influences IBSS mode negatively.

Greetings,
Jan

Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime

[Luis R. Rodriguez]

On Mon, Aug 02, 2010 at 01:35:36PM -0700, Jan Friedrich wrote:
> > What effect does this have? What issue does this fix? Is this a
> > fix which also needs to be propagated to the stable kernels, if so
> > why? etc.
> 
> The mactime is always set to 0. I encountered the problem in IBSS mode.
> In ieee80211_rx_bss_info (net/mac80211/ibss.c) the mactime is used to
> trigger an IBSS merge. At the moment the merge is always triggered.
> 
> I got the "beacon TSF higher than local TSF - IBSS merge with BSSID" log
> message with every beacon and so recognized this odd behaviour.
> 
> I think this has to be fixed and at least influences IBSS mode negatively.

Think? Does the patch actually fix something or do you think it fixes
something? Next time please include all these details on your patch
commit log mesage so its easy to understand the reasoning behind
the patch.

  Luis

Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime

[Jan Friedrich]

> Think? Does the patch actually fix something or do you think it fixes
> something? Next time please include all these details on your patch
> commit log mesage so its easy to understand the reasoning behind
> the patch.

It fixes the problem I described. Now the mactime is set correctly and
the IBSS merge is done only once and by the correct station. I noticed
no negative side effect and tested it with 10+ stations in IBSS mode.

Sorry for the inconvenience. Next time I will provide more information.

Greetings,
Jan

Re: [Patch] ath9k: fix erased ieee80211_rx_status.mactime

[Luis R. Rodriguez]

On Mon, Aug 02, 2010 at 01:48:49PM -0700, Jan Friedrich wrote:
> > Think? Does the patch actually fix something or do you think it fixes
> > something? Next time please include all these details on your patch
> > commit log mesage so its easy to understand the reasoning behind
> > the patch.
> 
> It fixes the problem I described. Now the mactime is set correctly and
> the IBSS merge is done only once and by the correct station. I noticed
> no negative side effect and tested it with 10+ stations in IBSS mode.
> 
> Sorry for the inconvenience. Next time I will provide more information.

no problem, can you resubmit but add all these details to the commit log?
ALso, is this a stable kernel fix, that is, does this fix need to be
propagated to the stable kernels, 2.6.35, 2.6.34, 2.6.33, 2.6.32?

  Luis

[Patch] ath9k: fix erased ieee80211_rx_status.mactime

[Jan Friedrich]

ath9k_rx_skb_preprocess nulls rxs and the mactime is never set again -
mactime is always 0. This causes problems in IBSS mode.

ieee80211_rx_bss_info uses mactime to decide if an IBSS merge is needed.
Without this patch the merge is triggered by each beacon received.

This can be recognized by the "beacon TSF higher than local TSF - IBSS
merge with BSSID" log message accompanying each beacon.

This problem was not completely fixed in commit
a6d2055b02dde1067075795274672720baadd3ca and is not a stable kernel fix.
It is solely intended for wireless-testing.

Signed-off-by: Jan Friedrich <jft@dev2day.de>
---
 drivers/net/wireless/ath/ath9k/recv.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c
b/drivers/net/wireless/ath/ath9k/recv.c
index da0cfe9..ae4bd30 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1140,6 +1140,11 @@ int ath_rx_tasklet(struct ath_softc *sc, int
flush, bool hp)
 		if (flush)
 			goto requeue;

+		retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
+						 rxs, &decrypt_error);
+		if (retval)
+			goto requeue;
+			
 		rxs->mactime = (tsf & ~0xffffffffULL) | rs.rs_tstamp;
 		if (rs.rs_tstamp > tsf_lower &&
 		    unlikely(rs.rs_tstamp - tsf_lower > 0x10000000))
@@ -1149,11 +1154,6 @@ int ath_rx_tasklet(struct ath_softc *sc, int
flush, bool hp)
 		    unlikely(tsf_lower - rs.rs_tstamp > 0x10000000))
 			rxs->mactime += 0x100000000ULL;

-		retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs,
-						 rxs, &decrypt_error);
-		if (retval)
-			goto requeue;
-
 		/* Ensure we always have an skb to requeue once we are done
 		 * processing the current buffer's skb */
 		requeue_skb = ath_rxbuf_alloc(common, common->rx_bufsize, GFP_ATOMIC);