[RFC] Extend netlink error codes

[RFC] Extend netlink error codes

[Thomas Graf]

The current situation with error codes sent back to a netlink
application is unsatisfactory. Most often, the application
receives EINVAL but has no idea what was wrong.

Here is my plan:

Introduce some macros:

#define NLERR_MAJ_MASK (0x7FFF0000U)
#define NLERR_MIN_MASK (0x0000FFFFU)
#define NLERR_MAJ(e) ((abs(e) & NLERR_MAJ_MASK) >> 16)
#define NLERR_MIN(e) (abs(e) & NLERR_MIN_MASK)
#define NLERR_MAKE(err, nle) ((((nle) << 16) & NLERR_MAJ_MASK)
		| ((err) & NLERR_MIN_MASK))

Specify netlink specific error codes, e.g.:

#define NLE_SUCCESS     0
#define NLE_NOQDISC     1
#define NLE_NOTCLASFUL  2
...

Predefined error messages could be provided by libnetlink
via nl_strerror().

Replace vague error return calls with something like:

return -NLERR_MAKE(EINVAL, NLE_NOQDISC);

Once in netlink_ack, split the combined error message again,
fill the existing errno into nlerrmsg.error and provide the
netlink specific error code in a newly introduced TLV. This
way, no binaries or old applications would break, but
applications can support it and provide more meanigful error
message with almost no effort.

Comments?

Re: [RFC] Extend netlink error codes

[David S. Miller]

On Sat, 11 Sep 2004 00:51:58 +0200
Thomas Graf <tgraf@suug.ch> wrote:

> Once in netlink_ack, split the combined error message again,
> fill the existing errno into nlerrmsg.error and provide the
> netlink specific error code in a newly introduced TLV. This
> way, no binaries or old applications would break, but
> applications can support it and provide more meanigful error
> message with almost no effort.

It sounds like it could work.

Jamal, any objections to his idea?

Re: [RFC] Extend netlink error codes

[jamal]

On Fri, 2004-09-10 at 19:07, David S. Miller wrote:
> On Sat, 11 Sep 2004 00:51:58 +0200
> Thomas Graf <tgraf@suug.ch> wrote:
> 
> > Once in netlink_ack, split the combined error message again,
> > fill the existing errno into nlerrmsg.error and provide the
> > netlink specific error code in a newly introduced TLV. This
> > way, no binaries or old applications would break, but
> > applications can support it and provide more meanigful error
> > message with almost no effort.
> 
> It sounds like it could work.
> 
> Jamal, any objections to his idea?

I think its a good idea.

so, questions:
classical ABI issues
a) old kernel vs new app
b) new kernel vs old app

i.e:
The use of existing error codes is still valuable. Existing apps will
do:
errno = msg->error
perror("some message");

and lastly:
c) how to make sure no conflicts ever with updates to errno.h in the
case of b)

one solution is to introduce a new flag NLM_F_NERR that signals kernel
that we have a new app. In such a case, a newer kernel gives a more
precise error code and sets that same flag; old kernels ignore such a
flag - and by not setting it newer apps understand to use old errno
scheme. In this case old apps should ignore the flag.

All this is assuming that previously unused flags followed principle of 
"Must be Zero on sending and ignore when receiving" principle for
reserved bits ;-> or as otherwise known as Postels rule these days "Be
liberal in what you accept and conservative in what you send"[1]

cheers,
jamal
 
[1] Jon Postel, RFC 1122

Re: [RFC] Extend netlink error codes

[Thomas Graf]

* jamal <1094866738.1042.53.camel@jzny.localdomain> 2004-09-10 21:38
> I think its a good idea.
> 
> so, questions:
> classical ABI issues
> a) old kernel vs new app
> b) new kernel vs old app

> and lastly:
> c) how to make sure no conflicts ever with updates to errno.h in the
> case of b)

Outdated error string table should result in a unknown error like
in strerror. 2^16 errno codes should be sufficient forever.

The only problem to be solved I see is where to specify the error
codes or how to distribute them between netlink users in kernel
context. Manage them as 512 blocks of 64 error codes and hand
them out to netlink users?

