[Bridge] [PATCH 01/31] bridge: Auto-load bridge module when socket opened.

[Bridge] [PATCH 01/31] bridge: Auto-load bridge module when socket opened.

[Scott James Remnant]

The bridge module is missing the net-pf-7 alias that would cause it to
be auto-loaded when a socket of that type is opened.  This patch adds
the alias.

Signed-off-by: Scott James Remnant <scott@canonical.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 net/bridge/br.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/bridge/br.c b/net/bridge/br.c
index 4d2c1f1..229daee 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -16,6 +16,7 @@
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/init.h>
+#include <linux/net.h>
 #include <linux/llc.h>
 #include <net/llc.h>
 #include <net/stp.h>
@@ -108,3 +109,4 @@ module_init(br_init)
 module_exit(br_deinit)
 MODULE_LICENSE("GPL");
 MODULE_VERSION(BR_VERSION);
+MODULE_ALIAS_NETPROTO(PF_BRIDGE);
-- 
1.6.0.5

Re: [Bridge] [PATCH 01/31] bridge: Auto-load bridge module when socket opened.

[Stephen Hemminger]

On Mon, 2 Mar 2009 16:40:10 +0000
Scott James Remnant <scott@canonical.com> wrote:

> The bridge module is missing the net-pf-7 alias that would cause it to
> be auto-loaded when a socket of that type is opened.  This patch adds
> the alias.
> 
> Signed-off-by: Scott James Remnant <scott@canonical.com>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  net/bridge/br.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bridge/br.c b/net/bridge/br.c
> index 4d2c1f1..229daee 100644
> --- a/net/bridge/br.c
> +++ b/net/bridge/br.c
> @@ -16,6 +16,7 @@
>  #include <linux/netdevice.h>
>  #include <linux/etherdevice.h>
>  #include <linux/init.h>
> +#include <linux/net.h>
>  #include <linux/llc.h>
>  #include <net/llc.h>
>  #include <net/stp.h>
> @@ -108,3 +109,4 @@ module_init(br_init)
>  module_exit(br_deinit)
>  MODULE_LICENSE("GPL");
>  MODULE_VERSION(BR_VERSION);
> +MODULE_ALIAS_NETPROTO(PF_BRIDGE);

Hmm. It looks okay, but there is not real direct socket interface
for PF_BRIDGE/AF_BRIDGE.

Re: [Bridge] [PATCH 01/31] bridge: Auto-load bridge module when socket opened.

[David Miller]

From: Scott James Remnant <scott@canonical.com>
Date: Mon, 2 Mar 2009 16:40:10 +0000

> The bridge module is missing the net-pf-7 alias that would cause it to
> be auto-loaded when a socket of that type is opened.  This patch adds
> the alias.
> 
> Signed-off-by: Scott James Remnant <scott@canonical.com>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

Bridging has no sockets, so this patch won't do anything.

Did you verify that these changes actually do something?
Or did you blindly run around peppering source files with
these annotations?

Re: [Bridge] [PATCH 01/31] bridge: Auto-load bridge module when socket opened.

[Stephen Hemminger]

On Fri, 13 Mar 2009 14:27:18 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Scott James Remnant <scott@canonical.com>
> Date: Mon, 2 Mar 2009 16:40:10 +0000
> 
> > The bridge module is missing the net-pf-7 alias that would cause it to
> > be auto-loaded when a socket of that type is opened.  This patch adds
> > the alias.
> > 
> > Signed-off-by: Scott James Remnant <scott@canonical.com>
> > Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> 
> Bridging has no sockets, so this patch won't do anything.
> 
> Did you verify that these changes actually do something?
> Or did you blindly run around peppering source files with
> these annotations?

I think this all started when I was debugging a problem where LLC
was not getting autoloaded properly. It turned out that distro's
like Debian (and therefore Ubuntu) had incorrect modprobe.d/alias file
entries. I raised the bug that, "hey you have wrong data and you
really don't need these alias entries at all anymore". There was some
comparison of what was being shipped versus what was in the kernel
and the Debian/Ubuntu guys stepped in to try and unify this.

Let's be supportive of Ubuntu maintainers actually working on upstream
resolution for a change. The normal distro maintainer is "patch and ignore".
In this case, many of these are correct, but several of them are cases
where there is no point in having a module alias because there is no
functiona user visible API. You can try and open a PF_BRIDGE socket, there just
is no point.

Re: [Bridge] [PATCH 01/31] bridge: Auto-load bridge module when socket opened.

[David Miller]

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 13 Mar 2009 15:12:00 -0700

> Let's be supportive of Ubuntu maintainers actually working on
> upstream resolution for a change. The normal distro maintainer is
> "patch and ignore".

This has nothing to do with being supportive or not.

If the changes are wrong, many of these in fact were, and they were
not even tested on top of it, I cannot be happy at all about that.

And that goes for anyone submitting patches here.

Re: [Bridge] [PATCH 01/31] bridge: Auto-load bridge module when socket opened.

[Scott James Remnant]

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

On Fri, 2009-03-13 at 14:27 -0700, David Miller wrote:

> Did you verify that these changes actually do something?
> Or did you blindly run around peppering source files with
> these annotations?
> 
I converted existing modprobe aliases that most distributions are
carrying around into the one-line kernel patches.

I tested the important ones such as the floppy and ch changes.

If the patches are wrong, which I personally suspected many of them
were, then it's a good example as to why distributions shouldn't hack
around the kernel in userspace, or keep their own patches.

So yes, they're a bit stinky (hell, I even fluffed up the covering mail
subject line <g>) - but distros including Ubuntu were trying to do these
aliases in modprobe rules, so it's a good thing to find out that they're
wrong; no?

Scott
-- 
Scott James Remnant
scott@canonical.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]