Skip to contentSkip to conversation

mac80211: rc algos broken

PATCH New Michael Buesch 3 participants 7 messages Dec 20, 2007 – Dec 21, 2007 mac80211

lore
Share & tools
lore

Michael Buesch

author
#0 lore raw

RC algos are broken in current wireless-2.6.
PID fails to load because it can not find any debugfs related symbols (debugfs is mounted and works fine). I tried adding an #include <linux/debugfs.h> to the code, but it doesn't help.

mac80211 also fails to load _any_ rc algorithm.
Both, PID and simple are compiled (after disabling debugfs stuff) and mac80211 fails to find, load or whatever any of them. The message "Failed to load rc algo blabla you know it" appears.

Thanks a lot for fixing this ;)

Signature · 1 line
Greetings Michael.
author
#1 lore raw
Quoted 5 lines · On Thursday 20 December 2007 20:20:51 Michael Buesch wrote:

RC algos are broken in current wireless-2.6.
PID fails to load because it can not find any debugfs related symbols (debugfs is mounted and works fine). I tried adding an #include <linux/debugfs.h> to the code, but it doesn't help.

Quoted 7 lines

mac80211 also fails to load _any_ rc algorithm.
Both, PID and simple are compiled (after disabling debugfs stuff) and mac80211 fails to find, load or whatever any of them. The message "Failed to load rc algo blabla you know it" appears.

Thanks a lot for fixing this ;)

Grepping the code it seems that nobody is ever calling ieee80211_rate_control_register(). I guess that it kind of a fatal problem. ;)

Signature · 1 line
Greetings Michael.
#2 lore raw
Quoted 2 lines · On Thu, 20 Dec 2007 20:35:45 +0100 Michael Buesch <[email protected]> wrote:

Grepping the code it seems that nobody is ever calling ieee80211_rate_control_register(). I guess that it kind of a fatal problem. ;)

Indeed. Really sorry for the breakage, I'm fixing that in a bit.

--
Ciao
Stefano

#4 lore raw

PATCH 1/2 rc80211-pid: fix debugfs missing symbols

In reply to Michael Buesch

Declare the module license, so that debugfs will happily export its symbols to us.

Signed-off-by Stefano Brivio <[email protected]>

---

net/mac80211/rc80211_pid_algo.c +5 -0
Index: wireless-2.6/net/mac80211/rc80211_pid_algo.c
===================================================================
@@ -12,12 +12,17 @@
 #include <linux/netdevice.h>
 #include <linux/types.h>
 #include <linux/skbuff.h>
+#include <linux/debugfs.h>
 
 #include <net/mac80211.h>
 #include "ieee80211_rate.h"
 
 #include "rc80211_pid.h"
 
