Patchwork Add libertas_disablemesh module parameter to disable mesh interface

login
register
mail settings
Submitter Sascha Silbe
Date May 11, 2011, 12:52 p.m.
Message ID <1305118354-17337-1-git-send-email-silbe@activitycentral.com>
Download mbox | patch
Permalink /patch/95169/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Sascha Silbe - May 11, 2011, 12:52 p.m.
This allows individual users and deployments to disable mesh support at
runtime, i.e. without having to build and maintain a custom kernel.

Based on a patch by Paul Fox <pgf@laptop.org>.
Signed-off-by: Sascha Silbe <silbe@activitycentral.com>
---
 drivers/net/wireless/libertas/main.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

The patch is based on the OLPC 2.6.35 kernel tree, but applies cleanly to
wireless-next.

--
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Randy.Dunlap - May 11, 2011, 5 p.m.
On Wed, 11 May 2011 14:52:34 +0200 Sascha Silbe wrote:

> This allows individual users and deployments to disable mesh support at
> runtime, i.e. without having to build and maintain a custom kernel.

I guess a user could want to do this on a per-driver basis, but ISTM that
it would be better to be something like a sysctl that applies to all (wireless)
drivers.

Do other wireless drivers have something like this?


> Based on a patch by Paul Fox <pgf@laptop.org>.
> Signed-off-by: Sascha Silbe <silbe@activitycentral.com>
> ---
>  drivers/net/wireless/libertas/main.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> The patch is based on the OLPC 2.6.35 kernel tree, but applies cleanly to
> wireless-next.
> 
> diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
> index 8445473..62069e2 100644
> --- a/drivers/net/wireless/libertas/main.c
> +++ b/drivers/net/wireless/libertas/main.c
> @@ -41,6 +41,10 @@ unsigned int lbs_debug;
>  EXPORT_SYMBOL_GPL(lbs_debug);
>  module_param_named(libertas_debug, lbs_debug, int, 0644);
> 
> +unsigned int lbs_disablemesh;
> +EXPORT_SYMBOL_GPL(lbs_disablemesh);
> +module_param_named(libertas_disablemesh, lbs_disablemesh, int, 0644);
> +
> 
>  /* This global structure is used to send the confirm_sleep command as
>   * fast as possible down to the firmware. */
> @@ -1086,7 +1090,10 @@ int lbs_start_card(struct lbs_private *priv)
> 
>  	lbs_update_channel(priv);
> 
> -	lbs_init_mesh(priv);
> +	if (!lbs_disablemesh)
> +		lbs_init_mesh(priv);
> +	else
> +		lbs_pr_info("%s: mesh disabled\n", dev->name);
> 
>  	/*
>  	 * While rtap isn't related to mesh, only mesh-enabled
> --


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Williams - May 12, 2011, 3:11 a.m.
On Wed, 2011-05-11 at 14:52 +0200, Sascha Silbe wrote:
> This allows individual users and deployments to disable mesh support at
> runtime, i.e. without having to build and maintain a custom kernel.

Does the mesh interface somehow cause problems, even when nothing is
using it?  I'd expect that if the mesh interface was simply left alone,
there would be no need to disable it.

Dan

> Based on a patch by Paul Fox <pgf@laptop.org>.
> Signed-off-by: Sascha Silbe <silbe@activitycentral.com>
> ---
>  drivers/net/wireless/libertas/main.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> The patch is based on the OLPC 2.6.35 kernel tree, but applies cleanly to
> wireless-next.
> 
> diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
> index 8445473..62069e2 100644
> --- a/drivers/net/wireless/libertas/main.c
> +++ b/drivers/net/wireless/libertas/main.c
> @@ -41,6 +41,10 @@ unsigned int lbs_debug;
>  EXPORT_SYMBOL_GPL(lbs_debug);
>  module_param_named(libertas_debug, lbs_debug, int, 0644);
> 
> +unsigned int lbs_disablemesh;
> +EXPORT_SYMBOL_GPL(lbs_disablemesh);
> +module_param_named(libertas_disablemesh, lbs_disablemesh, int, 0644);
> +
> 
>  /* This global structure is used to send the confirm_sleep command as
>   * fast as possible down to the firmware. */
> @@ -1086,7 +1090,10 @@ int lbs_start_card(struct lbs_private *priv)
> 
>  	lbs_update_channel(priv);
> 
> -	lbs_init_mesh(priv);
> +	if (!lbs_disablemesh)
> +		lbs_init_mesh(priv);
> +	else
> +		lbs_pr_info("%s: mesh disabled\n", dev->name);
> 
>  	/*
>  	 * While rtap isn't related to mesh, only mesh-enabled
> --
> 1.7.4.1
> 


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sascha Silbe - May 13, 2011, 1:16 p.m.
Excerpts from Dan Williams's message of Thu May 12 05:11:36 +0200 2011:
> On Wed, 2011-05-11 at 14:52 +0200, Sascha Silbe wrote:
> > This allows individual users and deployments to disable mesh support at
> > runtime, i.e. without having to build and maintain a custom kernel.

> Does the mesh interface somehow cause problems, even when nothing is
> using it?

Some people suspect it does, but there's no hard data showing that. But
then the problems are often hard to reproduce in the first place, so
proving a correlation with mesh is even harder.

The hardware based mesh support is based on an outdated draft of
802.11s and not interoperable with any other device AFAIK. For most
users Ad-hoc networks are the better option. Disabling mesh support as
low-level as possible makes it less likely that any remains are causing
trouble. With at least four layers (firmware, kernel, NM, Sugar)
involved in managing connectivity and one of the (firmware) being closed
source, I prefer to simplify things by eliminating three layers for
functionality we don't intend to use. It makes debugging (and
blaming ;) ) a lot easier.

In the field, mesh support is currently disabled using
/sys/class/net/eth0/lbs_mesh. However, it comes back after resume
(possibly only if powercycled) and needs to be disabled again by
post-resume hacks. Race conditions with NM are possible.

A user space option would be to teach NM to disable mesh support (at
runtime - we don't want to ship a custom NM package). I'd expect the
patch to be much more invasive than the one posted for libertas.

Sascha
Sascha Silbe - May 13, 2011, 1:26 p.m.
Excerpts from Randy Dunlap's message of Wed May 11 19:00:30 +0200 2011:
> On Wed, 11 May 2011 14:52:34 +0200 Sascha Silbe wrote:
> 
> > This allows individual users and deployments to disable mesh support at
> > runtime, i.e. without having to build and maintain a custom kernel.
> 
> I guess a user could want to do this on a per-driver basis, but ISTM that
> it would be better to be something like a sysctl that applies to all (wireless)
> drivers.

AFAIK libertas is the only driver with a mesh implementation based on an
outdated 802.11s draft. To clarify, we could rename the parameter to
lbs_disable_olpcmesh (or just disable_olpcmesh).
Disabling 802.11s (latest draft) based mesh support is a separate
matter.

Sascha
Dan Williams - May 19, 2011, 5:16 p.m.
On Fri, 2011-05-13 at 15:16 +0200, Sascha Silbe wrote:
> Excerpts from Dan Williams's message of Thu May 12 05:11:36 +0200 2011:
> > On Wed, 2011-05-11 at 14:52 +0200, Sascha Silbe wrote:
> > > This allows individual users and deployments to disable mesh support at
> > > runtime, i.e. without having to build and maintain a custom kernel.
> 
> > Does the mesh interface somehow cause problems, even when nothing is
> > using it?
> 
> Some people suspect it does, but there's no hard data showing that. But
> then the problems are often hard to reproduce in the first place, so
> proving a correlation with mesh is even harder.

That's not an excuse for not finding and fixing the problem though.
What problems are we actually talking about here?

> The hardware based mesh support is based on an outdated draft of
> 802.11s and not interoperable with any other device AFAIK. For most
> users Ad-hoc networks are the better option. Disabling mesh support as
> low-level as possible makes it less likely that any remains are causing
> trouble. With at least four layers (firmware, kernel, NM, Sugar)
> involved in managing connectivity and one of the (firmware) being closed
> source, I prefer to simplify things by eliminating three layers for
> functionality we don't intend to use. It makes debugging (and
> blaming ;) ) a lot easier.
> 
> In the field, mesh support is currently disabled using
> /sys/class/net/eth0/lbs_mesh. However, it comes back after resume
> (possibly only if powercycled) and needs to be disabled again by
> post-resume hacks. Race conditions with NM are possible.

That's a parameter handled by the driver; so shouldn't we make sure it's
respected again on resume?

> A user space option would be to teach NM to disable mesh support (at
> runtime - we don't want to ship a custom NM package). I'd expect the
> patch to be much more invasive than the one posted for libertas.

Not really, but we already have on/off for a bunch of other stuff, I
don't see why we can't add one for OLPC mesh.

Dan

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
index 8445473..62069e2 100644
--- a/drivers/net/wireless/libertas/main.c
+++ b/drivers/net/wireless/libertas/main.c
@@ -41,6 +41,10 @@  unsigned int lbs_debug;
 EXPORT_SYMBOL_GPL(lbs_debug);
 module_param_named(libertas_debug, lbs_debug, int, 0644);

+unsigned int lbs_disablemesh;
+EXPORT_SYMBOL_GPL(lbs_disablemesh);
+module_param_named(libertas_disablemesh, lbs_disablemesh, int, 0644);
+

 /* This global structure is used to send the confirm_sleep command as
  * fast as possible down to the firmware. */
@@ -1086,7 +1090,10 @@  int lbs_start_card(struct lbs_private *priv)

 	lbs_update_channel(priv);

-	lbs_init_mesh(priv);
+	if (!lbs_disablemesh)
+		lbs_init_mesh(priv);
+	else
+		lbs_pr_info("%s: mesh disabled\n", dev->name);

 	/*
 	 * While rtap isn't related to mesh, only mesh-enabled