Date: Sat, 5 Feb 2011 01:33:40 +0300 From: Alexander Zagrebin <alex@zagrebin.ru> To: Bernhard Schmidt <bschmidt@freebsd.org> Cc: freebsd-net@freebsd.org Subject: Re: if_run in hostap mode: issue with stations in the power save mode Message-ID: <20110204223339.GA12555@gw.zagrebin.ru> In-Reply-To: <201102040951.34201.bschmidt@freebsd.org> References: <20110204060808.GA97298@gw.zagrebin.ru> <201102040951.34201.bschmidt@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--s2ZSL+KKDSLx8OML
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hi!
On 04.02.2011 09:51:34 +0100, Bernhard Schmidt wrote:
> On Friday 04 February 2011 07:08:08 Alexander Zagrebin wrote:
> > I'm using an Ralink RT2870 based adapter (run(4) driver) in the
> > hostap mode. and I've noticed that if_run doesn't support stations
> > working in the power save mode (PSM). The reason is in lack of the
> > TIM in beacons. The attached patch adds this functionality and
> > completely fixes this issue. Despite the fact that patch is working,
> > it seems that it needs an additional work. For example, now the
> > result of ieee80211_beacon_update is ignored with a corresponding
> > message, but may be necessary to process it...
> >
> > Can somebody review it?
>
> That looks about right, good catch!
>
> Handling ieee80211_beacon_update()'s return value doesn't seem to be
> necessary, the mbuf's length is handled in the next few lines of code
> anyways, doesn't matter if it changed or not.
>
> Though, I have a some doubts about just restoring bo_flags is enough
> (Can't prove that with some obvious code, still..). It feels saner to me
> if we just reuse the whole mbuf, similar to what ath(4) does. Can you
> look at attached patch? Completely untested, so I'm not sure what does
> happen on e.g. changing the SSID.
I've tried the slightly modified version of your patch (see attached files),
and found that it is working too. Moreover, it looks more safe.
For example, suppose we need update the beacon due to a new TIM.
Immediately after updating, but before the beacon with a TIM will be
transmitted, we need update the beacon again for any other reason. With
the first version of the patch the beacon will completely recreated, so
a TIM will be lost. But if we are using the second version of the patch,
then TIM will be preserved.
I had the doubts about ability to change or hide/unhide the SSID, but it
works too. It seems that after `ifconfig wlan0 ssid ...` or `ifconfig wlan0
(hidessid|-hidessid)` the following occurs:
1. run_newstate() is called
2. run_newstate() invokes run_update_beacon_cb()
3. run_update_beacon_cb() invokes ieee80211_beacon_update()
But I couldn't understand where ieee80211_beacon_update() can change a SSID...
--
Alexander Zagrebin
--s2ZSL+KKDSLx8OML
Content-Type: text/x-patch; charset=us-ascii
Content-Disposition: attachment; filename="patch-if_run.c"
--- sys/dev/usb/wlan/if_run.c.orig 2011-01-21 08:28:14.000000000 +0300
+++ sys/dev/usb/wlan/if_run.c 2011-02-04 21:01:38.659409262 +0300
@@ -856,6 +856,8 @@
RUN_LOCK(sc);
+ m_freem(rvp->beacon_mbuf);
+
rvp_id = rvp->rvp_id;
sc->ratectl_run &= ~(1 << rvp_id);
sc->rvp_bmap &= ~(1 << rvp_id);
@@ -3903,6 +3905,7 @@
struct run_softc *sc = ic->ic_ifp->if_softc;
uint32_t i;
+ setbit(RUN_VAP(vap)->bo.bo_flags, item);
i = RUN_CMDQ_GET(&sc->cmdq_store);
DPRINTF("cmdq_store=%d\n", i);
sc->cmdq[i].func = run_update_beacon_cb;
@@ -3916,6 +3919,7 @@
run_update_beacon_cb(void *arg)
{
struct ieee80211vap *vap = arg;
+ struct run_vap *rvp = RUN_VAP(vap);
struct ieee80211com *ic = vap->iv_ic;
struct run_softc *sc = ic->ic_ifp->if_softc;
struct rt2860_txwi txwi;
@@ -3925,8 +3929,14 @@
if (vap->iv_bss->ni_chan == IEEE80211_CHAN_ANYC)
return;
- if ((m = ieee80211_beacon_alloc(vap->iv_bss, &RUN_VAP(vap)->bo)) == NULL)
- return;
+ if (rvp->beacon_mbuf == NULL) {
+ m = ieee80211_beacon_alloc(vap->iv_bss, &rvp->bo);
+ if (m == NULL) return;
+ rvp->beacon_mbuf = m;
+ } else {
+ m = rvp->beacon_mbuf;
+ ieee80211_beacon_update(vap->iv_bss, &rvp->bo, m, 1);
+ }
memset(&txwi, 0, sizeof txwi);
txwi.wcid = 0xff;
@@ -3941,13 +3951,11 @@
txwi.flags = RT2860_TX_TS;
txwi.xflags = RT2860_TX_NSEQ;
- run_write_region_1(sc, RT2860_BCN_BASE(RUN_VAP(vap)->rvp_id),
+ run_write_region_1(sc, RT2860_BCN_BASE(rvp->rvp_id),
(uint8_t *)&txwi, sizeof txwi);
- run_write_region_1(sc, RT2860_BCN_BASE(RUN_VAP(vap)->rvp_id) + sizeof txwi,
+ run_write_region_1(sc, RT2860_BCN_BASE(rvp->rvp_id) + sizeof txwi,
mtod(m, uint8_t *), (m->m_pkthdr.len + 1) & ~1); /* roundup len */
- m_freem(m);
-
return;
}
--s2ZSL+KKDSLx8OML
Content-Type: text/x-patch; charset=us-ascii
Content-Disposition: attachment; filename="patch-if_runvar.h"
--- sys/dev/usb/wlan/if_runvar.h.orig 2010-11-20 10:22:53.446928634 +0300
+++ sys/dev/usb/wlan/if_runvar.h 2011-02-04 20:03:08.090731320 +0300
@@ -121,6 +121,7 @@
struct run_vap {
struct ieee80211vap vap;
struct ieee80211_beacon_offsets bo;
+ struct mbuf *beacon_mbuf;
int (*newstate)(struct ieee80211vap *,
enum ieee80211_state, int);
--s2ZSL+KKDSLx8OML--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110204223339.GA12555>