+MODULE_DESCRIPTION("PID controller based rate control algorithm");
+MODULE_AUTHOR("Stefano Brivio");
+MODULE_AUTHOR("Mattias Nissler");
+MODULE_LICENSE("GPL");
 
 /* This is an implementation of a TX rate control algorithm that uses a PID
  * controller. Given a target failed frames rate, the controller decides about
net/mac80211/rc80211_pid_debugfs.c +1 -0
Index: wireless-2.6/net/mac80211/rc80211_pid_debugfs.c
===================================================================
@@ -11,6 +11,7 @@
 #include <linux/netdevice.h>
 #include <linux/types.h>
 #include <linux/skbuff.h>
+#include <linux/debugfs.h>
 
 #include <net/mac80211.h>
 #include "ieee80211_rate.h"
Signature · 2 lines
Ciao
Stefano
#5 lore raw

PATCH 2/2 mac80211: fix breakage

In reply to Michael Buesch

Fix breakage introduced by:

commit 14d9eab2febd9caa8821d2b578d84e961dda3103

Author: Stefano Brivio <[email protected]>
Date:   Wed Dec 19 01:26:16 2007 +0100

---

net/mac80211/ieee80211.c +26 -1
Index: wireless-2.6/net/mac80211/ieee80211.c
===================================================================
@@ -1323,21 +1323,46 @@ static int __init ieee80211_init(void)
 
 	BUILD_BUG_ON(sizeof(struct ieee80211_tx_packet_data) > sizeof(skb->cb));
 
+	ret = ieee80211_rate_control_register(&mac80211_rcsimple);
+	if (ret)
+		goto fail;
+
+#ifdef CONFIG_MAC80211_RC_PID
+	ret = ieee80211_rate_control_register(&mac80211_rcpid);
+	if (ret)
+		goto fail;
+#endif
+
 	ret = ieee80211_wme_register();
 	if (ret) {
 		printk(KERN_DEBUG "ieee80211_init: failed to "
 		       "initialize WME (err=%d)\n", ret);
-		return ret;
+		goto fail;
 	}
 
 	ieee80211_debugfs_netdev_init();
 	ieee80211_regdomain_init();
 
 	return 0;
+
+fail:
+	ieee80211_rate_control_unregister(&mac80211_rcsimple);
+
+#ifdef CONFIG_MAC80211_RCPID
+	ieee80211_rate_control_unregister(&mac80211_rcpid);
+#endif
+
+	return ret;
 }
 
 static void __exit ieee80211_exit(void)
 {
+	ieee80211_rate_control_unregister(&mac80211_rcsimple);
+
+#ifdef CONFIG_MAC80211_RC_PID
+	ieee80211_rate_control_unregister(&mac80211_rcpid);
+#endif
+
 	ieee80211_wme_unregister();
 	ieee80211_debugfs_netdev_exit();
 }
net/mac80211/ieee80211_rate.h +6 -0
Index: wireless-2.6/net/mac80211/ieee80211_rate.h
===================================================================
@@ -58,6 +58,12 @@ struct rate_control_ref {
 	struct kref kref;
 };
 
+/* default 'simple' algorithm */
+extern struct rate_control_ops mac80211_rcsimple;
+
+/* 'PID' algorithm */
+extern struct rate_control_ops mac80211_rcpid;
+
 int ieee80211_rate_control_register(struct rate_control_ops *ops);
 void ieee80211_rate_control_unregister(struct rate_control_ops *ops);
net/mac80211/ieee80211_rate.c +1 -2
Index: wireless-2.6/net/mac80211/ieee80211_rate.c
===================================================================
@@ -94,8 +94,7 @@ ieee80211_try_rate_control_ops_get(const
 	return ops;
 }
 
-/* Get the rate control algorithm. If `name' is NULL, get the first
- * available algorithm. */
+/* Get the rate control algorithm. */
 static struct rate_control_ops *
 ieee80211_rate_control_ops_get(const char *name)
 {
Signature · 2 lines
Ciao
Stefano
#6 lore raw

PATCH v2 2/2 mac80211: fix breakage

In reply to Michael Buesch

Properly fix breakage introduced by:

commit 14d9eab2febd9caa8821d2b578d84e961dda3103

Author: Stefano Brivio <[email protected]>
Date:   Wed Dec 19 01:26:16 2007 +0100

Always build rc80211-simple into mac80211, and let rc80211-pid to be selected through a modparam and built as a module. The default rate control algorithm to be used can be set into the kernel configuration as well.

Signed-off-by Stefano Brivio <[email protected]>

---
John,

sorry for the delay. In case you didn't merge the previous patch, this is the proper fix for the breakage.
---

net/mac80211/ieee80211.c +12 -1
Index: wireless-2.6/net/mac80211/ieee80211.c
===================================================================
@@ -1323,21 +1323,32 @@ static int __init ieee80211_init(void)
 
 	BUILD_BUG_ON(sizeof(struct ieee80211_tx_packet_data) > sizeof(skb->cb));
 
+	ret = ieee80211_rate_control_register(&mac80211_rcsimple);
+	if (ret)
+		goto fail;
+
 	ret = ieee80211_wme_register();
 	if (ret) {
 		printk(KERN_DEBUG "ieee80211_init: failed to "
 		       "initialize WME (err=%d)\n", ret);
-		return ret;
+		goto fail;
 	}
 
 	ieee80211_debugfs_netdev_init();
 	ieee80211_regdomain_init();
 
 	return 0;
+
+fail:
+	ieee80211_rate_control_unregister(&mac80211_rcsimple);
+
+	return ret;
 }
 
 static void __exit ieee80211_exit(void)
 {
+	ieee80211_rate_control_unregister(&mac80211_rcsimple);
+
 	ieee80211_wme_unregister();
 	ieee80211_debugfs_netdev_exit();
 }
net/mac80211/ieee80211_rate.h +3 -0
Index: wireless-2.6/net/mac80211/ieee80211_rate.h
===================================================================
@@ -58,6 +58,9 @@ struct rate_control_ref {
 	struct kref kref;
 };
 
