From owner-freebsd-net@FreeBSD.ORG Sat Feb 5 04:46:02 2011 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 39ECD1065673 for ; Sat, 5 Feb 2011 04:46:02 +0000 (UTC) (envelope-from moonlightakkiy@yahoo.ca) Received: from web39304.mail.mud.yahoo.com (web39304.mail.mud.yahoo.com [66.94.238.171]) by mx1.freebsd.org (Postfix) with SMTP id AE87D8FC15 for ; Sat, 5 Feb 2011 04:46:01 +0000 (UTC) Received: (qmail 93959 invoked by uid 60001); 5 Feb 2011 04:46:00 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.ca; s=s1024; t=1296881160; bh=huFq3B+d8Eld5CCrgFdpYSodmsTr97yM/LFES3p+RY0=; h=Message-ID:X-YMail-OSG:Received:X-Mailer:References:Date:From:Subject:To:Cc:In-Reply-To:MIME-Version:Content-Type; b=wESb59pHQVQp+T2uvNBeMBnyB/jIIilVBh7GQrsIRH3igGzLjR0/Ro+UvQ/8C+1RJDVlRaXUXoYXXrEhcVv0uNONmREzlBlNGM1eowiFi5RQ3bAUFak39yviHVqfOJGeiX4IT6o0Ks5chL/ekXx+nwC0ot9ThmQYMyuIjfluRlQ= DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=s1024; d=yahoo.ca; h=Message-ID:X-YMail-OSG:Received:X-Mailer:References:Date:From:Subject:To:Cc:In-Reply-To:MIME-Version:Content-Type; b=LxU1pv6AOEdRPpW949z1XCEeK6q9xsfWuuQxyr3IqRby/NSYDqFO80+9ASbFEHzfNJDeyxGqDYQPTFmhq1CnW2B5W0Lrrx+Kx5hbYAE6tf37F0z/OHYhM9jsQgkXhTe87+imPx8VVkefSolUOUfViN42vIcQKNtEYYwkAL2X7TI=; Message-ID: <147585.89029.qm@web39304.mail.mud.yahoo.com> X-YMail-OSG: PfvUXvwVM1m_bQE7lOxJtqz3_deMaiyX6h1PG6X_rLnxR_F sEQzbZVV2zpFwaIANachdYFdO8o_.OF6C2maWE7EoFnCmc9ikPVUYRlT1Ftx z.mcnMQqwFB7gtT50fgfX_yPurAUs1kxTAsje4wxorZOpuTqqacY28NMKkRM lCxotUkHRW4.sfSb.jqmr6QlnQ9v11E0pUdveKvYQVK8yU7lZ6dVIWAKSoXq i1FI41TSeb69Ll9y0WBbcfq3Rrp5sblf0x7Vt5ox7LQB_r8E0ouzXY8dYdU5 pCgH77w4NvLXqnFXXUiJgK23eqHa8SIre2HD8OAIykMtSX9d5h_WS Received: from [206.75.146.55] by web39304.mail.mud.yahoo.com via HTTP; Fri, 04 Feb 2011 20:45:59 PST X-Mailer: YahooMailRC/555 YahooMailWebService/0.8.108.291010 References: <20110204060808.GA97298@gw.zagrebin.ru> <201102040951.34201.bschmidt@freebsd.org> <20110204114021.GA237@gw.zagrebin.ru> <201102041314.17939.bschmidt@freebsd.org> Date: Fri, 4 Feb 2011 20:45:59 -0800 (PST) From: PseudoCylon To: Bernhard Schmidt , Alexander Zagrebin In-Reply-To: <201102041314.17939.bschmidt@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: freebsd-net@freebsd.org Subject: Re: if_run in hostap mode: issue with stations in the power save mode X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Feb 2011 04:46:02 -0000 ----- Original Message ---- > From: Bernhard Schmidt > To: Alexander Zagrebin > Cc: freebsd-net@freebsd.org; PseudoCylon > 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