[PATCH] ath9k: Fix FIF_BCN_PRBRESP_PROMISC handling

[PATCH] ath9k: Fix FIF_BCN_PRBRESP_PROMISC handling

[Luis R. Rodriguez]

This is a port of commit
91ed19f5f66a7fe544f0ec385e981f43491d1d5a
for 2.6.29.

Without this after scanning your device will set
the association ID to something bogus and what is
being reported is multicast/broadcast frame are not
being received. For details see this bug report:

https://bugzilla.redhat.com/show_bug.cgi?id=498502

>From the original commit:

So that a new created IBSS network
doesn't break on the first scan.

It seems to Sujith and me that this
stupid code unnecessary, too.

So remove it...

Reported-by: David Woodhouse <dwmw2@infradead.org>
Tested-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Alina Friedrichsen <x-alina@gmx.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Signed-off-by: Jouni Malinen <Jouni.Malinen@atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath9k/main.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
index 3c04044..1cc826b 100644
--- a/drivers/net/wireless/ath9k/main.c
+++ b/drivers/net/wireless/ath9k/main.c
@@ -2300,11 +2300,6 @@ static void ath9k_configure_filter(struct ieee80211_hw *hw,
 	rfilt = ath_calcrxfilter(sc);
 	ath9k_hw_setrxfilter(sc->sc_ah, rfilt);
 
-	if (changed_flags & FIF_BCN_PRBRESP_PROMISC) {
-		if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
-			ath9k_hw_write_associd(sc->sc_ah, ath_bcast_mac, 0);
-	}
-
 	DPRINTF(sc, ATH_DBG_CONFIG, "Set HW RX filter: 0x%x\n", sc->rx.rxfilter);
 }
 
-- 
1.6.2.2.446.gfbdc0

IPV6 testing... Re: [PATCH] ath9k: Fix FIF_BCN_PRBRESP_PROMISC handling

[David Woodhouse]

On Tue, 2009-05-05 at 17:04 -0700, Luis R. Rodriguez wrote:
> This is a port of commit
> 91ed19f5f66a7fe544f0ec385e981f43491d1d5a
> for 2.6.29.
> 
> Without this after scanning your device will set
> the association ID to something bogus and what is
> being reported is multicast/broadcast frame are not
> being received. For details see this bug report:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=498502
> 
> From the original commit:
> 
> So that a new created IBSS network
> doesn't break on the first scan.
> 
> It seems to Sujith and me that this
> stupid code unnecessary, too.
> 
> So remove it...
> 
> Reported-by: David Woodhouse <dwmw2@infradead.org>
> Tested-by: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Alina Friedrichsen <x-alina@gmx.net>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> Signed-off-by: Jouni Malinen <Jouni.Malinen@atheros.com>
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>

Btw, people ought to be _really_ embarrassed by this. It shows that you
never even bothered to try a simple IPv6 ping to/from a machine with
this driver.

You should _ALWAYS_ test network drivers with IPv6; it exercises
multicast paths that Legacy IP won't usually touch.