> one solution is to introduce a new flag NLM_F_NERR that signals kernel
> that we have a new app.

Another solution would be to add a new TLV to the ACK holding the
additional error code. However, I do like your idea better as it's
less work and easier for the userspace application.

Both, a) and b) are not problem in my eyes and can be easily solved:
  a) the user space application has to set NLM_F_ERR on all outgoing
     messages and check for the flag again in the NLMSG_ERROR handler and
     handle nlmsgerr::error differently.
  b) check nlh->nlmsg_flags & NLM_F_ERR in netlink_ack, 1-line change

Re: [RFC] Extend netlink error codes

[Andi Kleen]

On Sat, Sep 11, 2004 at 12:51:58AM +0200, Thomas Graf wrote:
> The current situation with error codes sent back to a netlink
> application is unsatisfactory. Most often, the application
> receives EINVAL but has no idea what was wrong.

[...]

IMHO it would be far better to just pass text errors
in a variable length packet back.  It's a bit plan9ish,
but it would work nicely here and be a bit improvement
(especially for the qdiscs) 

Otherwise you will end up with a mainteance nightmare of a
long list of error codes that needs to be updated for every new
subsystem.  And everybody who has a patch to add a new netlink
user would always fight with conflicts in this file.

I don't think an very specific error like
"CFQ subsystem parameter X is FOO, can be only upto BAR" 
can be nicely put into a global error file.
And most errors can only be handled by passing them to the user anyways,
text errors would be fine for that.

There could be still an errno provided for easy success/failure
checking.

-Andi

Re: [RFC] Extend netlink error codes

[Thomas Graf]

* Andi Kleen <20040911155839.GN4431@wotan.suse.de> 2004-09-11 17:58
> IMHO it would be far better to just pass text errors
> in a variable length packet back.  It's a bit plan9ish,
> but it would work nicely here and be a bit improvement
> (especially for the qdiscs) 

I had the same idea and the only good way to do so would
be to add a char * errbuf or alike to struct netlink_opt
and access it via skb.sk.sk_protinfo.

The bad side is that the for example cls and sch api don't
provide the skb to the implementing modules and therefore
they can't access the error buffer. I don't want to change
all netlink users because of this.

Changing all paths back to netlink_ack to provide a struct
containing the errno and an additional text error buffer is
no option for me either.

Correct me if I'm wrong.

> Otherwise you will end up with a mainteance nightmare of a
> long list of error codes that needs to be updated for every new
> subsystem.

I would suggst to split them up and assign blocks of free codes
to subsystems.

> And everybody who has a patch to add a new netlink
> user would always fight with conflicts in this file.

This is indeed a problem.

> I don't think an very specific error like
> "CFQ subsystem parameter X is FOO, can be only upto BAR" 
> can be nicely put into a global error file.

True, I would really like to have such error messages.

Re: [RFC] Extend netlink error codes

[Andi Kleen]

On Sat, Sep 11, 2004 at 06:24:33PM +0200, Thomas Graf wrote:
> * Andi Kleen <20040911155839.GN4431@wotan.suse.de> 2004-09-11 17:58
> > IMHO it would be far better to just pass text errors
> > in a variable length packet back.  It's a bit plan9ish,
> > but it would work nicely here and be a bit improvement
> > (especially for the qdiscs) 
> 
> I had the same idea and the only good way to do so would
> be to add a char * errbuf or alike to struct netlink_opt
> and access it via skb.sk.sk_protinfo.

I was thinking about a variable length netlink packet
that contains it and that is sent back to the application.  
No need to store it anywhere else.
> 
> The bad side is that the for example cls and sch api don't
> provide the skb to the implementing modules and therefore
> they can't access the error buffer. I don't want to change
> all netlink users because of this.

Indeed, that's an issue. Even with the retry packet you need
the sequence number. On the other hand there are not 
that many sched/cls modules and some minor changes to them
are probably ok.

In the worst case you can define new entry points and
keep the old ones for compatibility, but it is probably
not worth it to be that compatible because there are not
enough different users.

-Andi

Re: [RFC] Extend netlink error codes

