[PATCH 1/1] drivers/net: param for lapbether for specific ethernet device

[PATCH 1/1] drivers/net: param for lapbether for specific ethernet device

[Stephen Moorby]

 From 35607dd0607bca5322860703143c31cb04377764 Mon Sep 17 00:00:00 2001
From: Stephen Moorby <steve.moorby@ntlworld.com>
Date: Wed, 19 Jun 2013 08:22:51 +0100
Subject: [PATCH 1/1] drivers/net: param for lapbether for specific ethernet
  device

The lapbether driver binds to the first ethernet device that comes up.
This causes problems in hardware with multiple ethernet interfaces,
potentially resulting in the LAPB traffic on the wrong interface.
Have added a module parameter 'eth_dev' to specify the ethernet
interface over which lapbether should operate.  The module behaves
as before if no device is specified.

Patch created on linux-next 18-Jun-2013.
Tested on 2.6.32-45-generic.

Signed-off-by: Stephen Moorby <steve.moorby@ntlworld.com>
---
  drivers/net/wan/lapbether.c | 15 +++++++++++++--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wan/lapbether.c b/drivers/net/wan/lapbether.c
index a33a46f..510fc4a 100644
--- a/drivers/net/wan/lapbether.c
+++ b/drivers/net/wan/lapbether.c
@@ -47,6 +47,7 @@

  #include <net/x25device.h>

+static char *eth_dev;
  static const u8 bcast_addr[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };

  /* If this number is made larger, check that the temporary string buffer
@@ -381,8 +382,14 @@ static int lapbeth_device_event(struct 
notifier_block *this,
      switch (event) {
      case NETDEV_UP:
          /* New ethernet device -> new LAPB interface     */
-        if (lapbeth_get_x25_dev(dev) == NULL)
-            lapbeth_new_device(dev);
+        if (lapbeth_get_x25_dev(dev) == NULL) {
+            if (NULL != eth_dev) {
+                if (0 == strcmp(dev->name, eth_dev))
+                    lapbeth_new_device(dev);
+            } else {
+                lapbeth_new_device(dev);
+            }
+        }
          break;
      case NETDEV_DOWN:
          /* ethernet device closed -> close LAPB interface */
@@ -446,6 +453,10 @@ static void __exit lapbeth_cleanup_driver(void)
  }
  module_exit(lapbeth_cleanup_driver);

+module_param(eth_dev, charp, S_IRUGO);
+MODULE_PARM_DESC(eth_dev, "Ethernet device to use");
+
+
  MODULE_AUTHOR("Jonathan Naylor <g4klx@g4klx.demon.co.uk>");
  MODULE_DESCRIPTION("The unofficial LAPB over Ethernet driver");
  MODULE_LICENSE("GPL");
-- 
1.8.1.2

Re: [PATCH 1/1] drivers/net: param for lapbether for specific ethernet device

[David Miller]

Please no module parameters, provide a run-time, programmatic way
to configure this aspect.

Re: [PATCH 1/1] drivers/net: param for lapbether for specific ethernet device

[Sergei Shtylyov]

Hello.

On 19-06-2013 12:21, Stephen Moorby wrote:

>  From 35607dd0607bca5322860703143c31cb04377764 Mon Sep 17 00:00:00 2001
> From: Stephen Moorby <steve.moorby@ntlworld.com>
> Date: Wed, 19 Jun 2013 08:22:51 +0100
> Subject: [PATCH 1/1] drivers/net: param for lapbether for specific ethernet
>   device

    No need for this header.