+/* default 'simple' algorithm */
+extern struct rate_control_ops mac80211_rcsimple;
+
 int ieee80211_rate_control_register(struct rate_control_ops *ops);
 void ieee80211_rate_control_unregister(struct rate_control_ops *ops);
net/mac80211/ieee80211_rate.c +1 -2
Index: wireless-2.6/net/mac80211/ieee80211_rate.c
===================================================================
@@ -94,8 +94,7 @@ ieee80211_try_rate_control_ops_get(const
 	return ops;
 }
 
-/* Get the rate control algorithm. If `name' is NULL, get the first
- * available algorithm. */
+/* Get the rate control algorithm. */
 static struct rate_control_ops *
 ieee80211_rate_control_ops_get(const char *name)
 {
net/mac80211/Kconfig +0 -13
Index: wireless-2.6/net/mac80211/Kconfig
===================================================================
@@ -35,7 +35,6 @@ config MAC80211_RC_DEFAULT_PID
 config MAC80211_RC_DEFAULT_SIMPLE
 	bool "Simple rate control algorithm"
 	depends on MAC80211
-	select MAC80211_RC_SIMPLE
 	help
 	Select the simple rate control as the default rate
 	control algorithm. Note that this is a non-responsive,

@@ -62,18 +61,6 @@ config MAC80211_RC_PID
 	  Say Y or M unless you're sure you want to use a
 	  different rate control algorithm.
 
-config MAC80211_RC_SIMPLE
-	tristate "Simple rate control algorithm (DEPRECATED)"
-	default n
-	depends on MAC80211
-	help
-	  This option enables a very simple, non-responsive TX
-	  rate control algorithm. This algorithm is deprecated
-	  and will be removed from the kernel in near future.
-	  It has been replaced by the PID algorithm.
-
-	  Say N unless you know what you are doing.
-
 config MAC80211_LEDS
 	bool "Enable LED triggers"
 	depends on MAC80211 && LEDS_TRIGGERS
net/mac80211/rc80211_pid_algo.c +15 -1
Index: wireless-2.6/net/mac80211/rc80211_pid_algo.c
===================================================================
@@ -498,7 +498,8 @@ static void rate_control_pid_free_sta(vo
 	kfree(spinfo);
 }
 
-struct rate_control_ops mac80211_rcpid = {
+static struct rate_control_ops mac80211_rcpid = {
+	.module = THIS_MODULE,
 	.name = "pid",
 	.tx_status = rate_control_pid_tx_status,
 	.get_rate = rate_control_pid_get_rate,

@@ -513,3 +514,16 @@ struct rate_control_ops mac80211_rcpid =
 	.remove_sta_debugfs = rate_control_pid_remove_sta_debugfs,
 #endif
 };
+
+static int __init rate_control_pid_init(void)
+{
+	return ieee80211_rate_control_register(&mac80211_rcpid);
+}
+
+static void __exit rate_control_pid_exit(void)
+{
+	ieee80211_rate_control_unregister(&mac80211_rcpid);
+}
+
+subsys_initcall(rate_control_pid_init);
+module_exit(rate_control_pid_exit);
Signature · 2 lines
Ciao
Stefano
#3 lore raw
In reply to Stefano Brivio
Quoted 7 lines · Stefano Brivio
Quoted 2 lines · On Thu, 20 Dec 2007 20:35:45 +0100 Michael Buesch <[email protected]> wrote:

Grepping the code it seems that nobody is ever calling ieee80211_rate_control_register(). I guess that it kind of a fatal problem. ;)

Indeed. Really sorry for the breakage, I'm fixing that in a bit.

Stefano,

Thanks for the patches. I mostly adopted your patch 2/2, but then I did some Kconfig/Makefile hacking on top. What I have merged keeps the rc algorithms built into the mac80211 module instead of their own modules. You can include either or both of simple and pid to be included, or neither if EMBEDDED. And of course you can select the default, which is also modifiable via a module option. I think this is simpler than the other options, and it gets the tree working again. I'm still open to some options for building one or both of the algorithms as a module, but that can be worked-out later.

I have folded my changes back into your patches for the push to davem. On the 'everything' branch I just included one fix-up patch to minimize disruption to the 'git pull' developers. :-) That is currently the head of the everything branch -- please take a look at it for details of what I have changed versus your patches.

Thanks,

John

Signature · 2 lines
John W. Linville
[email protected]