[Thomas Graf]

* Andi Kleen <20040911165056.GQ4431@wotan.suse.de> 2004-09-11 18:50
> I was thinking about a variable length netlink packet
> that contains it and that is sent back to the application.  

All solutions discussed so far are based on sending a netlink
message back to the application. Changing the existing
nlmsgerr.error or adding a TLV with an error message doesn't
make much difference from an implementors view.

> No need to store it anywhere else.

Not storing it would mean to call netlink_ack from everywhere
which requires knowledge of the original netlink skb and the
original netlink message in order to get the pid and sequence
number right. (We can't use the pid assigned to the peer's
sock.) It would also mean to set a bit to avoid sending out
the ack again. Wrong?

> On the other hand there are not 
> that many sched/cls modules and some minor changes to them
> are probably ok.

True, but also think of xfrm, tcp_diag, netfilter, dnrmg. I haven't
checked how much work it would actually be to improve better
error messages for them but I plan to at least extend all
error codes related to the data passed via a netlink socket.

Will wait for some comments. I guess I would be swamped with
something like discussed above.

Re: [RFC] Extend netlink error codes

[Sam Leffler]

On Sep 11, 2004, at 9:24 AM, Thomas Graf wrote:

> * Andi Kleen <20040911155839.GN4431@wotan.suse.de> 2004-09-11 17:58
>> IMHO it would be far better to just pass text errors
>> in a variable length packet back.  It's a bit plan9ish,
>> but it would work nicely here and be a bit improvement
>> (especially for the qdiscs)
>
> I had the same idea and the only good way to do so would
> be to add a char * errbuf or alike to struct netlink_opt
> and access it via skb.sk.sk_protinfo.
>
> The bad side is that the for example cls and sch api don't
> provide the skb to the implementing modules and therefore
> they can't access the error buffer. I don't want to change
> all netlink users because of this.
>
> Changing all paths back to netlink_ack to provide a struct
> containing the errno and an additional text error buffer is
> no option for me either.
>
> Correct me if I'm wrong.
>
>> Otherwise you will end up with a mainteance nightmare of a
>> long list of error codes that needs to be updated for every new
>> subsystem.
>
> I would suggst to split them up and assign blocks of free codes
> to subsystems.
>
>> And everybody who has a patch to add a new netlink
>> user would always fight with conflicts in this file.
>
> This is indeed a problem.
>
>> I don't think an very specific error like
>> "CFQ subsystem parameter X is FOO, can be only upto BAR"
>> can be nicely put into a global error file.
>
> True, I would really like to have such error messages.

A technique used for mbuf packet tags in FreeBSD is to define a 32-bit 
cookie that uniquely identifies a module or ABI code and concatenate 
this with the 16-bit tag number.  The ABI code is defined as the 
date+time that the module is created, expressed as the number of 
seconds since the epoch (e.g. the output of date -u +'%s').  Tag 
numbers are meaningful only within the context of the module/ABI.  A 
default/compatibility cookie is used for compatibility with previously 
defined tag values.  This scheme effectively distributes management of 
tags (so far there have been no collisions).

Check the comments in sys/sys/mbuf.h on FreeBSD for more details.

	Sam

Re: [RFC] Extend netlink error codes

[jamal]

On Sat, 2004-09-11 at 14:48, Sam Leffler wrote:
> On Sep 11, 2004, at 9:24 AM, Thomas Graf wrote:
> 
> > * Andi Kleen <20040911155839.GN4431@wotan.suse.de> 2004-09-11 17:58
> >> IMHO it would be far better to just pass text errors
> >> in a variable length packet back.  It's a bit plan9ish,
> >> but it would work nicely here and be a bit improvement
> >> (especially for the qdiscs)
> >
> > I had the same idea and the only good way to do so would
> > be to add a char * errbuf or alike to struct netlink_opt
> > and access it via skb.sk.sk_protinfo.
> >
> > The bad side is that the for example cls and sch api don't
> > provide the skb to the implementing modules and therefore
> > they can't access the error buffer. I don't want to change
> > all netlink users because of this.
> >
> > Changing all paths back to netlink_ack to provide a struct
> > containing the errno and an additional text error buffer is
> > no option for me either.
> >
> > Correct me if I'm wrong.
> >

