Date: Fri, 4 Feb 2011 20:45:59 -0800 (PST) From: PseudoCylon <moonlightakkiy@yahoo.ca> To: Bernhard Schmidt <bschmidt@freebsd.org>, Alexander Zagrebin <alex@zagrebin.ru> Cc: freebsd-net@freebsd.org Subject: Re: if_run in hostap mode: issue with stations in the power save mode Message-ID: <147585.89029.qm@web39304.mail.mud.yahoo.com> In-Reply-To: <201102041314.17939.bschmidt@freebsd.org> References: <20110204060808.GA97298@gw.zagrebin.ru> <201102040951.34201.bschmidt@freebsd.org> <20110204114021.GA237@gw.zagrebin.ru> <201102041314.17939.bschmidt@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
----- Original Message ---- > From: Bernhard Schmidt <bschmidt@freebsd.org> > To: Alexander Zagrebin <alex@zagrebin.ru> > Cc: freebsd-net@freebsd.org; PseudoCylon <moonlightakkiy@yahoo.ca> > Sent: Fri, February 4, 2011 5:14:17 AM > Subject: Re: if_run in hostap mode: issue with stations in the power save mode > > On Friday 04 February 2011 12:40:21 Alexander Zagrebin wrote: > > 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 thought about such solution, and it looks more right, but I've > > decided just to add ieee80211_beacon_update() to make the patch > > clear. I'll try your patch a bit later, but I already have a > > question: on the first invocation of the run_update_beacon_cb() only > > ieee80211_beacon_alloc() will be called. So dynamic beacon contents > > will not updated. Is it a problem? > > I don't think it is. The work beacon_update does is handling changes to > bo_flags, which are only changed through calls to iv_update_beacon(), so > this is safe, because the driver itself does change bo_flags which is > immediately followed by the beacon update process. > I like the way mwl(4) handles it. http://fxr.watson.org/fxr/source/dev/mwl/if_mwl.c#L1927 though I don't know why it uses ieee80211_beacon_alloc() instead of _update() @@run_update_beacon(struct ieee80211vap *vap, int item) struct run_vap *rvp = RUN_VAP(vap); +int mcast = 0; uint32_t i; +KASSERT(vap != NULL, ("no beacon")); + +switch (item) { +case IEEE80211_BEACON_ERP: +run_updateslot(ic->ic_ifp); +break; +case IEEE80211_BEACON_HTINFO: +run_updateprot(ic); +break; +case IEEE80211_BEACON_TIM: +mcast = 1;/*TODO*/ +break; +default: + break; } +setbit(rvp->bo.bo_flags, item); +ieee80211_beacon_update(vap->iv_bss, &rvp->bo, rvp->bm, mcast); + i = RUN_CMDQ_GET(&sc->cmdq_store); DPRINTF("cmdq_store=%d\n", i); sc->cmdq[i].func = run_update_beacon_cb; It's been working fine updating protection mode in HT mode past a few days. (Some how, iwn(4) works fine with run(4), I cannot tell it works fin with power saving mode.) I'd like to move ieee80211_beacon_alloc() into iv_vap_alloc(). Then we don't need to test beacon_mbuf == NULL in run_update_beacon_cb(), and there is already switch we can use for conditionally alloc mem. > There is one expection I'm not sure about how to handle though, > slottime. This value might change based on nodes associating and > leaving, resulting in a call to ic_updateslot() which is currently > commentted out. > That's only because of LOR. I'm adding another call back function since run_updateprot() need to be deferred when it is called from run_update_beacon(). AK
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?147585.89029.qm>