Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Wed, 2006-16-08 at 17:04 +0400, Alexey Kuznetsov wrote: > Hello! > > > In one conversation with Alexey he told me there was some inspiration > > from pfkey in the semantics of it i.e processid. > > Inspiration, but not a copy. :-) Oh, absolutely. Netlink is way superior. I should have said perspiration instead of inspiration;-> Calling inspiration was being polite - it is as being polite as saying i was being economical with the truth[1] ;-> > Unlike pfkeyv2 it uses addressing usual for networking i.e. > struct sockaddr_nl. I think this needs to be captured somewhere. I dont know who is maintaining the man pages these days. cheers, jamal [1] A term i learnt from some British guy. They have ways with words those Brits. - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
Hello! > In one conversation with Alexey he told me there was some inspiration > from pfkey in the semantics of it i.e processid. Inspiration, but not a copy. :-) Unlike pfkeyv2 it uses addressing usual for networking i.e. struct sockaddr_nl. Alexey - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
Hello! > The netlink header pid is really akin to sadb_msg_pid from RFC 2367. > IMHO it should always be zero if the kernel is the originator of the > message. No. Analogue of sadb_msg_pid is nladdr.nl_pid. Netlink header pid is not originator of the message, but author of the change. The notion is ambiguous by definition, and the field is a little ambiguous. If the message is a plain ack or part of a dump, it is obviously pid of requestor. But if it is notification about change, it can be nl_pid of socket, which requested the operation, but may be 0. Of course, all the 0s sent only because I was lazy to track authorship, should be eliminated. Alexey - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Wed, 2006-16-08 at 14:08 +0200, Thomas Graf wrote: > * jamal <[EMAIL PROTECTED]> 2006-08-16 08:04 > > current->pid i think is coming out to be a bad idea. Thomas' patches > > revert it out. Again this has everything to do with the original idea > > what maps to pid now changing to socketid. > > It probably developed from autobind using current->tid. In one conversation with Alexey he told me there was some inspiration from pfkey in the semantics of it i.e processid. Obviously with many sockets on the same process etc, that assumption is no longer valid. On Wed, 2006-16-08 at 22:08 +1000, Herbert Xu wrote: > On Wed, Aug 16, 2006 at 08:04:24AM -0400, jamal wrote: > > > > What do you think of the idea of infact rewriting the pid to be that of > > the socket id? > > Rewriting it with the netlink socket address? That's fine by me as > long as there is a clear 1-to-1 relationship between the request > and the notification. you would have to call getpeername() to get a correct 1-1 mapping as is today when in doubt. What i was suggesting is notifications using the pid that would id the socket and would therefore require a getpeername() which identify the real socket it came from; if you are fine with what Thomas is doing, then this unnecessary since i was suggesting it as a compromise for consistency you pointed was lacking. cheers, jamal - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
* Herbert Xu <[EMAIL PROTECTED]> 2006-08-16 21:39 > For a broadcast notification, the nlmsg_pid field is meaningless > because the nlmsg_seq field is also meaningless. I'm not denying > that it wouldn't be useful to have the originator's socket address > in there. What I'm saying is that it's the wrong place to put > that information. It might not be the best place to put it considering the original intend of nlmsg_pid as you explained correctly. However, as you state yourself, the nlmsg_pid field is meaningless/unused for notifications so extending the definition of nlmsg_pid to have a special meaning for broadcasts doesn't harm anyone. When setting nlmsg_seq to the seq of the request it becomes a meaning together with nlmsg_pid as applications can then easly assign notifications to their own sent requests. Secondly we already have applications depending on this whereas the eventual breaking of aplications depending on nlmsg_pid == 0 is uncertain. - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
* jamal <[EMAIL PROTECTED]> 2006-08-16 08:04 > current->pid i think is coming out to be a bad idea. Thomas' patches > revert it out. Again this has everything to do with the original idea > what maps to pid now changing to socketid. It probably developed from autobind using current->tid. - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Wed, Aug 16, 2006 at 08:04:24AM -0400, jamal wrote:
>
> What do you think of the idea of infact rewriting the pid to be that of
> the socket id?
Rewriting it with the netlink socket address? That's fine by me as
long as there is a clear 1-to-1 relationship between the request
and the notification.
Cheers,
--
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.txt
-
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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Wed, 2006-16-08 at 21:39 +1000, Herbert Xu wrote: > So let's step back a bit and think about where does this pid really > come from. The field in question is nlmsg_pid. Its primary purpose > is to identify unicast transactions along with the field nlmsg_seq. > It was not designed to identify the origin of a broadcast kernel > notification to a third party. There are quiet a few things that netlink design intent was not intending to solve that became needed over time. This being one IMHO. Design intent and eventual (sometimes creative) use occasionally create an impedance ;-> Evolution is the only description i can think of. > For this purpose, the value of nlmsg_pid is set to the address of > the destination socket for a particular unicast message (also known > as the pid). Since we are talking history: The idea of it being a destination socket _was not_ design intent. It was evolution. I recall James Morris actually to be the first person whining about this ambiguity when coding up nfqueue. I cant remember who fixed it (I am inclined to think it was you;->) > That pid in turn has only a vague connection with the process ID > of the process owning the socket. For practical purposes, we > should not treat it as a process ID it can easily be claimed by > another process (think socket + bind + fork). If you want to be complete the kernel should "fix" the pid in netlink::sendmsg(). > For a broadcast notification, the nlmsg_pid field is meaningless > because the nlmsg_seq field is also meaningless. nlmsg_seq is meaningless; "seq" is again a bad noun. It should be "cookie". > I'm not denying > that it wouldn't be useful to have the originator's socket address > in there. What I'm saying is that it's the wrong place to put > that information. > > In any case, putting current->pid in this field is definitely > a bad idea because it only encourages people to confuse the > netlink pid with the process ID which can lead to security > problems later on. current->pid i think is coming out to be a bad idea. Thomas' patches revert it out. Again this has everything to do with the original idea what maps to pid now changing to socketid. What do you think of the idea of infact rewriting the pid to be that of the socket id? cheers, jamal - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Wed, 2006-16-08 at 14:05 +0200, Thomas Graf wrote: > Right, but he forgot the bits in IPv6 which I now fixed. The > changeset introducing those current->pid uses was definitely > simply wrong. I'm not questioning that :) Herbert, if you look at the thread as well I am no longer questioning that either ;-> cheers, jamal PS:- Would a topic of "things i wish netlink did better" be of interest for discussion (maybe for netconf)? (Un)fortunately, we are fixing some of them with genetlink;-> - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
* Herbert Xu <[EMAIL PROTECTED]> 2006-08-16 21:57 > On Wed, Aug 16, 2006 at 01:40:03PM +0200, Thomas Graf wrote: > > > > It was added to help quagga identify which route modifications > > are self caused. It's not possible to use rtm_protocol for this > > purpose as other applications can delete routes added by quagga. > > Actually it's not that bad. I just checked the quagga source and > the stuff it needs was already provided anyway, even before that > change. If I recall correctly the quagga folks asked to get the same behaviour for IPv6 routes as it was already done for IPv4 around the time of that bogus changeset. > In fact, the really bad bits in the changeset have already been > reverted by Alexey back in February :) Right, but he forgot the bits in IPv6 which I now fixed. The changeset introducing those current->pid uses was definitely simply wrong. I'm not questioning that :) - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Wed, Aug 16, 2006 at 01:40:03PM +0200, Thomas Graf wrote:
>
> It was added to help quagga identify which route modifications
> are self caused. It's not possible to use rtm_protocol for this
> purpose as other applications can delete routes added by quagga.
Actually it's not that bad. I just checked the quagga source and
the stuff it needs was already provided anyway, even before that
change.
In fact, the really bad bits in the changeset have already been
reverted by Alexey back in February :)
Cheers,
--
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.txt
-
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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
* Herbert Xu <[EMAIL PROTECTED]> 2006-08-16 21:12 > On Wed, Aug 16, 2006 at 12:58:56PM +0200, Thomas Graf wrote: > > > > All route and tc notifications already use the pid so applications > > can decide whether the event was caused by them. A notification > > is a reply to a request so it doesn't even violate RFC 2367. > > Actually most IPv4 notifications *do* set the pid to zero which is > the right thing to do for kernel-generated messages. > > You're right though that the IPv6 notification modified by this patch > does set the pid to the netlink originator. Looking back in history > it seems that this behaviour was only introduced last year to a subset > of notifications. It was added to help quagga identify which route modifications are self caused. It's not possible to use rtm_protocol for this purpose as other applications can delete routes added by quagga. > This inconsistency is very bad. IMHO this change (made last year) > should be reverted so that all kernel generated (broadcast) notifications > have the originator set to zero to match the source address of the > message. We can't just knowingly break quagga. I think it's a good thing to include the pid, it's additional information that is helpful to userspace. Userspace is already aware that the notifications are orignating from the kernel, we can't do userspace -> userspace communication anymore anyway. > Any notification that sets the netlink pid to current->pid is > *completely* bogus. Let me repeat this, the netlink pid is not > a process ID. Everyone is aware of that, actually these patches fix all occurences of current->pid by replacing them with a pid of 0. - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Wed, Aug 16, 2006 at 09:12:40PM +1000, herbert wrote:
>
> Any notification that sets the netlink pid to current->pid is
> *completely* bogus. Let me repeat this, the netlink pid is not
> a process ID.
BTW, I'm not having a go at either Thomas or Jamal. You guys
are oo the same side for once :).
I honestly believe that we have a misunderstanding here which needs
to be sorted out. It gets worse because that misunderstanding has
made it into the manpages package which only causes more confusion.
So let's step back a bit and think about where does this pid really
come from. The field in question is nlmsg_pid. Its primary purpose
is to identify unicast transactions along with the field nlmsg_seq.
It was not designed to identify the origin of a broadcast kernel
notification to a third party.
For this purpose, the value of nlmsg_pid is set to the address of
the destination socket for a particular unicast message (also known
as the pid).
That pid in turn has only a vague connection with the process ID
of the process owning the socket. For practical purposes, we
should not treat it as a process ID it can easily be claimed by
another process (think socket + bind + fork).
For a broadcast notification, the nlmsg_pid field is meaningless
because the nlmsg_seq field is also meaningless. I'm not denying
that it wouldn't be useful to have the originator's socket address
in there. What I'm saying is that it's the wrong place to put
that information.
In any case, putting current->pid in this field is definitely
a bad idea because it only encourages people to confuse the
netlink pid with the process ID which can lead to security
problems later on.
Cheers,
--
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.txt
-
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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
Hi Thomas:
On Wed, Aug 16, 2006 at 12:58:56PM +0200, Thomas Graf wrote:
>
> All route and tc notifications already use the pid so applications
> can decide whether the event was caused by them. A notification
> is a reply to a request so it doesn't even violate RFC 2367.
Actually most IPv4 notifications *do* set the pid to zero which is
the right thing to do for kernel-generated messages.
You're right though that the IPv6 notification modified by this patch
does set the pid to the netlink originator. Looking back in history
it seems that this behaviour was only introduced last year to a subset
of notifications.
This inconsistency is very bad. IMHO this change (made last year)
should be reverted so that all kernel generated (broadcast) notifications
have the originator set to zero to match the source address of the
message.
Any notification that sets the netlink pid to current->pid is
*completely* bogus. Let me repeat this, the netlink pid is not
a process ID.
Cheers,
--
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.txt
-
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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Wed, 2006-16-08 at 12:58 +0200, Thomas Graf wrote: > * Herbert Xu <[EMAIL PROTECTED]> 2006-08-16 12:58 > > I'm not comfortable with that change since it implies the message > > originated from a user-space process. > > > > The netlink header pid is really akin to sadb_msg_pid from RFC 2367. > > IMHO it should always be zero if the kernel is the originator of the > > message. > > All route and tc notifications already use the pid so applications > can decide whether the event was caused by them. A notification > is a reply to a request so it doesn't even violate RFC 2367. I would agree with Thomas on this. Regardless, I dont think that 2367 is really a glorified reference (that thing needs so much updating it is not funny). cheers, jamal - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
* Herbert Xu <[EMAIL PROTECTED]> 2006-08-16 12:58 > I'm not comfortable with that change since it implies the message > originated from a user-space process. > > The netlink header pid is really akin to sadb_msg_pid from RFC 2367. > IMHO it should always be zero if the kernel is the originator of the > message. All route and tc notifications already use the pid so applications can decide whether the event was caused by them. A notification is a reply to a request so it doesn't even violate RFC 2367. - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Wed, Aug 16, 2006 at 12:58:28PM +1000, Herbert Xu wrote:
>
> I'm not comfortable with that change since it implies the message
> originated from a user-space process.
>
> The netlink header pid is really akin to sadb_msg_pid from RFC 2367.
> IMHO it should always be zero if the kernel is the originator of the
> message.
Actually I think this has the potential to break applications that
ignore any messages with the original pid set to a non-zero value.
Cheers,
--
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.txt
-
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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
Thomas Graf <[EMAIL PROTECTED]> wrote:
>
> IPv4 address notifications originating from a netlink request now
> carry the netlink pid of the request instead of 0.
I'm not comfortable with that change since it implies the message
originated from a user-space process.
The netlink header pid is really akin to sadb_msg_pid from RFC 2367.
IMHO it should always be zero if the kernel is the originator of the
message.
Cheers,
--
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.txt
-
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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
* Hasso Tepper <[EMAIL PROTECTED]> 2006-08-15 15:08 > Thomas Graf wrote: > > However, I changed IPv4 addresses to provide the pid and support > > NLM_F_ECHO and the same will follow for IPv6 address notifications > > which will mean that quagga sees a different set of IPv4 address > > notifications. > > Can you explain what exactly is changed? IPv4 address notifications originating from a netlink request now carry the netlink pid of the request instead of 0. - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
Thomas Graf wrote: > However, I changed IPv4 addresses to provide the pid and support > NLM_F_ECHO and the same will follow for IPv6 address notifications > which will mean that quagga sees a different set of IPv4 address > notifications. Can you explain what exactly is changed? -- Hasso Tepper Elion Enterprises Ltd. [AS3249] Data Communication Network Administrator - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
* Alexey Kuznetsov <[EMAIL PROTECTED]> 2006-08-15 04:36 > > Some of these removals of current->pid will affect users such as quagga, > > zebra, vrrpd etc. > > If they survived cleanup in IPv4, they definitely will not feel cleanup > in IPv6. That was exactly my thought, the notifications in question provide a pid of 0 for their IPv4 counterparts therefore I considered it safe. However, I changed IPv4 addresses to provide the pid and support NLM_F_ECHO and the same will follow for IPv6 address notifications which will mean that quagga sees a different set of IPv4 address notifications. It would be a bug on their side if that would cause failure though. - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Tue, 2006-15-08 at 10:20 +0300, Hasso Tepper wrote: > Zebra, Quagga and Xorp treat the pid field in the netlink header as > unicast address of the netlink socket. As far as I know, that was the > original idea: > > http://marc.theaimsgroup.com/?l=linux-netdev&m=113943269432327&w=2 > > So, for Zebra, Quagga and Xorp users removals of current->pid mean just > closing potential races. You can't quarantee that ip/route/ifconfig will > not have the pid equal to the address of the netlink socket > Zebra/Quagga/Xorp owns. We've had this discussion before, havent we? ;-> Why do people like Alexey remember but i dont ;-> In any case if you are happy, I am happy too and i hope this settles it then ;-> cheers, jamal - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
jamal wrote: > CCing Hasso Tepper <[EMAIL PROTECTED]> > > Hasso, please comment on this thread (apologies, I forgot the other > guy's name; so if you can CC all the necessary suspects to comment, it > would help). I fully agree with Herbert Xu: > The pid field in the netlink header should be treated as an opaque > value. Any attempt to interpret it as the process ID is doomed to > failure. Zebra, Quagga and Xorp treat the pid field in the netlink header as unicast address of the netlink socket. As far as I know, that was the original idea: http://marc.theaimsgroup.com/?l=linux-netdev&m=113943269432327&w=2 So, for Zebra, Quagga and Xorp users removals of current->pid mean just closing potential races. You can't quarantee that ip/route/ifconfig will not have the pid equal to the address of the netlink socket Zebra/Quagga/Xorp owns. regards, -- Hasso - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Mon, 2006-14-08 at 22:46 -0400, jamal wrote: > On Tue, 2006-15-08 at 10:56 +1000, Herbert Xu wrote: > Thats all the quagga > and other folks were looking for in cases where it was ambigous. > Just to be clear, when Alexey says "that past discussion essentially closed the question." he is correct and i did not, hopefully, insuniate otherwise. The question i posed above is still valid though ;-> cheers, jamal - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Tue, 2006-15-08 at 10:56 +1000, Herbert Xu wrote:
> Agreed. The pid field in the netlink header should be treated as an
> opaque value. Any attempt to interpret it as the process ID is doomed
> to failure.
Not necessarily as a processid ("PID" is a really bad noun in that
sense); but rather as something meaningful of interpretation in regards
to the real origin of the executed change.
The concept of "whodunnit" is invaluable.
And a processid tends to be useful when nothing else is there to
identify the originator. Just saying "it is the kernel" (PID=0) when the
kernel just acted as a proxy of some user space app, is not useful all
the times, IMO.
[Routes, but not other functional blocks in rtnetlink, actually have a
field (called protocol) that says who added them]. Thats all the quagga
and other folks were looking for in cases where it was ambigous.
cheers,
jamal
-
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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
Alexey Kuznetsov <[EMAIL PROTECTED]> wrote:
>
>> IMO, I believe there is a strong case that can be made for events that
>> were caused by non-netlink users such as ioctls that could at least be
>> multicast with current->pid.
>
> Highly not desired.
Agreed. The pid field in the netlink header should be treated as an
opaque value. Any attempt to interpret it as the process ID is doomed
to failure.
Cheers,
--
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.txt
-
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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
Hello! > Some of these removals of current->pid will affect users such as quagga, > zebra, vrrpd etc. If they survived cleanup in IPv4, they definitely will not feel cleanup in IPv6. Thomas does great work, Jamal, do not worry. :-) > IMO, I believe there is a strong case that can be made for events that > were caused by non-netlink users such as ioctls that could at least be > multicast with current->pid. Highly not desired. > It would also be acceptable if the quagga etc folks could meet their > goals some other way. To all that I remember, that past discussion essentially closed the question. Alexey - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Tue, 2006-15-08 at 02:07 +0200, Thomas Graf wrote: > * jamal <[EMAIL PROTECTED]> 2006-08-14 19:43 > > If i am not mistaken: > > Some of these removals of current->pid will affect users such as quagga, > > zebra, vrrpd etc. If those specific users are not affected, please > > ignore the rest of my comments. > > I'm aware of that. The next patchset cleaning up fib6 and ifa6 will > provide means to pass on the required netlink information to these > notification functions. Good nuf. Thanks Thomas. cheers, jamal - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
* jamal <[EMAIL PROTECTED]> 2006-08-14 19:43 > On Mon, 2006-14-08 at 00:00 +0200, Thomas Graf wrote: > > plain text document attachment (rtnl_convert_ip6_addr) > > Fixes a wrong use of current->pid as netlink pid. > > If i am not mistaken: > Some of these removals of current->pid will affect users such as quagga, > zebra, vrrpd etc. If those specific users are not affected, please > ignore the rest of my comments. I'm aware of that. The next patchset cleaning up fib6 and ifa6 will provide means to pass on the required netlink information to these notification functions. - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
CCing Hasso Tepper <[EMAIL PROTECTED]> Hasso, please comment on this thread (apologies, I forgot the other guy's name; so if you can CC all the necessary suspects to comment, it would help). cheers, jamal On Mon, 2006-14-08 at 19:43 -0400, jamal wrote: > On Mon, 2006-14-08 at 00:00 +0200, Thomas Graf wrote: > > plain text document attachment (rtnl_convert_ip6_addr) > > Fixes a wrong use of current->pid as netlink pid. > > If i am not mistaken: > Some of these removals of current->pid will affect users such as quagga, > zebra, vrrpd etc. If those specific users are not affected, please > ignore the rest of my comments. > > I realize mr. Kuznetsov submitted at least one patch in the past to do > something similar to this. > IMO, I believe there is a strong case that can be made for events that > were caused by non-netlink users such as ioctls that could at least be > multicast with current->pid. In other words an exception case for only > this scenario. In such a case there is no unicast netlink message. > It would also be acceptable if the quagga etc folks could meet their > goals some other way. > > cheers, > jamal > > > - > 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 > - 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 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
On Mon, 2006-14-08 at 00:00 +0200, Thomas Graf wrote: > plain text document attachment (rtnl_convert_ip6_addr) > Fixes a wrong use of current->pid as netlink pid. If i am not mistaken: Some of these removals of current->pid will affect users such as quagga, zebra, vrrpd etc. If those specific users are not affected, please ignore the rest of my comments. I realize mr. Kuznetsov submitted at least one patch in the past to do something similar to this. IMO, I believe there is a strong case that can be made for events that were caused by non-netlink users such as ioctls that could at least be multicast with current->pid. In other words an exception case for only this scenario. In such a case there is no unicast netlink message. It would also be acceptable if the quagga etc folks could meet their goals some other way. cheers, jamal - 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
[PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()
Fixes a wrong use of current->pid as netlink pid.
Signed-off-by: Thomas Graf <[EMAIL PROTECTED]>
Index: net-2.6.19.git/net/ipv6/addrconf.c
===
--- net-2.6.19.git.orig/net/ipv6/addrconf.c
+++ net-2.6.19.git/net/ipv6/addrconf.c
@@ -73,6 +73,7 @@
#include
#include
#include
+#include
#include
#include
@@ -3280,20 +3281,23 @@ out_free:
static void inet6_ifa_notify(int event, struct inet6_ifaddr *ifa)
{
struct sk_buff *skb;
- int size = NLMSG_SPACE(sizeof(struct ifaddrmsg) +
INET6_IFADDR_RTA_SPACE);
+ int payload = sizeof(struct ifaddrmsg) + INET6_IFADDR_RTA_SPACE;
+ int err = -ENOBUFS;
- skb = alloc_skb(size, GFP_ATOMIC);
- if (!skb) {
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFADDR, ENOBUFS);
- return;
- }
- if (inet6_fill_ifaddr(skb, ifa, current->pid, 0, event, 0) < 0) {
+ skb = nlmsg_new(nlmsg_total_size(payload), GFP_ATOMIC);
+ if (skb == NULL)
+ goto errout;
+
+ err = inet6_fill_ifaddr(skb, ifa, 0, 0, event, 0);
+ if (err < 0) {
kfree_skb(skb);
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFADDR, EINVAL);
- return;
+ goto errout;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_IFADDR;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_IFADDR, GFP_ATOMIC);
+
+ err = rtnl_notify(skb, 0, RTNLGRP_IPV6_IFADDR, NULL, GFP_ATOMIC);
+errout:
+ if (err < 0)
+ rtnl_set_sk_err(RTNLGRP_IPV6_IFADDR, err);
}
static void inline ipv6_store_devconf(struct ipv6_devconf *cnf,
-
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
