I have two systems under test, both Dell laptops (a Latitude D630 and an
Inspiron 600m) both running Ubuntu 9.10 with the latest updates, and
bleeding edge compat-wireless-2009-11-02. I'm using identical AR9170
based D-Link DWA-160 USB 802.11adapters. I'm using nuttcp to measure
throughput. I'm running in ad-hoc mode. Both machines have the same
ar9170 files in /lib/firmware. The machines are sitting about 5 feet
apart in my office.
I'm having occasional problems where throughput drops through the floor
(0.5Mbps - 1.5Mbps). When I cat
/sys/kernel/debug/ieee80211/*/stations/*/rc_stats, one of the machines
lists the full set of rates, but the other only lists 1M and 54M. After
a period of time, that machine drops 54M and lists only one rate
(1Mbps), and the throughput listed by nuttcp drops accordingly. I
assume that, for whatever reason, the rates drop off the list and
minstrel uses the only one left available to it.
If I modify include/net/mac80211.h and force the inline function
rate_supported to always return 1, this fixes the problem. However, I
think this is a band aid around some other issue.
Any clues or ideas what the real issue might be here?
On Tue, 2009-11-24 at 09:05 -0800, Adam Wozniak wrote:
> In Ad-Hoc mode, it is possible for minstrel to be initialized without complete rate information.
> In these cases, throughput suffers because not all rates are tried. This patch tells minstrel to try all rates.
> This won't cause problems, because unsupported rates will simply fail, causing minstrel to set their probability to 0%.
> (Hopefully not whitespace damaged this time)
I still stand by this being the wrong thing to do
johannes
> Signed-off-by: Adam Wozniak <[email protected]>
> ---
> diff --git a/net/mac80211/rc80211_minstrel.c
> b/net/mac80211/rc80211_minstrel.c
> index 6e5d68b..3a8fe30 100644
> --- a/net/mac80211/rc80211_minstrel.c
> +++ b/net/mac80211/rc80211_minstrel.c
> @@ -395,8 +395,6 @@ minstrel_rate_init(void *priv, struct
> ieee80211_supported_band *sband,
> unsigned int tx_time_single;
> unsigned int cw = mp->cw_min;
>
> - if (!rate_supported(sta, sband->band, i))
> - continue;
> n++;
> memset(mr, 0, sizeof(*mr));
>
>
>
On Thursday 12 November 2009 20:43:22 Adam Wozniak wrote:
> I was hoping for more of an "ah-ha!" response. =)
>
> It worked well initially, but when I let it run overnight it fell back
> into that same failure mode.
>
great... :\
what about this patch?
---
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index fbffce9..bde89f7 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -250,6 +250,9 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
supp_rates = ieee80211_sta_get_rates(local, elems, band);
+ /* make sure mandatory rates are always added */
+ supp_rates |= ieee80211_mandatory_rates(local, band);
+
rcu_read_lock();
sta = sta_info_get(local, mgmt->sa);
@@ -257,9 +260,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
- /* make sure mandatory rates are always added */
- sta->sta.supp_rates[band] = supp_rates |
- ieee80211_mandatory_rates(local, band);
+ sta->sta.supp_rates[band] = supp_rates;
#ifdef CONFIG_MAC80211_IBSS_DEBUG
if (sta->sta.supp_rates[band] != prev_rates)
On Thursday 12 November 2009 23:38:07 Adam Wozniak wrote:
> I see what you're doing there.
> That didn't quite work, but I'm fairly confident this one will.
yep, what about the attached version? ...it's slightly different...
BTW: Use "diff -up" or "diff -uprN" to create patches. (or git :) )
---
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index fbffce9..7c6c170 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -246,9 +246,12 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
+ /* make sure mandatory rates are always added */
+ supp_rates = ieee80211_mandatory_rates(local, band);
+
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
- supp_rates = ieee80211_sta_get_rates(local, elems, band);
+ supp_rates |= ieee80211_sta_get_rates(local, elems, band);
rcu_read_lock();
@@ -257,9 +260,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
- /* make sure mandatory rates are always added */
- sta->sta.supp_rates[band] = supp_rates |
- ieee80211_mandatory_rates(local, band);
+ sta->sta.supp_rates[band] = supp_rates;
#ifdef CONFIG_MAC80211_IBSS_DEBUG
if (sta->sta.supp_rates[band] != prev_rates)
@@ -272,6 +273,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
(unsigned long long) supp_rates,
(unsigned long long) sta->sta.supp_rates[band]);
#endif
+ rate_control_rate_init(sta);
} else
ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
In your example where A&C can pass data, but not hear each other's
beacons, how is rate information passed between them? ProbeReq/Resp ?
Derek Smithies wrote:
> Hi,
> Statistics is a wonderful thing.
> Every node is required to send a beacon at time
> X+r, X+x+r, X+2x+r, X+3x+r, ......
>
> All nodes are agreed on the value of x (which is the beacon interval).
> All nodes are agreed on the value of X
> r is a random value, and is (from memory) 20 slots long.
>
> given this, all nodes work (to borrow an analogy from music) in time,
> or beat in sync with each other.
>
> Now, if a node hears a beacon on its BSSID inside that r period, the
> node will not transmit a beacon. This way, for a 20 node network in a
> room, you should only get 1 (or sometimes 2) beacons transmitted every
> beacon interval.
>
> If we assume that every node correctly attempts to send a beacon
> somewhere in that period r, and that somewhere is randomly
> distributed, then we will hear a beacon from most nodes, which is good
> enough.
>
> Consider the case of three nodes, A, B, C.
>
> A and B are turned on, and create an Adhoc network. A and B agree on
> a)supported rates
> b)the value of X, the value of x
> c)the channel
> d)the ESSSID
> and so start sending a beacon. Inside this beacon is BSSID. The BSSID
> is a random value. The particular random value used implies acceptance
> of a,b,c,d. Look at the name. Basic Service Set ID. The basic service
> set is the collection of those values a,b,c,d.
>
> Now, node C is turned on. A is positioned such that A&C cannot
> communicate. However, B can communicate with A&C.
>
> C is turned on, detects the presence of B, likes B's beacons, and
> agrees on all the settings in B's beacons. In other words, C likes and
> agrees with a,b,c,d.
> So C starts sending beacons with the same BSSID as B.
> At this point, it does not matter that A cannot set C's beacons. A and
> C have agreed on the BSSID.
>
> Change the story a little bit.
> In this locality, there is often a burst of 1ms of noise every 2ms.
> This means that most beacons are shotdown. However, data packets at
> 54Mbit/sec get through.
> A&B saw each others beacons and agreed on a BSSID. C was turned on,
> and agreed with the BSSID of B.
>
> C sends out data packets, with the BSSID of B. A sees the datapackets
> of C. Since the datapackets of C have A's BSSID, A has to accept them.
>
> Now, you see where this is all going. What is the meaning of a beacon
> containing a BSSID of all zero ?
> Further, you see that all nodes do need to send a beacon. This makes
> node discovery a little easier.
> Even though A and C cannot see each others beacons, they should still
> communicate as they have the same agreed on BSSID.
>
> Derek.
>
>
> On Mon, 16 Nov 2009, Adam Wozniak wrote:
>
>> This assumption seems too stoichastic. Reading 802.11-2007 section
>> 11.1.2.2, it doesn't seem that we're guaranteed to always receive
>> beacons from all stations. Stations will cancel their pending beacon
>> transmission if they receive a beacon before their random delay times
>> out. In the extreme case where the number of stations is very large,
>> it seems possible that you may never hear beacons for some stations.
>>
>> Johannes Berg wrote:
>>> On Mon, 2009-11-16 at 09:25 -0800, Adam Wozniak wrote:
>>>
>>>> If we have only three stations in an ad-hoc network, where all
>>>> three can hear the other two, only one of them should be beaconing,
>>>> correct?
>>>
>>> No, if they all behave correctly beaconing will be distributed.
>>>
>>> johannes
>>>
>>
>>
>>
>
On Wed, Nov 4, 2009 at 2:31 PM, Christian Lamparter
<[email protected]> wrote:
> On Wednesday 04 November 2009 23:20:07 Luis R. Rodriguez wrote:
>> On Wed, Nov 04, 2009 at 11:18:13PM +0100, Christian Lamparter wrote:
>> > On Wednesday 04 November 2009 22:50:46 Luis R. Rodriguez wrote:
>> > > On Wed, Nov 4, 2009 at 1:46 PM, Adam Wozniak <[email protected]> wrote:
>> > > >
>> > > > $ ls -la /lib/firmware/ar9170*
>> > > > -rw-r--r-- 1 root root 83968 2009-10-17 15:55 /lib/firmware/ar9170-1.fw
>> > > > -rw-r--r-- 1 root root 3508 2009-10-17 15:55 /lib/firmware/ar9170-2.fw
>> > > > -rw-r--r-- 1 root root 15960 2009-10-17 15:55 /lib/firmware/ar9170.fw
>> > > >
>> > > > It is unclear to me which are actually used.
>> > >
>> > > By default the ar9170.fw is tried first, if that fails then the others
>> > > are tried. The 2-stage firmware will not be tried if your device
>> > > cannot handle it but right now only the AVM Fritz devices can't handle
>> > > the 2-stage firmware.
>> > >
>> > > Christian should we just remove 2-stage fw support?
>> >
>> > I've moved the two-stage fw into the legacy section some time ago :)
>> > Maybe a add printk?
>>
>> Oh sorry didn't notice, I meant complete removal of its support on the
>> driver though :)
> Oh, I've no problem removing two-stage fw support.
Ok cool.
> But then, I don't know much about usability :-D and I've enough of
> people banging their heads against each other.
Not sure I got this part.
Luis
Johannes Berg wrote:
> On Tue, 2009-11-24 at 09:17 -0800, Adam Wozniak wrote:
>
>> I do not believe 1/2 was a hack. It really was possible to get to the
>> end of that function and use that variable without it being assigned the
>> correct value. Also, it is important to reinitialize the rate control
>> layer when the list of usable rates changes.
>>
> Ah, indeed. But I still don't think the rate control _init() function
> should be called again.
>
My first thought was to use rate_control_ops.rate_update (via rate.h's
rate_control_rate_update() function).
However, neither minstrel nor pid_algo implement it, and the only place
it is actually used is from ieee80211_enable_ht() (in mlme.c) when the
channel type changes.
So the only way (currently) to "update" is call _init() again.
Unless you want to make rate_control_rate_update() do this:
if (ref->ops->rate_update)
ref->ops->rate_update(ref->priv, sband, ista, priv_sta,
changed);
else if (ref->ops->rate_init)
ref->ops->rate_update(ref->priv, sband, ista, priv_sta);
But that seems equally distasteful.
On Wed, Nov 04, 2009 at 11:18:13PM +0100, Christian Lamparter wrote:
> On Wednesday 04 November 2009 22:50:46 Luis R. Rodriguez wrote:
> > On Wed, Nov 4, 2009 at 1:46 PM, Adam Wozniak <[email protected]> wrote:
> > >
> > > $ ls -la /lib/firmware/ar9170*
> > > -rw-r--r-- 1 root root 83968 2009-10-17 15:55 /lib/firmware/ar9170-1.fw
> > > -rw-r--r-- 1 root root 3508 2009-10-17 15:55 /lib/firmware/ar9170-2.fw
> > > -rw-r--r-- 1 root root 15960 2009-10-17 15:55 /lib/firmware/ar9170.fw
> > >
> > > It is unclear to me which are actually used.
> >
> > By default the ar9170.fw is tried first, if that fails then the others
> > are tried. The 2-stage firmware will not be tried if your device
> > cannot handle it but right now only the AVM Fritz devices can't handle
> > the 2-stage firmware.
> >
> > Christian should we just remove 2-stage fw support?
>
> I've moved the two-stage fw into the legacy section some time ago :)
> Maybe a add printk?
Oh sorry didn't notice, I meant complete removal of its support on the
driver though :)
Luis
This assumption seems too stoichastic. Reading 802.11-2007 section
11.1.2.2, it doesn't seem that we're guaranteed to always receive
beacons from all stations. Stations will cancel their pending beacon
transmission if they receive a beacon before their random delay times
out. In the extreme case where the number of stations is very large, it
seems possible that you may never hear beacons for some stations.
Johannes Berg wrote:
> On Mon, 2009-11-16 at 09:25 -0800, Adam Wozniak wrote:
>
>> If we have only three stations in an ad-hoc network, where all three can
>> hear the other two, only one of them should be beaconing, correct?
>>
>
> No, if they all behave correctly beaconing will be distributed.
>
> johannes
>
Reading through the 802.11 spec, it appears to me that "Supported rates"
(and "Extended Supported Rates" when number of rates > 8) is REQUIRED
for all management frames except authentication, deauthentication, and
action frames. (IEEE 802.11-2007, 7.2.3)
Do you know which frames in the mac80211 code are missing this required
information? Or was that conjecture?
Looking at mac80211/tx.c ieee80211_beacon_get_tim, it is not clear to me
how (or if) this rate information is being set for ad-hoc beacons.
Derek Smithies wrote:
> Some management frames don't contain a full report of the rates
> supported by the sender.
> My view is that node A (in this example) is incorrectly determining
> that B only supports the 1mb/sec rate. Consequently, node A fills the
> rate_supported array with one rate - 1mb/sec.
On Mon, 16 Nov 2009, Adam Wozniak wrote:
> It seems like the code I pointed out earlier in rx.c is certainly wrong then;
> bitrates used in absence of other information should not be guessed based on
> the received rate, but should come instead from the BSSID. Correct?
Yes.
that is what the BSSID is - the definition of the basis service set. To
use a particular BSSID is an implicit acceptance of the
>>>> a)supported rates
>>>> b)the value of X, the value of x
>>>> c)the channel
>>>> d)the ESSSID
from an existing network.
>
> Although, once you start these silly examples, it seems to me like the best
> thing to do is just assume the receiver can handle anything and let minstrel
> sort out what works and what doesn't. This has obvious problems for PID and
> similar rate algorithms.
These silly examples illustrate deficiencies in the spec.
We do have to know about them, as we need to know what will happen out
there.
> This has obvious problems for PID and similar rate algorithms.
it may do - but who uses PID now? My understanding was that Minstrel has
proven to be the better approach.
Derek.
=============================================================
> Derek Smithies wrote:
>> Hi,
>>
>> Referring back to my letter below, you see that when A&B agree on their
>> BSSID, this implies that A&B have agreed on:
>>
>>>> a)supported rates
>>>> b)the value of X, the value of x
>>>> c)the channel
>>>> d)the ESSSID
>>
>>
>> When B&C start talking, and C adopts the same BSSID as B, then we have (by
>> inference) that C has agreed to the same rates as A. There is no need to
>> pass rate information between C and A.
>>
>> ===========================
>> Ok, now here is a silly example. Suppose C supports bitrates b4,b5 and b6.
>>
>> B does not support bitrates b4,b5 and b6.
>>
>> Further, A does support bitrates b4,b5,b6
>>
>> This could be that B only does 1, 2, 5.5 and 11MBits/sec. But A&C do
>> everything up to 54Mbit/sec.
>>
>>
>> B has caused a problem cause he is deficient (not doing G-Rates)
>>
>> In one view, B should report that he has all the rates of A. B would then
>> hope that A's rate control algorithm will detect that B cannot do the
>> G-rates. Minstrel will do this fine. Stepup-Stepdown rate algorithms will
>> struggle here if their table is constructed in the wrong order.
>>
>> For this network, the BSSID defines the basic service set. you cannot have
>> nodes on the same BSSID who report as handling different rate sets.
>>
>>
>>
>> Derek.
>>
>>
>>
>> On Mon, 16 Nov 2009, Adam Wozniak wrote:
>>
>>> In your example where A&C can pass data, but not hear each other's
>>> beacons, how is rate information passed between them? ProbeReq/Resp ?
>>>
>>> Derek Smithies wrote:
>>>> Hi,
>>>> Statistics is a wonderful thing.
>>>> Every node is required to send a beacon at time
>>>> X+r, X+x+r, X+2x+r, X+3x+r, ......
>>>>
>>>> All nodes are agreed on the value of x (which is the beacon interval).
>>>> All nodes are agreed on the value of X
>>>> r is a random value, and is (from memory) 20 slots long.
>>>>
>>>> given this, all nodes work (to borrow an analogy from music) in time, or
>>>> beat in sync with each other.
>>>>
>>>> Now, if a node hears a beacon on its BSSID inside that r period, the node
>>>> will not transmit a beacon. This way, for a 20 node network in a room,
>>>> you should only get 1 (or sometimes 2) beacons transmitted every beacon
>>>> interval.
>>>>
>>>> If we assume that every node correctly attempts to send a beacon
>>>> somewhere in that period r, and that somewhere is randomly distributed,
>>>> then we will hear a beacon from most nodes, which is good enough.
>>>>
>>>> Consider the case of three nodes, A, B, C.
>>>>
>>>> A and B are turned on, and create an Adhoc network. A and B agree on
>>>> a)supported rates
>>>> b)the value of X, the value of x
>>>> c)the channel
>>>> d)the ESSSID
>>>> and so start sending a beacon. Inside this beacon is BSSID. The BSSID is
>>>> a random value. The particular random value used implies acceptance of
>>>> a,b,c,d. Look at the name. Basic Service Set ID. The basic service set is
>>>> the collection of those values a,b,c,d.
>>>>
>>>> Now, node C is turned on. A is positioned such that A&C cannot
>>>> communicate. However, B can communicate with A&C.
>>>>
>>>> C is turned on, detects the presence of B, likes B's beacons, and agrees
>>>> on all the settings in B's beacons. In other words, C likes and agrees
>>>> with a,b,c,d.
>>>> So C starts sending beacons with the same BSSID as B.
>>>> At this point, it does not matter that A cannot set C's beacons. A and C
>>>> have agreed on the BSSID.
>>>>
>>>> Change the story a little bit.
>>>> In this locality, there is often a burst of 1ms of noise every 2ms. This
>>>> means that most beacons are shotdown. However, data packets at 54Mbit/sec
>>>> get through.
>>>> A&B saw each others beacons and agreed on a BSSID. C was turned on, and
>>>> agreed with the BSSID of B.
>>>>
>>>> C sends out data packets, with the BSSID of B. A sees the datapackets of
>>>> C. Since the datapackets of C have A's BSSID, A has to accept them.
>>>>
>>>> Now, you see where this is all going. What is the meaning of a beacon
>>>> containing a BSSID of all zero ?
>>>> Further, you see that all nodes do need to send a beacon. This makes node
>>>> discovery a little easier.
>>>> Even though A and C cannot see each others beacons, they should still
>>>> communicate as they have the same agreed on BSSID.
>>>>
>>>> Derek.
>>>>
>>>>
>>>> On Mon, 16 Nov 2009, Adam Wozniak wrote:
>>>>
>>>>> This assumption seems too stoichastic. Reading 802.11-2007 section
>>>>> 11.1.2.2, it doesn't seem that we're guaranteed to always receive
>>>>> beacons from all stations. Stations will cancel their pending beacon
>>>>> transmission if they receive a beacon before their random delay times
>>>>> out. In the extreme case where the number of stations is very large, it
>>>>> seems possible that you may never hear beacons for some stations.
>>>>>
>>>>> Johannes Berg wrote:
>>>>>> On Mon, 2009-11-16 at 09:25 -0800, Adam Wozniak wrote:
>>>>>>
>>>>>>> If we have only three stations in an ad-hoc network, where all three
>>>>>>> can hear the other two, only one of them should be beaconing, correct?
>>>>>>
>>>>>> No, if they all behave correctly beaconing will be distributed.
>>>>>>
>>>>>> johannes
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
>
>
>
--
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/
"The only thing IE should be used for is to download Fire Fox"
"My favorite language is call STAR. It's extremely concise. It has
exactly one verb '*', which does exactly what I want at the moment."
--Larry Wall
Whoops, patch was slightly incorrect. This is better:
*** compat-wireless-2009-11-09/net/mac80211/ibss.c 2009-11-08
21:15:06.000000000 -0800
--- compat-wireless-2009-11-09b/net/mac80211/ibss.c 2009-11-12
14:39:12.391545084 -0800
***************
*** 246,254 ****
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
- supp_rates = ieee80211_sta_get_rates(local, elems, band);
rcu_read_lock();
--- 246,258 ----
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
+ supp_rates = ieee80211_sta_get_rates(local, elems, band);
+
+ /* make sure mandatory rates are always added */
+ supp_rates = supp_rates |= ieee80211_mandatory_rates(local, band);
+
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
rcu_read_lock();
***************
*** 257,268 ****
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
! /* make sure mandatory rates are always added */
! sta->sta.supp_rates[band] = supp_rates |
! ieee80211_mandatory_rates(local, band);
#ifdef CONFIG_MAC80211_IBSS_DEBUG
- if (sta->sta.supp_rates[band] != prev_rates)
printk(KERN_DEBUG "%s: updated supp_rates set "
"for %pM based on beacon info (0x%llx | "
"0x%llx -> 0x%llx)\n",
--- 261,270 ----
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
! sta->sta.supp_rates[band] = supp_rates;
+ if (sta->sta.supp_rates[band] != prev_rates) {
#ifdef CONFIG_MAC80211_IBSS_DEBUG
printk(KERN_DEBUG "%s: updated supp_rates set "
"for %pM based on beacon info (0x%llx | "
"0x%llx -> 0x%llx)\n",
***************
*** 272,277 ****
--- 274,285 ----
(unsigned long long) supp_rates,
(unsigned long long) sta->sta.supp_rates[band]);
#endif
+
+ /* TODO: implement rate_update in minstrel/pid,
+ * then change this to rate_control_rate_update.
+ */
+ rate_control_rate_init(sta);
+ }
} else
ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
supp_rates);
Adam Wozniak wrote:
> I see what you're doing there. That didn't quite work, but I'm fairly
> confident this one will. I'm running my long term test now. Note the
> added call to rate_control_init() when the rate is updated. This
> *should* be rate_control_rate_update, but it doesn't look to me like
> that method is implemented in minstrel or the PID code.
>
> *** compat-wireless-2009-11-09/net/mac80211/ibss.c 2009-11-08
> 21:15:06.000000000 -0800
> --- compat-wireless-2009-11-09b/net/mac80211/ibss.c 2009-11-12
> 14:29:16.308550923 -0800
> ***************
> *** 246,254 ****
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
> - supp_rates = ieee80211_sta_get_rates(local, elems, band);
>
> rcu_read_lock();
>
> --- 246,258 ----
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> + supp_rates = ieee80211_sta_get_rates(local, elems, band);
> +
> + /* make sure mandatory rates are always added */
> + supp_rates = supp_rates |= ieee80211_mandatory_rates(local, band);
> +
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
>
> rcu_read_lock();
>
> ***************
> *** 257,268 ****
> u32 prev_rates;
>
> prev_rates = sta->sta.supp_rates[band];
> ! /* make sure mandatory rates are always added */
> ! sta->sta.supp_rates[band] = supp_rates |
> ! ieee80211_mandatory_rates(local, band);
>
> #ifdef CONFIG_MAC80211_IBSS_DEBUG
> ! if (sta->sta.supp_rates[band] != prev_rates)
> printk(KERN_DEBUG "%s: updated supp_rates set "
> "for %pM based on beacon info (0x%llx | "
> "0x%llx -> 0x%llx)\n",
> --- 261,270 ----
> u32 prev_rates;
>
> prev_rates = sta->sta.supp_rates[band];
> ! sta->sta.supp_rates[band] = supp_rates;
>
> #ifdef CONFIG_MAC80211_IBSS_DEBUG
> ! if (sta->sta.supp_rates[band] != prev_rates) {
> printk(KERN_DEBUG "%s: updated supp_rates set "
> "for %pM based on beacon info (0x%llx | "
> "0x%llx -> 0x%llx)\n",
> ***************
> *** 271,276 ****
> --- 273,284 ----
> (unsigned long long) prev_rates,
> (unsigned long long) supp_rates,
> (unsigned long long) sta->sta.supp_rates[band]);
> +
> + /* TODO: implement rate_update in minstrel/pid,
> + * then change this to rate_control_rate_update.
> + */
> + rate_control_rate_init(sta);
> + }
> #endif
> } else
> ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
> supp_rates);
>
>
>
> Christian Lamparter wrote:
>> On Thursday 12 November 2009 20:43:22 Adam Wozniak wrote:
>>
>>> I was hoping for more of an "ah-ha!" response. =)
>>>
>>> It worked well initially, but when I let it run overnight it fell
>>> back into that same failure mode.
>>>
>>>
>>
>> great... :\
>>
>> what about this patch?
>> ---
>> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
>> index fbffce9..bde89f7 100644
>> --- a/net/mac80211/ibss.c
>> +++ b/net/mac80211/ibss.c
>> @@ -250,6 +250,9 @@ static void ieee80211_rx_bss_info(struct
>> ieee80211_sub_if_data *sdata,
>> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
>> supp_rates = ieee80211_sta_get_rates(local, elems, band);
>>
>> + /* make sure mandatory rates are always added */
>> + supp_rates |= ieee80211_mandatory_rates(local, band);
>> +
>> rcu_read_lock();
>>
>> sta = sta_info_get(local, mgmt->sa);
>> @@ -257,9 +260,7 @@ static void ieee80211_rx_bss_info(struct
>> ieee80211_sub_if_data *sdata,
>> u32 prev_rates;
>>
>> prev_rates = sta->sta.supp_rates[band];
>> - /* make sure mandatory rates are always added */
>> - sta->sta.supp_rates[band] = supp_rates |
>> - ieee80211_mandatory_rates(local, band);
>> + sta->sta.supp_rates[band] = supp_rates;
>>
>> #ifdef CONFIG_MAC80211_IBSS_DEBUG
>> if (sta->sta.supp_rates[band] != prev_rates)
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
I was hoping for more of an "ah-ha!" response. =)
It worked well initially, but when I let it run overnight it fell back
into that same failure mode.
Derek Smithies wrote:
> Hi,
>
>> Is it possible this is the problem? (note supp_rates is used at the
>> bottom of the function, outside the "if")
>
> Could be. Did it fix your problem?
>
> Derek.
>
> On Tue, 10 Nov 2009, Adam Wozniak wrote:
>
>> Is it possible this is the problem? (note supp_rates is used at the
>> bottom of the function, outside the "if")
>>
>> *** a/net/mac80211/ibss.c 2009-11-02 09:11:36.000000000 -0800
>> --- b/net/mac80211/ibss.c 2009-11-10 16:31:46.291563951 -0800
>> ***************
>> *** 246,254 ****
>> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
>> return;
>>
>> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
>> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
>> - supp_rates = ieee80211_sta_get_rates(local, elems, band);
>>
>> rcu_read_lock();
>>
>> --- 246,255 ----
>> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
>> return;
>>
>> + supp_rates = ieee80211_sta_get_rates(local, elems, band);
>> +
>> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
>> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
>>
>> rcu_read_lock();
>>
>>
>>
>> Derek Smithies wrote:
>>> Hi,
>>>> Do you know which frames in the mac80211 code are missing this
>>>> required information? Or was that conjecture?
>>>
>>> it was mostly conjecture. I had done some work with an earlier pull
>>> of compat wireless in adhoc. the rates tried by minstrel were bad -
>>> it was stuck at the slowest rate.
>>>
>>> I too had found that the fix was to adjust the array so that all
>>> rates were marked as supported.
>>>
>>> Derek
>>>
>>> On Tue, 10 Nov 2009, Adam Wozniak wrote:
>>>
>>>> Reading through the 802.11 spec, it appears to me that "Supported
>>>> rates" (and "Extended Supported Rates" when number of rates > 8) is
>>>> REQUIRED for all management frames except authentication,
>>>> deauthentication, and action frames. (IEEE 802.11-2007, 7.2.3)
>>>>
>>>> Do you know which frames in the mac80211 code are missing this
>>>> required information? Or was that conjecture?
>>>>
>>>> Looking at mac80211/tx.c ieee80211_beacon_get_tim, it is not clear
>>>> to me how (or if) this rate information is being set for ad-hoc
>>>> beacons.
>>>>
>>>> Derek Smithies wrote:
>>>>> Some management frames don't contain a full report of the rates
>>>>> supported by the sender.
>>>>> My view is that node A (in this example) is incorrectly
>>>>> determining that B only supports the 1mb/sec rate. Consequently,
>>>>> node A fills the rate_supported array with one rate - 1mb/sec.
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>
On Tue, 2009-11-24 at 10:34 -0800, Adam Wozniak wrote:
> Johannes Berg wrote:
> > Or you can probably call rate_exit and rate_init but I don't know how
> > the algorithms would react if you actually ask it for a rate during that
> > race window?
> >
> There is no rate_exit in rate_control_ops, only init and update. There
> is an rc80211_minstrel_exit() in rc80211_minstrel.c, but that just
> unregisters the module, which I don't think is what you want.
Hah, indeed, I thought it was symmetric, guess it's not.
However, that reminds me that I actually wanted to get rid of
rate_init() since it's mostly not useful to have it separate from
alloc_sta()... oh well.
I suppose if we document clearly that rate_init maybe called multiple
times and audit the existing algorithms the patch could be ok.
johannes
On Mon, 2009-11-16 at 09:57 -0800, Adam Wozniak wrote:
> This assumption seems too stoichastic. Reading 802.11-2007 section
> 11.1.2.2, it doesn't seem that we're guaranteed to always receive
> beacons from all stations. Stations will cancel their pending beacon
> transmission if they receive a beacon before their random delay times
> out. In the extreme case where the number of stations is very large, it
> seems possible that you may never hear beacons for some stations.
That is indeed possible, shouldn't happen in the scenario you were
outlining (three stations) though. It's more likely that one of them
doesn't implement beaconing correctly.
It would make sense either way though to send probe requests and
reacting to them and probe responses, if you want to implement that.
johannes
My guess from this is that I'm using the single stage:
# dmesg | grep ar9170
[ 273.608189] usb 2-1: firmware: requesting ar9170.fw
[ 274.019650] Registered led device: ar9170-phy1::tx
[ 274.019698] Registered led device: ar9170-phy1::assoc
[ 274.019751] usbcore: registered new interface driver ar9170usb
Luis R. Rodriguez wrote:
> On Wed, Nov 4, 2009 at 1:46 PM, Adam Wozniak <[email protected]> wrote:
>
>> $ ls -la /lib/firmware/ar9170*
>> -rw-r--r-- 1 root root 83968 2009-10-17 15:55 /lib/firmware/ar9170-1.fw
>> -rw-r--r-- 1 root root 3508 2009-10-17 15:55 /lib/firmware/ar9170-2.fw
>> -rw-r--r-- 1 root root 15960 2009-10-17 15:55 /lib/firmware/ar9170.fw
>>
>> It is unclear to me which are actually used.
>>
>
> By default the ar9170.fw is tried first, if that fails then the others
> are tried. The 2-stage firmware will not be tried if your device
> cannot handle it but right now only the AVM Fritz devices can't handle
> the 2-stage firmware.
>
> Christian should we just remove 2-stage fw support?
>
> Luis
>
In Ad-Hoc mode, it is possible for minstrel to be initialized without
complete rate information. In these cases, throughput suffers because
not all rates are tried. This patch tells minstrel to try all rates.
This won't cause problems, because unsupported rates will simply fail,
causing minstrel to set their probability to 0%.
Signed-off-by: Adam Wozniak <[email protected]>
---
diff --git a/net/mac80211/rc80211_minstrel.c
b/net/mac80211/rc80211_minstrel.c
index 6e5d68b..3a8fe30 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -395,8 +395,6 @@ minstrel_rate_init(void *priv, struct
ieee80211_supported_band *sband,
unsigned int tx_time_single;
unsigned int cw = mp->cw_min;
- if (!rate_supported(sta, sband->band, i))
- continue;
n++;
memset(mr, 0, sizeof(*mr));
On Tue, 2009-11-24 at 09:55 -0800, Adam Wozniak wrote:
> Johannes Berg wrote:
> > On Tue, 2009-11-24 at 09:17 -0800, Adam Wozniak wrote:
> >
> >> I do not believe 1/2 was a hack. It really was possible to get to the
> >> end of that function and use that variable without it being assigned the
> >> correct value. Also, it is important to reinitialize the rate control
> >> layer when the list of usable rates changes.
> >>
> > Ah, indeed. But I still don't think the rate control _init() function
> > should be called again.
> >
> My first thought was to use rate_control_ops.rate_update (via rate.h's
> rate_control_rate_update() function).
>
> However, neither minstrel nor pid_algo implement it, and the only place
> it is actually used is from ieee80211_enable_ht() (in mlme.c) when the
> channel type changes.
>
> So the only way (currently) to "update" is call _init() again.
I think I'd rather require that they implement it then, and add a new
flag that says "got new info on supported rates".
Or you can probably call rate_exit and rate_init but I don't know how
the algorithms would react if you actually ask it for a rate during that
race window?
johannes
It seems like the code I pointed out earlier in rx.c is certainly wrong
then; bitrates used in absence of other information should not be
guessed based on the received rate, but should come instead from the
BSSID. Correct?
Although, once you start these silly examples, it seems to me like the
best thing to do is just assume the receiver can handle anything and let
minstrel sort out what works and what doesn't. This has obvious problems
for PID and similar rate algorithms.
Derek Smithies wrote:
> Hi,
>
> Referring back to my letter below, you see that when A&B agree on
> their BSSID, this implies that A&B have agreed on:
>
>>> a)supported rates
>>> b)the value of X, the value of x
>>> c)the channel
>>> d)the ESSSID
>
>
> When B&C start talking, and C adopts the same BSSID as B, then we have
> (by inference) that C has agreed to the same rates as A. There is no
> need to pass rate information between C and A.
>
> ===========================
> Ok, now here is a silly example. Suppose C supports bitrates b4,b5 and
> b6.
>
> B does not support bitrates b4,b5 and b6.
>
> Further, A does support bitrates b4,b5,b6
>
> This could be that B only does 1, 2, 5.5 and 11MBits/sec. But A&C do
> everything up to 54Mbit/sec.
>
>
> B has caused a problem cause he is deficient (not doing G-Rates)
>
> In one view, B should report that he has all the rates of A. B would
> then hope that A's rate control algorithm will detect that B cannot do
> the G-rates. Minstrel will do this fine. Stepup-Stepdown rate
> algorithms will struggle here if their table is constructed in the
> wrong order.
>
> For this network, the BSSID defines the basic service set. you cannot
> have nodes on the same BSSID who report as handling different rate sets.
>
>
>
> Derek.
>
>
>
> On Mon, 16 Nov 2009, Adam Wozniak wrote:
>
>> In your example where A&C can pass data, but not hear each other's
>> beacons, how is rate information passed between them? ProbeReq/Resp ?
>>
>> Derek Smithies wrote:
>>> Hi,
>>> Statistics is a wonderful thing.
>>> Every node is required to send a beacon at time
>>> X+r, X+x+r, X+2x+r, X+3x+r, ......
>>>
>>> All nodes are agreed on the value of x (which is the beacon interval).
>>> All nodes are agreed on the value of X
>>> r is a random value, and is (from memory) 20 slots long.
>>>
>>> given this, all nodes work (to borrow an analogy from music) in
>>> time, or beat in sync with each other.
>>>
>>> Now, if a node hears a beacon on its BSSID inside that r period, the
>>> node will not transmit a beacon. This way, for a 20 node network in
>>> a room, you should only get 1 (or sometimes 2) beacons transmitted
>>> every beacon interval.
>>>
>>> If we assume that every node correctly attempts to send a beacon
>>> somewhere in that period r, and that somewhere is randomly
>>> distributed, then we will hear a beacon from most nodes, which is
>>> good enough.
>>>
>>> Consider the case of three nodes, A, B, C.
>>>
>>> A and B are turned on, and create an Adhoc network. A and B agree on
>>> a)supported rates
>>> b)the value of X, the value of x
>>> c)the channel
>>> d)the ESSSID
>>> and so start sending a beacon. Inside this beacon is BSSID. The
>>> BSSID is a random value. The particular random value used implies
>>> acceptance of a,b,c,d. Look at the name. Basic Service Set ID. The
>>> basic service set is the collection of those values a,b,c,d.
>>>
>>> Now, node C is turned on. A is positioned such that A&C cannot
>>> communicate. However, B can communicate with A&C.
>>>
>>> C is turned on, detects the presence of B, likes B's beacons, and
>>> agrees on all the settings in B's beacons. In other words, C likes
>>> and agrees with a,b,c,d.
>>> So C starts sending beacons with the same BSSID as B.
>>> At this point, it does not matter that A cannot set C's beacons. A
>>> and C have agreed on the BSSID.
>>>
>>> Change the story a little bit.
>>> In this locality, there is often a burst of 1ms of noise every 2ms.
>>> This means that most beacons are shotdown. However, data packets at
>>> 54Mbit/sec get through.
>>> A&B saw each others beacons and agreed on a BSSID. C was turned on,
>>> and agreed with the BSSID of B.
>>>
>>> C sends out data packets, with the BSSID of B. A sees the
>>> datapackets of C. Since the datapackets of C have A's BSSID, A has
>>> to accept them.
>>>
>>> Now, you see where this is all going. What is the meaning of a
>>> beacon containing a BSSID of all zero ?
>>> Further, you see that all nodes do need to send a beacon. This makes
>>> node discovery a little easier.
>>> Even though A and C cannot see each others beacons, they should
>>> still communicate as they have the same agreed on BSSID.
>>>
>>> Derek.
>>>
>>>
>>> On Mon, 16 Nov 2009, Adam Wozniak wrote:
>>>
>>>> This assumption seems too stoichastic. Reading 802.11-2007 section
>>>> 11.1.2.2, it doesn't seem that we're guaranteed to always receive
>>>> beacons from all stations. Stations will cancel their pending
>>>> beacon transmission if they receive a beacon before their random
>>>> delay times out. In the extreme case where the number of stations
>>>> is very large, it seems possible that you may never hear beacons
>>>> for some stations.
>>>>
>>>> Johannes Berg wrote:
>>>>> On Mon, 2009-11-16 at 09:25 -0800, Adam Wozniak wrote:
>>>>>
>>>>>> If we have only three stations in an ad-hoc network, where all
>>>>>> three can hear the other two, only one of them should be
>>>>>> beaconing, correct?
>>>>>
>>>>> No, if they all behave correctly beaconing will be distributed.
>>>>>
>>>>> johannes
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>
Hi,
Statistics is a wonderful thing.
Every node is required to send a beacon at time
X+r, X+x+r, X+2x+r, X+3x+r, ......
All nodes are agreed on the value of x (which is the beacon interval).
All nodes are agreed on the value of X
r is a random value, and is (from memory) 20 slots long.
given this, all nodes work (to borrow an analogy from music) in time, or
beat in sync with each other.
Now, if a node hears a beacon on its BSSID inside that r period, the node
will not transmit a beacon. This way, for a 20 node network in a room, you
should only get 1 (or sometimes 2) beacons transmitted every beacon
interval.
If we assume that every node correctly attempts to send a beacon somewhere
in that period r, and that somewhere is randomly distributed, then we will
hear a beacon from most nodes, which is good enough.
Consider the case of three nodes, A, B, C.
A and B are turned on, and create an Adhoc network. A and B agree on
a)supported rates
b)the value of X, the value of x
c)the channel
d)the ESSSID
and so start sending a beacon. Inside this beacon is BSSID. The BSSID is
a random value. The particular random value used implies acceptance of
a,b,c,d. Look at the name. Basic Service Set ID. The basic service set is
the collection of those values a,b,c,d.
Now, node C is turned on. A is positioned such that A&C cannot
communicate. However, B can communicate with A&C.
C is turned on, detects the presence of B, likes B's beacons, and agrees
on all the settings in B's beacons. In other words, C likes and agrees
with a,b,c,d.
So C starts sending beacons with the same BSSID as B.
At this point, it does not matter that A cannot set C's beacons. A and C
have agreed on the BSSID.
Change the story a little bit.
In this locality, there is often a burst of 1ms of noise every 2ms. This
means that most beacons are shotdown. However, data packets at 54Mbit/sec
get through.
A&B saw each others beacons and agreed on a BSSID.
C was turned on, and agreed with the BSSID of B.
C sends out data packets, with the BSSID of B. A sees the datapackets of
C. Since the datapackets of C have A's BSSID, A has to accept them.
Now, you see where this is all going. What is the meaning of a beacon
containing a BSSID of all zero ?
Further, you see that all nodes do need to send a beacon. This makes node
discovery a little easier.
Even though A and C cannot see each others beacons, they should still
communicate as they have the same agreed on BSSID.
Derek.
On Mon, 16 Nov 2009, Adam Wozniak wrote:
> This assumption seems too stoichastic. Reading 802.11-2007 section 11.1.2.2,
> it doesn't seem that we're guaranteed to always receive beacons from all
> stations. Stations will cancel their pending beacon transmission if they
> receive a beacon before their random delay times out. In the extreme case
> where the number of stations is very large, it seems possible that you may
> never hear beacons for some stations.
>
> Johannes Berg wrote:
>> On Mon, 2009-11-16 at 09:25 -0800, Adam Wozniak wrote:
>>
>>> If we have only three stations in an ad-hoc network, where all three can
>>> hear the other two, only one of them should be beaconing, correct?
>>
>> No, if they all behave correctly beaconing will be distributed.
>>
>> johannes
>>
>
>
>
--
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/
"The only thing IE should be used for is to download Fire Fox"
"My favorite language is call STAR. It's extremely concise. It has
exactly one verb '*', which does exactly what I want at the moment."
--Larry Wall
On Wed, Nov 4, 2009 at 7:53 AM, Christian Lamparter
<[email protected]> wrote:
> On Wednesday 04 November 2009 02:13:49 Adam Wozniak wrote:
>> I have two systems under test, both Dell laptops (a Latitude D630 and an
>> Inspiron 600m) both running Ubuntu 9.10 with the latest updates, and
>> bleeding edge compat-wireless-2009-11-02. I'm using identical AR9170
>> based D-Link DWA-160 USB 802.11adapters. I'm using nuttcp to measure
>> throughput. I'm running in ad-hoc mode. Both machines have the same
>> ar9170 files in /lib/firmware. The machines are sitting about 5 feet
>> apart in my office.
>>
>> I'm having occasional problems where throughput drops through the floor
>> (0.5Mbps - 1.5Mbps). When I cat
>> /sys/kernel/debug/ieee80211/*/stations/*/rc_stats, one of the machines
>> lists the full set of rates, but the other only lists 1M and 54M. After
>> a period of time, that machine drops 54M and lists only one rate
>> (1Mbps), and the throughput listed by nuttcp drops accordingly. I
>> assume that, for whatever reason, the rates drop off the list and
>> minstrel uses the only one left available to it.
>>
>> If I modify include/net/mac80211.h and force the inline function
>> rate_supported to always return 1, this fixes the problem. However, I
>> think this is a band aid around some other issue.
>>
>> Any clues or ideas what the real issue might be here?
> no, but there are a few knobs you can play with...
>
> First of: more debug data
>
> 1. enable some ibss-related verbose messages
> open config.mk (in your compat-wireless directory)
> and get rid of the leading "# " <-- "sharp + space"
> in front of # CONFIG_MAC80211_IBSS_DEBUG=y
> (rebuild, reload, retest & checkout dmesg for a _flood_
> of potential interesting data)
If you can just use wireless-testing directly for debugging I
recommend that as we don't use mconf and friends for cofiguration
selection so we cannot be sure the config changes you make to enable
something are indeed agreeable by current upstream Kconfig rules. That
is, unless you know what you are doing I'd advise against using
debugging on compat-wireless.
Luis
These patches fix problems with rate selection in Ad-Hoc mode
[PATCH 1/2] mac80211: supp_rates initialization and rate control
notification
[PATCH 2/2] mac80211: minstrel try all rates
What's the next step in making sure this patch makes it into the real world?
Adam Wozniak wrote:
> In either case, having minstrel use all rates doesn't seem like it
> should hurt much. This is the patch I'm running with now, and it
> seems to work. The changes in ibss.c make sure supp_rates is
> initialized properly, and that rate control is called when a change
> occurs. If other stuff is working right (?) this should help PID and
> other rate control algorithms. The changes in rc80211_minstrel.c make
> it ignore the supported rates set and just try everything.
>
>
> diff -upr compat-wireless-2009-11-17/net/mac80211/ibss.c
> compat-wireless-2009-11-17a/net/mac80211/ibss.c
> --- compat-wireless-2009-11-17/net/mac80211/ibss.c 2009-11-16
> 21:17:21.000000000 -0800
> +++ compat-wireless-2009-11-17a/net/mac80211/ibss.c 2009-11-17
> 09:01:54.287720290 -0800
> @@ -246,9 +246,13 @@ static void ieee80211_rx_bss_info(struct
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> + supp_rates = ieee80211_sta_get_rates(local, elems, band);
> +
> + /* make sure mandatory rates are always added */
> + supp_rates |= ieee80211_mandatory_rates(local, band);
> +
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
> - supp_rates = ieee80211_sta_get_rates(local, elems, band);
>
> rcu_read_lock();
>
> @@ -257,12 +261,10 @@ static void ieee80211_rx_bss_info(struct
> u32 prev_rates;
>
> prev_rates = sta->sta.supp_rates[band];
> - /* make sure mandatory rates are always added */
> - sta->sta.supp_rates[band] = supp_rates |
> - ieee80211_mandatory_rates(local, band);
> + sta->sta.supp_rates[band] = supp_rates;
>
> + if (sta->sta.supp_rates[band] != prev_rates) {
> #ifdef CONFIG_MAC80211_IBSS_DEBUG
> - if (sta->sta.supp_rates[band] != prev_rates)
> printk(KERN_DEBUG "%s: updated supp_rates set "
> "for %pM based on beacon info (0x%llx | "
> "0x%llx -> 0x%llx)\n",
> @@ -272,6 +274,8 @@ static void ieee80211_rx_bss_info(struct
> (unsigned long long) supp_rates,
> (unsigned long long) sta->sta.supp_rates[band]);
> #endif
> + rate_control_rate_init(sta);
> + }
> } else
> ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
> supp_rates);
>
> @@ -415,6 +419,15 @@ struct sta_info *ieee80211_ibss_add_sta(
> sta->sta.supp_rates[band] = supp_rates |
> ieee80211_mandatory_rates(local, band);
>
> +#ifdef CONFIG_MAC80211_IBSS_DEBUG
> + printk(KERN_DEBUG "%s: initialized supp_rates set "
> + "for %pM (0x%llx) (band %d)\n",
> + sdata->dev->name,
> + sta->sta.addr,
> + (unsigned long long) sta->sta.supp_rates[band],
> + band);
> +#endif
> +
> rate_control_rate_init(sta);
>
> if (sta_info_insert(sta))
> diff -upr compat-wireless-2009-11-17/net/mac80211/rc80211_minstrel.c
> compat-wireless-2009-11-17a/net/mac80211/rc80211_minstrel.c
> --- compat-wireless-2009-11-17/net/mac80211/rc80211_minstrel.c
> 2009-11-16 21:17:21.000000000 -0800
> +++ compat-wireless-2009-11-17a/net/mac80211/rc80211_minstrel.c
> 2009-11-17 09:02:25.544628968 -0800
> @@ -395,8 +395,6 @@ minstrel_rate_init(void *priv, struct ie
> unsigned int tx_time_single;
> unsigned int cw = mp->cw_min;
>
> - if (!rate_supported(sta, sband->band, i))
> - continue;
> n++;
> memset(mr, 0, sizeof(*mr));
On Tue, 2009-11-24 at 09:05 -0800, Adam Wozniak wrote:
> Previously, not all code paths set supp_rates, and a rate change did not reinitialize the rate control layer.
> This patch fixes those issues. (Hopefully not whitespace damaged this time)
All my other comments still stand.
johannes
> Signed-off-by: Adam Wozniak <[email protected]>
> ---
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index 10d1385..474f66d 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -246,9 +246,13 @@ static void ieee80211_rx_bss_info(struct
> ieee80211_sub_if_data *sdata,
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> + supp_rates = ieee80211_sta_get_rates(local, elems, band);
> +
> + /* make sure mandatory rates are always added */
> + supp_rates |= ieee80211_mandatory_rates(local, band);
> +
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
> - supp_rates = ieee80211_sta_get_rates(local, elems, band);
>
> rcu_read_lock();
>
> @@ -257,12 +261,10 @@ static void ieee80211_rx_bss_info(struct
> ieee80211_sub_if_data *sdata,
> u32 prev_rates;
>
> prev_rates = sta->sta.supp_rates[band];
> - /* make sure mandatory rates are always added */
> - sta->sta.supp_rates[band] = supp_rates |
> - ieee80211_mandatory_rates(local, band);
> + sta->sta.supp_rates[band] = supp_rates;
>
> + if (sta->sta.supp_rates[band] != prev_rates) {
> #ifdef CONFIG_MAC80211_IBSS_DEBUG
> - if (sta->sta.supp_rates[band] != prev_rates)
> printk(KERN_DEBUG "%s: updated supp_rates set "
> "for %pM based on beacon info (0x%llx | "
> "0x%llx -> 0x%llx)\n",
> @@ -272,6 +274,8 @@ static void ieee80211_rx_bss_info(struct
> ieee80211_sub_if_data *sdata,
> (unsigned long long) supp_rates,
> (unsigned long long) sta->sta.supp_rates[band]);
> #endif
> + rate_control_rate_init(sta);
> + }
> } else
> ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
>
> @@ -415,6 +419,15 @@ struct sta_info *ieee80211_ibss_add_sta(struct
> ieee80211_sub_if_data *sdata,
> sta->sta.supp_rates[band] = supp_rates |
> ieee80211_mandatory_rates(local, band);
>
> +#ifdef CONFIG_MAC80211_IBSS_DEBUG
> + printk(KERN_DEBUG "%s: initialized supp_rates set "
> + "for %pM (0x%llx) (band %d)\n",
> + sdata->dev->name,
> + sta->sta.addr,
> + (unsigned long long) sta->sta.supp_rates[band],
> + band);
> +#endif
> +
> rate_control_rate_init(sta);
>
> if (sta_info_insert(sta))
>
>
If we have only three stations in an ad-hoc network, where all three can
hear the other two, only one of them should be beaconing, correct? If
this is true, it's not clear to me how the non beaconing stations will
update their rate information about each other based on the beacon. It
seems like, in the case we're "absent other information", we also need
to send a probe request, OR we need to get the bitrate information from
the next probe request we receive from them (neither of which we seem to
be doing).
Johannes Berg wrote:
> On Fri, 2009-11-13 at 14:35 -0800, Adam Wozniak wrote:
>
>
>> I've traced it down to this bit in rx.c in prepare_for_handlers():
>>
>> case NL80211_IFTYPE_ADHOC:
>> [ stuff deleted ]
>> } else if (!rx->sta) {
>> int rate_idx;
>> if (rx->status->flag & RX_FLAG_HT)
>> rate_idx = 0; /* TODO: HT rates */
>> else
>> rate_idx = rx->status->rate_idx;
>>
>> rx->sta = ieee80211_ibss_add_sta(sdata, bssid,
>> hdr->addr2,
>> BIT(rate_idx));
>> }
>> break;
>>
>> I don't think this is right. I know the issue is here, because if I
>> change to "BIT(rate_idx) | 0xfff" the problem corrects. Either we need
>> to (a) set it properly here or (b) make sure something else happens
>> before or after. I'm not sure we have enough context here to do (a).
>>
>
> Hmm. This BIT(..) was basically here to ensure we have at least a single
> good rate absent other information. If we do not receive a probe or
> beacon frame from that peer at least once, but add it to our IBSS only
> due to a single received data frame, we add the bitrate that this frame
> was transmitted at so we have one rate that we know it can transmit (and
> presumably also receive).
>
> What should happen is that once it starts beaconing we pick up a beacon
> and extend our set of known good rates.
>
> johannes
>
Hi,
Referring back to my letter below, you see that when A&B agree on their
BSSID, this implies that A&B have agreed on:
>> a)supported rates
>> b)the value of X, the value of x
>> c)the channel
>> d)the ESSSID
When B&C start talking, and C adopts the same BSSID as B, then we have (by
inference) that C has agreed to the same rates as A. There is no need to
pass rate information between C and A.
===========================
Ok, now here is a silly example.
Suppose C supports bitrates b4,b5 and b6.
B does not support bitrates b4,b5 and b6.
Further, A does support bitrates b4,b5,b6
This could be that B only does 1, 2, 5.5 and 11MBits/sec. But A&C do
everything up to 54Mbit/sec.
B has caused a problem cause he is deficient (not doing G-Rates)
In one view, B should report that he has all the rates of A. B would then
hope that A's rate control algorithm will detect that B cannot do the
G-rates. Minstrel will do this fine. Stepup-Stepdown rate algorithms will
struggle here if their table is constructed in the wrong order.
For this network, the BSSID defines the basic service set. you cannot have
nodes on the same BSSID who report as handling different rate sets.
Derek.
On Mon, 16 Nov 2009, Adam Wozniak wrote:
> In your example where A&C can pass data, but not hear each other's beacons,
> how is rate information passed between them? ProbeReq/Resp ?
>
> Derek Smithies wrote:
>> Hi,
>> Statistics is a wonderful thing.
>> Every node is required to send a beacon at time
>> X+r, X+x+r, X+2x+r, X+3x+r, ......
>>
>> All nodes are agreed on the value of x (which is the beacon interval).
>> All nodes are agreed on the value of X
>> r is a random value, and is (from memory) 20 slots long.
>>
>> given this, all nodes work (to borrow an analogy from music) in time, or
>> beat in sync with each other.
>>
>> Now, if a node hears a beacon on its BSSID inside that r period, the node
>> will not transmit a beacon. This way, for a 20 node network in a room, you
>> should only get 1 (or sometimes 2) beacons transmitted every beacon
>> interval.
>>
>> If we assume that every node correctly attempts to send a beacon somewhere
>> in that period r, and that somewhere is randomly distributed, then we will
>> hear a beacon from most nodes, which is good enough.
>>
>> Consider the case of three nodes, A, B, C.
>>
>> A and B are turned on, and create an Adhoc network. A and B agree on
>> a)supported rates
>> b)the value of X, the value of x
>> c)the channel
>> d)the ESSSID
>> and so start sending a beacon. Inside this beacon is BSSID. The BSSID is a
>> random value. The particular random value used implies acceptance of
>> a,b,c,d. Look at the name. Basic Service Set ID. The basic service set is
>> the collection of those values a,b,c,d.
>>
>> Now, node C is turned on. A is positioned such that A&C cannot communicate.
>> However, B can communicate with A&C.
>>
>> C is turned on, detects the presence of B, likes B's beacons, and agrees on
>> all the settings in B's beacons. In other words, C likes and agrees with
>> a,b,c,d.
>> So C starts sending beacons with the same BSSID as B.
>> At this point, it does not matter that A cannot set C's beacons. A and C
>> have agreed on the BSSID.
>>
>> Change the story a little bit.
>> In this locality, there is often a burst of 1ms of noise every 2ms. This
>> means that most beacons are shotdown. However, data packets at 54Mbit/sec
>> get through.
>> A&B saw each others beacons and agreed on a BSSID. C was turned on, and
>> agreed with the BSSID of B.
>>
>> C sends out data packets, with the BSSID of B. A sees the datapackets of C.
>> Since the datapackets of C have A's BSSID, A has to accept them.
>>
>> Now, you see where this is all going. What is the meaning of a beacon
>> containing a BSSID of all zero ?
>> Further, you see that all nodes do need to send a beacon. This makes node
>> discovery a little easier.
>> Even though A and C cannot see each others beacons, they should still
>> communicate as they have the same agreed on BSSID.
>>
>> Derek.
>>
>>
>> On Mon, 16 Nov 2009, Adam Wozniak wrote:
>>
>>> This assumption seems too stoichastic. Reading 802.11-2007 section
>>> 11.1.2.2, it doesn't seem that we're guaranteed to always receive beacons
>>> from all stations. Stations will cancel their pending beacon transmission
>>> if they receive a beacon before their random delay times out. In the
>>> extreme case where the number of stations is very large, it seems possible
>>> that you may never hear beacons for some stations.
>>>
>>> Johannes Berg wrote:
>>>> On Mon, 2009-11-16 at 09:25 -0800, Adam Wozniak wrote:
>>>>
>>>>> If we have only three stations in an ad-hoc network, where all three can
>>>>> hear the other two, only one of them should be beaconing, correct?
>>>>
>>>> No, if they all behave correctly beaconing will be distributed.
>>>>
>>>> johannes
>>>>
>>>
>>>
>>>
>>
>
>
>
--
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/
"The only thing IE should be used for is to download Fire Fox"
"My favorite language is call STAR. It's extremely concise. It has
exactly one verb '*', which does exactly what I want at the moment."
--Larry Wall
In Ad-Hoc mode, it is possible for minstrel to be initialized without complete rate information.
In these cases, throughput suffers because not all rates are tried. This patch tells minstrel to try all rates.
This won't cause problems, because unsupported rates will simply fail, causing minstrel to set their probability to 0%.
(Hopefully not whitespace damaged this time)
Signed-off-by: Adam Wozniak <[email protected]>
---
diff --git a/net/mac80211/rc80211_minstrel.c
b/net/mac80211/rc80211_minstrel.c
index 6e5d68b..3a8fe30 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -395,8 +395,6 @@ minstrel_rate_init(void *priv, struct
ieee80211_supported_band *sband,
unsigned int tx_time_single;
unsigned int cw = mp->cw_min;
- if (!rate_supported(sta, sband->band, i))
- continue;
n++;
memset(mr, 0, sizeof(*mr));
On Tue, 2009-11-24 at 10:43 -0800, Adam Wozniak wrote:
> Johannes Berg wrote:
> > However, that reminds me that I actually wanted to get rid of
> > rate_init() since it's mostly not useful to have it separate from
> > alloc_sta()... oh well.
> >
> > I suppose if we document clearly that rate_init maybe called multiple
> > times and audit the existing algorithms the patch could be ok.
> >
> As far as I can tell, minstrel_rate_init does not do any allocation.
> The pid_algo version does not appear to do any allocation either.
There are three more -- iwlwifi (2x) and ath9k
johannes
Johannes Berg wrote:
> On Tue, 2009-11-24 at 10:43 -0800, Adam Wozniak wrote:
>
>> As far as I can tell, minstrel_rate_init does not do any allocation.
>> The pid_algo version does not appear to do any allocation either.
>>
>
> There are three more -- iwlwifi (2x) and ath9k
>
Ugh. Why aren't these in with the mac80211 code? Do they rely on
something hardware specific?
I could not find any allocations in the rate_init in any of these:
./drivers/net/wireless/iwlwifi/iwl-3945-rs.c
./drivers/net/wireless/iwlwifi/iwl-agn-rs.c
./drivers/net/wireless/ath/ath9k/rc.c
On Mon, 2009-11-23 at 16:57 -0800, Adam Wozniak wrote:
> In Ad-Hoc mode, it is possible for minstrel to be initialized without
> complete rate information. In these cases, throughput suffers because
> not all rates are tried. This patch tells minstrel to try all rates.
> This won't cause problems, because unsupported rates will simply fail,
> causing minstrel to set their probability to 0%.
Both your patches are completely whitespace damaged.
I really don't like this patch anyhow though, it's at best a hackish
workaround around an issue that should just be fixed instead.
Will take another look at the other patch tomorrow.
johannes
> Signed-off-by: Adam Wozniak <[email protected]>
> ---
> diff --git a/net/mac80211/rc80211_minstrel.c
> b/net/mac80211/rc80211_minstrel.c
> index 6e5d68b..3a8fe30 100644
> --- a/net/mac80211/rc80211_minstrel.c
> +++ b/net/mac80211/rc80211_minstrel.c
> @@ -395,8 +395,6 @@ minstrel_rate_init(void *priv, struct
> ieee80211_supported_band *sband,
> unsigned int tx_time_single;
> unsigned int cw = mp->cw_min;
>
> - if (!rate_supported(sta, sband->band, i))
> - continue;
> n++;
> memset(mr, 0, sizeof(*mr));
>
>
>
Previously, not all code paths set supp_rates, and a rate change did not
reinitialize the rate control layer.
This patch fixes those issues.
Signed-off-by: Adam Wozniak <[email protected]>
---
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 10d1385..474f66d 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -246,9 +246,13 @@ static void ieee80211_rx_bss_info(struct
ieee80211_sub_if_data *sdata,
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
+ supp_rates = ieee80211_sta_get_rates(local, elems, band);
+
+ /* make sure mandatory rates are always added */
+ supp_rates |= ieee80211_mandatory_rates(local, band);
+
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
- supp_rates = ieee80211_sta_get_rates(local, elems, band);
rcu_read_lock();
@@ -257,12 +261,10 @@ static void ieee80211_rx_bss_info(struct
ieee80211_sub_if_data *sdata,
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
- /* make sure mandatory rates are always added */
- sta->sta.supp_rates[band] = supp_rates |
- ieee80211_mandatory_rates(local, band);
+ sta->sta.supp_rates[band] = supp_rates;
+ if (sta->sta.supp_rates[band] != prev_rates) {
#ifdef CONFIG_MAC80211_IBSS_DEBUG
- if (sta->sta.supp_rates[band] != prev_rates)
printk(KERN_DEBUG "%s: updated supp_rates set "
"for %pM based on beacon info (0x%llx | "
"0x%llx -> 0x%llx)\n",
@@ -272,6 +274,8 @@ static void ieee80211_rx_bss_info(struct
ieee80211_sub_if_data *sdata,
(unsigned long long) supp_rates,
(unsigned long long) sta->sta.supp_rates[band]);
#endif
+ rate_control_rate_init(sta);
+ }
} else
ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
supp_rates);
@@ -415,6 +419,15 @@ struct sta_info *ieee80211_ibss_add_sta(struct
ieee80211_sub_if_data *sdata,
sta->sta.supp_rates[band] = supp_rates |
ieee80211_mandatory_rates(local, band);
+#ifdef CONFIG_MAC80211_IBSS_DEBUG
+ printk(KERN_DEBUG "%s: initialized supp_rates set "
+ "for %pM (0x%llx) (band %d)\n",
+ sdata->dev->name,
+ sta->sta.addr,
+ (unsigned long long) sta->sta.supp_rates[band],
+ band);
+#endif
+
rate_control_rate_init(sta);
if (sta_info_insert(sta))
Adam Wozniak wrote:
> Johannes Berg wrote:
>> Both your patches are completely whitespace damaged.
> Can you explain what you mean by that?
Ah, xterm cut&paste converting tabs to space. Will try that again.
On Tue, 2009-11-24 at 09:17 -0800, Adam Wozniak wrote:
> I do not believe 1/2 was a hack. It really was possible to get to the
> end of that function and use that variable without it being assigned the
> correct value. Also, it is important to reinitialize the rate control
> layer when the list of usable rates changes.
Ah, indeed. But I still don't think the rate control _init() function
should be called again.
johannes
Johannes Berg wrote:
> Or you can probably call rate_exit and rate_init but I don't know how
> the algorithms would react if you actually ask it for a rate during that
> race window?
>
There is no rate_exit in rate_control_ops, only init and update. There
is an rc80211_minstrel_exit() in rc80211_minstrel.c, but that just
unregisters the module, which I don't think is what you want.
Eep, I can already tell this isn't going to work out. The ewma
probability reported in rc_stats is always 0.0, and the success and
attempts columns are always zero, as if the minstrel layer is constantly
being reinitialized. My previous patch was better.
--Adam
Christian Lamparter wrote:
> On Thursday 12 November 2009 23:38:07 Adam Wozniak wrote:
>
>> I see what you're doing there.
>> That didn't quite work, but I'm fairly confident this one will.
>>
> yep, what about the attached version? ...it's slightly different...
>
> BTW: Use "diff -up" or "diff -uprN" to create patches. (or git :) )
> ---
>
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index fbffce9..7c6c170 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -246,9 +246,12 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> + /* make sure mandatory rates are always added */
> + supp_rates = ieee80211_mandatory_rates(local, band);
> +
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
> - supp_rates = ieee80211_sta_get_rates(local, elems, band);
> + supp_rates |= ieee80211_sta_get_rates(local, elems, band);
>
> rcu_read_lock();
>
> @@ -257,9 +260,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
> u32 prev_rates;
>
> prev_rates = sta->sta.supp_rates[band];
> - /* make sure mandatory rates are always added */
> - sta->sta.supp_rates[band] = supp_rates |
> - ieee80211_mandatory_rates(local, band);
> + sta->sta.supp_rates[band] = supp_rates;
>
> #ifdef CONFIG_MAC80211_IBSS_DEBUG
> if (sta->sta.supp_rates[band] != prev_rates)
> @@ -272,6 +273,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
> (unsigned long long) supp_rates,
> (unsigned long long) sta->sta.supp_rates[band]);
> #endif
> + rate_control_rate_init(sta);
> } else
> ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
>
>
$ ls -la /lib/firmware/ar9170*
-rw-r--r-- 1 root root 83968 2009-10-17 15:55 /lib/firmware/ar9170-1.fw
-rw-r--r-- 1 root root 3508 2009-10-17 15:55 /lib/firmware/ar9170-2.fw
-rw-r--r-- 1 root root 15960 2009-10-17 15:55 /lib/firmware/ar9170.fw
It is unclear to me which are actually used. I will try removing the
two stage firmware files and see what happens.
Christian Lamparter wrote:
> On Wednesday 04 November 2009 22:01:39 Derek Smithies wrote:
>
>> Hi,
>> On Wed, 4 Nov 2009, Christian Lamparter wrote:
>>
>>
>>> On Wednesday 04 November 2009 02:13:49 Adam Wozniak wrote:
>>>
>>>> I have two systems under test, both Dell laptops (a Latitude D630 and an
>>>> Inspiron 600m) both running Ubuntu 9.10 with the latest updates, and
>>>> bleeding edge compat-wireless-2009-11-02. I'm using identical AR9170
>>>> based D-Link DWA-160 USB 802.11adapters. I'm using nuttcp to measure
>>>> throughput. I'm running in ad-hoc mode. Both machines have the same
>>>> ar9170 files in /lib/firmware. The machines are sitting about 5 feet
>>>> apart in my office.
>>>>
> by the way: I forgot to ask, but which firmware do you use?
> If you still have *two - stage*, then get rid of it.
> Since one-stage fws contain a few fixes for most temporarily MAC/BB-hiccups.
>
>
>>>> I'm having occasional problems where throughput drops through the floor
>>>> (0.5Mbps - 1.5Mbps). When I cat
>>>> /sys/kernel/debug/ieee80211/*/stations/*/rc_stats, one of the machines
>>>> lists the full set of rates, but the other only lists 1M and 54M. After
>>>> a period of time, that machine drops 54M and lists only one rate
>>>> (1Mbps), and the throughput listed by nuttcp drops accordingly. I
>>>> assume that, for whatever reason, the rates drop off the list and
>>>> minstrel uses the only one left available to it.
>>>>
>>>> If I modify include/net/mac80211.h and force the inline function
>>>> rate_supported to always return 1, this fixes the problem. However, I
>>>> think this is a band aid around some other issue.
>>>>
>>>> Any clues or ideas what the real issue might be here?
>>>>
>> My guess::
>>
>> When an adhoc node (call it A) merges with a second adhoc node (call it
>> B) there is a capability comparison.
>> Node A looks at the rates supported by B and says,
>> "I must only transmit at rates supported by B"
>>
>> Some management frames don't contain a full report of the rates supported
>> by the sender.
>> My view is that node A (in this example) is incorrectly determining that B
>> only supports the 1mb/sec rate. Consequently, node A fills the
>> rate_supported array with one rate - 1mb/sec.
>>
> well, that's the thing... it sounds like something in cfg80211/mac80211 has
> gone wrong. Since ibss supported/basic rates IEs should always include all
> mandatory rates for the given band & mode. Therefore you should see the
> 2Mbit, 11Mbit, 6MBit, 12Mbit 24Mbit rates in rc_stats array as well.
>
>
>> =====
>>
> =====
>
>
>> There is no evidence that Minstrel is doing anything wrong.
>>
> ?but no one said it was minstrel fault? And it clearly isn't.
>
> But something OT: do you have already thoughts about
> _extending_ minstrel to support 802.11n MCS rates?
>
> The current endeavor is stuck and needs a kick-start.
> This is partly because of a hen-egg problem:
> no driver <-> no 11n rc. But it should be easy to get
> 11n capable hw now (e.g. Mikrotik's R52N) and
> ath9k should be the perfect testing platform right now.
>
> nbd has/had some thought about grouping rates and options
> (e.g SGI/40MHz) together to reduce the number of rates to
> improve the _search for best tp_ time. But dunno, maybe he
> has already something better than the proof-of-concept I wrote earlier.
>
> Regards,
> Chr
>
Johannes Berg wrote:
> No, point a) is incorrect -- it is the _basic_ rate set only, not the
> entire rate set. Hence not knowing which rates a station actually
> supports.
>
In either case, having minstrel use all rates doesn't seem like it
should hurt much. This is the patch I'm running with now, and it seems
to work. The changes in ibss.c make sure supp_rates is initialized
properly, and that rate control is called when a change occurs. If
other stuff is working right (?) this should help PID and other rate
control algorithms. The changes in rc80211_minstrel.c make it ignore
the supported rates set and just try everything.
diff -upr compat-wireless-2009-11-17/net/mac80211/ibss.c
compat-wireless-2009-11-17a/net/mac80211/ibss.c
--- compat-wireless-2009-11-17/net/mac80211/ibss.c 2009-11-16
21:17:21.000000000 -0800
+++ compat-wireless-2009-11-17a/net/mac80211/ibss.c 2009-11-17
09:01:54.287720290 -0800
@@ -246,9 +246,13 @@ static void ieee80211_rx_bss_info(struct
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
+ supp_rates = ieee80211_sta_get_rates(local, elems, band);
+
+ /* make sure mandatory rates are always added */
+ supp_rates |= ieee80211_mandatory_rates(local, band);
+
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
- supp_rates = ieee80211_sta_get_rates(local, elems, band);
rcu_read_lock();
@@ -257,12 +261,10 @@ static void ieee80211_rx_bss_info(struct
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
- /* make sure mandatory rates are always added */
- sta->sta.supp_rates[band] = supp_rates |
- ieee80211_mandatory_rates(local, band);
+ sta->sta.supp_rates[band] = supp_rates;
+ if (sta->sta.supp_rates[band] != prev_rates) {
#ifdef CONFIG_MAC80211_IBSS_DEBUG
- if (sta->sta.supp_rates[band] != prev_rates)
printk(KERN_DEBUG "%s: updated supp_rates set "
"for %pM based on beacon info (0x%llx | "
"0x%llx -> 0x%llx)\n",
@@ -272,6 +274,8 @@ static void ieee80211_rx_bss_info(struct
(unsigned long long) supp_rates,
(unsigned long long) sta->sta.supp_rates[band]);
#endif
+ rate_control_rate_init(sta);
+ }
} else
ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
supp_rates);
@@ -415,6 +419,15 @@ struct sta_info *ieee80211_ibss_add_sta(
sta->sta.supp_rates[band] = supp_rates |
ieee80211_mandatory_rates(local, band);
+#ifdef CONFIG_MAC80211_IBSS_DEBUG
+ printk(KERN_DEBUG "%s: initialized supp_rates set "
+ "for %pM (0x%llx) (band %d)\n",
+ sdata->dev->name,
+ sta->sta.addr,
+ (unsigned long long) sta->sta.supp_rates[band],
+ band);
+#endif
+
rate_control_rate_init(sta);
if (sta_info_insert(sta))
diff -upr compat-wireless-2009-11-17/net/mac80211/rc80211_minstrel.c
compat-wireless-2009-11-17a/net/mac80211/rc80211_minstrel.c
--- compat-wireless-2009-11-17/net/mac80211/rc80211_minstrel.c
2009-11-16 21:17:21.000000000 -0800
+++ compat-wireless-2009-11-17a/net/mac80211/rc80211_minstrel.c
2009-11-17 09:02:25.544628968 -0800
@@ -395,8 +395,6 @@ minstrel_rate_init(void *priv, struct ie
unsigned int tx_time_single;
unsigned int cw = mp->cw_min;
- if (!rate_supported(sta, sband->band, i))
- continue;
n++;
memset(mr, 0, sizeof(*mr));
Johannes Berg wrote:
> However, that reminds me that I actually wanted to get rid of
> rate_init() since it's mostly not useful to have it separate from
> alloc_sta()... oh well.
>
> I suppose if we document clearly that rate_init maybe called multiple
> times and audit the existing algorithms the patch could be ok.
>
As far as I can tell, minstrel_rate_init does not do any allocation.
The pid_algo version does not appear to do any allocation either.
--Adam Wozniak
I see what you're doing there. That didn't quite work, but I'm fairly
confident this one will. I'm running my long term test now. Note the
added call to rate_control_init() when the rate is updated. This
*should* be rate_control_rate_update, but it doesn't look to me like
that method is implemented in minstrel or the PID code.
*** compat-wireless-2009-11-09/net/mac80211/ibss.c 2009-11-08
21:15:06.000000000 -0800
--- compat-wireless-2009-11-09b/net/mac80211/ibss.c 2009-11-12
14:29:16.308550923 -0800
***************
*** 246,254 ****
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
- supp_rates = ieee80211_sta_get_rates(local, elems, band);
rcu_read_lock();
--- 246,258 ----
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
+ supp_rates = ieee80211_sta_get_rates(local, elems, band);
+
+ /* make sure mandatory rates are always added */
+ supp_rates = supp_rates |= ieee80211_mandatory_rates(local, band);
+
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
rcu_read_lock();
***************
*** 257,268 ****
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
! /* make sure mandatory rates are always added */
! sta->sta.supp_rates[band] = supp_rates |
! ieee80211_mandatory_rates(local, band);
#ifdef CONFIG_MAC80211_IBSS_DEBUG
! if (sta->sta.supp_rates[band] != prev_rates)
printk(KERN_DEBUG "%s: updated supp_rates set "
"for %pM based on beacon info (0x%llx | "
"0x%llx -> 0x%llx)\n",
--- 261,270 ----
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
! sta->sta.supp_rates[band] = supp_rates;
#ifdef CONFIG_MAC80211_IBSS_DEBUG
! if (sta->sta.supp_rates[band] != prev_rates) {
printk(KERN_DEBUG "%s: updated supp_rates set "
"for %pM based on beacon info (0x%llx | "
"0x%llx -> 0x%llx)\n",
***************
*** 271,276 ****
--- 273,284 ----
(unsigned long long) prev_rates,
(unsigned long long) supp_rates,
(unsigned long long) sta->sta.supp_rates[band]);
+
+ /* TODO: implement rate_update in minstrel/pid,
+ * then change this to rate_control_rate_update.
+ */
+ rate_control_rate_init(sta);
+ }
#endif
} else
ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
supp_rates);
Christian Lamparter wrote:
> On Thursday 12 November 2009 20:43:22 Adam Wozniak wrote:
>
>> I was hoping for more of an "ah-ha!" response. =)
>>
>> It worked well initially, but when I let it run overnight it fell back
>> into that same failure mode.
>>
>>
>
> great... :\
>
> what about this patch?
> ---
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index fbffce9..bde89f7 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -250,6 +250,9 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
> supp_rates = ieee80211_sta_get_rates(local, elems, band);
>
> + /* make sure mandatory rates are always added */
> + supp_rates |= ieee80211_mandatory_rates(local, band);
> +
> rcu_read_lock();
>
> sta = sta_info_get(local, mgmt->sa);
> @@ -257,9 +260,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
> u32 prev_rates;
>
> prev_rates = sta->sta.supp_rates[band];
> - /* make sure mandatory rates are always added */
> - sta->sta.supp_rates[band] = supp_rates |
> - ieee80211_mandatory_rates(local, band);
> + sta->sta.supp_rates[band] = supp_rates;
>
> #ifdef CONFIG_MAC80211_IBSS_DEBUG
> if (sta->sta.supp_rates[band] != prev_rates)
>
On Wednesday 04 November 2009 02:13:49 Adam Wozniak wrote:
> I have two systems under test, both Dell laptops (a Latitude D630 and an
> Inspiron 600m) both running Ubuntu 9.10 with the latest updates, and
> bleeding edge compat-wireless-2009-11-02. I'm using identical AR9170
> based D-Link DWA-160 USB 802.11adapters. I'm using nuttcp to measure
> throughput. I'm running in ad-hoc mode. Both machines have the same
> ar9170 files in /lib/firmware. The machines are sitting about 5 feet
> apart in my office.
>
> I'm having occasional problems where throughput drops through the floor
> (0.5Mbps - 1.5Mbps). When I cat
> /sys/kernel/debug/ieee80211/*/stations/*/rc_stats, one of the machines
> lists the full set of rates, but the other only lists 1M and 54M. After
> a period of time, that machine drops 54M and lists only one rate
> (1Mbps), and the throughput listed by nuttcp drops accordingly. I
> assume that, for whatever reason, the rates drop off the list and
> minstrel uses the only one left available to it.
>
> If I modify include/net/mac80211.h and force the inline function
> rate_supported to always return 1, this fixes the problem. However, I
> think this is a band aid around some other issue.
>
> Any clues or ideas what the real issue might be here?
no, but there are a few knobs you can play with...
First of: more debug data
1. enable some ibss-related verbose messages
open config.mk (in your compat-wireless directory)
and get rid of the leading "# " <-- "sharp + space"
in front of # CONFIG_MAC80211_IBSS_DEBUG=y
(rebuild, reload, retest & checkout dmesg for a _flood_
of potential interesting data)
2. do you think you can get a capture any beacons, auth, assoc, reassoc
management frames form the misbehaving setup?
3. cat /sys/kernel/debug/ieee80211/*/stations/*/rc_stats perhaps?
(yeah, I know... sort of pointless, but people have different believes
and somehow they have the bad tendency to really _stick_ to it!)
Regards,
Chr
Please everyone in this thread, trim your quotes.
On Tue, 2009-11-17 at 13:20 +1300, Derek Smithies wrote:
> Yes.
> that is what the BSSID is - the definition of the basis service set. To
> use a particular BSSID is an implicit acceptance of the
> >>>> a)supported rates
> >>>> b)the value of X, the value of x
> >>>> c)the channel
> >>>> d)the ESSSID
> from an existing network.
No, point a) is incorrect -- it is the _basic_ rate set only, not the
entire rate set. Hence not knowing which rates a station actually
supports.
johannes
Hi,
> Do you know which frames in the mac80211 code are missing this required
> information? Or was that conjecture?
it was mostly conjecture. I had done some work with an earlier pull of
compat wireless in adhoc. the rates tried by minstrel were bad - it was
stuck at the slowest rate.
I too had found that the fix was to adjust the array so that all rates
were marked as supported.
Derek
On Tue, 10 Nov 2009, Adam Wozniak wrote:
> Reading through the 802.11 spec, it appears to me that "Supported rates" (and
> "Extended Supported Rates" when number of rates > 8) is REQUIRED for all
> management frames except authentication, deauthentication, and action frames.
> (IEEE 802.11-2007, 7.2.3)
>
> Do you know which frames in the mac80211 code are missing this required
> information? Or was that conjecture?
>
> Looking at mac80211/tx.c ieee80211_beacon_get_tim, it is not clear to me how
> (or if) this rate information is being set for ad-hoc beacons.
>
> Derek Smithies wrote:
>> Some management frames don't contain a full report of the rates supported
>> by the sender.
>> My view is that node A (in this example) is incorrectly determining that B
>> only supports the 1mb/sec rate. Consequently, node A fills the
>> rate_supported array with one rate - 1mb/sec.
>
>
>
--
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/
"The only thing IE should be used for is to download Fire Fox"
"My favorite language is call STAR. It's extremely concise. It has
exactly one verb '*', which does exactly what I want at the moment."
--Larry Wall
On Wed, Nov 4, 2009 at 1:46 PM, Adam Wozniak <[email protected]> wrote:
>
> $ ls -la /lib/firmware/ar9170*
> -rw-r--r-- 1 root root 83968 2009-10-17 15:55 /lib/firmware/ar9170-1.fw
> -rw-r--r-- 1 root root 3508 2009-10-17 15:55 /lib/firmware/ar9170-2.fw
> -rw-r--r-- 1 root root 15960 2009-10-17 15:55 /lib/firmware/ar9170.fw
>
> It is unclear to me which are actually used.
By default the ar9170.fw is tried first, if that fails then the others
are tried. The 2-stage firmware will not be tried if your device
cannot handle it but right now only the AVM Fritz devices can't handle
the 2-stage firmware.
Christian should we just remove 2-stage fw support?
Luis
On Mon, 2009-11-23 at 12:21 -0800, Adam Wozniak wrote:
> What's the next step in making sure this patch makes it into the real world?
In that form, it never will. You need to create it against
wireless-testing, give it a good changelog, sign it off, make it not
white-space damaged, etc.:
http://wireless.kernel.org/en/developers/Documentation/SubmittingPatches
Besides, some of it really is a hack.
johannes
On Thu, 2009-11-12 at 14:41 -0800, Adam Wozniak wrote:
> Whoops, patch was slightly incorrect. This is better:
>
> *** compat-wireless-2009-11-09/net/mac80211/ibss.c 2009-11-08
> 21:15:06.000000000 -0800
> --- compat-wireless-2009-11-09b/net/mac80211/ibss.c 2009-11-12
> 14:39:12.391545084 -0800
> ***************
> *** 246,254 ****
[snip]
it would help if you used diff -u
johannes
Is it possible this is the problem? (note supp_rates is used at the
bottom of the function, outside the "if")
*** a/net/mac80211/ibss.c 2009-11-02 09:11:36.000000000 -0800
--- b/net/mac80211/ibss.c 2009-11-10 16:31:46.291563951 -0800
***************
*** 246,254 ****
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
- supp_rates = ieee80211_sta_get_rates(local, elems, band);
rcu_read_lock();
--- 246,255 ----
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
+ supp_rates = ieee80211_sta_get_rates(local, elems, band);
+
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
rcu_read_lock();
Derek Smithies wrote:
> Hi,
>> Do you know which frames in the mac80211 code are missing this
>> required information? Or was that conjecture?
>
> it was mostly conjecture. I had done some work with an earlier pull of
> compat wireless in adhoc. the rates tried by minstrel were bad - it
> was stuck at the slowest rate.
>
> I too had found that the fix was to adjust the array so that all rates
> were marked as supported.
>
> Derek
>
> On Tue, 10 Nov 2009, Adam Wozniak wrote:
>
>> Reading through the 802.11 spec, it appears to me that "Supported
>> rates" (and "Extended Supported Rates" when number of rates > 8) is
>> REQUIRED for all management frames except authentication,
>> deauthentication, and action frames. (IEEE 802.11-2007, 7.2.3)
>>
>> Do you know which frames in the mac80211 code are missing this
>> required information? Or was that conjecture?
>>
>> Looking at mac80211/tx.c ieee80211_beacon_get_tim, it is not clear to
>> me how (or if) this rate information is being set for ad-hoc beacons.
>>
>> Derek Smithies wrote:
>>> Some management frames don't contain a full report of the rates
>>> supported by the sender.
>>> My view is that node A (in this example) is incorrectly determining
>>> that B only supports the 1mb/sec rate. Consequently, node A fills
>>> the rate_supported array with one rate - 1mb/sec.
>>
>>
>>
>
Johannes Berg wrote:
> it would help if you used diff -u
> johannes
>
Will do.
I think the patch below is most correct. However, I still have problems
when a third station joins the ad-hoc network.
When the third station joins, it doesn't always show a complete set of
bitrates in the rc_stats files for the other two stations. For one (I
assume this is the beacon OR whoever sent a probe response) it shows
all, but for the other, it shows only one or two. This is remarkably
similar to the failure mode I was seeing before, so much so that there
were probably cases where I confused the two issues.
I've traced it down to this bit in rx.c in prepare_for_handlers():
case NL80211_IFTYPE_ADHOC:
[ stuff deleted ]
} else if (!rx->sta) {
int rate_idx;
if (rx->status->flag & RX_FLAG_HT)
rate_idx = 0; /* TODO: HT rates */
else
rate_idx = rx->status->rate_idx;
rx->sta = ieee80211_ibss_add_sta(sdata, bssid,
hdr->addr2,
BIT(rate_idx));
}
break;
I don't think this is right. I know the issue is here, because if I
change to "BIT(rate_idx) | 0xfff" the problem corrects. Either we need
to (a) set it properly here or (b) make sure something else happens
before or after. I'm not sure we have enough context here to do (a).
Thoughts?
patch for ibss.c :
--- compat-wireless-2009-11-09/net/mac80211/ibss.c 2009-11-08
21:15:06.000000000 -0800
+++ compat-wireless-2009-11-09d/net/mac80211/ibss.c 2009-11-13
14:17:37.935556965 -0800
@@ -246,9 +246,13 @@ static void ieee80211_rx_bss_info(struct
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
+ supp_rates = ieee80211_sta_get_rates(local, elems, band);
+
+ /* make sure mandatory rates are always added */
+ supp_rates |= ieee80211_mandatory_rates(local, band);
+
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
- supp_rates = ieee80211_sta_get_rates(local, elems, band);
rcu_read_lock();
@@ -257,12 +261,10 @@ static void ieee80211_rx_bss_info(struct
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
- /* make sure mandatory rates are always added */
- sta->sta.supp_rates[band] = supp_rates |
- ieee80211_mandatory_rates(local, band);
+ sta->sta.supp_rates[band] = supp_rates;
+ if (sta->sta.supp_rates[band] != prev_rates) {
#ifdef CONFIG_MAC80211_IBSS_DEBUG
- if (sta->sta.supp_rates[band] != prev_rates)
printk(KERN_DEBUG "%s: updated supp_rates set "
"for %pM based on beacon info (0x%llx | "
"0x%llx -> 0x%llx)\n",
@@ -272,6 +274,8 @@ static void ieee80211_rx_bss_info(struct
(unsigned long long) supp_rates,
(unsigned long long) sta->sta.supp_rates[band]);
#endif
+ rate_control_rate_init(sta);
+ }
} else
ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
supp_rates);
@@ -415,6 +419,15 @@ struct sta_info *ieee80211_ibss_add_sta(
sta->sta.supp_rates[band] = supp_rates |
ieee80211_mandatory_rates(local, band);
+#ifdef CONFIG_MAC80211_IBSS_DEBUG
+ printk(KERN_DEBUG "%s: initialized supp_rates set "
+ "for %pM (0x%llx) (band %d)\n",
+ sdata->dev->name,
+ sta->sta.addr,
+ (unsigned long long) sta->sta.supp_rates[band],
+ band);
+#endif
+
rate_control_rate_init(sta);
if (sta_info_insert(sta))
Johannes Berg wrote:
> Both your patches are completely whitespace damaged.
Can you explain what you mean by that?
Johannes Berg wrote:
> Ok, but can they all deal with calling get_rate and rate_init at the
> same time? I think that could happen now, no?
>
I understand your concern. I do not know the code well enough to answer
that question.
It seems like, if that is a real concern, it should be simple enough to
guarantee that would never happen with a mutex. But this is getting out
of scope for the amount of time I have budgeted to pursue this issue.
These patches fix problems with rate selection in Ad-Hoc mode. Hopefully not whitespace damaged this time.
[PATCH v2 1/2] mac80211: supp_rates initialization and rate control notification
[PATCH v2 2/2] mac80211: minstrel try all rates
On Mon, 2009-11-16 at 09:25 -0800, Adam Wozniak wrote:
> If we have only three stations in an ad-hoc network, where all three can
> hear the other two, only one of them should be beaconing, correct?
No, if they all behave correctly beaconing will be distributed.
johannes
On Wednesday 04 November 2009 22:01:39 Derek Smithies wrote:
> Hi,
> On Wed, 4 Nov 2009, Christian Lamparter wrote:
>
> > On Wednesday 04 November 2009 02:13:49 Adam Wozniak wrote:
> >> I have two systems under test, both Dell laptops (a Latitude D630 and an
> >> Inspiron 600m) both running Ubuntu 9.10 with the latest updates, and
> >> bleeding edge compat-wireless-2009-11-02. I'm using identical AR9170
> >> based D-Link DWA-160 USB 802.11adapters. I'm using nuttcp to measure
> >> throughput. I'm running in ad-hoc mode. Both machines have the same
> >> ar9170 files in /lib/firmware. The machines are sitting about 5 feet
> >> apart in my office.
by the way: I forgot to ask, but which firmware do you use?
If you still have *two - stage*, then get rid of it.
Since one-stage fws contain a few fixes for most temporarily MAC/BB-hiccups.
> >> I'm having occasional problems where throughput drops through the floor
> >> (0.5Mbps - 1.5Mbps). When I cat
> >> /sys/kernel/debug/ieee80211/*/stations/*/rc_stats, one of the machines
> >> lists the full set of rates, but the other only lists 1M and 54M. After
> >> a period of time, that machine drops 54M and lists only one rate
> >> (1Mbps), and the throughput listed by nuttcp drops accordingly. I
> >> assume that, for whatever reason, the rates drop off the list and
> >> minstrel uses the only one left available to it.
> >>
> >> If I modify include/net/mac80211.h and force the inline function
> >> rate_supported to always return 1, this fixes the problem. However, I
> >> think this is a band aid around some other issue.
> >>
> >> Any clues or ideas what the real issue might be here?
>
> My guess::
>
> When an adhoc node (call it A) merges with a second adhoc node (call it
> B) there is a capability comparison.
> Node A looks at the rates supported by B and says,
> "I must only transmit at rates supported by B"
>
> Some management frames don't contain a full report of the rates supported
> by the sender.
> My view is that node A (in this example) is incorrectly determining that B
> only supports the 1mb/sec rate. Consequently, node A fills the
> rate_supported array with one rate - 1mb/sec.
well, that's the thing... it sounds like something in cfg80211/mac80211 has
gone wrong. Since ibss supported/basic rates IEs should always include all
mandatory rates for the given band & mode. Therefore you should see the
2Mbit, 11Mbit, 6MBit, 12Mbit 24Mbit rates in rc_stats array as well.
> =====
=====
> There is no evidence that Minstrel is doing anything wrong.
?but no one said it was minstrel fault? And it clearly isn't.
But something OT: do you have already thoughts about
_extending_ minstrel to support 802.11n MCS rates?
The current endeavor is stuck and needs a kick-start.
This is partly because of a hen-egg problem:
no driver <-> no 11n rc. But it should be easy to get
11n capable hw now (e.g. Mikrotik's R52N) and
ath9k should be the perfect testing platform right now.
nbd has/had some thought about grouping rates and options
(e.g SGI/40MHz) together to reduce the number of rates to
improve the _search for best tp_ time. But dunno, maybe he
has already something better than the proof-of-concept I wrote earlier.
Regards,
Chr
On Wed, Nov 4, 2009 at 1:53 PM, Adam Wozniak <[email protected]> wrote:
> My guess from this is that I'm using the single stage:
>
> # dmesg | grep ar9170
> [ 273.608189] usb 2-1: firmware: requesting ar9170.fw
> [ 274.019650] Registered led device: ar9170-phy1::tx
> [ 274.019698] Registered led device: ar9170-phy1::assoc
> [ 274.019751] usbcore: registered new interface driver ar9170usb
Affirmative.
Luis
I'll let that run tonight as well. Although I think the comment about
using rate_control_rate_update instead of rate_control_rate_init would
be important to future developers. I also notice that mlme.c calls
rate_control_rate_update. I'm not sure what that does. Someone
familiar with that may want to investigate, it's probably a big deal if
someone changes bands and all the supported rates change.
Alternately, it may be ok just to stick a stub in like this for now:
static void minstrel_rate_update(void *priv, struct
ieee80211_supported_band *sband,
struct ieee80211_sta *sta, void *priv_sta, u32 changed) {
minstrel_rate_init(priv, sband, sta, priv_sta);
}
( and update mac80211_minstrel )
Thoughts?
--Adam
Christian Lamparter wrote:
> On Thursday 12 November 2009 23:38:07 Adam Wozniak wrote:
>
>> I see what you're doing there.
>> That didn't quite work, but I'm fairly confident this one will.
>>
> yep, what about the attached version? ...it's slightly different...
>
> BTW: Use "diff -up" or "diff -uprN" to create patches. (or git :) )
> ---
>
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index fbffce9..7c6c170 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -246,9 +246,12 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> + /* make sure mandatory rates are always added */
> + supp_rates = ieee80211_mandatory_rates(local, band);
> +
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
> - supp_rates = ieee80211_sta_get_rates(local, elems, band);
> + supp_rates |= ieee80211_sta_get_rates(local, elems, band);
>
> rcu_read_lock();
>
> @@ -257,9 +260,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
> u32 prev_rates;
>
> prev_rates = sta->sta.supp_rates[band];
> - /* make sure mandatory rates are always added */
> - sta->sta.supp_rates[band] = supp_rates |
> - ieee80211_mandatory_rates(local, band);
> + sta->sta.supp_rates[band] = supp_rates;
>
> #ifdef CONFIG_MAC80211_IBSS_DEBUG
> if (sta->sta.supp_rates[band] != prev_rates)
> @@ -272,6 +273,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
> (unsigned long long) supp_rates,
> (unsigned long long) sta->sta.supp_rates[band]);
> #endif
> + rate_control_rate_init(sta);
> } else
> ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
>
>
On Tue, 2009-11-24 at 11:44 -0800, Adam Wozniak wrote:
> Johannes Berg wrote:
> > On Tue, 2009-11-24 at 10:43 -0800, Adam Wozniak wrote:
> >
> >> As far as I can tell, minstrel_rate_init does not do any allocation.
> >> The pid_algo version does not appear to do any allocation either.
> >>
> >
> > There are three more -- iwlwifi (2x) and ath9k
> >
> Ugh. Why aren't these in with the mac80211 code? Do they rely on
> something hardware specific?
Yeah ... they should go away.
> I could not find any allocations in the rate_init in any of these:
Ok, but can they all deal with calling get_rate and rate_init at the
same time? I think that could happen now, no?
johannes
Hi,
> Is it possible this is the problem? (note supp_rates is used at the bottom of
> the function, outside the "if")
Could be. Did it fix your problem?
Derek.
On Tue, 10 Nov 2009, Adam Wozniak wrote:
> Is it possible this is the problem? (note supp_rates is used at the bottom of
> the function, outside the "if")
>
> *** a/net/mac80211/ibss.c 2009-11-02 09:11:36.000000000 -0800
> --- b/net/mac80211/ibss.c 2009-11-10 16:31:46.291563951 -0800
> ***************
> *** 246,254 ****
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
> - supp_rates = ieee80211_sta_get_rates(local, elems, band);
>
> rcu_read_lock();
>
> --- 246,255 ----
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> + supp_rates = ieee80211_sta_get_rates(local, elems, band);
> +
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
>
> rcu_read_lock();
>
>
>
> Derek Smithies wrote:
>> Hi,
>>> Do you know which frames in the mac80211 code are missing this required
>>> information? Or was that conjecture?
>>
>> it was mostly conjecture. I had done some work with an earlier pull of
>> compat wireless in adhoc. the rates tried by minstrel were bad - it was
>> stuck at the slowest rate.
>>
>> I too had found that the fix was to adjust the array so that all rates were
>> marked as supported.
>>
>> Derek
>>
>> On Tue, 10 Nov 2009, Adam Wozniak wrote:
>>
>>> Reading through the 802.11 spec, it appears to me that "Supported rates"
>>> (and "Extended Supported Rates" when number of rates > 8) is REQUIRED for
>>> all management frames except authentication, deauthentication, and action
>>> frames. (IEEE 802.11-2007, 7.2.3)
>>>
>>> Do you know which frames in the mac80211 code are missing this required
>>> information? Or was that conjecture?
>>>
>>> Looking at mac80211/tx.c ieee80211_beacon_get_tim, it is not clear to me
>>> how (or if) this rate information is being set for ad-hoc beacons.
>>>
>>> Derek Smithies wrote:
>>>> Some management frames don't contain a full report of the rates supported
>>>> by the sender.
>>>> My view is that node A (in this example) is incorrectly determining that
>>>> B only supports the 1mb/sec rate. Consequently, node A fills the
>>>> rate_supported array with one rate - 1mb/sec.
>>>
>>>
>>>
>>
>
>
>
--
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/
"The only thing IE should be used for is to download Fire Fox"
"My favorite language is call STAR. It's extremely concise. It has
exactly one verb '*', which does exactly what I want at the moment."
--Larry Wall
On Mon, 2009-11-23 at 16:57 -0800, Adam Wozniak wrote:
> Previously, not all code paths set supp_rates, and a rate change did not
> reinitialize the rate control layer.
> This patch fixes those issues.
>
> Signed-off-by: Adam Wozniak <[email protected]>
> ---
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index 10d1385..474f66d 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -246,9 +246,13 @@ static void ieee80211_rx_bss_info(struct
> ieee80211_sub_if_data *sdata,
> if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
> return;
>
> + supp_rates = ieee80211_sta_get_rates(local, elems, band);
> +
> + /* make sure mandatory rates are always added */
> + supp_rates |= ieee80211_mandatory_rates(local, band);
> +
> if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
> memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
> - supp_rates = ieee80211_sta_get_rates(local, elems, band);
This change is pointless -- if we don't get into this if() we don't care
about the value of supp_rates.
> @@ -257,12 +261,10 @@ static void ieee80211_rx_bss_info(struct
> ieee80211_sub_if_data *sdata,
> u32 prev_rates;
>
> prev_rates = sta->sta.supp_rates[band];
> - /* make sure mandatory rates are always added */
> - sta->sta.supp_rates[band] = supp_rates |
> - ieee80211_mandatory_rates(local, band);
> + sta->sta.supp_rates[band] = supp_rates;
That's just as pointless, it doesn't matter whether you add the
mandatory rates here or earlier.
> + if (sta->sta.supp_rates[band] != prev_rates) {
> #ifdef CONFIG_MAC80211_IBSS_DEBUG
> - if (sta->sta.supp_rates[band] != prev_rates)
> printk(KERN_DEBUG "%s: updated supp_rates set "
> "for %pM based on beacon info (0x%llx | "
> "0x%llx -> 0x%llx)\n",
> @@ -272,6 +274,8 @@ static void ieee80211_rx_bss_info(struct
> ieee80211_sub_if_data *sdata,
> (unsigned long long) supp_rates,
> (unsigned long long) sta->sta.supp_rates[band]);
> #endif
> + rate_control_rate_init(sta);
> + }
> } else
> ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
> supp_rates);
And that's not really right -- I don't think we should be calling
_init() over and over again. This could be a call to
rate_control_rate_update() with a certain flag instead.
johannes
On Fri, 2009-11-13 at 14:35 -0800, Adam Wozniak wrote:
> I've traced it down to this bit in rx.c in prepare_for_handlers():
>
> case NL80211_IFTYPE_ADHOC:
> [ stuff deleted ]
> } else if (!rx->sta) {
> int rate_idx;
> if (rx->status->flag & RX_FLAG_HT)
> rate_idx = 0; /* TODO: HT rates */
> else
> rate_idx = rx->status->rate_idx;
>
> rx->sta = ieee80211_ibss_add_sta(sdata, bssid,
> hdr->addr2,
> BIT(rate_idx));
> }
> break;
>
> I don't think this is right. I know the issue is here, because if I
> change to "BIT(rate_idx) | 0xfff" the problem corrects. Either we need
> to (a) set it properly here or (b) make sure something else happens
> before or after. I'm not sure we have enough context here to do (a).
Hmm. This BIT(..) was basically here to ensure we have at least a single
good rate absent other information. If we do not receive a probe or
beacon frame from that peer at least once, but add it to our IBSS only
due to a single received data frame, we add the bitrate that this frame
was transmitted at so we have one rate that we know it can transmit (and
presumably also receive).
What should happen is that once it starts beaconing we pick up a beacon
and extend our set of known good rates.
johannes
On Wednesday 04 November 2009 23:20:07 Luis R. Rodriguez wrote:
> On Wed, Nov 04, 2009 at 11:18:13PM +0100, Christian Lamparter wrote:
> > On Wednesday 04 November 2009 22:50:46 Luis R. Rodriguez wrote:
> > > On Wed, Nov 4, 2009 at 1:46 PM, Adam Wozniak <[email protected]> wrote:
> > > >
> > > > $ ls -la /lib/firmware/ar9170*
> > > > -rw-r--r-- 1 root root 83968 2009-10-17 15:55 /lib/firmware/ar9170-1.fw
> > > > -rw-r--r-- 1 root root 3508 2009-10-17 15:55 /lib/firmware/ar9170-2.fw
> > > > -rw-r--r-- 1 root root 15960 2009-10-17 15:55 /lib/firmware/ar9170.fw
> > > >
> > > > It is unclear to me which are actually used.
> > >
> > > By default the ar9170.fw is tried first, if that fails then the others
> > > are tried. The 2-stage firmware will not be tried if your device
> > > cannot handle it but right now only the AVM Fritz devices can't handle
> > > the 2-stage firmware.
> > >
> > > Christian should we just remove 2-stage fw support?
> >
> > I've moved the two-stage fw into the legacy section some time ago :)
> > Maybe a add printk?
>
> Oh sorry didn't notice, I meant complete removal of its support on the
> driver though :)
Oh, I've no problem removing two-stage fw support.
But then, I don't know much about usability :-D and I've enough of
people banging their heads against each other.
Regards,
Chr
Johannes Berg wrote:
> I really don't like this patch anyhow though, it's at best a hackish
> workaround around an issue that should just be fixed instead.
>
> Will take another look at the other patch tomorrow.
>
I concur 2/2 is a hack. I'm not an 802.11 ad-hoc expert, and I haven't
been allocated time to pursue the issue any more deeply than this. It
is still not clear to me how the list of usable rates is supposed to be
determined when the IBSS is composed of stations with heterogeneous rate
capabilities, beaconing is stochastically distributed, and not all nodes
can hear all beacons, much less what is wrong with the way the current
code is doing it.
I do not believe 1/2 was a hack. It really was possible to get to the
end of that function and use that variable without it being assigned the
correct value. Also, it is important to reinitialize the rate control
layer when the list of usable rates changes.
I would certainly be in favor of a better patch that resolves all the
issues.
--Adam Wozniak
Hi,
On Wed, 4 Nov 2009, Christian Lamparter wrote:
> On Wednesday 04 November 2009 02:13:49 Adam Wozniak wrote:
>> I have two systems under test, both Dell laptops (a Latitude D630 and an
>> Inspiron 600m) both running Ubuntu 9.10 with the latest updates, and
>> bleeding edge compat-wireless-2009-11-02. I'm using identical AR9170
>> based D-Link DWA-160 USB 802.11adapters. I'm using nuttcp to measure
>> throughput. I'm running in ad-hoc mode. Both machines have the same
>> ar9170 files in /lib/firmware. The machines are sitting about 5 feet
>> apart in my office.
>>
>> I'm having occasional problems where throughput drops through the floor
>> (0.5Mbps - 1.5Mbps). When I cat
>> /sys/kernel/debug/ieee80211/*/stations/*/rc_stats, one of the machines
>> lists the full set of rates, but the other only lists 1M and 54M. After
>> a period of time, that machine drops 54M and lists only one rate
>> (1Mbps), and the throughput listed by nuttcp drops accordingly. I
>> assume that, for whatever reason, the rates drop off the list and
>> minstrel uses the only one left available to it.
>>
>> If I modify include/net/mac80211.h and force the inline function
>> rate_supported to always return 1, this fixes the problem. However, I
>> think this is a band aid around some other issue.
>>
>> Any clues or ideas what the real issue might be here?
My guess::
When an adhoc node (call it A) merges with a second adhoc node (call it
B) there is a capability comparison.
Node A looks at the rates supported by B and says,
"I must only transmit at rates supported by B"
Some management frames don't contain a full report of the rates supported
by the sender.
My view is that node A (in this example) is incorrectly determining that B
only supports the 1mb/sec rate. Consequently, node A fills the
rate_supported array with one rate - 1mb/sec.
=====
There is no evidence that Minstrel is doing anything wrong.
The original poster's report of examining
>> /sys/kernel/debug/ieee80211/*/stations/*/rc_stats
is very important, as it shows exactly what minstrel is doing.
When examining network problems, one should always check the rc_stats
file. This tells you much about what is going on in the radio medium.
Derek.
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/
"The only thing IE should be used for is to download Fire Fox"
"My favorite language is call STAR. It's extremely concise. It has
exactly one verb '*', which does exactly what I want at the moment."
--Larry Wall
Previously, not all code paths set supp_rates, and a rate change did not reinitialize the rate control layer.
This patch fixes those issues. (Hopefully not whitespace damaged this time)
Signed-off-by: Adam Wozniak <[email protected]>
---
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 10d1385..474f66d 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -246,9 +246,13 @@ static void ieee80211_rx_bss_info(struct
ieee80211_sub_if_data *sdata,
if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
return;
+ supp_rates = ieee80211_sta_get_rates(local, elems, band);
+
+ /* make sure mandatory rates are always added */
+ supp_rates |= ieee80211_mandatory_rates(local, band);
+
if (sdata->vif.type == NL80211_IFTYPE_ADHOC && elems->supp_rates &&
memcmp(mgmt->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0) {
- supp_rates = ieee80211_sta_get_rates(local, elems, band);
rcu_read_lock();
@@ -257,12 +261,10 @@ static void ieee80211_rx_bss_info(struct
ieee80211_sub_if_data *sdata,
u32 prev_rates;
prev_rates = sta->sta.supp_rates[band];
- /* make sure mandatory rates are always added */
- sta->sta.supp_rates[band] = supp_rates |
- ieee80211_mandatory_rates(local, band);
+ sta->sta.supp_rates[band] = supp_rates;
+ if (sta->sta.supp_rates[band] != prev_rates) {
#ifdef CONFIG_MAC80211_IBSS_DEBUG
- if (sta->sta.supp_rates[band] != prev_rates)
printk(KERN_DEBUG "%s: updated supp_rates set "
"for %pM based on beacon info (0x%llx | "
"0x%llx -> 0x%llx)\n",
@@ -272,6 +274,8 @@ static void ieee80211_rx_bss_info(struct
ieee80211_sub_if_data *sdata,
(unsigned long long) supp_rates,
(unsigned long long) sta->sta.supp_rates[band]);
#endif
+ rate_control_rate_init(sta);
+ }
} else
ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
@@ -415,6 +419,15 @@ struct sta_info *ieee80211_ibss_add_sta(struct
ieee80211_sub_if_data *sdata,
sta->sta.supp_rates[band] = supp_rates |
ieee80211_mandatory_rates(local, band);
+#ifdef CONFIG_MAC80211_IBSS_DEBUG
+ printk(KERN_DEBUG "%s: initialized supp_rates set "
+ "for %pM (0x%llx) (band %d)\n",
+ sdata->dev->name,
+ sta->sta.addr,
+ (unsigned long long) sta->sta.supp_rates[band],
+ band);
+#endif
+
rate_control_rate_init(sta);
if (sta_info_insert(sta))
Adam Wozniak wrote:
> It seems like the code I pointed out earlier in rx.c is certainly wrong
> then; bitrates used in absence of other information should not be
> guessed based on the received rate, but should come instead from the
> BSSID. Correct?
>
> Although, once you start these silly examples, it seems to me like the
> best thing to do is just assume the receiver can handle anything and let
> minstrel sort out what works and what doesn't. This has obvious problems
> for PID and similar rate algorithms.
In mesh networks here in Berlin and elsewhere, some people are running
with beacons disabled ('ahdemo' mode), so it would be nice if mac80211
could deal with this as well, by testing rates in absence of beacon
information.
- Felix
On Wednesday 04 November 2009 22:50:46 Luis R. Rodriguez wrote:
> On Wed, Nov 4, 2009 at 1:46 PM, Adam Wozniak <[email protected]> wrote:
> >
> > $ ls -la /lib/firmware/ar9170*
> > -rw-r--r-- 1 root root 83968 2009-10-17 15:55 /lib/firmware/ar9170-1.fw
> > -rw-r--r-- 1 root root 3508 2009-10-17 15:55 /lib/firmware/ar9170-2.fw
> > -rw-r--r-- 1 root root 15960 2009-10-17 15:55 /lib/firmware/ar9170.fw
> >
> > It is unclear to me which are actually used.
>
> By default the ar9170.fw is tried first, if that fails then the others
> are tried. The 2-stage firmware will not be tried if your device
> cannot handle it but right now only the AVM Fritz devices can't handle
> the 2-stage firmware.
>
> Christian should we just remove 2-stage fw support?
I've moved the two-stage fw into the legacy section some time ago :)
Maybe a add printk?
Regards,
Chr