2007-06-02 18:52:16

by Larry Finger

[permalink] [raw]
Subject: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

The initial rate for STA's using rc80211_simple is set to the last
rate in the rate table. This is too fast for the bcm43xx-mac80211
driver. To correct this situation without affecting any other driver,
an initial_rate parameter is added to the rc80211_simple module. This
parameter is set to 10 times the desired initial rate. If the parameter
is zero or not set, the original behavior is kept. If the parameter
does not match a valid rate, the first rate in the table will be set.

Signed-off-by: Larry Finger <[email protected]>
---

Index: wireless-dev/net/mac80211/rc80211_simple.c
===================================================================
--- wireless-dev.orig/net/mac80211/rc80211_simple.c
+++ wireless-dev/net/mac80211/rc80211_simple.c
@@ -31,6 +31,10 @@

MODULE_ALIAS("rc80211_default");

+static int modparam_initial_rate;
+module_param_named(initial_rate, modparam_initial_rate, int, 0444);
+MODULE_PARM_DESC(initial_rate, "set initial rate in Mbs*10");
+
static void rate_control_rate_inc(struct ieee80211_local *local,
struct sta_info *sta)
{
@@ -283,14 +287,18 @@ static void rate_control_simple_rate_ini
int i;
sta->txrate = 0;
mode = local->oper_hw_mode;
- /* TODO: what is a good starting rate for STA? About middle? Maybe not
- * the lowest or the highest rate.. Could consider using RSSI from
- * previous packets? Need to have IEEE 802.1X auth succeed immediately
- * after assoc.. */
+ /* The starting rate for STA is set to the last rate in the table
+ * unless the module was loaded with the 'initial_rate' parameter,
+ * which is set to 10 times the desired rate. If an illegal value
+ * is used for the parameter, the first rate in the table will be used. */
for (i = 0; i < mode->num_rates; i++) {
if ((sta->supp_rates & BIT(i)) &&
(mode->rates[i].flags & IEEE80211_RATE_SUPPORTED))
- sta->txrate = i;
+ if (modparam_initial_rate) {
+ if (mode->rates[i].rate == modparam_initial_rate)
+ sta->txrate = i;
+ } else
+ sta->txrate = i;
}
}



2007-06-04 20:02:08

by James Ketrenos

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

James Ketrenos wrote:
> Larry Finger wrote:
>> The initial rate for STA's using rc80211_simple is set to the last
>> rate in the rate table. This is too fast for the bcm43xx-mac80211
>> driver.
>> To correct this situation without affecting any other driver,
>> an initial_rate parameter is added to the rc80211_simple module. This
>> parameter is set to 10 times the desired initial rate. If the parameter
>> is zero or not set, the original behavior is kept. If the parameter
>> does not match a valid rate, the first rate in the table will be set.
>
> Initial rate selection should be based on the RSSI of the last frame(s)
> received from the target network. One way to do that would be for the
> driver to map a minimum RSSI value to each rate. If the RSSI for the
> target network is greater than the value listed, that rate could be used
> for initial selection. The ieee80211_rate with the highest 'rate',
> which is in the supported rates mask and with a min_rssi <= the AP's
> reported RSSI, is selected.
>
> The mechanism by which the magic values for min_rssi are determined for
> each rate is more observational than scientific--and is vendor
> specific. You can pick a set of values, but different hardware and
> environments will yield different levels of success / failure. Some
> hardware can be very lenient if it has automatic hardware retry. Other
> hardware will need to be very conservative (since, as you point out, the
> first few frames Tx/Rx'd are important)

I didn't state it above -- doing what is described requires adding one unsigned char (min_rssi) to ieee80211_rate in mac80211.h and then modifying rc80211_simple to take advantage of that information (so if you're grepping for min_rssi and can't find it, that's why...)

James

>
> James
>
>>
>> Signed-off-by: Larry Finger <[email protected]>
> -
> 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

2007-06-07 22:19:30

by Jiri Benc

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

On Thu, 07 Jun 2007 16:22:52 -0500, Larry Finger wrote:
> I'm more than a little confused. First you tell me that setting the initial rate with a module
> parameter is bad because it sets a value for all _MY_ devices, then you say that you will allow a
> change that would affect _EVERYONE'S_ devices. Huh?

