From: Patrick McHardy <kaber@trash•net>
To: David Miller <davem@davemloft•net>
Cc: netdev@vger•kernel.org
Subject: Re: net: rtnetlink: support specifying device flags on device creation
Date: Mon, 01 Mar 2010 16:24:23 +0100 [thread overview]
Message-ID: <4B8BDC27.2060803@trash.net> (raw)
In-Reply-To: <20100227.025128.186425822.davem@davemloft.net>
David Miller wrote:
> From: Patrick McHardy <kaber@trash•net>
> Date: Fri, 26 Feb 2010 17:34:49 +0100 (MET)
>
>> The following patches add support to specify the device flags (like UP) when
>> creating a new device through rtnl_link. This requires to surpress netlink
>> notifications until the device is fully configured in order to not confuse
>> userspace when changing the flags fails and registration has to be undone.
>> Once the device is configured, a single NEWLINK message with the full state
>> is sent.
>>
>> The individual patch changelogs describe the necessary changes in more detail.
>
> All applied, but two things:
>
> 1) This patch set was harder to review because there were no
> default initializations of the new enumeration you added
> to struct netdev.
>
> I know RTNL_LINK_INITIALIZED is probably zero by C enumeration
> rules, and the zeroing out of new netdev objects gives us this,
> but I only figured that out after some time.
>
> It deserved at least a commit message mention in patch #2.
Agreed. The reason to use the value 0 for RTNL_LINK_INITIALIZED
was that it avoids touching drivers that can register netdevices
through non rtnl_link paths, like bonding, ifb or dummy.
> 2) I would really appreciate you forming your patch postings
> properly. I have to edit them every single time
>
> You put the whole output of "git show" or "git format-patch" into
> your email body. That doesn't work, we don't want all of those
> commit ID etc. lines in there. It also causes every line of your
> commit messages to be indented by 4 spaces.
>
> Your email body should just contain the unindented commit message
> and the signoffs, then the patch itself.
>
> Your Subject lines are also not setup properly. Because the
> "net X/N:" thing isn't in [] brackets, it ends up in the
> commit message header lines when I feed your emails to
> "git am".
>
> All of this could be avoided if you used git send-email but
> I realize that a lot of people dislike that for one reason
> or another (myself included), but if you're going to compose
> the emails by hand you ought to make it look the same (syntax
> wise) as what git send-email would have emitted.
I'll either switch to git-send-email or make sure my script
works properly with git-am.
prev parent reply other threads:[~2010-03-01 15:24 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-26 16:34 net: rtnetlink: support specifying device flags on device creation Patrick McHardy
2010-02-26 16:34 ` net 01/04: rtnetlink: ignore NETDEV_PRE_UP notifier in rtnetlink_event() Patrick McHardy
2010-02-26 16:34 ` net 02/04: rtnetlink: handle rtnl_link netlink notifications manually Patrick McHardy
2010-02-26 16:34 ` net 03/04: dev: support deferring device flag change notifications Patrick McHardy
2010-02-26 16:34 ` net 04/04: rtnetlink: support specifying device flags on device creation Patrick McHardy
2010-02-27 10:51 ` net: " David Miller
2010-03-01 15:24 ` Patrick McHardy [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B8BDC27.2060803@trash.net \
--to=kaber@trash$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox