[PATCH] mac80211: fix rates setup on IBSS merge

[PATCH] mac80211: fix rates setup on IBSS merge

[Bruno Randolf]

when an IBSS merge happened, the supported rates for the newly added station
were left empty, causing the rate control module to be initialized with only
the basic rates.

also the section of the ibss code which deals with updating supported rates for
an already existing station fails to inform the rate control module about the
new rates. as i don't know how to fix this (minstrel does not have an update
function), i have just added a comment for now.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---

 net/mac80211/ibss.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index f3e9424..b840d90 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -276,6 +276,8 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
 				    (unsigned long long) sta->sta.supp_rates[band]);
 #endif
 			rcu_read_unlock();
+
+			/* FIXME: update rate control */
 		} else {
 			rcu_read_unlock();
 			ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
@@ -370,6 +372,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
 		       sdata->name, mgmt->bssid);
 #endif
 		ieee80211_sta_join_ibss(sdata, bss);
+		supp_rates = ieee80211_sta_get_rates(local, elems, band);
 		ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
 				       supp_rates, GFP_KERNEL);
 	}

Re: [PATCH] mac80211: fix rates setup on IBSS merge

[Luis R. Rodriguez]

On Tue, Feb 23, 2010 at 1:51 AM, Bruno Randolf <br1@einfach.org> wrote:
> when an IBSS merge happened, the supported rates for the newly added station
> were left empty, causing the rate control module to be initialized with only
> the basic rates.
>
> also the section of the ibss code which deals with updating supported rates for
> an already existing station fails to inform the rate control module about the
> new rates. as i don't know how to fix this (minstrel does not have an update
> function), i have just added a comment for now.
>
> Signed-off-by: Bruno Randolf <br1@einfach.org>

This seems like a stable fix, if it applies can you please resend with a

Cc: stable@kernel.org

on the commit log entry right below your own SOB.

  Luis

Re: [PATCH] mac80211: fix rates setup on IBSS merge

[Benoit PAPILLAULT]

Luis R. Rodriguez a écrit :
> On Tue, Feb 23, 2010 at 1:51 AM, Bruno Randolf <br1@einfach.org> wrote:
>   
>> when an IBSS merge happened, the supported rates for the newly added station
>> were left empty, causing the rate control module to be initialized with only
>> the basic rates.
>>
>> also the section of the ibss code which deals with updating supported rates for
>> an already existing station fails to inform the rate control module about the
>> new rates. as i don't know how to fix this (minstrel does not have an update
>> function), i have just added a comment for now.
>>
>> Signed-off-by: Bruno Randolf <br1@einfach.org>
>>     
>
> This seems like a stable fix, if it applies can you please resend with a
>
> Cc: stable@kernel.org
>
> on the commit log entry right below your own SOB.
>
>   Luis
>   
Hi Bruno,

I think the root cause is that IBSS neighbor entries are added whenever 
we received any packet from a neighbor. However, the supported rates are 
only available in the beacon/probe responses. I think one fix is to only 
add IBSS neighbors on beacon/probe response reception (and moreover, 
beacon/probe responses contains the timestamp that is needed for IBSS 
merge).

Basically, I removed the call to ieee80211_ibss_add_sta in 
net/mac80211/rx.c.

Can you try that and tell me about the result?

In fact, I did this change to have HT rates in IBSS mode (but I'm still 
missing some stuff).

Regards,
Benoit

Re: [PATCH] mac80211: fix rates setup on IBSS merge

[Bruno Randolf]

On Thursday 25 February 2010 08:56:40 Benoit PAPILLAULT wrote:
> Luis R. Rodriguez a écrit :
> > On Tue, Feb 23, 2010 at 1:51 AM, Bruno Randolf <br1@einfach.org> wrote:
> >> when an IBSS merge happened, the supported rates for the newly added
> >> station were left empty, causing the rate control module to be
> >> initialized with only the basic rates.
> >> 
> >> also the section of the ibss code which deals with updating supported
> >> rates for an already existing station fails to inform the rate control
> >> module about the new rates. as i don't know how to fix this (minstrel
> >> does not have an update function), i have just added a comment for now.
> >> 
> >> Signed-off-by: Bruno Randolf <br1@einfach.org>
> > 
> > This seems like a stable fix, if it applies can you please resend with a
> > 
> > Cc: stable@kernel.org
> > 
> > on the commit log entry right below your own SOB.
> > 
> >   Luis
> 
> Hi Bruno,
> 
> I think the root cause is that IBSS neighbor entries are added whenever
> we received any packet from a neighbor. However, the supported rates are
> only available in the beacon/probe responses. I think one fix is to only
> add IBSS neighbors on beacon/probe response reception (and moreover,
> beacon/probe responses contains the timestamp that is needed for IBSS
> merge).
> 
> Basically, I removed the call to ieee80211_ibss_add_sta in
> net/mac80211/rx.c.

i think we need to keep that.

it can happen in normal IBSS operation, that we get a data packet from a node 
which we did not yet receive a beacon from. for example: he might be part of 
the IBSS, correctly merged and all, but have deferred from sending a beacon 
the last couple of beacon intervals because some other node sent a beacon 
first. or we might have missed his beacons due to interference. nevertheless 
we should try to be able to communicate with him.

