[PATCH] ath9k: Add sanity check for beacon_int in adhoc/mesh case

[PATCH] ath9k: Add sanity check for beacon_int in adhoc/mesh case

[Jouni Malinen]

It looks like mac80211 can request the driver to start beaconing with
a beacon interval of zero in some cases (at least for mesh point). This
does not sound correct and something may need to be fixed in
mac80211. However, taken into account the unpleasantness of getting
stuck in an infinite busy loop with rtnl_lock held, let's add a quick
workaround in the driver to avoid the worst symptom while someone more
familiar with the mesh implementation can figure out what should be done
with mac80211 as far as beacon interval configuration is concerned.

Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>

---
 drivers/net/wireless/ath/ath9k/beacon.c |    8 ++++++++
 1 file changed, 8 insertions(+)

--- wireless-testing.orig/drivers/net/wireless/ath/ath9k/beacon.c	2009-05-28 19:13:54.000000000 +0300
+++ wireless-testing/drivers/net/wireless/ath/ath9k/beacon.c	2009-05-28 19:14:58.000000000 +0300
@@ -673,6 +673,14 @@ static void ath_beacon_config_adhoc(stru
 
 	intval = conf->beacon_interval & ATH9K_BEACON_PERIOD;
 
+	/*
+	 * It looks like mac80211 may end up using beacon interval of zero in
+	 * some cases (at least for mesh point). Avoid getting into an
+	 * infinite loop by using a bit safer value instead..
+	 */
+	if (intval == 0)
+		intval = 100;
+
 	/* Pull nexttbtt forward to reflect the current TSF */
 
 	nexttbtt = TSF_TO_TU(sc->beacon.bc_tstamp >> 32, sc->beacon.bc_tstamp);

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] ath9k: Add sanity check for beacon_int in adhoc/mesh case

[Kalle Valo]

Jouni Malinen <jouni.malinen@atheros.com> writes:

> +	/*
> +	 * It looks like mac80211 may end up using beacon interval of zero in
> +	 * some cases (at least for mesh point). Avoid getting into an
> +	 * infinite loop by using a bit safer value instead..
> +	 */
> +	if (intval == 0)
> +		intval = 100;

Maybe a big fat warning here would help with debugging the issue?

-- 
Kalle Valo

Re: [PATCH] ath9k: Add sanity check for beacon_int in adhoc/mesh case

[Jouni Malinen]

On Mon, 2009-06-01 at 06:30 -0700, Kalle Valo wrote:
> Jouni Malinen <jouni.malinen@atheros.com> writes:
> 
> > +     /*
> > +      * It looks like mac80211 may end up using beacon interval of zero in
> > +      * some cases (at least for mesh point). Avoid getting into an
> > +      * infinite loop by using a bit safer value instead..
> > +      */
> > +     if (intval == 0)
> > +             intval = 100;
> 
> Maybe a big fat warning here would help with debugging the issue?

If we agree that this should never happen, such a thing could be added.
Though, it would probably be better to place that in mac80211 so that it
would catch the issue with any driver. The beacon_int=0 notification is
not really in any way specific to ath9k; only this workaround for driver
behavior is.

- Jouni

Re: [PATCH] ath9k: Add sanity check for beacon_int in adhoc/mesh case

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

On Mon, 2009-06-01 at 17:05 +0300, Jouni Malinen wrote:
> On Mon, 2009-06-01 at 06:30 -0700, Kalle Valo wrote:
> > Jouni Malinen <jouni.malinen@atheros.com> writes:
> > 
> > > +     /*
> > > +      * It looks like mac80211 may end up using beacon interval of zero in
> > > +      * some cases (at least for mesh point). Avoid getting into an
> > > +      * infinite loop by using a bit safer value instead..
> > > +      */
> > > +     if (intval == 0)
> > > +             intval = 100;
> > 
> > Maybe a big fat warning here would help with debugging the issue?
> 
> If we agree that this should never happen, such a thing could be added.
> Though, it would probably be better to place that in mac80211 so that it
> would catch the issue with any driver. The beacon_int=0 notification is
> not really in any way specific to ath9k; only this workaround for driver
> behavior is.

Has anyone identified why this happens? :) I'm not particularly happy
with the mesh configuration code anyway, but this is a stupid thing that
should be trivial to fix.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]