2007-12-20 19:23:03

by Michael Büsch

[permalink] [raw]
Subject: mac80211: rc algos broken

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

--
Greetings Michael.


2007-12-20 23:14:04

by Stefano Brivio

[permalink] [raw]
Subject: [PATCH 2/2] mac80211: fix breakage

Fix breakage introduced by:

commit 14d9eab2febd9caa8821d2b578d84e961dda3103
Author: Stefano Brivio <[email protected]>
Date: Wed Dec 19 01:26:16 2007 +0100
---
Index: wireless-2.6/net/mac80211/ieee80211.c
===================================================================
--- wireless-2.6.orig/net/mac80211/ieee80211.c
+++ 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();
}
Index: wireless-2.6/net/mac80211/ieee80211_rate.h
===================================================================
--- wireless-2.6.orig/net/mac80211/ieee80211_rate.h
+++ 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);

Index: wireless-2.6/net/mac80211/ieee80211_rate.c
===================================================================
--- wireless-2.6.orig/net/mac80211/ieee80211_rate.c
+++ 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)
{



--
Ciao
Stefano

2007-12-21 14:28:56

by John W. Linville

[permalink] [raw]
Subject: Re: mac80211: rc algos broken

On Thu, Dec 20, 2007 at 09:51:50PM +0100, Stefano Brivio wrote:
> 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
--
John W. Linville
[email protected]

2007-12-20 23:53:48

by Stefano Brivio

[permalink] [raw]
Subject: [PATCH v2 2/2] mac80211: fix breakage

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.
---
Index: wireless-2.6/net/mac80211/ieee80211.c
===================================================================
--- wireless-2.6.orig/net/mac80211/ieee80211.c
+++ 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();
}
Index: wireless-2.6/net/mac80211/ieee80211_rate.h
===================================================================
--- wireless-2.6.orig/net/mac80211/ieee80211_rate.h
+++ 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);

Index: wireless-2.6/net/mac80211/ieee80211_rate.c
===================================================================
--- wireless-2.6.orig/net/mac80211/ieee80211_rate.c
+++ 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)
{
Index: wireless-2.6/net/mac80211/Kconfig
===================================================================
--- wireless-2.6.orig/net/mac80211/Kconfig
+++ 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
Index: wireless-2.6/net/mac80211/rc80211_pid_algo.c
===================================================================
--- wireless-2.6.orig/net/mac80211/rc80211_pid_algo.c
+++ 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);



--
Ciao
Stefano

2007-12-20 19:38:00

by Michael Büsch

[permalink] [raw]
Subject: Re: mac80211: rc algos broken

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.
>
> 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. ;)

--
Greetings Michael.

2007-12-20 23:12:05

by Stefano Brivio

[permalink] [raw]
Subject: [PATCH 1/2] rc80211-pid: fix debugfs missing symbols

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

Signed-off-by: Stefano Brivio <[email protected]>
---
Index: wireless-2.6/net/mac80211/rc80211_pid_algo.c
===================================================================
--- wireless-2.6.orig/net/mac80211/rc80211_pid_algo.c
+++ 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
Index: wireless-2.6/net/mac80211/rc80211_pid_debugfs.c
===================================================================
--- wireless-2.6.orig/net/mac80211/rc80211_pid_debugfs.c
+++ 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"



--
Ciao
Stefano

2007-12-20 20:55:36

by Stefano Brivio

[permalink] [raw]
Subject: Re: mac80211: rc algos broken

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