PANIC - SWBMISS (9.0-CURRENT)
Edgar Martinez
emartinez at kbcnetworks.com
Thu Sep 29 22:36:59 UTC 2011
Fixed all panics..please dbl check to see what else I broke in the process..
*** ORIG/ieee80211_proto.c Thu Sep 29 05:58:01 2011
--- ieee80211_proto.c Thu Sep 29 11:51:21 2011
***************
*** 1433,1443 ****
* Software beacon miss handling. Check if any beacons
* were received in the last period. If not post a
* beacon miss; otherwise reset the counter.
*/
void
! ieee80211_swbmiss(void *arg)
{
struct ieee80211vap *vap = arg;
struct ieee80211com *ic = vap->iv_ic;
/* XXX sleep state? */
--- 1433,1443 ----
* Software beacon miss handling. Check if any beacons
* were received in the last period. If not post a
* beacon miss; otherwise reset the counter.
*/
void
! ieee80211_swbmiss_locked(void *arg)
{
struct ieee80211vap *vap = arg;
struct ieee80211com *ic = vap->iv_ic;
/* XXX sleep state? */
***************
*** 1463,1472 ****
--- 1463,1483 ----
vap->iv_swbmiss_count = 0;
callout_reset(&vap->iv_swbmiss, vap->iv_swbmiss_period,
ieee80211_swbmiss, vap);
}
+ void
+ ieee80211_swbmiss(void *arg)
+ {
+ struct ieee80211vap *vap = arg;
+ struct ieee80211com *ic = vap->iv_ic;
+
+ IEEE80211_LOCK(ic);
+ ieee80211_swbmiss_locked(vap);
+ IEEE80211_UNLOCK(ic);
+ }
+
/*
* Start an 802.11h channel switch. We record the parameters,
* mark the operation pending, notify each vap through the
* beacon update mechanism so it can update the beacon frame
* contents, and then switch vap's to CSA state to block outbound
*** ORIG/ieee80211_proto.h Thu Sep 29 05:58:01 2011
--- ieee80211_proto.h Thu Sep 29 11:55:55 2011
***************
*** 310,319 ****
--- 310,320 ----
void ieee80211_stop_all(struct ieee80211com *);
void ieee80211_suspend_all(struct ieee80211com *);
void ieee80211_resume_all(struct ieee80211com *);
void ieee80211_dturbo_switch(struct ieee80211vap *, int newflags);
void ieee80211_swbmiss(void *arg);
+ void ieee80211_swbmiss_locked(void *arg);
void ieee80211_beacon_miss(struct ieee80211com *);
int ieee80211_new_state(struct ieee80211vap *, enum ieee80211_state, int);
void ieee80211_print_essid(const uint8_t *, int);
void ieee80211_dump_pkt(struct ieee80211com *,
const uint8_t *, int, int, int);
*** ORIG/ieee80211_sta.c Thu Sep 29 05:58:01 2011
--- ieee80211_sta.c Thu Sep 29 12:11:41 2011
***************
*** 64,73 ****
--- 64,74 ----
#define IEEE80211_RATE2MBS(r) (((r) & IEEE80211_RATE_VAL) / 2)
static void sta_vattach(struct ieee80211vap *);
static void sta_beacon_miss(struct ieee80211vap *);
+ static void sta_beacon_miss_locked(struct ieee80211vap *);
static int sta_newstate(struct ieee80211vap *, enum ieee80211_state, int);
static int sta_input(struct ieee80211_node *, struct mbuf *, int, int);
static void sta_recv_mgmt(struct ieee80211_node *, struct mbuf *,
int subtype, int rssi, int nf);
static void sta_recv_ctl(struct ieee80211_node *, struct mbuf *, int subtype);
***************
*** 103,113 ****
* Handle a beacon miss event. The common code filters out
* spurious events that can happen when scanning and/or before
* reaching RUN state.
*/
static void
! sta_beacon_miss(struct ieee80211vap *vap)
{
struct ieee80211com *ic = vap->iv_ic;
KASSERT((ic->ic_flags & IEEE80211_F_SCAN) == 0, ("scanning"));
KASSERT(vap->iv_state >= IEEE80211_S_RUN,
--- 104,114 ----
* Handle a beacon miss event. The common code filters out
* spurious events that can happen when scanning and/or before
* reaching RUN state.
*/
static void
! sta_beacon_miss_locked(struct ieee80211vap *vap)
{
struct ieee80211com *ic = vap->iv_ic;
KASSERT((ic->ic_flags & IEEE80211_F_SCAN) == 0, ("scanning"));
KASSERT(vap->iv_state >= IEEE80211_S_RUN,
***************
*** 172,181 ****
--- 173,193 ----
*/
ieee80211_new_state(vap, IEEE80211_S_SCAN, 0);
}
}
+ void
+ sta_beacon_miss(struct ieee80211vap *vap)
+ {
+ struct ieee80211com *ic = vap->iv_ic;
+
+ IEEE80211_LOCK(ic);
+ sta_beacon_miss_locked(vap);
+ IEEE80211_UNLOCK(ic);
+ }
+
+
/*
* Handle deauth with reason. We retry only for
* the cases where we might succeed. Otherwise
* we downgrade the ap and scan.
*/
-----Original Message-----
From: adrian.chadd at gmail.com [mailto:adrian.chadd at gmail.com] On Behalf Of Adrian Chadd
Sent: Wednesday, September 28, 2011 10:56 PM
To: Edgar Martinez
Cc: freebsd-wireless at freebsd.org
Subject: Re: PANIC - SWBMISS (9.0-CURRENT)
Hm, how are the interfaces configured? You're doing wds, I wonder how
the heck that works. :)
There's a locking problem with how software beacon miss is handled:
* sta_beacon_miss doesn't grab any lock, so I think its possible that
the swbmiss callout is being run on another CPU whilst
sta_becaon_miss() stops the callout and changes the state to S_ASSOC
or S_SCAN
* sta_newstate() grabs the ic lock, then it changes the state
(vap->iv_state = nstate) before it cancels the callout. Again, the
problem here is that the swbmiss callout may be scheduled during this
and there's no locking in sta_beacon_miss.
What should the solution be?
* should sta_beacon_miss (and the tdma one too?) grab the ic lock? i
think so, but I'd have to audit all the functions that it calls to
ensure it can be called with the ic lock held. In fact, I did a 30
second audit and it can't just hold the ic lock - as the calls to
ieeee80211_new_state() need the ic lock to be not held as it grabs the
lock itself. So we can't hold the lock for the whole function.
* should ieee80211_swbmiss() be called with the ic lock held? I think
so. That way if something external changes state, it should first grab
the lock, change the state, then cancel the swbmiss timer.
What I think should happen is that the beacon miss handler should be
called with the ic lock held. That way a state change can't occur
whilst its processing. That's going to take a bit of auditing though.
So here, try this patch; it may make things worse, it may make things
slightly better. I'm not sure. It just tries to eliminate a couple of
the race conditions that I found and makes sure ieee80211_swbmiss is
called with the ic lock held. I haven't tried to fix the more general
problem though.
Adrian
[adrian at pcbsd-2547] /data/freebsd/mips/if_ath_tx/src/sys/net80211> svn
diff ieee80211_sta.c ieee80211_tdma.c ieee80211_proto.c
Index: ieee80211_sta.c
===================================================================
--- ieee80211_sta.c (revision 225723)
+++ ieee80211_sta.c (working copy)
@@ -145,7 +145,9 @@
return;
}
+ IEEE80211_LOCK(ic);
callout_stop(&vap->iv_swbmiss);
+ IEEE80211_UNLOCK(ic);
vap->iv_bmiss_count = 0;
vap->iv_stats.is_beacon_miss++;
if (vap->iv_roaming == IEEE80211_ROAMING_AUTO) {
Index: ieee80211_tdma.c
===================================================================
--- ieee80211_tdma.c (revision 225610)
+++ ieee80211_tdma.c (working copy)
@@ -295,7 +295,9 @@
"beacon miss, mode %u state %s\n",
vap->iv_opmode, ieee80211_state_name[vap->iv_state]);
+ IEEE80211_LOCK(ic);
callout_stop(&vap->iv_swbmiss);
+ IEEE80211_UNLOCK(ic);
if (ts->tdma_peer != NULL) { /* XXX? can this be null? */
ieee80211_notify_node_leave(vap->iv_bss);
Index: ieee80211_proto.c
===================================================================
--- ieee80211_proto.c (revision 225610)
+++ ieee80211_proto.c (working copy)
@@ -193,7 +193,8 @@
vap->iv_rtsthreshold = IEEE80211_RTS_DEFAULT;
vap->iv_fragthreshold = IEEE80211_FRAG_DEFAULT;
vap->iv_bmiss_max = IEEE80211_BMISS_MAX;
- callout_init(&vap->iv_swbmiss, CALLOUT_MPSAFE);
+ callout_init_mtx(&vap->iv_swbmiss, IEEE80211_LOCK_OBJ(ic),
+ CALLOUT_MPSAFE);
callout_init(&vap->iv_mgtsend, CALLOUT_MPSAFE);
TASK_INIT(&vap->iv_nstate_task, 0, ieee80211_newstate_cb, vap);
TASK_INIT(&vap->iv_swbmiss_task, 0, beacon_swmiss, vap);
@@ -1448,6 +1449,8 @@
struct ieee80211vap *vap = arg;
struct ieee80211com *ic = vap->iv_ic;
+ IEEE80211_LOCK_ASSERT(ic);
+
/* XXX sleep state? */
KASSERT(vap->iv_state == IEEE80211_S_RUN,
("wrong state %d", vap->iv_state));
More information about the freebsd-wireless
mailing list