Re: [Bugme-new] [Bug 9545] New: Cannot bring up a bridge interface without a MAC address set

Re: [Bugme-new] [Bug 9545] New: Cannot bring up a bridge interface without a MAC address set

[Andrew Morton]

(please respond via emailed reply-to-all, not via the bugzilla web
interface).

On Tue, 11 Dec 2007 11:04:55 -0800 (PST)
bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=9545
> 
>            Summary: Cannot bring up a bridge interface without a MAC address
>                     set
>            Product: Networking
>            Version: 2.5
>      KernelVersion: 2.6.24-0.81.rc4.git7.fc9
>           Platform: All
>         OS/Version: Linux
>               Tree: Fedora
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Other
>         AssignedTo: acme@ghostprotocols.net
>         ReportedBy: berrange@redhat.com
> 
> 
> Most recent kernel where this bug did not occur: Any 2.6.23 or earlier
> Distribution: Fedora 9 rawhide
> Hardware Environment: Intel(R) Core(TM)2 Duo CPU     E6850, x86_64
> Software Environment:  2.6.24-0.81.rc4.git7.fc9 #1 SMP  x86_64 GNU/Linux
> Problem Description:
> It is not possible to bring up a bridge interface unless one first assigns a
> MAC address to it. This is a regression from earlier kernels where one could
> always bring up a bridge device immediately after creating it. The bridge
> should not require a MAC address because it is not going to be configured with
> any IP addr - in my scenario I merely wish to use it to connect a number of
> 'tap' devices associated with KVM guests.
> 
> Steps to reproduce:
> # brctl addbr demobr
> # ifconfig demobr up
> SIOCSIFFLAGS: Invalid argument
> 
> It is failing on the ioctl to bring up the interface
> 
> ioctl(4, SIOCGIFFLAGS, {ifr_name="demobr",
> ifr_flags=IFF_BROADCAST|IFF_MULTICAST}) = 0
> ioctl(4, SIOCSIFFLAGS, 0x7fff38a6f180)  = -1 EINVAL (Invalid argument)
> 
> 
> The following Fedora kernel BZ has the original problem report:
> 
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=390991
> 
> The problem appears to have been introduced by this patch:
> 
> Commit bada339ba24dee9e143bfb42e1dc61f146619846
> Author: Jeff Garzik <jgarzik@redhat.com>
> Date:   Tue Oct 23 20:19:37 2007 -0700
> 
>     [NET]: Validate device addr prior to interface-up
> 
>     Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 8726589..f861555 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1007,17 +1007,20 @@ int dev_open(struct net_device *dev)
>          *      Call device private open method
>          */
>         set_bit(__LINK_STATE_START, &dev->state);
> -       if (dev->open) {
> +
> +       if (dev->validate_addr)
> +               ret = dev->validate_addr(dev);
> 
> 
> 
> Which rejects MAC addresses with all 0s
> 
> Herbert Xu tells me...
> 
> "This is clearly a bug in the upstream bridge device.  It should override the
>  default address validator so that when there are no devices attached that a
>  zero MAC address is allowed."
> 

Rafael, another for the regression list please.

Re: [Bugme-new] [Bug 9545] New: Cannot bring up a bridge interface without a MAC address set

[Stephen Hemminger]

On Tue, 11 Dec 2007 13:26:14 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> 
> (please respond via emailed reply-to-all, not via the bugzilla web
> interface).
> 
> On Tue, 11 Dec 2007 11:04:55 -0800 (PST)
> bugme-daemon@bugzilla.kernel.org wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=9545
> > 
> >            Summary: Cannot bring up a bridge interface without a MAC address
> >                     set
> >            Product: Networking
> >            Version: 2.5
> >      KernelVersion: 2.6.24-0.81.rc4.git7.fc9
> >           Platform: All
> >         OS/Version: Linux
> >               Tree: Fedora
> >             Status: NEW
> >           Severity: normal
> >           Priority: P1
> >          Component: Other
> >         AssignedTo: acme@ghostprotocols.net
> >         ReportedBy: berrange@redhat.com
> > 
> > 
> > Most recent kernel where this bug did not occur: Any 2.6.23 or earlier
> > Distribution: Fedora 9 rawhide
> > Hardware Environment: Intel(R) Core(TM)2 Duo CPU     E6850, x86_64
> > Software Environment:  2.6.24-0.81.rc4.git7.fc9 #1 SMP  x86_64 GNU/Linux
> > Problem Description:
> > It is not possible to bring up a bridge interface unless one first assigns a
> > MAC address to it. This is a regression from earlier kernels where one could
> > always bring up a bridge device immediately after creating it. The bridge
> > should not require a MAC address because it is not going to be configured with
> > any IP addr - in my scenario I merely wish to use it to connect a number of
> > 'tap' devices associated with KVM guests.
> > 
> > Steps to reproduce:
> > # brctl addbr demobr
> > # ifconfig demobr up
> > SIOCSIFFLAGS: Invalid argument

The tap devices have to have addresses don't they. So bringing up an empty
bridge is meaningless. If you just add the device first then it will work.

Could be fixed to prevent errors from existing scripts but it is not a complete showstopper.
The problem is that when device is brought up it propogates events up to
other layers and applications, these layers will then query and see a bogus
address.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [Bugme-new] [Bug 9545] New: Cannot bring up a bridge interface without a MAC address set

[Andrew Morton]

On Tue, 11 Dec 2007 14:52:43 -0800
Stephen Hemminger <shemminger@linux-foundation.org> wrote:

> On Tue, 11 Dec 2007 13:26:14 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > 
> > (please respond via emailed reply-to-all, not via the bugzilla web
> > interface).
> > 
> > On Tue, 11 Dec 2007 11:04:55 -0800 (PST)
> > bugme-daemon@bugzilla.kernel.org wrote:
> > 
> > > http://bugzilla.kernel.org/show_bug.cgi?id=9545
> > > 
> > >            Summary: Cannot bring up a bridge interface without a MAC address
> > >                     set
> > >            Product: Networking
> > >            Version: 2.5
> > >      KernelVersion: 2.6.24-0.81.rc4.git7.fc9
> > >           Platform: All
> > >         OS/Version: Linux
> > >               Tree: Fedora
> > >             Status: NEW
> > >           Severity: normal
> > >           Priority: P1
> > >          Component: Other
> > >         AssignedTo: acme@ghostprotocols.net
> > >         ReportedBy: berrange@redhat.com
> > > 
> > > 
> > > Most recent kernel where this bug did not occur: Any 2.6.23 or earlier
> > > Distribution: Fedora 9 rawhide
> > > Hardware Environment: Intel(R) Core(TM)2 Duo CPU     E6850, x86_64
> > > Software Environment:  2.6.24-0.81.rc4.git7.fc9 #1 SMP  x86_64 GNU/Linux
> > > Problem Description:
> > > It is not possible to bring up a bridge interface unless one first assigns a
> > > MAC address to it. This is a regression from earlier kernels where one could
> > > always bring up a bridge device immediately after creating it. The bridge
> > > should not require a MAC address because it is not going to be configured with
> > > any IP addr - in my scenario I merely wish to use it to connect a number of
> > > 'tap' devices associated with KVM guests.
> > > 
> > > Steps to reproduce:
> > > # brctl addbr demobr
> > > # ifconfig demobr up
> > > SIOCSIFFLAGS: Invalid argument
> 
> The tap devices have to have addresses don't they. So bringing up an empty
> bridge is meaningless. If you just add the device first then it will work.
> 
> Could be fixed to prevent errors from existing scripts but it is not a complete showstopper.
> The problem is that when device is brought up it propogates events up to
> other layers and applications, these layers will then query and see a bogus
> address.
> 

If the fix to make bridge compatible with 2.6.23 behaviour isn't too
gruesome then I'd have thought it'd be worth doing it?

[PATCH] bridge: assign random address

[Stephen Hemminger]

Assigning a valid random address to bridge device solves problems
when bridge device is brought up before adding real device to bridge.
When the first real device is added to the bridge, it's address
will overide the bridges random address.

Note: any device added to a bridge must already have a valid
ethernet address.
 br_add_if -> br_fdb_insert -> fdb_insert -> is_valid_ether_addr

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

--- a/net/bridge/br_device.c	2007-10-16 16:48:21.000000000 -0700
+++ b/net/bridge/br_device.c	2007-12-11 15:36:52.000000000 -0800
@@ -157,8 +157,7 @@ static struct ethtool_ops br_ethtool_ops
 
 void br_dev_setup(struct net_device *dev)
 {
-	memset(dev->dev_addr, 0, ETH_ALEN);
-
+	random_ether_addr(dev->dev_addr);
 	ether_setup(dev);
 
 	dev->do_ioctl = br_dev_ioctl;

Re: [PATCH] bridge: assign random address

[Andrew Morton]

On Tue, 11 Dec 2007 15:48:35 -0800
Stephen Hemminger <shemminger@linux-foundation.org> wrote:

> Assigning a valid random address to bridge device solves problems
> when bridge device is brought up before adding real device to bridge.
> When the first real device is added to the bridge, it's address
> will overide the bridges random address.
> 
> Note: any device added to a bridge must already have a valid
> ethernet address.
>  br_add_if -> br_fdb_insert -> fdb_insert -> is_valid_ether_addr
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> --- a/net/bridge/br_device.c	2007-10-16 16:48:21.000000000 -0700
> +++ b/net/bridge/br_device.c	2007-12-11 15:36:52.000000000 -0800
> @@ -157,8 +157,7 @@ static struct ethtool_ops br_ethtool_ops
>  
>  void br_dev_setup(struct net_device *dev)
>  {
> -	memset(dev->dev_addr, 0, ETH_ALEN);
> -
> +	random_ether_addr(dev->dev_addr);
>  	ether_setup(dev);
>  
>  	dev->do_ioctl = br_dev_ioctl;

I'd have thought that a comment is needed here as it is rather unobvious
what that code is there for.

Re: [Bugme-new] [Bug 9545] New: Cannot bring up a bridge interface without a MAC address set

[Herbert Xu]

On Tue, Dec 11, 2007 at 02:52:43PM -0800, Stephen Hemminger wrote:
>
> The tap devices have to have addresses don't they. So bringing up an empty
> bridge is meaningless. If you just add the device first then it will work.

Actually bringing up a bridge with no constituents is useful for
a bridge that's made up of only virtual interfaces.  Since each
vritual interface may be created or destroyed at run-time it'd
be quite awkward to check every time to see if that's the last
or first and act differently on the bridge.

More importantly constiuents can be added to and removed from a
bridge without taking it down.

> Could be fixed to prevent errors from existing scripts but it is not a complete showstopper.

Well this stops FC8 working with Xen so for that it's a showstopper :)

> The problem is that when device is brought up it propogates events up to
> other layers and applications, these layers will then query and see a bogus
> address.

What exactly would it break for this scenario though?

Cheersk,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [Bugme-new] [Bug 9545] New: Cannot bring up a bridge interface without a MAC address set

[Stephen Hemminger]

On Wed, 12 Dec 2007 09:51:05 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Tue, Dec 11, 2007 at 02:52:43PM -0800, Stephen Hemminger wrote:
> >
> > The tap devices have to have addresses don't they. So bringing up an empty
> > bridge is meaningless. If you just add the device first then it will work.
> 
> Actually bringing up a bridge with no constituents is useful for
> a bridge that's made up of only virtual interfaces.  Since each
> vritual interface may be created or destroyed at run-time it'd
> be quite awkward to check every time to see if that's the last
> or first and act differently on the bridge.
> 
> More importantly constiuents can be added to and removed from a
> bridge without taking it down.
> 
> > Could be fixed to prevent errors from existing scripts but it is not a complete showstopper.
> 
> Well this stops FC8 working with Xen so for that it's a showstopper :)
> 
> > The problem is that when device is brought up it propogates events up to
> > other layers and applications, these layers will then query and see a bogus
> > address.
> 
> What exactly would it break for this scenario though?

Well with earlier kernels, ipv6 and others would see an invalid address (all zeros).
That could be a problem if some netlink watching program or udev script
propagated that value into a database or management interface. But now
using a random value, that won't happen.


-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH] bridge: assign random address

[David Miller]

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Tue, 11 Dec 2007 15:48:35 -0800

> Subject: Re: [PATCH] bridge: assign random address

"bridge" should all-caps and in brackets, "assign random address"
should be capitalized like a proper english sentence with a period at
the end.

These are changelog messages for a mature and professional software
project, not random comments amongst teenagers on an IRC channel.

> Assigning a valid random address to bridge device solves problems
> when bridge device is brought up before adding real device to bridge.
> When the first real device is added to the bridge, it's address
> will overide the bridges random address.
> 
> Note: any device added to a bridge must already have a valid
> ethernet address.
>  br_add_if -> br_fdb_insert -> fdb_insert -> is_valid_ether_addr
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

Applied to net-2.6, thanks.

Re: [PATCH] bridge: assign random address

[Andrew Morton]

On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Tue, 11 Dec 2007 15:48:35 -0800
> 
> > Subject: Re: [PATCH] bridge: assign random address
> 
> "bridge" should all-caps and in brackets,

No, "bridge" should not be in [].  Lots of people's patch-receiving scripts
assume that any text in [] is to be removed as the patch is committed.  It
contains text which is only relevant to the particular email which carried
the patch.  Stuff like "patch" and "4/5" and "linux-2.6.23", etc.

> "assign random address"
> should be capitalized like a proper english sentence with a period at
> the end.

Actually I usually remove the caps and the waste-of-space period, but
that's much less important than the brackets abuse.  The bracket convention
is quite useful and I've often wondered why I need to edit the patch title
when I merge up patches from net developers ;)

Re: [PATCH] bridge: assign random address

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Sun, 16 Dec 2007 14:29:15 -0800

> On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <davem@davemloft.net> wrote:
> 
> > From: Stephen Hemminger <shemminger@linux-foundation.org>
> > Date: Tue, 11 Dec 2007 15:48:35 -0800
> > 
> > > Subject: Re: [PATCH] bridge: assign random address
> > 
> > "bridge" should all-caps and in brackets,
> 
> No, "bridge" should not be in [].  Lots of people's patch-receiving scripts
> assume that any text in [] is to be removed as the patch is committed.  It
> contains text which is only relevant to the particular email which carried
> the patch.  Stuff like "patch" and "4/5" and "linux-2.6.23", etc.

I don't use scripts, I edit it by hand.  And when I do ever use
scripts I will make sure they accomodate "[$SUBSYSTEM]" format
subject lines, you can be sure.

And you can even make those scripts happy by doing:

[Patch 1/7] [SUBSYSTEM]: Foo bar baz...

And if you haven't noticed over the past few years, this is
is the convention we've been using in the networking.

I munge every one of your (and everyone else's) changelog entry
headers this way.  Without exception, every single one.

So when you don't follow this convention, you make more typing
and more work for me.  The more patches I get from someone
the more important it is for this convention to be followed.

I find it very hard to believe that you haven't once looked
at the hundreds of patches I've applied of your's and not
noticed how I reformat everything.

Re: [PATCH] bridge: assign random address

[Andrew Morton]

On Sun, 16 Dec 2007 15:26:06 -0800 (PST) David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Sun, 16 Dec 2007 14:29:15 -0800
> 
> > On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <davem@davemloft.net> wrote:
> > 
> > > From: Stephen Hemminger <shemminger@linux-foundation.org>
> > > Date: Tue, 11 Dec 2007 15:48:35 -0800
> > > 
> > > > Subject: Re: [PATCH] bridge: assign random address
> > > 
> > > "bridge" should all-caps and in brackets,
> > 
> > No, "bridge" should not be in [].  Lots of people's patch-receiving scripts
> > assume that any text in [] is to be removed as the patch is committed.  It
> > contains text which is only relevant to the particular email which carried
> > the patch.  Stuff like "patch" and "4/5" and "linux-2.6.23", etc.
> 
> I don't use scripts, I edit it by hand.  And when I do ever use
> scripts I will make sure they accomodate "[$SUBSYSTEM]" format
> subject lines, you can be sure.
> 
> And you can even make those scripts happy by doing:
> 
> [Patch 1/7] [SUBSYSTEM]: Foo bar baz...
> 
> And if you haven't noticed over the past few years, this is
> is the convention we've been using in the networking.
> 
> I munge every one of your (and everyone else's) changelog entry
> headers this way.  Without exception, every single one.
> 
> So when you don't follow this convention, you make more typing
> and more work for me.  The more patches I get from someone
> the more important it is for this convention to be followed.
> 
> I find it very hard to believe that you haven't once looked
> at the hundreds of patches I've applied of your's and not
> noticed how I reformat everything.

Of course I have.  And I believe it to be incorrect for the reasons
which I clearly stated.

Take a look at the git logs, see what most other people are doing.

Re: [PATCH] bridge: assign random address

[Randy Dunlap]

On Sun, 16 Dec 2007 15:26:06 -0800 (PST) David Miller wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Sun, 16 Dec 2007 14:29:15 -0800
> 
> > On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <davem@davemloft.net> wrote:
> > 
> > > From: Stephen Hemminger <shemminger@linux-foundation.org>
> > > Date: Tue, 11 Dec 2007 15:48:35 -0800
> > > 
> > > > Subject: Re: [PATCH] bridge: assign random address
> > > 
> > > "bridge" should all-caps and in brackets,
> > 
> > No, "bridge" should not be in [].  Lots of people's patch-receiving scripts
> > assume that any text in [] is to be removed as the patch is committed.  It
> > contains text which is only relevant to the particular email which carried
> > the patch.  Stuff like "patch" and "4/5" and "linux-2.6.23", etc.
> 
> I don't use scripts, I edit it by hand.  And when I do ever use
> scripts I will make sure they accomodate "[$SUBSYSTEM]" format
> subject lines, you can be sure.
> 
> And you can even make those scripts happy by doing:
> 
> [Patch 1/7] [SUBSYSTEM]: Foo bar baz...
> 
> And if you haven't noticed over the past few years, this is
> is the convention we've been using in the networking.
> 
> I munge every one of your (and everyone else's) changelog entry
> headers this way.  Without exception, every single one.
> 
> So when you don't follow this convention, you make more typing
> and more work for me.  The more patches I get from someone
> the more important it is for this convention to be followed.
> 
> I find it very hard to believe that you haven't once looked
> at the hundreds of patches I've applied of your's and not
> noticed how I reformat everything.

I have noticed the difference in networking vs. rest-of-kernel.
Rest-of-kernel generally follows the canonical format in
Documentation/SubmittingPatches:

14) The canonical patch format

The canonical patch subject line is:

    Subject: [PATCH 001/123] subsystem: summary phrase


---
~Randy

Re: [PATCH] bridge: assign random address

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Sun, 16 Dec 2007 15:34:42 -0800

> Take a look at the git logs, see what most other people are doing.

You're talking bucking a convention that has been used
for all networking changes since we starting using real
revision control.

I've shown how the subject lines can be done in a way
that both satisfies the scripts you're worried about
and keeps the networking changes looking the way they
have for 5+ years.

What's the reason to change again?

Re: [PATCH] bridge: assign random address

[Andrew Morton]

On Sun, 16 Dec 2007 15:40:18 -0800 (PST) David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Sun, 16 Dec 2007 15:34:42 -0800
> 
> > Take a look at the git logs, see what most other people are doing.
> 
> You're talking bucking a convention that has been used
> for all networking changes since we starting using real
> revision control.
> 
> I've shown how the subject lines can be done in a way
> that both satisfies the scripts you're worried about
> and keeps the networking changes looking the way they
> have for 5+ years.
> 
> What's the reason to change again?

I see no particular reason to change - it's just one of those things.

Two third of commits don't use [subsystem] and 90% don't use trailing
period.  Reasons to change would be a) consistency and b) the time it takes
to occasionally fix up patches which use [subsystem] as I earlier
described.

I don't think these are terribly important, really.

Re: [PATCH] bridge: assign random address

[Jeff Garzik]

David Miller wrote:
> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Sun, 16 Dec 2007 14:29:15 -0800
> 
>> On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <davem@davemloft.net> wrote:
>>
>>> From: Stephen Hemminger <shemminger@linux-foundation.org>
>>> Date: Tue, 11 Dec 2007 15:48:35 -0800
>>>
>>>> Subject: Re: [PATCH] bridge: assign random address
>>> "bridge" should all-caps and in brackets,
>> No, "bridge" should not be in [].  Lots of people's patch-receiving scripts
>> assume that any text in [] is to be removed as the patch is committed.  It
>> contains text which is only relevant to the particular email which carried
>> the patch.  Stuff like "patch" and "4/5" and "linux-2.6.23", etc.
> 
> I don't use scripts, I edit it by hand.  And when I do ever use
> scripts I will make sure they accomodate "[$SUBSYSTEM]" format
> subject lines, you can be sure.
> 
> And you can even make those scripts happy by doing:
> 
> [Patch 1/7] [SUBSYSTEM]: Foo bar baz...

The most popular tool is git-am, which I and many others use.

git-am will snip "[SUBSYSTEM]" in the example that you give.

Until Linus's official mail import tool (git-am) changes, I agree with 
Andrew -- since Andrew is simply describing the de facto standard as it 
exists today:  [] gets eaten.

That's why documentation like Documentation/SubmittingPatches and 
http://linux.yyz.us/patch-format.html indicate "subsystem: " rather than 
"[SUBSYSTEM]":  it's compatible with Linus's widely used mail import tool.

	Jeff

Re: [PATCH] bridge: assign random address

[Andrew Morton]

On Sun, 16 Dec 2007 20:46:24 -0500 Jeff Garzik <jeff@garzik.org> wrote:

> David Miller wrote:
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Date: Sun, 16 Dec 2007 14:29:15 -0800
> > 
> >> On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <davem@davemloft.net> wrote:
> >>
> >>> From: Stephen Hemminger <shemminger@linux-foundation.org>
> >>> Date: Tue, 11 Dec 2007 15:48:35 -0800
> >>>
> >>>> Subject: Re: [PATCH] bridge: assign random address
> >>> "bridge" should all-caps and in brackets,
> >> No, "bridge" should not be in [].  Lots of people's patch-receiving scripts
> >> assume that any text in [] is to be removed as the patch is committed.  It
> >> contains text which is only relevant to the particular email which carried
> >> the patch.  Stuff like "patch" and "4/5" and "linux-2.6.23", etc.
> > 
> > I don't use scripts, I edit it by hand.  And when I do ever use
> > scripts I will make sure they accomodate "[$SUBSYSTEM]" format
> > subject lines, you can be sure.
> > 
> > And you can even make those scripts happy by doing:
> > 
> > [Patch 1/7] [SUBSYSTEM]: Foo bar baz...
> 
> The most popular tool is git-am, which I and many others use.
> 
> git-am will snip "[SUBSYSTEM]" in the example that you give.
> 
> Until Linus's official mail import tool (git-am) changes, I agree with 
> Andrew -- since Andrew is simply describing the de facto standard as it 
> exists today:  [] gets eaten.

I didn't know that.

> That's why documentation like Documentation/SubmittingPatches and 
> http://linux.yyz.us/patch-format.html indicate "subsystem: " rather than 
> "[SUBSYSTEM]":  it's compatible with Linus's widely used mail import tool.
> 

People are tossing all sorts of metadata into [] nowadays...

grep '^Subject:' lkmo-folder | grep '\[.*\[' | grep -v Re:

says stuff like

Subject: [PATCH 5/7] Security: Change current->fs[ug]id to current_fs[ug]id()
Subject: [PATCH 00/28] Permit filesystem local caching [try #2]
Subject: [PATCH 04/28] KEYS: Add keyctl function to get a security label [try
Subject: [PATCH 05/28] Security: Change current->fs[ug]id to
Subject: [PATCH 02/28] KEYS: Check starting keyring as part of search [try #2]
Subject: [PATCH 21/28] NFS: Display local caching state [try #2]
Subject: [PATCH 17/28] CacheFiles: Export things for CacheFiles [try #2]
Subject: [PATCH 13/28] CacheFiles: Add missing copy_page export for ia64 [try
Subject: [PATCH 19/28] NFS: Use local caching [try #2]
Subject: [PATCH 12/28] FS-Cache: Generic filesystem caching facility [try #2]
Subject: [PATCH 23/28] AFS: Add TestSetPageError() [try #2]
Subject: [PATCH 22/28] fcrypt endianness misannotations [try #2]
Subject: [PATCH 26/28] AF_RXRPC: Save the operation ID for debugging [try #2]
Subject: [PATCH 25/28] AFS: Improve handling of a rejected writeback [try #2]
Subject: [PATCH 27/28] AFS: Implement shared-writable mmap [try #2]
Subject: [PATCH 28/28] FS-Cache: Make kAFS use FS-Cache [try #2]
Subject: [RFC] [PATCH] A clean approach to writeout throttling
Subject: [RFC][POWERPC] Provide a way to protect 4k subpages when
Subject: [PATCH 2/3] [PATCH] unify common parts of segment.h
Subject: [PATCH 1/3] [PATCH] put get_kernel_rpl in a common location
Subject: [PATCH 3/3] [PATCH] remove arch specific segment headers
Subject: [patch 2.6.24-rc4-mm 1/6] gpiolib: add gpio_desc[]
Subject: [PATCH][SCSI] hptiop: add more adapter models and other fixes
Subject: [PATCH][for -mm] fix accounting in vmscan.c for memory controller
Subject: [DOC][for -mm] update Documentation/controller/memory.txt
Subject: [PATCH] [NET]: Fix Ooops of napi net_rx_action.
Subject: RE: [PATCH] [NET]: Fix Ooops of napi net_rx_action.
Subject: RE: [PATCH] [NET]: Fix Ooops of napi net_rx_action.
Subject: [PATCH 7/7] [NETDEV]: myri10ge Fix possible causing oops of net_rx_action
Subject: [PATCH 4/7] [NETDEV]: ixgbe Fix possible causing oops of net_rx_action
Subject: [PATCH 5/7] [NETDEV]: e100 Fix possible causing oops of net_rx_action
Subject: [PATCH 3/7] [NETDEV]: ixgb Fix possible causing oops of net_rx_action
Subject: [PATCH 1/7] [NETDEV]: e1000 Fix possible causing oops of net_rx_action
Subject: [PATCH 2/7] [NETDEV]: e1000e Fix possible causing oops of net_rx_action
Subject: [PATCH 6/7] [NETDEV]: tehuti Fix possible causing oops of net_rx_action
Subject: [PATCH][SCSI] resend: hptiop: add more adapter models and other fixes
Subject: [PATCH/RFC] [POWERPC] Add fixed-phy support for fs_enet
Subject: [PATCH][NETDEV]: remove netif_running() check from myri10ge_poll()
Subject: [RFC] [PATCH -mm] agp: remove uid comparison as security check
Subject: [RFC] [PATCH -mm] agp: remove uid comparison as security check
Subject: [RFC] [PATCH -mm] reiser4: replace uid==0 check with capability
Subject: [RFC] [PATCH -mm] reiser4: replace uid==0 check with capability
Subject: [RFC] [PATCH -mm] oom_kill: remove uid==0 checks
Subject: [RFC] [PATCH -mm] oom_kill: remove uid==0 checks
Subject: [PATCH][MMC] Fix wrong EXT_CSD_REV handling
Subject: [PATCH] x86: move interrupts[] to .rodata/.init.data
Subject: RE: [PATCH 1/7] [NETDEV]: e1000 Fix possible causing oops of net_rx_action
Subject: [PATCH][KJ] 8250: remove unnecessary variable tmout from  wait_for_xmitr()
Subject: [PATCH][rewrite with goto error handling] Bluetooth: hci_sysfs
Subject: [RFC][PATCH] fix bus error when trying to access anon & shared page created by mremap()[BUG:8691]
Subject: [RFC] [patch 2/2] Refuse kprobe insertion on __init section code
Subject: [RFC] [patch 1/2] add non_init_kernel_text_address
Subject: [PATCH] [TCP]: Fix fack_count miscountings (multiple places)
Subject: RE: [i2c] [PATCH 2.6.24-rc4-mm 1/2] gpiolib: basic support for 16-bit PCA9539 GPIO expander[
Subject: [PATCH][RT] 2.6.24-rc5-rt1 drivers/dma/ioat_dma.c compile fix


Kinda funny.  I (and I bet lots of others) spend a lot of time fixing,
cleaning up and totally rewriting patch titles.

Re: [PATCH] bridge: assign random address

[David Miller]

From: Jeff Garzik <jeff@garzik.org>
Date: Sun, 16 Dec 2007 20:46:24 -0500

> David Miller wrote:
> > [Patch 1/7] [SUBSYSTEM]: Foo bar baz...
> 
> The most popular tool is git-am, which I and many others use.
> 
> git-am will snip "[SUBSYSTEM]" in the example that you give.

It should only snip the first "[Patch X/Y] " part, or the manual is
buggy :-)

Re: [PATCH] bridge: assign random address

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Sun, 16 Dec 2007 18:55:48 -0800

> Kinda funny.  I (and I bet lots of others) spend a lot of time
> fixing, cleaning up and totally rewriting patch titles.

It's probably the majority of the typing I perform to apply a patch
except for the folks who format things the way that works best for
me.

I realize that I can't get every patch submitter to conform to what I
want, but at least I try to nudge the people who submit the most stuff
to me.

And it's just as much to "help me out" as it is that I want the
changelogs header lines to look consistent.  I'm going to enforce that
consistency anyways. :-)

But if people submit patches in a way that makes life difficult for
the subsystem maintainer, and keeps doing so after being asked to
adjust their submissions a little bit, well... some patches might get
mysteriously ignored.

When you're submitting a lot of patches you're chewing up a LOT of
someone's bandwidth.  I often spend at least 45 minutes a day on
network namespace patches, as one specific example.  That's a large
investment of time to give someone.

In return it's asking very little to "make your subject lines look
like this, it helps me a lot, kthx".

Re: [PATCH] bridge: assign random address

[Stephen Hemminger]

On Sun, 16 Dec 2007 14:29:15 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <davem@davemloft.net> wrote:
> 
> > From: Stephen Hemminger <shemminger@linux-foundation.org>
> > Date: Tue, 11 Dec 2007 15:48:35 -0800
> > 
> > > Subject: Re: [PATCH] bridge: assign random address
> > 
> > "bridge" should all-caps and in brackets,
> 
> No, "bridge" should not be in [].  Lots of people's patch-receiving scripts
> assume that any text in [] is to be removed as the patch is committed.  It
> contains text which is only relevant to the particular email which carried
> the patch.  Stuff like "patch" and "4/5" and "linux-2.6.23", etc.
> 
> > "assign random address"
> > should be capitalized like a proper english sentence with a period at
> > the end.
> 
> Actually I usually remove the caps and the waste-of-space period, but
> that's much less important than the brackets abuse.  The bracket convention
> is quite useful and I've often wondered why I need to edit the patch title
> when I merge up patches from net developers ;)
> 

I try to follow the title convention that Jeff was promoting.

It works well because he is dealing with many different drivers.


-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH] bridge: assign random address

[Jeff Garzik]

David Miller wrote:
> From: Jeff Garzik <jeff@garzik.org>
> Date: Sun, 16 Dec 2007 20:46:24 -0500
> 
>> David Miller wrote:
>>> [Patch 1/7] [SUBSYSTEM]: Foo bar baz...
>> The most popular tool is git-am, which I and many others use.
>>
>> git-am will snip "[SUBSYSTEM]" in the example that you give.
> 
> It should only snip the first "[Patch X/Y] " part, or the manual is
> buggy :-)

Manual is buggy.

	[jgarzik@pretzel netdev-2.6]$ grep '^Subject:' /g/tmp/mbox
	Subject: [PATCH 1/2][FOO][BAR][BAZ] tulip: napi full quantum bug

is merged as

	commit e34e20a3ae1ec30856427d260f454b8984ebced2
	Author: Stephen Hemminger <shemminger@linux-foundation.org>
	Date:   Thu Dec 13 09:35:45 2007 -0800

	    tulip: napi full quantum bug

And that matches existing practice, where people put tons of 
not-to-be-merged metadata into the subject lines.

Regards,

	Jeff