A simple 'ping6 -I wlan0 fe80::217:c4ff:fe5e:dd55' doesn't even need any
infrastructure or prior setup (except for looking in 'ifconfig' to see
the link-local IPv6 address of the interface you want to ping, since my
father's laptop is unlikely to be on your local link).

And running radvd with a simple config like this is also trivial, and
lets you verify that the machines on the subnet really do pick up
addresses and routes. It doesn't need an IPv6 connection to the outside
world; the fec0::/16 addresses it advertises are site-local:

	interface eth0
	{
		AdvSendAdvert on;
		MinRtrAdvInterval 30;
		MaxRtrAdvInterval 100;
		prefix fec0::/64
		{
			AdvOnLink on;
			AdvAutonomous on;
			AdvRouterAddr off;
		};
	};

For that matter, setting up a real tunnel to somewhere like sixxs.net or
he.net (or just using 6to4) is fairly bloody trivial too.

There's no excuse for shipping stuff that fails even the most basic
testing.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: IPV6 testing... Re: [PATCH] ath9k: Fix FIF_BCN_PRBRESP_PROMISC handling

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Sat, 09 May 2009 09:49:30 +0100

> Btw, people ought to be _really_ embarrassed by this. It shows that you
> never even bothered to try a simple IPv6 ping to/from a machine with
> this driver.
> 
> You should _ALWAYS_ test network drivers with IPv6; it exercises
> multicast paths that Legacy IP won't usually touch.

I pretty much never test IPV6 and I'm not embarassed.  Really, I
simply do not use it and have no interest in IPV6 at all.  It's simply
not on my radar.  I'll work on ipv6 bugs, but as far as using it
actively and testing every driver I write with it, NO WAY.

And that is OK.

Nobody else should be embarrased about failing to test ipv6 either.
Don't be rediculious David.

The community will test that eventually, find the problems, and the
bugs will get fixed.  End of story.

Re: IPV6 testing... Re: [PATCH] ath9k: Fix FIF_BCN_PRBRESP_PROMISC handling

[David Woodhouse]

On Sat, 2009-05-09 at 12:29 -0700, David Miller wrote:
> I pretty much never test IPV6 and I'm not embarassed.  Really, I
> simply do not use it and have no interest in IPV6 at all.  It's simply
> not on my radar.  I'll work on ipv6 bugs, but as far as using it
> actively and testing every driver I write with it, NO WAY.

I wasn't suggesting that you should care about IPv6, per se.

I was suggesting that you should care about Ethernet multicast.

And the simplest way of testing that seems to be to use IPv6.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[PATCH] ath9k: Fix FIF_BCN_PRBRESP_PROMISC handling

[Alina Friedrichsen]

So that a new created IBSS network
doesn't break on the first scan.

Signed-off-by: Alina Friedrichsen <x-alina@gmx.net>
---
diff --git a/drivers/net/wireless/ath9k/hw.c b/drivers/net/wireless/ath=
9k/hw.c
index 2acbb84..cedf00a 100644
--- a/drivers/net/wireless/ath9k/hw.c
+++ b/drivers/net/wireless/ath9k/hw.c
@@ -3696,9 +3696,18 @@ void ath9k_hw_setbssidmask(struct ath_softc *sc)
=20
 void ath9k_hw_write_associd(struct ath_softc *sc)
 {
-	REG_WRITE(sc->sc_ah, AR_BSS_ID0, get_unaligned_le32(sc->curbssid));
-	REG_WRITE(sc->sc_ah, AR_BSS_ID1, get_unaligned_le16(sc->curbssid + 4)=
 |
-		  ((sc->curaid & 0x3fff) << AR_BSS_ID1_AID_S));
+	const u8 *curbssid =3D sc->curbssid;
+	u16 curaid =3D sc->curaid;
+
+	/* implement FIF_BCN_PRBRESP_PROMISC handling */
+	if(sc->sc_ah->bcn_prbresp_promisc) {
+		curbssid =3D ath_bcast_mac;
+		curaid =3D 0;
+	}
+
+	REG_WRITE(sc->sc_ah, AR_BSS_ID0, get_unaligned_le32(curbssid));
+	REG_WRITE(sc->sc_ah, AR_BSS_ID1, get_unaligned_le16(curbssid + 4) |
+		  ((curaid & 0x3fff) << AR_BSS_ID1_AID_S));
 }
=20
 u64 ath9k_hw_gettsf64(struct ath_hw *ah)
diff --git a/drivers/net/wireless/ath9k/hw.h b/drivers/net/wireless/ath=
9k/hw.h
index 5ec416b..4d930c7 100644
--- a/drivers/net/wireless/ath9k/hw.h
+++ b/drivers/net/wireless/ath9k/hw.h
@@ -441,6 +441,7 @@ struct ath_hw {
 	u32 btactive_gpio;
 	u32 wlanactive_gpio;
 	u32 ah_flags;
+	bool bcn_prbresp_promisc;
=20
 	enum nl80211_iftype opmode;
 	enum ath9k_power_mode power_mode;
diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/a=
th9k/main.c
index f5f5739..c0d5336 100644
--- a/drivers/net/wireless/ath9k/main.c
+++ b/drivers/net/wireless/ath9k/main.c
@@ -2397,11 +2397,12 @@ static void ath9k_configure_filter(struct ieee8=
0211_hw *hw,
 	ath9k_hw_setrxfilter(sc->sc_ah, rfilt);
=20
 	if (changed_flags & FIF_BCN_PRBRESP_PROMISC) {
-		if (*total_flags & FIF_BCN_PRBRESP_PROMISC) {
-			memcpy(sc->curbssid, ath_bcast_mac, ETH_ALEN);
-			sc->curaid =3D 0;
-			ath9k_hw_write_associd(sc);
-		}
+		if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
+			sc->sc_ah->bcn_prbresp_promisc =3D true;
+		else
+			sc->sc_ah->bcn_prbresp_promisc =3D false;
+
+		ath9k_hw_write_associd(sc);
 	}
=20
 	DPRINTF(sc, ATH_DBG_CONFIG, "Set HW RX filter: 0x%x\n", sc->rx.rxfilt=
er);

--=20
Computer Bild Tarifsieger! GMX FreeDSL - Telefonanschluss + DSL
f=FCr nur 17,95 =BF/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K11308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ath9k: Fix FIF_BCN_PRBRESP_PROMISC handling

[Sujith]

Alina Friedrichsen wrote:
> diff --git a/drivers/net/wireless/ath9k/hw.h b/drivers/net/wireless/ath9k/hw.h
> index 5ec416b..4d930c7 100644
> --- a/drivers/net/wireless/ath9k/hw.h
> +++ b/drivers/net/wireless/ath9k/hw.h
> @@ -441,6 +441,7 @@ struct ath_hw {
>  	u32 btactive_gpio;
>  	u32 wlanactive_gpio;
>  	u32 ah_flags;
> +	bool bcn_prbresp_promisc;
>  
>  	enum nl80211_iftype opmode;
>  	enum ath9k_power_mode power_mode;
> diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
> index f5f5739..c0d5336 100644
> --- a/drivers/net/wireless/ath9k/main.c
> +++ b/drivers/net/wireless/ath9k/main.c
> @@ -2397,11 +2397,12 @@ static void ath9k_configure_filter(struct ieee80211_hw *hw,
>  	ath9k_hw_setrxfilter(sc->sc_ah, rfilt);
>  
>  	if (changed_flags & FIF_BCN_PRBRESP_PROMISC) {
> -		if (*total_flags & FIF_BCN_PRBRESP_PROMISC) {
> -			memcpy(sc->curbssid, ath_bcast_mac, ETH_ALEN);
> -			sc->curaid = 0;
> -			ath9k_hw_write_associd(sc);
> -		}
> +		if (*total_flags & FIF_BCN_PRBRESP_PROMISC)
> +			sc->sc_ah->bcn_prbresp_promisc = true;
> +		else
> +			sc->sc_ah->bcn_prbresp_promisc = false;
> +
> +		ath9k_hw_write_associd(sc);
>  	}

You don't need a new variable in ath_hal. The RX filter is already cached in
ath_softc during a configure_filter() call.

Sujith

Re: [PATCH] ath9k: Fix FIF_BCN_PRBRESP_PROMISC handling

[Alina Friedrichsen]

Hallo Sujith!

> You don't need a new variable in ath_hal. The RX filter is already ca=
ched
> in
> ath_softc during a configure_filter() call.

I'm asking currently myself, if this code needed at all? Or can it comp=
letely removed?

Sending and receiving probe requests and responses works without it, to=
o. What is the need for this code?

Regards
Alina

--=20
Computer Bild Tarifsieger! GMX FreeDSL - Telefonanschluss + DSL
f=FCr nur 17,95 =BF/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K11308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html