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

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

[Larry Finger]

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 <Larry.Finger@lwfinger.net>
---

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;
 	}
 }
 

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

[Olivier Cornu]

2007/6/2, Larry Finger <Larry.Finger@lwfinger.net>:
> 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

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

[Michael Buesch]

On Sunday 03 June 2007 12:33:36 Olivier Cornu wrote:
> 2007/6/2, Larry Finger <Larry.Finger@lwfinger.net>:
> > 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.

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

[stefano.brivio]

Quoting Michael Buesch <mb@bu3sch.de>:

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

6M.


--
Ciao
Stefano

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

[Larry Finger]

Michael Buesch wrote:
> On Sunday 03 June 2007 12:33:36 Olivier Cornu wrote:
>> 2007/6/2, Larry Finger <Larry.Finger@lwfinger.net>:
>>> 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

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

[Larry Finger]

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

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

[James Ketrenos]

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 <Larry.Finger@lwfinger.net>

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

[James Ketrenos]

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 <Larry.Finger@lwfinger.net>
> -
> 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

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

[Larry Finger]

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

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

[Jiri Benc]

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

[PATCH] mac80211: allow changing of rate control algorithm

[Jiri Benc]

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 <jbenc@suse.cz>

---
 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

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

[Jiri Benc]

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 <jbenc@suse.cz>

---
 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 <jbenc@suse.cz>
+ *
+ * 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

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

[Jouni Malinen]

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

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

[Jiri Benc]

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

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

[Larry Finger]

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 <jbenc@suse.cz>

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

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

[Jiri Benc]

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

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 344 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

[Larry Finger]

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

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

[Larry Finger]

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

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

[Jiri Benc]

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