> The lapbether driver binds to the first ethernet device that comes up.
> This causes problems in hardware with multiple ethernet interfaces,
> potentially resulting in the LAPB traffic on the wrong interface.
> Have added a module parameter 'eth_dev' to specify the ethernet
> interface over which lapbether should operate.  The module behaves
> as before if no device is specified.
>
> Patch created on linux-next 18-Jun-2013.
> Tested on 2.6.32-45-generic.
>
> Signed-off-by: Stephen Moorby <steve.moorby@ntlworld.com>
> ---
>   drivers/net/wan/lapbether.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wan/lapbether.c b/drivers/net/wan/lapbether.c
> index a33a46f..510fc4a 100644
> --- a/drivers/net/wan/lapbether.c
> +++ b/drivers/net/wan/lapbether.c
[...]
> @@ -381,8 +382,14 @@ static int lapbeth_device_event(struct
> notifier_block *this,

    Your patch is line-wrapped.

>       switch (event) {
>       case NETDEV_UP:
>           /* New ethernet device -> new LAPB interface     */
> -        if (lapbeth_get_x25_dev(dev) == NULL)
> -            lapbeth_new_device(dev);
> +        if (lapbeth_get_x25_dev(dev) == NULL) {
> +            if (NULL != eth_dev) {
> +                if (0 == strcmp(dev->name, eth_dev))
> +                    lapbeth_new_device(dev);
> +            } else {
> +                lapbeth_new_device(dev);
> +            }
> +        }

    It seems the patch is also white space damaged, i.e. tabs converted 
to spaces.

WBR, Sergei

Re: [PATCH 1/1] drivers/net: param for lapbether for specific ethernet device

[Ben Hutchings]

On Wed, 2013-06-19 at 09:21 +0100, Stephen Moorby wrote:
>  From 35607dd0607bca5322860703143c31cb04377764 Mon Sep 17 00:00:00 2001
> From: Stephen Moorby <steve.moorby@ntlworld.com>
> Date: Wed, 19 Jun 2013 08:22:51 +0100
> Subject: [PATCH 1/1] drivers/net: param for lapbether for specific ethernet
>   device
> 
> The lapbether driver binds to the first ethernet device that comes up.
> This causes problems in hardware with multiple ethernet interfaces,
> potentially resulting in the LAPB traffic on the wrong interface.
> Have added a module parameter 'eth_dev' to specify the ethernet
> interface over which lapbether should operate.  The module behaves
> as before if no device is specified.
[...]

The proper interface for creating and destroying layered devices is
rtnetlink.

The current behaviour is weird and it seems like it would be hard for
userland to reliably get the LAPB interface(s) into the right state.  So
maybe the auto-creation should be removed, or, for backward
compatibility (and at the risk of annoying David) it should be possible
to disable it by setting a module parameter.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

[PATCH 1/1] drivers/net: param for lapbether for specific ethernet device

[Stephen Moorby]

The lapbether driver binds to the first ethernet device that comes up.
This causes problems in hardware with multiple ethernet interfaces,
potentially resulting in the LAPB traffic on the wrong interface.
Have added a module parameter 'eth_dev' to specify the ethernet
interface over which lapbether should operate.  The module behaves
as before if no device is specified.

Patch created on linux-next 18-Jun-2013.
Tested on 2.6.32-45-generic.

Signed-off-by: Stephen Moorby <steve.moorby@ntlworld.com>
---
 drivers/net/wan/lapbether.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wan/lapbether.c b/drivers/net/wan/lapbether.c
index a33a46f..510fc4a 100644
--- a/drivers/net/wan/lapbether.c
+++ b/drivers/net/wan/lapbether.c
@@ -47,6 +47,7 @@
 
 #include <net/x25device.h>
 
+static char *eth_dev;
 static const u8 bcast_addr[6] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
 
 /* If this number is made larger, check that the temporary string buffer
@@ -381,8 +382,14 @@ static int lapbeth_device_event(struct notifier_block *this,
 	switch (event) {
 	case NETDEV_UP:
 		/* New ethernet device -> new LAPB interface	 */
-		if (lapbeth_get_x25_dev(dev) == NULL)
-			lapbeth_new_device(dev);
+		if (lapbeth_get_x25_dev(dev) == NULL) {
+			if (NULL != eth_dev) {
+				if (0 == strcmp(dev->name, eth_dev))
+					lapbeth_new_device(dev);
+			} else {
+				lapbeth_new_device(dev);
+			}
+		}
 		break;
 	case NETDEV_DOWN:	
 		/* ethernet device closed -> close LAPB interface */
@@ -446,6 +453,10 @@ static void __exit lapbeth_cleanup_driver(void)
 }
 module_exit(lapbeth_cleanup_driver);
 
+module_param(eth_dev, charp, S_IRUGO);
+MODULE_PARM_DESC(eth_dev, "Ethernet device to use");
+
+
 MODULE_AUTHOR("Jonathan Naylor <g4klx@g4klx.demon.co.uk>");
 MODULE_DESCRIPTION("The unofficial LAPB over Ethernet driver");
 MODULE_LICENSE("GPL");
-- 
1.8.1.2

Re: [PATCH 1/1] drivers/net: param for lapbether for specific ethernet device

[David Miller]

From: Stephen Moorby <steve.moorby@ntlworld.com>
Date: Wed, 19 Jun 2013 16:26:53 +0100

> The lapbether driver binds to the first ethernet device that comes up.
> This causes problems in hardware with multiple ethernet interfaces,
> potentially resulting in the LAPB traffic on the wrong interface.
> Have added a module parameter 'eth_dev' to specify the ethernet
> interface over which lapbether should operate.  The module behaves
> as before if no device is specified.
> 
> Patch created on linux-next 18-Jun-2013.
> Tested on 2.6.32-45-generic.
> 
> Signed-off-by: Stephen Moorby <steve.moorby@ntlworld.com>

I rejected this patch last night, stating that module parameters are
deeply discouraged and therefore you need to create a runtime mechanism
to control this.

Re: [PATCH 1/1] drivers/net: param for lapbether for specific ethernet device

[Stephen Moorby]

I will be using an Ubuntu 10.04 kernel with the 'eth_dev' module 
parameter patch.  I appreciate a better solution needs to be designed 
but it is not one I can contribute to.  I still advocate the patch as an 
improvement on the current non functional situation.

On 19/06/13 20:18, David Miller wrote:
> From: Stephen Moorby <steve.moorby@ntlworld.com>
> Date: Wed, 19 Jun 2013 16:26:53 +0100
>
>> The lapbether driver binds to the first ethernet device that comes up.
>> This causes problems in hardware with multiple ethernet interfaces,
>> potentially resulting in the LAPB traffic on the wrong interface.
>> Have added a module parameter 'eth_dev' to specify the ethernet
>> interface over which lapbether should operate.  The module behaves
>> as before if no device is specified.
>>
>> Patch created on linux-next 18-Jun-2013.
>> Tested on 2.6.32-45-generic.
>>
>> Signed-off-by: Stephen Moorby <steve.moorby@ntlworld.com>
> I rejected this patch last night, stating that module parameters are
> deeply discouraged and therefore you need to create a runtime mechanism
> to control this.
>

Re: [PATCH 1/1] drivers/net: param for lapbether for specific ethernet device

[David Miller]

From: Stephen Moorby <steve.moorby@ntlworld.com>
Date: Wed, 19 Jun 2013 21:19:31 +0100

> I will be using an Ubuntu 10.04 kernel with the 'eth_dev' module
> parameter patch.  I appreciate a better solution needs to be designed
> but it is not one I can contribute to.  I still advocate the patch as
> an improvement on the current non functional situation.

We don't put poorly designed changes into the tree simply because
a properly implemented version doesn't exist yet.