Sam, I'm not entirely happy with the Makefile hacks. Can you take a look
at the comment there and tell me whether there's any way to convince
Kbuild to resolve the rc80211_pid.o even when rc80211_pid.o isn't in
obj-y or obj-m but within mac80211-y?
Also, I'm not really happy with the ieee80211_rate.h #if but I don't see
any way around that.
Below is the patch, comments welcome. I think this is the way we want
it, tristate for all rate control algorithms regardless of whether
mac80211 is modular (where =y then means "build algorithm into
mac80211") and forcing one of the algorithms to y to do exactly that.
From: Johannes Berg <[email protected]>
Subject: [PATCH] mac80211: better rate control algorithm selection
This patch changes mac80211's Kconfig/Makefile to:
* select between the PID and the SIMPLE rate control
algorithm as default
* always allow tri-state for the rate control algorithms,
building those that are selected 'y' into the mac80211
module (if that is a module, otherwise all into the kernel)
* force the default rate control algorithm to be built into
mac80211
It also makes both rate control algorithms proper modules again
with MODULE_LICENSE etc.
Only if EMBEDDED is the user allowed to select "NONE" as default
which will cause no algorithm to be selected, this will work
only when the driver brings one itself (e.g. iwlwifi drivers).
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/Kconfig | 37 ++++++++++++++++++-------------------
net/mac80211/Makefile | 40 ++++++++++++++++++++++++++++++++++------
net/mac80211/ieee80211.c | 34 +++++++++++-----------------------
net/mac80211/ieee80211_rate.h | 38 ++++++++++++++++++++++++++++++++------
net/mac80211/rc80211_pid_algo.c | 24 ++++++++++++++++++++++--
net/mac80211/rc80211_simple.c | 21 ++++++++++++++++++++-
6 files changed, 137 insertions(+), 57 deletions(-)
--- everything.orig/net/mac80211/Kconfig 2007-12-21 12:32:03.258062609 +0100
+++ everything/net/mac80211/Kconfig 2007-12-21 16:38:32.983806424 +0100
@@ -13,25 +13,17 @@ config MAC80211
This option enables the hardware independent IEEE 802.11
networking stack.
-config MAC80211_RC_DEFAULT_CHOICE
- bool "Choose default rate control algorithm" if EMBEDDED
- default y
- depends on MAC80211
- ---help---
- This options enables selection of a default rate control
- algorithm to be built into the mac80211 module. Alternate
- rate control algorithms might be built into the mac80211
- module as well.
+menu "Rate control algorithm selection"
+ depends on MAC80211 != n
choice
prompt "Default rate control algorithm"
default MAC80211_RC_DEFAULT_PID
- depends on MAC80211 && MAC80211_RC_DEFAULT_CHOICE
---help---
This option selects the default rate control algorithm
mac80211 will use. Note that this default can still be
overriden through the ieee80211_default_rc_algo module
- parameter.
+ parameter if different algorithms are available.
config MAC80211_RC_DEFAULT_PID
bool "PID controller based rate control algorithm"
@@ -50,19 +42,27 @@ config MAC80211_RC_DEFAULT_SIMPLE
dumb algorithm. You should choose the PID rate control
instead.
+config MAC80211_RC_DEFAULT_NONE
+ bool "No default algorithm"
+ depends on EMBEDDED
+ help
+ Selecting this option will select no default algorithm
+ and allow you to not build any. Do not choose this
+ option unless you know your driver comes with another
+ suitable algorithm.
endchoice
+comment "Selecting 'y' for an algorithm will"
+comment "build the algorithm into mac80211."
+
config MAC80211_RC_DEFAULT
string
- depends on MAC80211
default "pid" if MAC80211_RC_DEFAULT_PID
default "simple" if MAC80211_RC_DEFAULT_SIMPLE
default ""
config MAC80211_RC_PID
- bool "PID controller based rate control algorithm"
- default y
- depends on MAC80211
+ tristate "PID controller based rate control algorithm"
---help---
This option enables a TX rate control algorithm for
mac80211 that uses a PID controller to select the TX
@@ -72,16 +72,15 @@ config MAC80211_RC_PID
different rate control algorithm.
config MAC80211_RC_SIMPLE
- bool "Simple rate control algorithm (DEPRECATED)"
- default n
- depends on MAC80211
+ tristate "Simple rate control algorithm (DEPRECATED)"
---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.
+ and will be removed from the kernel in the near future.
It has been replaced by the PID algorithm.
Say N unless you know what you are doing.
+endmenu
config MAC80211_LEDS
bool "Enable LED triggers"
--- everything.orig/net/mac80211/Makefile 2007-12-21 13:09:22.008062120 +0100
+++ everything/net/mac80211/Makefile 2007-12-21 16:58:21.893811361 +0100
@@ -1,17 +1,44 @@
obj-$(CONFIG_MAC80211) += mac80211.o
+#
+# Rate control algorithms
+#
+# Build in those that are 'y' and build as modules those that are 'm'
+# Kconfig enforces (unless EMBEDDED) that one is always 'y'
+#
+mac80211-rc-$(CONFIG_MAC80211_RC_SIMPLE) += rc80211_simple.o
+# Hmm. I'd like to not do this but Kbuild doesn't resolve
+# the rc80211_pid.o into rc80211_pid-y when it isn't right
+# within the objs-m/objs-y list...
+ifeq ($(CONFIG_MAC80211_RC_PID),y)
+mac80211-rc-$(CONFIG_MAC80211_RC_PID) += $(rc80211_pid-y)
+else
+ifeq ($(CONFIG_MAC80211_RC_PID),m)
+mac80211-rc-$(CONFIG_MAC80211_RC_PID) += rc80211_pid.o
+endif
+endif
+
+rc80211_pid-y += rc80211_pid_algo.o
+rc80211_pid-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
+
+# extra #defines for the header file
+CFLAGS_rc80211_simple.o += -DRC80211_SIMPLE_COMPILE
+CFLAGS_rc80211_pid_algo.o += -DRC80211_PID_COMPILE
+
+# build the ones selected 'm' as modules
+obj-m += $(mac80211-rc-m)
+
+#
+# mac80211 building
+#
mac80211-objs-$(CONFIG_MAC80211_LEDS) += ieee80211_led.o
mac80211-objs-$(CONFIG_NET_SCHED) += wme.o
-mac80211-objs-$(CONFIG_MAC80211_RC_SIMPLE) += rc80211_simple.o
-mac80211-objs-$(CONFIG_MAC80211_RC_PID) += rc80211_pid_algo.o
-mac80211-debugfs-objs-$(CONFIG_MAC80211_RC_PID) += rc80211_pid_debugfs.o
mac80211-objs-$(CONFIG_MAC80211_DEBUGFS) += \
debugfs.o \
debugfs_sta.o \
debugfs_netdev.o \
- debugfs_key.o \
- $(mac80211-debugfs-objs-y)
+ debugfs_key.o
mac80211-objs := \
ieee80211.o \
@@ -32,4 +59,5 @@ mac80211-objs := \
key.o \
util.o \
event.o \
- $(mac80211-objs-y)
+ $(mac80211-objs-y) \
+ $(mac80211-rc-y)
--- everything.orig/net/mac80211/rc80211_simple.c 2007-12-21 13:10:50.258063802 +0100
+++ everything/net/mac80211/rc80211_simple.c 2007-12-21 13:41:07.702167209 +0100
@@ -13,6 +13,7 @@
#include <linux/slab.h>
#include <linux/skbuff.h>
#include <linux/compiler.h>
+#include <linux/module.h>
#include <net/mac80211.h>
#include "ieee80211_i.h"
@@ -349,7 +350,7 @@ static void rate_control_simple_remove_s
}
#endif
-struct rate_control_ops mac80211_rcsimple = {
+static struct rate_control_ops mac80211_rcsimple = {
.name = "simple",
.tx_status = rate_control_simple_tx_status,
.get_rate = rate_control_simple_get_rate,
@@ -364,3 +365,21 @@ struct rate_control_ops mac80211_rcsimpl
.remove_sta_debugfs = rate_control_simple_remove_sta_debugfs,
#endif
};
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Simple rate control algorithm");
+
+int __init rc80211_simple_init(void)
+{
+ return ieee80211_rate_control_register(&mac80211_rcsimple);
+}
+
+void __exit rc80211_simple_exit(void)
+{
+ ieee80211_rate_control_unregister(&mac80211_rcsimple);
+}
+
+#ifdef CONFIG_MAC80211_RC_SIMPLE_MODULE
+module_init(rc80211_simple_init);
+module_exit(rc80211_simple_exit);
+#endif
--- everything.orig/net/mac80211/ieee80211_rate.h 2007-12-21 13:41:31.842169001 +0100
+++ everything/net/mac80211/ieee80211_rate.h 2007-12-21 16:31:46.463806804 +0100
@@ -58,12 +58,6 @@ 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);
@@ -170,4 +164,36 @@ int ieee80211_init_rate_ctrl_alg(struct
const char *name);
void rate_control_deinitialize(struct ieee80211_local *local);
+
+/* Rate control algorithms */
+#if defined(RC80211_SIMPLE_COMPILE) || \
+ (defined(CONFIG_MAC80211_RC_SIMPLE) && \
+ !defined(CONFIG_MAC80211_RC_SIMPLE_MODULE))
+extern int rc80211_simple_init(void);
+extern void rc80211_simple_exit(void);
+#else
+static inline int rc80211_simple_init(void)
+{
+ return 0;
+}
+static inline void rc80211_simple_exit(void)
+{
+}
+#endif
+
+#if defined(RC80211_PID_COMPILE) || \
+ (defined(CONFIG_MAC80211_RC_PID) && \
+ !defined(CONFIG_MAC80211_RC_PID_MODULE))
+extern int rc80211_pid_init(void);
+extern void rc80211_pid_exit(void);
+#else
+static inline int rc80211_pid_init(void)
+{
+ return 0;
+}
+static inline void rc80211_pid_exit(void)
+{
+}
+#endif
+
#endif /* IEEE80211_RATE_H */
--- everything.orig/net/mac80211/ieee80211.c 2007-12-21 13:42:28.272180773 +0100
+++ everything/net/mac80211/ieee80211.c 2007-12-21 13:47:34.412168892 +0100
@@ -1323,23 +1323,19 @@ static int __init ieee80211_init(void)
BUILD_BUG_ON(sizeof(struct ieee80211_tx_packet_data) > sizeof(skb->cb));
-#ifdef CONFIG_MAC80211_RC_SIMPLE
- ret = ieee80211_rate_control_register(&mac80211_rcsimple);
+ ret = rc80211_simple_init();
if (ret)
goto fail;
-#endif
-#ifdef CONFIG_MAC80211_RC_PID
- ret = ieee80211_rate_control_register(&mac80211_rcpid);
+ ret = rc80211_pid_init();
if (ret)
- goto fail;
-#endif
+ goto fail_simple;
ret = ieee80211_wme_register();
if (ret) {
printk(KERN_DEBUG "ieee80211_init: failed to "
"initialize WME (err=%d)\n", ret);
- goto fail;
+ goto fail_pid;
}
ieee80211_debugfs_netdev_init();
@@ -1347,26 +1343,18 @@ static int __init ieee80211_init(void)
return 0;
-fail:
-
-#ifdef CONFIG_MAC80211_RC_SIMPLE
- ieee80211_rate_control_unregister(&mac80211_rcsimple);
-#endif
-#ifdef CONFIG_MAC80211_RC_PID
- ieee80211_rate_control_unregister(&mac80211_rcpid);
-#endif
-
+ fail_pid:
+ rc80211_simple_exit();
+ fail_simple:
+ rc80211_pid_exit();
+ fail:
return ret;
}
static void __exit ieee80211_exit(void)
{
-#ifdef CONFIG_MAC80211_RC_SIMPLE
- ieee80211_rate_control_unregister(&mac80211_rcsimple);
-#endif
-#ifdef CONFIG_MAC80211_RC_PID
- ieee80211_rate_control_unregister(&mac80211_rcpid);
-#endif
+ rc80211_simple_exit();
+ rc80211_pid_exit();
ieee80211_wme_unregister();
ieee80211_debugfs_netdev_exit();
--- everything.orig/net/mac80211/rc80211_pid_algo.c 2007-12-21 13:12:20.118094673 +0100
+++ everything/net/mac80211/rc80211_pid_algo.c 2007-12-21 16:33:14.133809245 +0100
@@ -12,7 +12,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"
@@ -493,7 +493,7 @@ static void rate_control_pid_free_sta(vo
kfree(spinfo);
}
-struct rate_control_ops mac80211_rcpid = {
+static struct rate_control_ops mac80211_rcpid = {
.name = "pid",
.tx_status = rate_control_pid_tx_status,
.get_rate = rate_control_pid_get_rate,
@@ -508,3 +508,23 @@ struct rate_control_ops mac80211_rcpid =
.remove_sta_debugfs = rate_control_pid_remove_sta_debugfs,
#endif
};
+
+MODULE_DESCRIPTION("PID controller based rate control algorithm");
+MODULE_AUTHOR("Stefano Brivio");
+MODULE_AUTHOR("Mattias Nissler");
+MODULE_LICENSE("GPL");
+
+int __init rc80211_pid_init(void)
+{
+ return ieee80211_rate_control_register(&mac80211_rcpid);
+}
+
+void __exit rc80211_pid_exit(void)
+{
+ ieee80211_rate_control_unregister(&mac80211_rcpid);
+}
+
+#ifdef CONFIG_MAC80211_RC_PID_MODULE
+module_init(rc80211_pid_init);
+module_exit(rc80211_pid_exit);
+#endif
> I think that rc80211-simple is broken. A user may just want not to have a
> real rate control algorithm (i.e. no need for rates above the lowest one or
> certainty of perfect signal). Currently, that user would choose
> rc80211-simple, I guess, and well, this is almost fine, as he could
> manually set a rate. But let's say it's an embedded device, and small
> footprint is a must. Why would he need to use any RC algorithm (even other
> than rc80211-simple, as your Kconfig changes allow for this) then? mac80211
> currently fails if no RC algorithms are available. So, I'd say, let's fix
> mac80211, so that it can work without any RC algorithm. And the default in
> this case could just be to set the lowest rate. That's what I called a
> dummy RC algorithm.
Well, even an embedded device needs a rate control algorithm. Me
allowing to build none into mac80211 is mainly for when you want to ship
your device with intel wireless hardware and that has an algorithm you
*know* will be used *anyway*.
johannes
This patch changes mac80211's Kconfig/Makefile to:
* select between the PID and the SIMPLE rate control
algorithm as default
* always allow tri-state for the rate control algorithms,
building those that are selected 'y' into the mac80211
module (if that is a module, otherwise all into the kernel)
* force the default rate control algorithm to be built into
mac80211
It also makes both rate control algorithms proper modules again
with MODULE_LICENSE etc.
Only if EMBEDDED is the user allowed to select "NONE" as default
which will cause no algorithm to be selected, this will work
only when the driver brings one itself (e.g. iwlwifi drivers).
Signed-off-by: Johannes Berg <[email protected]>
---
v2 adds a fallback to the built-in one (see last hunk of patch)
net/mac80211/Kconfig | 37 ++++++++++++++++++-------------------
net/mac80211/Makefile | 40 ++++++++++++++++++++++++++++++++++------
net/mac80211/ieee80211.c | 34 +++++++++++-----------------------
net/mac80211/ieee80211_rate.c | 4 ++++
net/mac80211/ieee80211_rate.h | 38 ++++++++++++++++++++++++++++++++------
net/mac80211/rc80211_pid_algo.c | 24 ++++++++++++++++++++++--
net/mac80211/rc80211_simple.c | 21 ++++++++++++++++++++-
7 files changed, 141 insertions(+), 57 deletions(-)
--- everything.orig/net/mac80211/Kconfig 2007-12-21 22:08:43.845117730 +0100
+++ everything/net/mac80211/Kconfig 2007-12-21 22:20:07.425118001 +0100
@@ -13,25 +13,17 @@ config MAC80211
This option enables the hardware independent IEEE 802.11
networking stack.
-config MAC80211_RC_DEFAULT_CHOICE
- bool "Choose default rate control algorithm" if EMBEDDED
- default y
- depends on MAC80211
- ---help---
- This options enables selection of a default rate control
- algorithm to be built into the mac80211 module. Alternate
- rate control algorithms might be built into the mac80211
- module as well.
+menu "Rate control algorithm selection"
+ depends on MAC80211 != n
choice
prompt "Default rate control algorithm"
default MAC80211_RC_DEFAULT_PID
- depends on MAC80211 && MAC80211_RC_DEFAULT_CHOICE
---help---
This option selects the default rate control algorithm
mac80211 will use. Note that this default can still be
overriden through the ieee80211_default_rc_algo module
- parameter.
+ parameter if different algorithms are available.
config MAC80211_RC_DEFAULT_PID
bool "PID controller based rate control algorithm"
@@ -50,19 +42,27 @@ config MAC80211_RC_DEFAULT_SIMPLE
dumb algorithm. You should choose the PID rate control
instead.
+config MAC80211_RC_DEFAULT_NONE
+ bool "No default algorithm"
+ depends on EMBEDDED
+ help
+ Selecting this option will select no default algorithm
+ and allow you to not build any. Do not choose this
+ option unless you know your driver comes with another
+ suitable algorithm.
endchoice
+comment "Selecting 'y' for an algorithm will"
+comment "build the algorithm into mac80211."
+
config MAC80211_RC_DEFAULT
string
- depends on MAC80211
default "pid" if MAC80211_RC_DEFAULT_PID
default "simple" if MAC80211_RC_DEFAULT_SIMPLE
default ""
config MAC80211_RC_PID
- bool "PID controller based rate control algorithm"
- default y
- depends on MAC80211
+ tristate "PID controller based rate control algorithm"
---help---
This option enables a TX rate control algorithm for
mac80211 that uses a PID controller to select the TX
@@ -72,16 +72,15 @@ config MAC80211_RC_PID
different rate control algorithm.
config MAC80211_RC_SIMPLE
- bool "Simple rate control algorithm (DEPRECATED)"
- default n
- depends on MAC80211
+ tristate "Simple rate control algorithm (DEPRECATED)"
---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.
+ and will be removed from the kernel in the near future.
It has been replaced by the PID algorithm.
Say N unless you know what you are doing.
+endmenu
config MAC80211_LEDS
bool "Enable LED triggers"
--- everything.orig/net/mac80211/Makefile 2007-12-21 22:08:44.005120985 +0100
+++ everything/net/mac80211/Makefile 2007-12-21 22:24:45.655121257 +0100
@@ -1,17 +1,44 @@
obj-$(CONFIG_MAC80211) += mac80211.o
+#
+# Rate control algorithms
+#
+# Build in those that are 'y' and build as modules those that are 'm'
+# Kconfig enforces (unless EMBEDDED) that one is always 'y'
+#
+mac80211-rc-$(CONFIG_MAC80211_RC_SIMPLE) += rc80211_simple.o
+# Hmm. I'd like to not do this but Kbuild doesn't resolve
+# the rc80211_pid.o into rc80211_pid-y when it isn't right
+# within the objs-m/objs-y list...
+ifeq ($(CONFIG_MAC80211_RC_PID),y)
+mac80211-rc-$(CONFIG_MAC80211_RC_PID) += $(rc80211_pid-y)
+else
+ifeq ($(CONFIG_MAC80211_RC_PID),m)
+mac80211-rc-$(CONFIG_MAC80211_RC_PID) += rc80211_pid.o
+endif
+endif
+
+rc80211_pid-y += rc80211_pid_algo.o
+rc80211_pid-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
+
+# extra #defines for the header file
+CFLAGS_rc80211_simple.o += -DRC80211_SIMPLE_COMPILE
+CFLAGS_rc80211_pid_algo.o += -DRC80211_PID_COMPILE
+
+# build the ones selected 'm' as modules
+obj-m += $(mac80211-rc-m)
+
+#
+# mac80211 building
+#
mac80211-objs-$(CONFIG_MAC80211_LEDS) += ieee80211_led.o
mac80211-objs-$(CONFIG_NET_SCHED) += wme.o
-mac80211-objs-$(CONFIG_MAC80211_RC_SIMPLE) += rc80211_simple.o
-mac80211-objs-$(CONFIG_MAC80211_RC_PID) += rc80211_pid_algo.o
-mac80211-debugfs-objs-$(CONFIG_MAC80211_RC_PID) += rc80211_pid_debugfs.o
mac80211-objs-$(CONFIG_MAC80211_DEBUGFS) += \
debugfs.o \
debugfs_sta.o \
debugfs_netdev.o \
- debugfs_key.o \
- $(mac80211-debugfs-objs-y)
+ debugfs_key.o
mac80211-objs := \
ieee80211.o \
@@ -32,4 +59,5 @@ mac80211-objs := \
key.o \
util.o \
event.o \
- $(mac80211-objs-y)
+ $(mac80211-objs-y) \
+ $(mac80211-rc-y)
--- everything.orig/net/mac80211/rc80211_simple.c 2007-12-21 22:08:44.125121799 +0100
+++ everything/net/mac80211/rc80211_simple.c 2007-12-21 22:24:45.915119955 +0100
@@ -13,6 +13,7 @@
#include <linux/slab.h>
#include <linux/skbuff.h>
#include <linux/compiler.h>
+#include <linux/module.h>
#include <net/mac80211.h>
#include "ieee80211_i.h"
@@ -349,7 +350,7 @@ static void rate_control_simple_remove_s
}
#endif
-struct rate_control_ops mac80211_rcsimple = {
+static struct rate_control_ops mac80211_rcsimple = {
.name = "simple",
.tx_status = rate_control_simple_tx_status,
.get_rate = rate_control_simple_get_rate,
@@ -364,3 +365,21 @@ struct rate_control_ops mac80211_rcsimpl
.remove_sta_debugfs = rate_control_simple_remove_sta_debugfs,
#endif
};
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Simple rate control algorithm");
+
+int __init rc80211_simple_init(void)
+{
+ return ieee80211_rate_control_register(&mac80211_rcsimple);
+}
+
+void __exit rc80211_simple_exit(void)
+{
+ ieee80211_rate_control_unregister(&mac80211_rcsimple);
+}
+
+#ifdef CONFIG_MAC80211_RC_SIMPLE_MODULE
+module_init(rc80211_simple_init);
+module_exit(rc80211_simple_exit);
+#endif
--- everything.orig/net/mac80211/ieee80211_rate.h 2007-12-21 22:08:44.325119792 +0100
+++ everything/net/mac80211/ieee80211_rate.h 2007-12-21 22:24:45.555121853 +0100
@@ -58,12 +58,6 @@ 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);
@@ -170,4 +164,36 @@ int ieee80211_init_rate_ctrl_alg(struct
const char *name);
void rate_control_deinitialize(struct ieee80211_local *local);
+
+/* Rate control algorithms */
+#if defined(RC80211_SIMPLE_COMPILE) || \
+ (defined(CONFIG_MAC80211_RC_SIMPLE) && \
+ !defined(CONFIG_MAC80211_RC_SIMPLE_MODULE))
+extern int rc80211_simple_init(void);
+extern void rc80211_simple_exit(void);
+#else
+static inline int rc80211_simple_init(void)
+{
+ return 0;
+}
+static inline void rc80211_simple_exit(void)
+{
+}
+#endif
+
+#if defined(RC80211_PID_COMPILE) || \
+ (defined(CONFIG_MAC80211_RC_PID) && \
+ !defined(CONFIG_MAC80211_RC_PID_MODULE))
+extern int rc80211_pid_init(void);
+extern void rc80211_pid_exit(void);
+#else
+static inline int rc80211_pid_init(void)
+{
+ return 0;
+}
+static inline void rc80211_pid_exit(void)
+{
+}
+#endif
+
#endif /* IEEE80211_RATE_H */
--- everything.orig/net/mac80211/ieee80211.c 2007-12-21 22:08:44.505121311 +0100
+++ everything/net/mac80211/ieee80211.c 2007-12-21 22:24:49.145136990 +0100
@@ -1323,23 +1323,19 @@ static int __init ieee80211_init(void)
BUILD_BUG_ON(sizeof(struct ieee80211_tx_packet_data) > sizeof(skb->cb));
-#ifdef CONFIG_MAC80211_RC_SIMPLE
- ret = ieee80211_rate_control_register(&mac80211_rcsimple);
+ ret = rc80211_simple_init();
if (ret)
goto fail;
-#endif
-#ifdef CONFIG_MAC80211_RC_PID
- ret = ieee80211_rate_control_register(&mac80211_rcpid);
+ ret = rc80211_pid_init();
if (ret)
- goto fail;
-#endif
+ goto fail_simple;
ret = ieee80211_wme_register();
if (ret) {
printk(KERN_DEBUG "ieee80211_init: failed to "
"initialize WME (err=%d)\n", ret);
- goto fail;
+ goto fail_pid;
}
ieee80211_debugfs_netdev_init();
@@ -1347,26 +1343,18 @@ static int __init ieee80211_init(void)
return 0;
-fail:
-
-#ifdef CONFIG_MAC80211_RC_SIMPLE
- ieee80211_rate_control_unregister(&mac80211_rcsimple);
-#endif
-#ifdef CONFIG_MAC80211_RC_PID
- ieee80211_rate_control_unregister(&mac80211_rcpid);
-#endif
-
+ fail_pid:
+ rc80211_simple_exit();
+ fail_simple:
+ rc80211_pid_exit();
+ fail:
return ret;
}
static void __exit ieee80211_exit(void)
{
-#ifdef CONFIG_MAC80211_RC_SIMPLE
- ieee80211_rate_control_unregister(&mac80211_rcsimple);
-#endif
-#ifdef CONFIG_MAC80211_RC_PID
- ieee80211_rate_control_unregister(&mac80211_rcpid);
-#endif
+ rc80211_simple_exit();
+ rc80211_pid_exit();
ieee80211_wme_unregister();
ieee80211_debugfs_netdev_exit();
--- everything.orig/net/mac80211/rc80211_pid_algo.c 2007-12-21 22:08:44.165120877 +0100
+++ everything/net/mac80211/rc80211_pid_algo.c 2007-12-21 22:24:45.985121365 +0100
@@ -12,7 +12,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"
@@ -493,7 +493,7 @@ static void rate_control_pid_free_sta(vo
kfree(spinfo);
}
-struct rate_control_ops mac80211_rcpid = {
+static struct rate_control_ops mac80211_rcpid = {
.name = "pid",
.tx_status = rate_control_pid_tx_status,
.get_rate = rate_control_pid_get_rate,
@@ -508,3 +508,23 @@ struct rate_control_ops mac80211_rcpid =
.remove_sta_debugfs = rate_control_pid_remove_sta_debugfs,
#endif
};
+
+MODULE_DESCRIPTION("PID controller based rate control algorithm");
+MODULE_AUTHOR("Stefano Brivio");
+MODULE_AUTHOR("Mattias Nissler");
+MODULE_LICENSE("GPL");
+
+int __init rc80211_pid_init(void)
+{
+ return ieee80211_rate_control_register(&mac80211_rcpid);
+}
+
+void __exit rc80211_pid_exit(void)
+{
+ ieee80211_rate_control_unregister(&mac80211_rcpid);
+}
+
+#ifdef CONFIG_MAC80211_RC_PID_MODULE
+module_init(rc80211_pid_init);
+module_exit(rc80211_pid_exit);
+#endif
--- everything.orig/net/mac80211/ieee80211_rate.c 2007-12-21 22:25:51.275120389 +0100
+++ everything/net/mac80211/ieee80211_rate.c 2007-12-21 22:37:40.115118707 +0100
@@ -115,6 +115,10 @@ ieee80211_rate_control_ops_get(const cha
/* try default if specific alg requested but not found */
ops = ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo);
+ /* try built-in one if specific alg requested but not found */
+ if (!ops && strlen(CONFIG_MAC80211_RC_DEFAULT))
+ ops = ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT);
+
return ops;
}
On Fri, 21 Dec 2007 22:43:11 +0100
Johannes Berg <[email protected]> wrote:
> This patch changes mac80211's Kconfig/Makefile to:
> * select between the PID and the SIMPLE rate control
> algorithm as default
> * always allow tri-state for the rate control algorithms,
> building those that are selected 'y' into the mac80211
> module (if that is a module, otherwise all into the kernel)
> * force the default rate control algorithm to be built into
> mac80211
>
> It also makes both rate control algorithms proper modules again
> with MODULE_LICENSE etc.
>
> Only if EMBEDDED is the user allowed to select "NONE" as default
> which will cause no algorithm to be selected, this will work
> only when the driver brings one itself (e.g. iwlwifi drivers).
>
> Signed-off-by: Johannes Berg <[email protected]>
Acked-by: Stefano Brivio <[email protected]>
--
Ciao
Stefano
On Fri, 21 Dec 2007 17:09:35 +0100
Johannes Berg <[email protected]> wrote:
> Below is the patch, comments welcome. I think this is the way we want
> it, tristate for all rate control algorithms regardless of whether
> mac80211 is modular (where =y then means "build algorithm into
> mac80211") and forcing one of the algorithms to y to do exactly that.
What about avoiding to force any algorithm to y, and instead always build a
dummy (a few lines of code which would just set the rate to the lowest
available) rate control algorithm into mac80211?
Looks good otherwise.
--
Ciao
Stefano
On Fri, 21 Dec 2007 21:43:36 +0100
Johannes Berg <[email protected]> wrote:
> > What about avoiding to force any algorithm to y, and instead always build a
> > dummy (a few lines of code which would just set the rate to the lowest
> > available) rate control algorithm into mac80211?
>
> Why?
I think that rc80211-simple is broken. A user may just want not to have a
real rate control algorithm (i.e. no need for rates above the lowest one or
certainty of perfect signal). Currently, that user would choose
rc80211-simple, I guess, and well, this is almost fine, as he could
manually set a rate. But let's say it's an embedded device, and small
footprint is a must. Why would he need to use any RC algorithm (even other
than rc80211-simple, as your Kconfig changes allow for this) then? mac80211
currently fails if no RC algorithms are available. So, I'd say, let's fix
mac80211, so that it can work without any RC algorithm. And the default in
this case could just be to set the lowest rate. That's what I called a
dummy RC algorithm.
--
Ciao
Stefano
Hi Johannes.
On Fri, Dec 21, 2007 at 05:09:35PM +0100, Johannes Berg wrote:
> Sam, I'm not entirely happy with the Makefile hacks. Can you take a look
> at the comment there and tell me whether there's any way to convince
> Kbuild to resolve the rc80211_pid.o even when rc80211_pid.o isn't in
> obj-y or obj-m but within mac80211-y?
>
> Also, I'm not really happy with the ieee80211_rate.h #if but I don't see
> any way around that.
>
> Below is the patch, comments welcome. I think this is the way we want
> it, tristate for all rate control algorithms regardless of whether
> mac80211 is modular (where =y then means "build algorithm into
> mac80211") and forcing one of the algorithms to y to do exactly that.
>
> From: Johannes Berg <[email protected]>
> Subject: [PATCH] mac80211: better rate control algorithm selection
>
> This patch changes mac80211's Kconfig/Makefile to:
> * select between the PID and the SIMPLE rate control
> algorithm as default
> * always allow tri-state for the rate control algorithms,
> building those that are selected 'y' into the mac80211
> module (if that is a module, otherwise all into the kernel)
> * force the default rate control algorithm to be built into
> mac80211
>
> It also makes both rate control algorithms proper modules again
> with MODULE_LICENSE etc.
>
> Only if EMBEDDED is the user allowed to select "NONE" as default
> which will cause no algorithm to be selected, this will work
> only when the driver brings one itself (e.g. iwlwifi drivers).
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> net/mac80211/Kconfig | 37 ++++++++++++++++++-------------------
> net/mac80211/Makefile | 40 ++++++++++++++++++++++++++++++++++------
> net/mac80211/ieee80211.c | 34 +++++++++++-----------------------
> net/mac80211/ieee80211_rate.h | 38 ++++++++++++++++++++++++++++++++------
> net/mac80211/rc80211_pid_algo.c | 24 ++++++++++++++++++++++--
> net/mac80211/rc80211_simple.c | 21 ++++++++++++++++++++-
> 6 files changed, 137 insertions(+), 57 deletions(-)
>
> --- everything.orig/net/mac80211/Kconfig 2007-12-21 12:32:03.258062609 +0100
> +++ everything/net/mac80211/Kconfig 2007-12-21 16:38:32.983806424 +0100
> @@ -13,25 +13,17 @@ config MAC80211
> This option enables the hardware independent IEEE 802.11
> networking stack.
>
> -config MAC80211_RC_DEFAULT_CHOICE
> - bool "Choose default rate control algorithm" if EMBEDDED
> - default y
> - depends on MAC80211
> - ---help---
> - This options enables selection of a default rate control
> - algorithm to be built into the mac80211 module. Alternate
> - rate control algorithms might be built into the mac80211
> - module as well.
> +menu "Rate control algorithm selection"
> + depends on MAC80211 != n
>
> choice
> prompt "Default rate control algorithm"
> default MAC80211_RC_DEFAULT_PID
> - depends on MAC80211 && MAC80211_RC_DEFAULT_CHOICE
> ---help---
> This option selects the default rate control algorithm
> mac80211 will use. Note that this default can still be
> overriden through the ieee80211_default_rc_algo module
> - parameter.
> + parameter if different algorithms are available.
>
> config MAC80211_RC_DEFAULT_PID
> bool "PID controller based rate control algorithm"
> @@ -50,19 +42,27 @@ config MAC80211_RC_DEFAULT_SIMPLE
> dumb algorithm. You should choose the PID rate control
> instead.
>
> +config MAC80211_RC_DEFAULT_NONE
> + bool "No default algorithm"
> + depends on EMBEDDED
> + help
> + Selecting this option will select no default algorithm
> + and allow you to not build any. Do not choose this
> + option unless you know your driver comes with another
> + suitable algorithm.
> endchoice
>
> +comment "Selecting 'y' for an algorithm will"
> +comment "build the algorithm into mac80211."
> +
> config MAC80211_RC_DEFAULT
> string
> - depends on MAC80211
> default "pid" if MAC80211_RC_DEFAULT_PID
> default "simple" if MAC80211_RC_DEFAULT_SIMPLE
> default ""
>
> config MAC80211_RC_PID
> - bool "PID controller based rate control algorithm"
> - default y
> - depends on MAC80211
> + tristate "PID controller based rate control algorithm"
> ---help---
> This option enables a TX rate control algorithm for
> mac80211 that uses a PID controller to select the TX
> @@ -72,16 +72,15 @@ config MAC80211_RC_PID
> different rate control algorithm.
>
> config MAC80211_RC_SIMPLE
> - bool "Simple rate control algorithm (DEPRECATED)"
> - default n
> - depends on MAC80211
> + tristate "Simple rate control algorithm (DEPRECATED)"
> ---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.
> + and will be removed from the kernel in the near future.
> It has been replaced by the PID algorithm.
>
> Say N unless you know what you are doing.
> +endmenu
>
> config MAC80211_LEDS
> bool "Enable LED triggers"
> --- everything.orig/net/mac80211/Makefile 2007-12-21 13:09:22.008062120 +0100
> +++ everything/net/mac80211/Makefile 2007-12-21 16:58:21.893811361 +0100
> @@ -1,17 +1,44 @@
> obj-$(CONFIG_MAC80211) += mac80211.o
>
> +#
> +# Rate control algorithms
> +#
> +# Build in those that are 'y' and build as modules those that are 'm'
> +# Kconfig enforces (unless EMBEDDED) that one is always 'y'
> +#
> +mac80211-rc-$(CONFIG_MAC80211_RC_SIMPLE) += rc80211_simple.o
> +# Hmm. I'd like to not do this but Kbuild doesn't resolve
> +# the rc80211_pid.o into rc80211_pid-y when it isn't right
> +# within the objs-m/objs-y list...
> +ifeq ($(CONFIG_MAC80211_RC_PID),y)
> +mac80211-rc-$(CONFIG_MAC80211_RC_PID) += $(rc80211_pid-y)
> +else
> +ifeq ($(CONFIG_MAC80211_RC_PID),m)
> +mac80211-rc-$(CONFIG_MAC80211_RC_PID) += rc80211_pid.o
> +endif
> +endif
> +
> +rc80211_pid-y += rc80211_pid_algo.o
> +rc80211_pid-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
> +
> +# extra #defines for the header file
> +CFLAGS_rc80211_simple.o += -DRC80211_SIMPLE_COMPILE
> +CFLAGS_rc80211_pid_algo.o += -DRC80211_PID_COMPILE
> +
> +# build the ones selected 'm' as modules
> +obj-m += $(mac80211-rc-m)
> +
> +#
> +# mac80211 building
> +#
> mac80211-objs-$(CONFIG_MAC80211_LEDS) += ieee80211_led.o
> mac80211-objs-$(CONFIG_NET_SCHED) += wme.o
> -mac80211-objs-$(CONFIG_MAC80211_RC_SIMPLE) += rc80211_simple.o
> -mac80211-objs-$(CONFIG_MAC80211_RC_PID) += rc80211_pid_algo.o
>
> -mac80211-debugfs-objs-$(CONFIG_MAC80211_RC_PID) += rc80211_pid_debugfs.o
> mac80211-objs-$(CONFIG_MAC80211_DEBUGFS) += \
> debugfs.o \
> debugfs_sta.o \
> debugfs_netdev.o \
> - debugfs_key.o \
> - $(mac80211-debugfs-objs-y)
> + debugfs_key.o
>
> mac80211-objs := \
> ieee80211.o \
> @@ -32,4 +59,5 @@ mac80211-objs := \
> key.o \
> util.o \
> event.o \
> - $(mac80211-objs-y)
> + $(mac80211-objs-y) \
> + $(mac80211-rc-y)
This looks overly complicated.
Reading your patch several times rsulted in this conclusions:
rc80211_simple.o is build-in or module decided alone by CONFIG_MAC80211_SIMPLE
and the relation to MAC80211_RC_PID is just bogus.
MAC80211_RC_PID build-in =>
rc80211_pid_algo as build-in
rc80211_pid_debugfs as build-in if CONFIG_MAC80211_DEBUGFS equals y (is enabled)
MAC80211_RC_PID module =>
rc80211_pid.o as a module
rc80211_pid_algo.o and rc80211_pid_debugfs.o are ignored
The rest looks like ordinary relations - but expressed a bit ackward.
Following is a more clean approach that uses the mac80211-y syntax to specify
the .o files used as part of the module.
# The mac80211 module
obj-$(CONFIG_MAC80211) += mac80211.o
# If RC algorithm in build-in
rate-rc-y := rc80211_pid_algo.o
rate-rc-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
# If RC algorithm is modular
rate-rc-m := rc80211_pid.o
mac80211-y += ieee80211.o key.o util.o event.o
mac80211-$(CONFIG_MAC80211_LEDS) += ieee80211_led.o
mac80211-$(CONFIG_NET_SCHED) += wme.o
mac80211-$(CONFIG_MAC80211_DEBUGFS) += debugfs.o debugfs_sta.o
mac80211-$(CONFIG_MAC80211_DEBUGFS) += debugfs_netdev.o debugfs_key.o
# And this part select the rate algorithm
mac80211-$(CONFIG_MAC80211_SIMPLE) += rc80211_simple.o
mac80211-$(CONFIG_MAC80211_RC_PID) += $(rate-rc-$(CONFIG_MAC80211_RC_PID))
# Modular rate algorithm was assigned to mac80211-m - make it a separate module
obj-m += $(mac80211-m)
Let me know if this are in line with what you tried to achieve
or you need more feedback.
And sorry for the late answer.
Sam
> What about avoiding to force any algorithm to y, and instead always build a
> dummy (a few lines of code which would just set the rate to the lowest
> available) rate control algorithm into mac80211?
Why?
johannes
On Mon, Dec 31, 2007 at 12:11:20AM +0100, Sam Ravnborg wrote:
> # If RC algorithm in build-in
> rate-rc-y := rc80211_pid_algo.o
> rate-rc-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
>
> # If RC algorithm is modular
> rate-rc-m := rc80211_pid.o
>
> mac80211-y += ieee80211.o key.o util.o event.o
> mac80211-$(CONFIG_MAC80211_LEDS) += ieee80211_led.o
> mac80211-$(CONFIG_NET_SCHED) += wme.o
> mac80211-$(CONFIG_MAC80211_DEBUGFS) += debugfs.o debugfs_sta.o
> mac80211-$(CONFIG_MAC80211_DEBUGFS) += debugfs_netdev.o debugfs_key.o
>
> # And this part select the rate algorithm
> mac80211-$(CONFIG_MAC80211_SIMPLE) += rc80211_simple.o
> mac80211-$(CONFIG_MAC80211_RC_PID) += $(rate-rc-$(CONFIG_MAC80211_RC_PID))
>
> # Modular rate algorithm was assigned to mac80211-m - make it a separate module
> obj-m += $(mac80211-m)
Why don't we just build the rate control algorithms entirely separate
useing obj-$(FOO) ? If they're modular that lets them be their own
module, and if they're built-in they'll still go into built-in.o.
But why do we want to have rc80211_pid.o only if modular and
rc80211_pid_algo.o only if built-in? This seems like a complete mess
to me.
On Wednesday 02 January 2008 09:52:14 Johannes Berg wrote:
>
> > Building an algorithm optionally into mac80211 seems rather odd. In
> > case mac80211 the algorithms should be their own modules and you should
> > use request_module to make sure at least one is loaded.
>
> We actually do that but people still manage to mess it up because they
> don't install the modules properly or whatever.
>
> > Or if you
> > really want to make sure one is alway avaiabable always build the
> > simplest one directly and unconditionally into mac80211.ko.
>
> That'd be an alternative too, I think we have something like that now. I
> don't really like it that much though because for 99% of people it's
> just dead code. Anyway, I'll look into Sam's mail and do a new patch
> depending on that.
What about simply _not_ failing the initialization of mac80211, if no
rc algo is available? I mean, we can just use a fixed rate we get through
WEXT and stuff. Simply start at 1M on init in that case and stay at a fixed
1M until the user sets a different fixed rate through WEXT.
--
Greetings Michael.
> What about simply _not_ failing the initialization of mac80211, if no
> rc algo is available? I mean, we can just use a fixed rate we get through
> WEXT and stuff. Simply start at 1M on init in that case and stay at a fixed
> 1M until the user sets a different fixed rate through WEXT.
Not sure. I think I'd prefer not to have all special cases all over, I'd
much prefer a new rc80211_none or so then.
johannes
> Why don't we just build the rate control algorithms entirely separate
> useing obj-$(FOO) ? If they're modular that lets them be their own
> module, and if they're built-in they'll still go into built-in.o.
Yeah. Except that they can't be built-in if mac80211 isn't, and we've
repeatedly had users f' up their installations and bitterly report to us
that mac80211 doesn't work because no rate control algorithm is
available so we wanted to force having one built into mac80211.ko unless
specifically turned off (only with EMBEDDED).
johannes
On Wed, 02 Jan 2008 16:27:32 +0100
Johannes Berg <[email protected]> wrote:
> > What about simply _not_ failing the initialization of mac80211, if no
> > rc algo is available? I mean, we can just use a fixed rate we get through
> > WEXT and stuff. Simply start at 1M on init in that case and stay at a fixed
> > 1M until the user sets a different fixed rate through WEXT.
>
> Not sure. I think I'd prefer not to have all special cases all over, I'd
> much prefer a new rc80211_none or so then.
Agreed. But again, that should be built into mac80211, if we want to work
around issues caused by lack of module autoloading. And to me it looks like
'rc80211_none' implies yet another 'real' rate control algorithm.
I propose to just replace a few lines of code in mac80211 in order to look
for the lowest available rate and set it instead of failing. mac80211 should
[1] correctly deal with forced rates, so that would be everything we need.
[1] I know it doesn't right now. That should be fixed first anyway.
--
Ciao
Stefano
On Wednesday 02 January 2008 16:27:32 Johannes Berg wrote:
>
> > What about simply _not_ failing the initialization of mac80211, if no
> > rc algo is available? I mean, we can just use a fixed rate we get through
> > WEXT and stuff. Simply start at 1M on init in that case and stay at a fixed
> > 1M until the user sets a different fixed rate through WEXT.
>
> Not sure. I think I'd prefer not to have all special cases all over, I'd
> much prefer a new rc80211_none or so then.
That's fine with me, too. It's just a different implementation of the same idea.
--
Greetings Michael.
On Wed, Jan 02, 2008 at 06:45:01PM +0100, Johannes Berg wrote:
>
> > > +# objects for PID algorithm
> > > +rc80211_pid-y := rc80211_pid_algo.o
> > > +rc80211_pid-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
>
> > > +# build helper for PID algorithm
> > > +rc-pid-y := $(rc80211_pid-y)
> >
> > Is this extra indirection ( rc80211_pid-y ) a preparation
> > for future rate algorithms or was it just coped from my proposal?
> > It is not needed and it does not help readability.
>
> As far as I can tell rc80211_pid-y is needed to build rc80211_pid.o from
> rc80211_pid_algo.o and rc80211_pid_debugfs.o for the modular case, no?
Correct - I just missed that when reading your patch.
And I guess this was one of the things I got wrong in my first proposal too.
Looks good then.
> Hence, I just wanted to keep it symmetric for the non-modular case.
>
> > A second note - but more as a personal taste..
> > I prefer += assignment in preference of continued lines.
> > So
> > +mac80211-y := \
> > ieee80211.o \
> > ieee80211_ioctl.o \
> > sta_info.o \
> >
> > would become:
> > +mac80211-y := ieee80211.o ieee80211_ioctl.o
> > +mac80211-y += sta_info.o
>
> Yeah, I was hoping the diff would be smaller this way because it was
> already this way before.
Understood.
Sam
Hi Johannes.
Lo much better. One small comment.
>
> config MAC80211_LEDS
> bool "Enable LED triggers"
> --- everything.orig/net/mac80211/Makefile 2007-12-23 10:12:08.658937771 +0100
> +++ everything/net/mac80211/Makefile 2008-01-02 15:07:52.568414117 +0100
> @@ -1,19 +1,15 @@
> obj-$(CONFIG_MAC80211) += mac80211.o
>
> -mac80211-objs-$(CONFIG_MAC80211_LEDS) += ieee80211_led.o
> -mac80211-objs-$(CONFIG_NET_SCHED) += wme.o
> -mac80211-objs-$(CONFIG_MAC80211_RC_SIMPLE) += rc80211_simple.o
> -mac80211-objs-$(CONFIG_MAC80211_RC_PID) += rc80211_pid_algo.o
> +# objects for PID algorithm
> +rc80211_pid-y := rc80211_pid_algo.o
> +rc80211_pid-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
>
> -mac80211-debugfs-objs-$(CONFIG_MAC80211_RC_PID) += rc80211_pid_debugfs.o
> -mac80211-objs-$(CONFIG_MAC80211_DEBUGFS) += \
> - debugfs.o \
> - debugfs_sta.o \
> - debugfs_netdev.o \
> - debugfs_key.o \
> - $(mac80211-debugfs-objs-y)
> +# build helper for PID algorithm
> +rc-pid-y := $(rc80211_pid-y)
Is this extra indirection ( rc80211_pid-y ) a preparation
for future rate algorithms or was it just coped from my proposal?
It is not needed and it does not help readability.
A second note - but more as a personal taste..
I prefer += assignment in preference of continued lines.
So
+mac80211-y := \
ieee80211.o \
ieee80211_ioctl.o \
sta_info.o \
would become:
+mac80211-y := ieee80211.o ieee80211_ioctl.o
+mac80211-y += sta_info.o
etc.
Sam
> Building an algorithm optionally into mac80211 seems rather odd. In
> case mac80211 the algorithms should be their own modules and you should
> use request_module to make sure at least one is loaded.
We actually do that but people still manage to mess it up because they
don't install the modules properly or whatever.
> Or if you
> really want to make sure one is alway avaiabable always build the
> simplest one directly and unconditionally into mac80211.ko.
That'd be an alternative too, I think we have something like that now. I
don't really like it that much though because for 99% of people it's
just dead code. Anyway, I'll look into Sam's mail and do a new patch
depending on that.
johannes
> > +# objects for PID algorithm
> > +rc80211_pid-y := rc80211_pid_algo.o
> > +rc80211_pid-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
> > +# build helper for PID algorithm
> > +rc-pid-y := $(rc80211_pid-y)
>
> Is this extra indirection ( rc80211_pid-y ) a preparation
> for future rate algorithms or was it just coped from my proposal?
> It is not needed and it does not help readability.
As far as I can tell rc80211_pid-y is needed to build rc80211_pid.o from
rc80211_pid_algo.o and rc80211_pid_debugfs.o for the modular case, no?
Hence, I just wanted to keep it symmetric for the non-modular case.
> A second note - but more as a personal taste..
> I prefer += assignment in preference of continued lines.
> So
> +mac80211-y := \
> ieee80211.o \
> ieee80211_ioctl.o \
> sta_info.o \
>
> would become:
> +mac80211-y := ieee80211.o ieee80211_ioctl.o
> +mac80211-y += sta_info.o
Yeah, I was hoping the diff would be smaller this way because it was
already this way before.
johannes
Hi Sam,
Thanks for your reply.
> MAC80211_RC_PID build-in =>
> rc80211_pid_algo as build-in
> rc80211_pid_debugfs as build-in if CONFIG_MAC80211_DEBUGFS equals y (is enabled)
>
> MAC80211_RC_PID module =>
> rc80211_pid.o as a module
> rc80211_pid_algo.o and rc80211_pid_debugfs.o are ignored
Well, 'ignored' may be too strong since rc80211_pid.o is built from the
latter two objects.
> # If RC algorithm in build-in
> rate-rc-y := rc80211_pid_algo.o
> rate-rc-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
>
> # If RC algorithm is modular
> rate-rc-m := rc80211_pid.o
I don't think that will work since the system won't know how to build
rc80211_pid.o. I have another patch inspired by yours which is much
better than my first, just need to test it.
johannes
On Wed, Jan 02, 2008 at 09:41:15AM +0100, Johannes Berg wrote:
>
> > Why don't we just build the rate control algorithms entirely separate
> > useing obj-$(FOO) ? If they're modular that lets them be their own
> > module, and if they're built-in they'll still go into built-in.o.
>
> Yeah. Except that they can't be built-in if mac80211 isn't, and we've
> repeatedly had users f' up their installations and bitterly report to us
> that mac80211 doesn't work because no rate control algorithm is
> available so we wanted to force having one built into mac80211.ko unless
> specifically turned off (only with EMBEDDED).
Building an algorithm optionally into mac80211 seems rather odd. In
case mac80211 the algorithms should be their own modules and you should
use request_module to make sure at least one is loaded. Or if you
really want to make sure one is alway avaiabable always build the
simplest one directly and unconditionally into mac80211.ko.
>
> johannes
---end quoted text---