just reuse skb->cb[] and intepret it in rtnetlink in the case of
failures.

> A technique used for mbuf packet tags in FreeBSD is to define a 32-bit 
> cookie that uniquely identifies a module or ABI code and concatenate 
> this with the 16-bit tag number.  The ABI code is defined as the 
> date+time that the module is created, expressed as the number of 
> seconds since the epoch (e.g. the output of date -u +'%s').  Tag 
> numbers are meaningful only within the context of the module/ABI.  A 
> default/compatibility cookie is used for compatibility with previously 
> defined tag values.  This scheme effectively distributes management of 
> tags (so far there have been no collisions).

This sounds similar to the skb tags of different sorts that exist.
It does however bring out a good point into the discussion.
I think we need to identify errors by IDs - dont think we can avoid
that. The IDs could be the T in TLV; their scope is per socket level (eg
at rtnetlink); This means there are 16 bits space per socket type.
The next level to addressing is at each submodule level eg qdisc,
ipv4route, etc. Assuming we have 8 bits there that leaves upto 8 bits
per submodule for different error types. 256 error codes per submodule
liek a qdisc sounds very reasonable to me. We could reserve the last
entry for future extensions. Example:

socket: rtnetlink
submodule: qdisc (module 1)
error id: Q_NASTYTHING which is 0x10
qdisc_errors[Q_NASTYTHING] is "something really nasty just happened"

Then T = 0x110
L length(qdisc_errors[Q_NASTYTHING])
V = qdisc_errors[Q_NASTYTHING]

BTW, there was someone from IBM a while back who was talking about
having drivers send error messages via netlink; someone needs to look at
the ideas they have maybe we can borrow something.


cheers,
jamal

Re: [RFC] Extend netlink error codes

[David S. Miller]

On Sat, 11 Sep 2004 18:24:33 +0200
Thomas Graf <tgraf@suug.ch> wrote:

> > I don't think an very specific error like
> > "CFQ subsystem parameter X is FOO, can be only upto BAR" 
> > can be nicely put into a global error file.
> 
> True, I would really like to have such error messages.

I'd love to have them too.

But I know internationalization folks are going to
scream, although it will obviously be better than
no messages at all.

Re: [RFC] Extend netlink error codes

[Andi Kleen]

On Sun, Sep 12, 2004 at 04:37:44PM -0700, David S. Miller wrote:
> On Sat, 11 Sep 2004 18:24:33 +0200
> Thomas Graf <tgraf@suug.ch> wrote:
> 
> > > I don't think an very specific error like
> > > "CFQ subsystem parameter X is FOO, can be only upto BAR" 
> > > can be nicely put into a global error file.
> > 
> > True, I would really like to have such error messages.
> 
> I'd love to have them too.
> 
> But I know internationalization folks are going to
> scream, although it will obviously be better than
> no messages at all.

I actually don't think it will be a big issue for them.
They already translate messages by mapping text to other
text. The same thing could be done for kernel messages,
by just marking them in the kernel somehow that they
can be automatically extracted from the source and then 
later mapping them to the translations in user space. 

But I wouldn't do it until someone actually complains
about it.

-Andi

Re: [RFC] Extend netlink error codes

[David S. Miller]

On Mon, 13 Sep 2004 08:26:13 +0200
Andi Kleen <ak@suse.de> wrote:

> I actually don't think it will be a big issue for them.
> They already translate messages by mapping text to other
> text. The same thing could be done for kernel messages,
> by just marking them in the kernel somehow that they
> can be automatically extracted from the source and then 
> later mapping them to the translations in user space. 

I understand, but I'm specifically talking about this
"synchronization" issue.

Anyways, I think strings are fine.

Re: [RFC] Extend netlink error codes

[Thomas Graf]

* jamal <1094937035.2344.189.camel@jzny.localdomain> 2004-09-11 17:10
> On Sat, 2004-09-11 at 14:48, Sam Leffler wrote:
> just reuse skb->cb[] and intepret it in rtnetlink in the case of
> failures.