The first part was not specifically about the change, but about the
approach used for switching on the feature. Module parameters are not
intended for such things. Almost always (not counting debugging stuff) when
you use a module parameter you do something wrong (yes, that includes
ieee80211_regdom and ieee80211_japan_5ghz parameters in mac80211. And yes,
I'm aware there are some rare cases where module parameters are useful).

The actual change is another thing. I don't think that setting of the
initial rate matters much - if the algorithm is good enough, it will
quickly pick up the best rate independently of the initial rate.

> I wrote the code the way I did because I thought
> it was much better to allow a local solution rather than a global one.

Use a specific rate control algorithm then.

Thanks,

Jiri

--
Jiri Benc
SUSE Labs

2007-06-03 18:51:03

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

Olivier Cornu wrote:
>
> What if two different mac80211 device drivers need two different initial
> rates?

This situation has not changed, except that the user gets to pick the initial rate, not the software
designer of rc80211_simple.

Larry

2007-06-08 00:50:47

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: rc80211_lowest, a dumb rate control algorithm

Jiri Benc wrote:
> This allows the lowest rate to be used all time.
>
> To switch to this algorithm:
> echo -n lowest > {your_debugfs_mount_point}/ieee80211/phy?/rate_ctrl_alg
>
> Signed-off-by: Jiri Benc <[email protected]>

I do not think this particular rate control algorithm is needed. A patch has been submitted that
will allow the user to use the SIOCSIWRATE ioctl to lock in a rate. In addition, a patch that
changes rc80211_simple to set the initial rate to the lowest supported value has also been
submitted. With these two alternatives, everything that this proposed module does is supported in
other ways.

Larry


2007-06-03 19:14:36

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

Michael Buesch wrote:
> On Sunday 03 June 2007 12:33:36 Olivier Cornu wrote:
>> 2007/6/2, Larry Finger <[email protected]>:
>>> The initial rate for STA's using rc80211_simple is set to the last
>>> rate in the rate table. This is too fast for the bcm43xx-mac80211
>>> driver. To correct this situation without affecting any other driver,
>>> an initial_rate parameter is added to the rc80211_simple module.
>> What if two different mac80211 device drivers need two different initial rates?
>
> I'm not sure if that's useful at all. The algorithm will migrate
> away from the initial value very quickly anyway. What about simply
> using 1MBit for B-PHYs and (what's the lowest OFDM rate?) 9M? for
> G and A PHYs.

Although the algorithm will migrate away from the initial value very quickly, it doesn't happen
quickly enough for the current version of bcm43xx-mac80211 to authenticate.

I tried setting the lowest OFDM rate (6M) and it didn't work either. There is a timeout in
wpa_supplicant waiting for the WPA handshake. For my BCM4311 to make a connection, the rate must
start at 1M.

Larry



2007-06-04 19:32:36

by James Ketrenos

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

Larry Finger wrote:
> The initial rate for STA's using rc80211_simple is set to the last
> rate in the rate table. This is too fast for the bcm43xx-mac80211
> driver.
>
> To correct this situation without affecting any other driver,
> an initial_rate parameter is added to the rc80211_simple module. This
> parameter is set to 10 times the desired initial rate. If the parameter
> is zero or not set, the original behavior is kept. If the parameter
> does not match a valid rate, the first rate in the table will be set.

Initial rate selection should be based on the RSSI of the last frame(s) received from the target network. One way to do that would be for the driver to map a minimum RSSI value to each rate. If the RSSI for the target network is greater than the value listed, that rate could be used for initial selection. The ieee80211_rate with the highest 'rate', which is in the supported rates mask and with a min_rssi <= the AP's reported RSSI, is selected.

The mechanism by which the magic values for min_rssi are determined for each rate is more observational than scientific--and is vendor specific. You can pick a set of values, but different hardware and environments will yield different levels of success / failure. Some hardware can be very lenient if it has automatic hardware retry. Other hardware will need to be very conservative (since, as you point out, the first few frames Tx/Rx'd are important)

James

>
> Signed-off-by: Larry Finger <[email protected]>

2007-06-03 10:46:03

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

On Sunday 03 June 2007 12:33:36 Olivier Cornu wrote:
> 2007/6/2, Larry Finger <[email protected]>:
> > The initial rate for STA's using rc80211_simple is set to the last
> > rate in the rate table. This is too fast for the bcm43xx-mac80211
> > driver. To correct this situation without affecting any other driver,
> > an initial_rate parameter is added to the rc80211_simple module.
>
> What if two different mac80211 device drivers need two different initial rates?

I'm not sure if that's useful at all. The algorithm will migrate
away from the initial value very quickly anyway. What about simply
using 1MBit for B-PHYs and (what's the lowest OFDM rate?) 9M? for
G and A PHYs.

--
Greetings Michael.

2007-06-08 09:06:12

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: rc80211_lowest, a dumb rate control algorithm

On Fri, 2007-06-08 at 10:59 +0200, Jiri Benc wrote:

> > I do not think this particular rate control algorithm is needed. A patch has been submitted that
> > will allow the user to use the SIOCSIWRATE ioctl to lock in a rate. In addition, a patch that
> > changes rc80211_simple to set the initial rate to the lowest supported value has also been
> > submitted. With these two alternatives, everything that this proposed module does is supported in
> > other ways.
>
> Ok.

Actually, I'd vote for having this rate control algorithm anyway because
it's basically a rate control algorithm template you can copy and
modify. rc80211_simple is quite a bit more sophisticated over that.

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-06-08 08:58:55

by Jiri Benc

[permalink] [raw]
Subject: Re: [PATCH] mac80211: rc80211_lowest, a dumb rate control algorithm

On Thu, 07 Jun 2007 19:50:45 -0500, Larry Finger wrote:
> I do not think this particular rate control algorithm is needed. A patch has been submitted that
> will allow the user to use the SIOCSIWRATE ioctl to lock in a rate. In addition, a patch that
> changes rc80211_simple to set the initial rate to the lowest supported value has also been
> submitted. With these two alternatives, everything that this proposed module does is supported in
> other ways.

Ok.

I'd still like to apply the patch allowing to change rate control
algorithm, but I'd like to hear Johannes' comments first - I suspect he
won't like what I did with his macros :-)

Jiri

--
Jiri Benc
SUSE Labs

2007-06-03 10:33:38

by Olivier Cornu

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

2007/6/2, Larry Finger <[email protected]>:
> The initial rate for STA's using rc80211_simple is set to the last
> rate in the rate table. This is too fast for the bcm43xx-mac80211
> driver. To correct this situation without affecting any other driver,
> an initial_rate parameter is added to the rc80211_simple module.

What if two different mac80211 device drivers need two different initial rates?

--
Olivier Cornu

2007-06-07 20:53:49

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH] mac80211: rc80211_lowest, a dumb rate control algorithm

On Thu, Jun 07, 2007 at 10:22:10PM +0200, Jiri Benc wrote:
> This allows the lowest rate to be used all time.

> +MODULE_DESCRIPTION("Forced 1 mbps rate control module for mac80211");

That "1 mbps" should be "lowest supported". In case of 802.11a, this is
likely to be 6 Mbps.

--
Jouni Malinen PGP id EFC895FA

2007-06-07 21:22:54

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

Jiri Benc wrote:
>
> Don't misuse module parameters, please. If you have more devices, this
> single parameter will affect all of them. That's not a good approach,
> especially when the value you are trying to change can be changed on
> the fly.
--- snip ---
> However, I'm not against setting the initial value to the lowest rate
> available. As Johannes pointed out, if the rate control algorithm isn't
> able to quickly migrate to higher rates, it's a good candidate for
> rewrite.

I'm more than a little confused. First you tell me that setting the initial rate with a module
parameter is bad because it sets a value for all _MY_ devices, then you say that you will allow a
change that would affect _EVERYONE'S_ devices. Huh? I wrote the code the way I did because I thought
it was much better to allow a local solution rather than a global one.

Larry


2007-06-07 21:06:04

by Jiri Benc

[permalink] [raw]
Subject: Re: [PATCH] mac80211: rc80211_lowest, a dumb rate control algorithm

On Thu, 7 Jun 2007 13:54:31 -0700, Jouni Malinen wrote:
> On Thu, Jun 07, 2007 at 10:22:10PM +0200, Jiri Benc wrote:
> > This allows the lowest rate to be used all time.
>
> > +MODULE_DESCRIPTION("Forced 1 mbps rate control module for mac80211");
>
> That "1 mbps" should be "lowest supported". In case of 802.11a, this is
> likely to be 6 Mbps.

True. I was thinking too much about bcm43xx when writing that. But the
code should be OK, it's mostly copied from rc80211_simple :-)

Thanks,

Jiri

--
Jiri Benc
SUSE Labs

2007-06-04 11:44:13

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

On Sat, 2007-06-02 at 13:51 -0500, Larry Finger wrote:
> The initial rate for STA's using rc80211_simple is set to the last
> rate in the rate table. This is too fast for the bcm43xx-mac80211
> driver. To correct this situation without affecting any other driver,
> an initial_rate parameter is added to the rc80211_simple module. This
> parameter is set to 10 times the desired initial rate. If the parameter
> is zero or not set, the original behavior is kept. If the parameter
> does not match a valid rate, the first rate in the table will be set.

I think I'd like to see your "start at 1M" patch merged rather than this
new API. If rc80211_simple migrates quickly enough then it'll migrate to
54M quickly without causing much performance loss, if not we need a
better rate control algorithm anyway.

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-06-07 20:22:04

by Jiri Benc

[permalink] [raw]
Subject: [PATCH] mac80211: rc80211_lowest, a dumb rate control algorithm

This allows the lowest rate to be used all time.

To switch to this algorithm:
echo -n lowest > {your_debugfs_mount_point}/ieee80211/phy?/rate_ctrl_alg

Signed-off-by: Jiri Benc <[email protected]>

---
net/mac80211/Makefile | 2
net/mac80211/rc80211_lowest.c | 105 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+), 1 deletion(-)

--- mac80211-dev.orig/net/mac80211/Makefile
+++ mac80211-dev/net/mac80211/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_MAC80211) += mac80211.o rc80211_simple.o
+obj-$(CONFIG_MAC80211) += mac80211.o rc80211_simple.o rc80211_lowest.o

mac80211-objs-$(CONFIG_MAC80211_LEDS) += ieee80211_led.o
mac80211-objs-$(CONFIG_MAC80211_DEBUGFS) += debugfs.o debugfs_sta.o debugfs_netdev.o debugfs_key.o
--- /dev/null
+++ mac80211-dev/net/mac80211/rc80211_lowest.c
@@ -0,0 +1,105 @@
+/*
+ * Copyright 2002-2005, Instant802 Networks, Inc.
+ * Copyright 2005, Devicescape Software, Inc.
+ * Copyright (c) 2006-2007 Jiri Benc <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/netdevice.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/skbuff.h>
+#include <linux/compiler.h>
+
+#include <net/mac80211.h>
+#include "ieee80211_i.h"
+#include "ieee80211_rate.h"
+#include "debugfs.h"
+
+static void rate_control_lowest_tx_status(void *priv, struct net_device *dev,
+ struct sk_buff *skb,
+ struct ieee80211_tx_status *status)
+{
+}
+
+static struct ieee80211_rate *
+rate_control_lowest_get_rate(void *priv, struct net_device *dev,
+ struct sk_buff *skb,
+ struct rate_control_extra *extra)
+{
+ struct ieee80211_hw_mode *mode = extra->mode;
+ int i;
+
+ for (i = 0; i < mode->num_rates; i++) {
+ struct ieee80211_rate *rate = &mode->rates[i];
+
+ if (rate->flags & IEEE80211_RATE_SUPPORTED)
+ return rate;
+ }
+ return &mode->rates[0];
+}
+
+static void rate_control_lowest_rate_init(void *priv, void *priv_sta,
+ struct ieee80211_local *local,
+ struct sta_info *sta)
+{
+ sta->txrate = 0;
+}
+
+static void *rate_control_lowest_alloc(struct ieee80211_local *local)
+{
+ return local;
+}
+
+static void rate_control_lowest_free(void *priv)
+{
+}
+
+static void rate_control_lowest_clear(void *priv)
+{
+}
+
+static void *rate_control_lowest_alloc_sta(void *priv, gfp_t gfp)
+{
+ return priv;
+}
+
+static void rate_control_lowest_free_sta(void *priv, void *priv_sta)
+{
+}
+
+static struct rate_control_ops rate_control_lowest = {
+ .module = THIS_MODULE,
+ .name = "lowest",
+ .tx_status = rate_control_lowest_tx_status,
+ .get_rate = rate_control_lowest_get_rate,
+ .rate_init = rate_control_lowest_rate_init,
+ .clear = rate_control_lowest_clear,
+ .alloc = rate_control_lowest_alloc,
+ .free = rate_control_lowest_free,
+ .alloc_sta = rate_control_lowest_alloc_sta,
+ .free_sta = rate_control_lowest_free_sta,
+};
+
+static int __init rate_control_lowest_init(void)
+{
+ return ieee80211_rate_control_register(&rate_control_lowest);
+}
+
+
+static void __exit rate_control_lowest_exit(void)
+{
+ ieee80211_rate_control_unregister(&rate_control_lowest);
+}
+
+
+module_init(rate_control_lowest_init);
+module_exit(rate_control_lowest_exit);
+
+MODULE_DESCRIPTION("Forced 1 mbps rate control module for mac80211");
+MODULE_LICENSE("GPL");


--
Jiri Benc
SUSE Labs

2007-06-03 11:25:15

by Stefano Brivio

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

Quoting Michael Buesch <[email protected]>:

> (what's the lowest OFDM rate?) 9M?

6M.


--
Ciao
Stefano




2007-06-05 12:40:34

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

James Ketrenos wrote:
> James Ketrenos wrote:
>> Larry Finger wrote:
>>> The initial rate for STA's using rc80211_simple is set to the last
>>> rate in the rate table. This is too fast for the bcm43xx-mac80211
>>> driver.
>>> To correct this situation without affecting any other driver,
>>> an initial_rate parameter is added to the rc80211_simple module. This
>>> parameter is set to 10 times the desired initial rate. If the parameter
>>> is zero or not set, the original behavior is kept. If the parameter
>>> does not match a valid rate, the first rate in the table will be set.
>>
>> Initial rate selection should be based on the RSSI of the last
>> frame(s) received from the target network. One way to do that would
>> be for the driver to map a minimum RSSI value to each rate. If the
>> RSSI for the target network is greater than the value listed, that
>> rate could be used for initial selection. The ieee80211_rate with the
>> highest 'rate', which is in the supported rates mask and with a
>> min_rssi <= the AP's reported RSSI, is selected.
>>
>> The mechanism by which the magic values for min_rssi are determined
>> for each rate is more observational than scientific--and is vendor
>> specific. You can pick a set of values, but different hardware and
>> environments will yield different levels of success / failure. Some
>> hardware can be very lenient if it has automatic hardware retry.
>> Other hardware will need to be very conservative (since, as you point
>> out, the first few frames Tx/Rx'd are important)
>
> I didn't state it above -- doing what is described requires adding one
> unsigned char (min_rssi) to ieee80211_rate in mac80211.h and then
> modifying rc80211_simple to take advantage of that information (so if
> you're grepping for min_rssi and can't find it, that's why...)

I agree that the initial rate selection should be based on the RSSI of the last frame(s), but that
will take a long time to develop - both to get the necessary code into mac80211 and the driver
support for min_rssi. I'm not sure we who work with bcm43xx have that time. Although the mac80211
version of our driver is not yet in mainline, it has been included in FC7, which changes the nature
and the skills of the users. For many users, me included, the driver cannot associate and
authenticate with the "out of the box" version of mac80211. I posted a private patch on the bcm43xx
mailing list that set the initial rate to 1 Mbs, which allowed my system to connect. A number of
others reported back that this change also allowed them to connect much more reliably. I considered
submitting that patch, but decided that the new module parameter would be better for at least two
reasons: (1) any driver that has no difficulty connecting with the current scheme would not be
affected, and (2) it is a lot easier for users to edit /etc/modprobe.conf.local and add the line
"options rc80211_simple initial_rate=10" than it would be to download the kernel sources, patch them
for the equivalent change, and build the revised kernel.

I'm still waiting for one of the mac80211 developers to comment on whether starting at 1 Mbs would
place an undue speed penalty on a device that is capable of operating at 54 Mbs.

Larry



2007-06-08 09:03:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: rc80211_lowest, a dumb rate control algorithm

On Fri, 2007-06-08 at 10:59 +0200, Jiri Benc wrote:

> I'd still like to apply the patch allowing to change rate control
> algorithm, but I'd like to hear Johannes' comments first - I suspect he
> won't like what I did with his macros :-)

Oh, I don't care much about those macros, just wanted to save myself
some typing ;)

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-06-07 20:19:33

by Jiri Benc

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add module parameter for setting initial rate in rc80211_simple

On Sat, 02 Jun 2007 13:51:59 -0500, Larry Finger wrote:
> The initial rate for STA's using rc80211_simple is set to the last
> rate in the rate table. This is too fast for the bcm43xx-mac80211
> driver. To correct this situation without affecting any other driver,
> an initial_rate parameter is added to the rc80211_simple module. This
> parameter is set to 10 times the desired initial rate. If the parameter
> is zero or not set, the original behavior is kept. If the parameter
> does not match a valid rate, the first rate in the table will be set.

Don't misuse module parameters, please. If you have more devices, this
single parameter will affect all of them. That's not a good approach,
especially when the value you are trying to change can be changed on
the fly.

I understand the problems you have with tx power control, but modifying
a stack because of a buggy driver (I know it's not your nor anyone
else's fault and nothing can be done with this now) isn't a right way
to go.

The correct approach is to write a new rate control module. Note that
this should be still understood as a temporary solution, as you have no
factual reason for depending on a specific rate control module.

See the following patches.

However, I'm not against setting the initial value to the lowest rate
available. As Johannes pointed out, if the rate control algorithm isn't
able to quickly migrate to higher rates, it's a good candidate for
rewrite.

Thanks,

Jiri

--
Jiri Benc
SUSE Labs

2007-06-07 20:21:07

by Jiri Benc

[permalink] [raw]
Subject: [PATCH] mac80211: allow changing of rate control algorithm

Possibility to change rate control algorithm was lost during the conversion
to debugfs. This patch adds it again.

Of course, this should be done by cfg80211. After it's implemented there,
this can go away again.

Signed-off-by: Jiri Benc <[email protected]>

---
net/mac80211/debugfs.c | 61 ++++++++++++++++++++++++++++++++++-----------
net/mac80211/ieee80211_i.h | 1
2 files changed, 48 insertions(+), 14 deletions(-)

--- mac80211-dev.orig/net/mac80211/debugfs.c
+++ mac80211-dev/net/mac80211/debugfs.c
@@ -13,6 +13,16 @@
#include "ieee80211_rate.h"
#include "debugfs.h"

+static inline int rtnl_lock_local(struct ieee80211_local *local)
+{
+ rtnl_lock();
+ if (unlikely(local->reg_state != IEEE80211_DEV_REGISTERED)) {
+ rtnl_unlock();
+ return -ENODEV;
+ }
+ return 0;
+}
+
int mac80211_open_file_generic(struct inode *inode, struct file *file)
{
file->private_data = inode->i_private;
@@ -56,7 +66,7 @@ static const struct file_operations mode
.open = mac80211_open_file_generic,
};

-#define DEBUGFS_READONLY_FILE(name, buflen, fmt, value...) \
+#define DEBUGFS_READ(name, buflen, fmt, value...) \
static ssize_t name## _read(struct file *file, char __user *userbuf, \
size_t count, loff_t *ppos) \
{ \
@@ -67,16 +77,20 @@ static ssize_t name## _read(struct file
res = scnprintf(buf, buflen, fmt "\n", ##value); \
return simple_read_from_buffer(userbuf, count, ppos, buf, res); \
} \
- \
+
+#define DEBUGFS_READONLY_FILE(name, buflen, fmt, value...) \
+DEBUGFS_READ(name, buflen, fmt, ## value) \
static const struct file_operations name## _ops = { \
.read = name## _read, \
.open = mac80211_open_file_generic, \
};

-#define DEBUGFS_ADD(name) \
- local->debugfs.name = debugfs_create_file(#name, 0444, phyd, \
+#define DEBUGFS_ADD_MODE(name, mode) \
+ local->debugfs.name = debugfs_create_file(#name, mode, phyd, \
local, &name## _ops);

+#define DEBUGFS_ADD(name) DEBUGFS_ADD_MODE(name, 0444)
+
#define DEBUGFS_DEL(name) \
debugfs_remove(local->debugfs.name); \
local->debugfs.name = NULL;
@@ -113,21 +127,38 @@ DEBUGFS_READONLY_FILE(wep_iv, 20, "%#06x
DEBUGFS_READONLY_FILE(tx_power_reduction, 20, "%d.%d dBm",
local->hw.conf.tx_power_reduction / 10,
local->hw.conf.tx_power_reduction & 10);
-DEBUGFS_READONLY_FILE(rate_ctrl_alg, 100, "%s",
- local->rate_ctrl ? local->rate_ctrl->ops->name : "<unset>");

-/* statistics stuff */
+DEBUGFS_READ(rate_ctrl_alg, 100, "%s",
+ local->rate_ctrl ? local->rate_ctrl->ops->name : "<unset>");

-static inline int rtnl_lock_local(struct ieee80211_local *local)
+static ssize_t rate_ctrl_alg_write(struct file *file, const char __user *userbuf,
+ size_t count, loff_t *ppos)
{
- rtnl_lock();
- if (unlikely(local->reg_state != IEEE80211_DEV_REGISTERED)) {
- rtnl_unlock();
- return -ENODEV;
- }
- return 0;
+ struct ieee80211_local *local = file->private_data;
+ char buf[64];
+ ssize_t buf_size;
+ int res;
+
+ buf_size = min(count, ARRAY_SIZE(buf) - 1);
+ if (copy_from_user(buf, userbuf, buf_size))
+ return -EFAULT;
+ buf[buf_size] = '\0';
+ res = rtnl_lock_local(local);
+ if (res)
+ return res;
+ res = ieee80211_init_rate_ctrl_alg(local, buf);
+ rtnl_unlock();
+ return res < 0 ? res : buf_size;
}

+static const struct file_operations rate_ctrl_alg_ops = {
+ .read = rate_ctrl_alg_read,
+ .write = rate_ctrl_alg_write,
+ .open = mac80211_open_file_generic,
+};
+
+/* statistics stuff */
+
#define DEBUGFS_STATS_FILE(name, buflen, fmt, value...) \
DEBUGFS_READONLY_FILE(stats_ ##name, buflen, fmt, ##value)

@@ -318,6 +349,7 @@ void debugfs_hw_add(struct ieee80211_loc
DEBUGFS_ADD(mode);
DEBUGFS_ADD(wep_iv);
DEBUGFS_ADD(tx_power_reduction);
+ DEBUGFS_ADD_MODE(rate_ctrl_alg, 0644);
DEBUGFS_ADD(modes);

statsd = debugfs_create_dir("statistics", phyd);
@@ -383,6 +415,7 @@ void debugfs_hw_del(struct ieee80211_loc
DEBUGFS_DEL(mode);
DEBUGFS_DEL(wep_iv);
DEBUGFS_DEL(tx_power_reduction);
+ DEBUGFS_DEL(rate_ctrl_alg);
DEBUGFS_DEL(modes);

DEBUGFS_STATS_DEL(transmitted_fragment_count);
--- mac80211-dev.orig/net/mac80211/ieee80211_i.h
+++ mac80211-dev/net/mac80211/ieee80211_i.h
@@ -649,6 +649,7 @@ struct ieee80211_local {
struct dentry *total_ps_buffered;
struct dentry *mode;
struct dentry *wep_iv;
+ struct dentry *rate_ctrl_alg;
struct dentry *tx_power_reduction;
struct dentry *modes;
struct dentry *statistics;


--
Jiri Benc
SUSE Labs

2007-06-08 12:27:24

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: rc80211_lowest, a dumb rate control algorithm

Johannes Berg wrote:
> On Fri, 2007-06-08 at 10:59 +0200, Jiri Benc wrote:
>
>>> I do not think this particular rate control algorithm is needed. A patch has been submitted that
>>> will allow the user to use the SIOCSIWRATE ioctl to lock in a rate. In addition, a patch that
>>> changes rc80211_simple to set the initial rate to the lowest supported value has also been
>>> submitted. With these two alternatives, everything that this proposed module does is supported in
>>> other ways.
>> Ok.
>
> Actually, I'd vote for having this rate control algorithm anyway because
> it's basically a rate control algorithm template you can copy and
> modify. rc80211_simple is quite a bit more sophisticated over that.
>
> johannes

If the comments include the idea that this is an template for the rate control mechanism, I withdraw
my objection.

Larry