New: Cannot bring up a bridge interface without a MAC address set
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]/
(please respond via emailed reply-to-all, not via the bugzilla web interface).
Quoted 74 lines · On Tue, 11 Dec 2007 11:04:55 -0800 (PST) [email protected] 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: [email protected] ReportedBy: [email protected]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 SMPx86_64GNU/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 argumentIt is failing on the ioctl to bring up the interface
ioctl(4, SIOCGIFFLAGS, {ifr_name="demobr", ifr_flags=IFF_BROADCAST|IFF_MULTICAST})= 0ioctl(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
bada339ba24dee9e143bfb42e1dc61f146619846Author: Jeff Garzik <[email protected]> Date: Tue Oct 23 20:19:37 2007 -0700[NET]: Validate device addr prior to interface-up
Signed-off-by Jeff Garzik <[email protected]>Signed-off-by David S. Miller <[email protected]>net/core/dev.c +3
-1@@ -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
Quoted 6 lines
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.
Quoted 41 lines · On Tue, 11 Dec 2007 13:26:14 -0800 Andrew Morton <[email protected]>…
(please respond via emailed reply-to-all, not via the bugzilla web interface).
Quoted 34 lines · On Tue, 11 Dec 2007 11:04:55 -0800 (PST) [email protected] 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: [email protected] ReportedBy: [email protected]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 SMPx86_64GNU/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.
Signature · 1 line
Stephen Hemminger <[email protected]>
Quoted 45 lines · On Tue, 11 Dec 2007 14:52:43 -0800 Stephen Hemminger…
Quoted 41 lines · On Tue, 11 Dec 2007 13:26:14 -0800 Andrew Morton <[email protected]>…
(please respond via emailed reply-to-all, not via the bugzilla web interface).
Quoted 34 lines · On Tue, 11 Dec 2007 11:04:55 -0800 (PST) [email protected] 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: [email protected] ReportedBy: [email protected]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 SMPx86_64GNU/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
Quoted 8 lines
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 1/1 bridge: assign random address
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
net/bridge/br_device.c +1 -2
@@ -157,8 +157,7 @@ static struct ethtool_ops br_ethtool_opsvoid 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
Quoted 14 lines · On Tue, 11 Dec 2007 15:48:35 -0800 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_addrSigned-off-by Stephen Hemminger <[email protected]>
net/bridge/br_device.c static struct ethtool_ops br_ethtool_ops · line 163
Quoted 9 lines
net/bridge/br_device.c +1
-2@@ -157,8 +157,7 @@ static struct ethtool_ops br_ethtool_opsvoid 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: [PATCH] bridge: assign random address
In reply to Stephen HemmingerFrom: Stephen Hemminger <[email protected]>
Date: Tue, 11 Dec 2007 15:48:35 -0800
Quoted 1 line
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.
Quoted 10 lines
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_addrSigned-off-by Stephen Hemminger <[email protected]>
Applied to net-2.6, thanks.
Re: [PATCH] bridge: assign random address
Quoted 6 lines · On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[email protected]>…
From: Stephen Hemminger <[email protected]>
Date: Tue, 11 Dec 2007 15:48:35 -0800Quoted 1 line
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.
Quoted 3 lines
"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 ;)
David Miller
maintainerRe: [PATCH] bridge: assign random address
From: Andrew Morton <[email protected]>
Date: Sun, 16 Dec 2007 14:29:15 -0800
Quoted 5 lines · On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[email protected]>…
Quoted 4 lines
From: Stephen Hemminger <[email protected]>
Date: Tue, 11 Dec 2007 15:48:35 -0800
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
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
Quoted 31 lines · On Sun, 16 Dec 2007 15:26:06 -0800 (PST) David Miller <[email protected]>…
From: Andrew Morton <[email protected]>
Date: Sun, 16 Dec 2007 14:29:15 -0800Quoted 5 lines · On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[email protected]>…
Quoted 4 lines
From: Stephen Hemminger <[email protected]>
Date: Tue, 11 Dec 2007 15:48:35 -0800Quoted 8 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 7 lines
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.
David Miller
maintainerRe: [PATCH] bridge: assign random address
From: Andrew Morton <[email protected]>
Date: Sun, 16 Dec 2007 15:34:42 -0800
Quoted 1 line
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
Quoted 9 lines · On Sun, 16 Dec 2007 15:40:18 -0800 (PST) David Miller <[email protected]>…
From: Andrew Morton <[email protected]>
Date: Sun, 16 Dec 2007 15:34:42 -0800Quoted 1 line
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.
Quoted 6 lines
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
In reply to David MillerQuoted 31 lines · On Sun, 16 Dec 2007 15:26:06 -0800 (PST) David Miller wrote:
From: Andrew Morton <[email protected]>
Date: Sun, 16 Dec 2007 14:29:15 -0800Quoted 5 lines · On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[email protected]>…
Quoted 4 lines
From: Stephen Hemminger <[email protected]>
Date: Tue, 11 Dec 2007 15:48:35 -0800Quoted 8 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 7 lines
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
In reply to David MillerQuoted 15 lines · David Miller
From: Andrew Morton <[email protected]>
Date: Sun, 16 Dec 2007 14:29:15 -0800Quoted 11 lines · On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[email protected]>…
Quoted 6 lines
From: Stephen Hemminger <[email protected]>
Date: Tue, 11 Dec 2007 15:48:35 -0800Quoted 1 line
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.
Quoted 7 lines
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
Quoted 24 lines · On Sun, 16 Dec 2007 20:46:24 -0500 Jeff Garzik <[email protected]> wrote:
Quoted 15 lines · David Miller
From: Andrew Morton <[email protected]>
Date: Sun, 16 Dec 2007 14:29:15 -0800Quoted 11 lines · On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[email protected]>…
Quoted 6 lines
From: Stephen Hemminger <[email protected]>
Date: Tue, 11 Dec 2007 15:48:35 -0800Quoted 1 line
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.
Quoted 7 lines
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...
Quoted 7 lines
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.
Quoted 4 lines
That's why documentation like
Documentation/SubmittingPatchesand
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.
David Miller
maintainerRe: [PATCH] bridge: assign random address
From: Andrew Morton <[email protected]>
Date: Sun, 16 Dec 2007 18:55:48 -0800
Quoted 2 lines
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".
David Miller
maintainerRe: [PATCH] bridge: assign random address
In reply to Jeff GarzikFrom: Jeff Garzik <[email protected]>
Date: Sun, 16 Dec 2007 20:46:24 -0500
Quoted 6 lines
Quoted 1 line · David Miller
[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
Quoted 11 lines · David Miller
From: Jeff Garzik <[email protected]>
Date: Sun, 16 Dec 2007 20:46:24 -0500Quoted 5 lines
Quoted 1 line · David Miller
[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 <[email protected]> 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
Re: [PATCH] bridge: assign random address
In reply to Andrew MortonQuoted 14 lines · On Sun, 16 Dec 2007 14:29:15 -0800 Andrew Morton <[email protected]>…
Quoted 6 lines · On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[email protected]>…
From: Stephen Hemminger <[email protected]>
Date: Tue, 11 Dec 2007 15:48:35 -0800Quoted 1 line
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.
Quoted 9 lines
Quoted 3 lines
"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.
Signature · 1 line
Stephen Hemminger <[email protected]>
Quoted 3 lines · Stephen Hemminger
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.
Quoted 1 line
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 :)
Quoted 3 lines
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,
Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txtQuoted 16 lines · On Wed, 12 Dec 2007 09:51:05 +0800 Herbert Xu <[email protected]>…
Quoted 3 lines · Stephen Hemminger
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.
Quoted 1 line
Could be fixed to prevent errors from existing scripts but it is not a complete showstopper.
Quoted 7 lines
Well this stops FC8 working with Xen so for that it's a showstopper :)
Quoted 3 lines
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.
Signature · 1 line
Stephen Hemminger <[email protected]>