Re: [PATCH] bridge: assign random address

2007-12-11 Thread Andrew Morton
On Tue, 11 Dec 2007 15:48:35 -0800
Stephen Hemminger <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
> 
> --- a/net/bridge/br_device.c  2007-10-16 16:48:21.0 -0700
> +++ b/net/bridge/br_device.c  2007-12-11 15:36:52.0 -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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bridge: assign random address

2007-12-16 Thread David Miller
From: Stephen Hemminger <[EMAIL PROTECTED]>
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 <[EMAIL PROTECTED]>

Applied to net-2.6, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bridge: assign random address

2007-12-16 Thread Andrew Morton
On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[EMAIL PROTECTED]> wrote:

> From: Stephen Hemminger <[EMAIL PROTECTED]>
> 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 ;)

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


Re: [PATCH] bridge: assign random address

2007-12-16 Thread David Miller
From: Andrew Morton <[EMAIL PROTECTED]>
Date: Sun, 16 Dec 2007 14:29:15 -0800

> On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[EMAIL PROTECTED]> 
> wrote:
> 
> > From: Stephen Hemminger <[EMAIL PROTECTED]>
> > 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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bridge: assign random address

2007-12-16 Thread Andrew Morton
On Sun, 16 Dec 2007 15:26:06 -0800 (PST) David Miller <[EMAIL PROTECTED]> wrote:

> From: Andrew Morton <[EMAIL PROTECTED]>
> Date: Sun, 16 Dec 2007 14:29:15 -0800
> 
> > On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[EMAIL PROTECTED]> 
> > wrote:
> > 
> > > From: Stephen Hemminger <[EMAIL PROTECTED]>
> > > 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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bridge: assign random address

2007-12-16 Thread Randy Dunlap
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 -0800
> 
> > On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[EMAIL PROTECTED]> 
> > wrote:
> > 
> > > From: Stephen Hemminger <[EMAIL PROTECTED]>
> > > 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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bridge: assign random address

2007-12-16 Thread David Miller
From: Andrew Morton <[EMAIL PROTECTED]>
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?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bridge: assign random address

2007-12-16 Thread Andrew Morton
On Sun, 16 Dec 2007 15:40:18 -0800 (PST) David Miller <[EMAIL PROTECTED]> wrote:

> From: Andrew Morton <[EMAIL PROTECTED]>
> 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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bridge: assign random address

2007-12-16 Thread Jeff Garzik

David Miller wrote:

From: Andrew Morton <[EMAIL PROTECTED]>
Date: Sun, 16 Dec 2007 14:29:15 -0800


On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[EMAIL PROTECTED]> wrote:


From: Stephen Hemminger <[EMAIL PROTECTED]>
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




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


Re: [PATCH] bridge: assign random address

2007-12-16 Thread Andrew Morton
On Sun, 16 Dec 2007 20:46:24 -0500 Jeff Garzik <[EMAIL PROTECTED]> wrote:

> David Miller wrote:
> > From: Andrew Morton <[EMAIL PROTECTED]>
> > Date: Sun, 16 Dec 2007 14:29:15 -0800
> > 
> >> On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[EMAIL PROTECTED]> 
> >> wrote:
> >>
> >>> From: Stephen Hemminger <[EMAIL PROTECTED]>
> >>> 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

Re: [PATCH] bridge: assign random address

2007-12-16 Thread David Miller
From: Jeff Garzik <[EMAIL PROTECTED]>
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 :-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bridge: assign random address

2007-12-16 Thread David Miller
From: Andrew Morton <[EMAIL PROTECTED]>
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".

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


Re: [PATCH] bridge: assign random address

2007-12-17 Thread Stephen Hemminger
On Sun, 16 Dec 2007 14:29:15 -0800
Andrew Morton <[EMAIL PROTECTED]> wrote:

> On Sun, 16 Dec 2007 13:37:17 -0800 (PST) David Miller <[EMAIL PROTECTED]> 
> wrote:
> 
> > From: Stephen Hemminger <[EMAIL PROTECTED]>
> > 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 <[EMAIL PROTECTED]>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] bridge: assign random address

2007-12-17 Thread Jeff Garzik

David Miller wrote:

From: Jeff Garzik <[EMAIL PROTECTED]>
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.

[EMAIL PROTECTED] 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


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