also there are many broken implementation out there (prominent example: 
madwifi) which fail to send beacons at some times. in order to communicate 
with then we also need to add stations on receiving data packets.

so i think the proper way is to add an update function to the rate control 
module. (i will try to do that and send in another patch.)

bruno

Re: [PATCH] mac80211: fix rates setup on IBSS merge

[Benoit PAPILLAULT]

Bruno Randolf a écrit :
> On Thursday 25 February 2010 08:56:40 Benoit PAPILLAULT wrote:
>   
>> Luis R. Rodriguez a écrit :
>>     
>>> On Tue, Feb 23, 2010 at 1:51 AM, Bruno Randolf <br1@einfach.org> wrote:
>>>       
>>>> when an IBSS merge happened, the supported rates for the newly added
>>>> station were left empty, causing the rate control module to be
>>>> initialized with only the basic rates.
>>>>
>>>> also the section of the ibss code which deals with updating supported
>>>> rates for an already existing station fails to inform the rate control
>>>> module about the new rates. as i don't know how to fix this (minstrel
>>>> does not have an update function), i have just added a comment for now.
>>>>
>>>> Signed-off-by: Bruno Randolf <br1@einfach.org>
>>>>         
>>> This seems like a stable fix, if it applies can you please resend with a
>>>
>>> Cc: stable@kernel.org
>>>
>>> on the commit log entry right below your own SOB.
>>>
>>>   Luis
>>>       
>> Hi Bruno,
>>
>> I think the root cause is that IBSS neighbor entries are added whenever
>> we received any packet from a neighbor. However, the supported rates are
>> only available in the beacon/probe responses. I think one fix is to only
>> add IBSS neighbors on beacon/probe response reception (and moreover,
>> beacon/probe responses contains the timestamp that is needed for IBSS
>> merge).
>>
>> Basically, I removed the call to ieee80211_ibss_add_sta in
>> net/mac80211/rx.c.
>>     
>
> i think we need to keep that.
>
> it can happen in normal IBSS operation, that we get a data packet from a node 
> which we did not yet receive a beacon from. for example: he might be part of 
> the IBSS, correctly merged and all, but have deferred from sending a beacon 
> the last couple of beacon intervals because some other node sent a beacon 
> first. or we might have missed his beacons due to interference. nevertheless 
> we should try to be able to communicate with him.
>
> also there are many broken implementation out there (prominent example: 
> madwifi) which fail to send beacons at some times. in order to communicate 
> with then we also need to add stations on receiving data packets.
>
> so i think the proper way is to add an update function to the rate control 
> module. (i will try to do that and send in another patch.)
>
> bruno
>
>   
For 802.11n for instance, only the beacons/probe responses say the 
sender is 802.11n. If we received a data packets from an unknown node, 
maybe we could simply do a probe request in order to get its probe 
response (despite the fact the 802.11 requires that only one node 
replies to probe request in IBSS, this is impossible to implement in 
practice). This will overcome if the sender node has not sent beacons 
for various reason (including shared beaconing).

Just my 2 cents.

Regards,
Benoit

Re: [PATCH] mac80211: fix rates setup on IBSS merge

[Adam Wozniak]

I noticed this a while back and tried (unsuccessfully) to get some 
patches pushed through for it.  Your patch looks cleaner than mine (I 
don't think I understood as much at the time as I do now).

You can look through the linux-wireless archives for some of my old 
posts.  Things to note:

For minstrel, calling rate_control_init() is effectively an update.  A 
workable solution may be to add a minstrel rate_control_update() that 
simply calls init().  I don't *think* PID has an update either, and 
don't forget about the cases where the rate control algorithm is handled 
in the firmware for the device.

Flow control in ieee80211_rx_bss_info() could use some cleanup.  It's 
possible to reach the end and use uninitialized variables.

To solve the problem for my devices, I used a patch to minstrel which 
actually ignores the rates in the beacon, under the assumption that if 
the other STA can't use them, they'll get 0% probability in the rate 
table.  This seems to work well for me.

--Adam

Bruno Randolf wrote:
> when an IBSS merge happened, the supported rates for the newly added station
> were left empty, causing the rate control module to be initialized with only
> the basic rates.
>
> also the section of the ibss code which deals with updating supported rates for
> an already existing station fails to inform the rate control module about the
> new rates. as i don't know how to fix this (minstrel does not have an update
> function), i have just added a comment for now.
>
> Signed-off-by: Bruno Randolf <br1@einfach.org>
> ---
>
>  net/mac80211/ibss.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index f3e9424..b840d90 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -276,6 +276,8 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
>  				    (unsigned long long) sta->sta.supp_rates[band]);
>  #endif
>  			rcu_read_unlock();
> +
> +			/* FIXME: update rate control */
>  		} else {
>  			rcu_read_unlock();
>  			ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
> @@ -370,6 +372,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
>  		       sdata->name, mgmt->bssid);
>  #endif
>  		ieee80211_sta_join_ibss(sdata, bss);
> +		supp_rates = ieee80211_sta_get_rates(local, elems, band);
>  		ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
>  				       supp_rates, GFP_KERNEL);
>  	}
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>