Good idea, however it means to change a lot of APIs used by modules
to provide the skb or a pointer to the error string buffer.

I still think it would be best to transport the error code via
the return value in order to not change any static APIs, but
I guess that 32bit is not enough to store errno and the id
your propose below.

> I think we need to identify errors by IDs - dont think we can avoid
> that. The IDs could be the T in TLV; their scope is per socket level (eg
> at rtnetlink); This means there are 16 bits space per socket type.
> The next level to addressing is at each submodule level eg qdisc,
> ipv4route, etc. Assuming we have 8 bits there that leaves upto 8 bits
> per submodule for different error types. 256 error codes per submodule
> liek a qdisc sounds very reasonable to me. We could reserve the last
> entry for future extensions. Example:
> 
> socket: rtnetlink
> submodule: qdisc (module 1)
> error id: Q_NASTYTHING which is 0x10
> qdisc_errors[Q_NASTYTHING] is "something really nasty just happened"

Sounds reasonable to me.

> BTW, there was someone from IBM a while back who was talking about
> having drivers send error messages via netlink; someone needs to look at
> the ideas they have maybe we can borrow something.

http://lwn.net/Articles/39164/

I read trough the patch and what they basically provide is a way to send
data to a netlink socket. The data can be collected via an own netlink
socket by subscribing to it or use the user space daemon.

They provide stuff like:

int
evl_printf(const char *facility, int event_type, int severity,
	const char *fmt, ...)

usage could be something like:

evl_printf("qdisc", Q_NASTYTHING, ...

We could use it this way:
- Leave the existing error system as it is
- Make the netlink users send out error messages on their own.
  pid and sequence number of the original netlink message could be
  provided.
- User space applications could fetch error messages from that socket
  and assign them to their own actions by sequence number and pid.

Pros:
 - Almost no changes to the APIs
 - Easy to implement
 - No problem with distribution of error codes
 - Very easy to implement formatted error strings.

Cons:
 - More work for user space
 - How does user space know if there will be an extended error?
   If unknown, how long to wait?
 - Should we allow sending of errors to this socket while no
   netlink message is being processed?

Idea: Make user space applications poll on the socket and
just print the errors async. This would allow netlink users to
use it for more than just error handling.

Re: [RFC] Extend netlink error codes

[David S. Miller]

On Mon, 13 Sep 2004 22:36:57 +0200
Thomas Graf <tgraf@suug.ch> wrote:

> > BTW, there was someone from IBM a while back who was talking about
> > having drivers send error messages via netlink; someone needs to look at
> > the ideas they have maybe we can borrow something.
> 
> http://lwn.net/Articles/39164/

I think since the inclusion of IBM's work is iffy at best,
we should architect our facility without assuming it's presence.

Re: [RFC] Extend netlink error codes

[jamal]

On Mon, 2004-09-13 at 16:36, Thomas Graf wrote:
> * jamal <1094937035.2344.189.camel@jzny.localdomain> 2004-09-11 17:10
> > just reuse skb->cb[] and intepret it in rtnetlink in the case of
> > failures.
> 
> Good idea, however it means to change a lot of APIs used by modules
> to provide the skb or a pointer to the error string buffer.
> 
> I still think it would be best to transport the error code via
> the return value in order to not change any static APIs, but
> I guess that 32bit is not enough to store errno and the id
> your propose below.

The problem with using the return codes is you have to know somehow all
the error variants within netlink code itself. If you return a TLV
skb->cb[] then the netlink code could blindly forward it to user space.
I dont think theres a lot of changes to APIs. the calls from netlink
already pass the skb to all modules. The dumps are speacial since the
->cb[] is actually used.
Of course you could do it with return codes, probably do some smart
things in the netlink code without making it too knowledgeable
but you may need to have a global table.

> Idea: Make user space applications poll on the socket and
> just print the errors async. This would allow netlink users to
> use it for more than just error handling.

I think i agree with what  Dave says in his other email - we should
probably just borrow any useful ideas they have wirthout complicating
things on our part. It does seem too tangential after reading it.

cheers,
jamal