[PATCH RESUBMIT2 net-next 1/2] IPv6 routing, NLM_F_* flag support: warn if NEWROUTE lacks of both CREATE and REPLACE flags

[PATCH RESUBMIT2 net-next 1/2] IPv6 routing, NLM_F_* flag support: warn if NEWROUTE lacks of both CREATE and REPLACE flags

[Matti Vaittinen]

The support for NLM_F_* flags at IPv6 routing requests.

If RTM_NEWROUTE request, has neither NLM_F_CREATE, nor NLM_F_REPLACE
specified, a warning is printed but no error is returned.

Patch created against linux-3.2-rc1



Signed-off-by: Matti Vaittinen <Mazziesaccount@gmail.com>
--
diff -uNr Linux-3.2-rc1.orig/net/ipv6/route.c Linux-3.2-rc1.new/net/ipv6/route.c
--- Linux-3.2-rc1.orig/net/ipv6/route.c	2011-11-10 08:44:18.000000000 +0200
+++ Linux-3.2-rc1.new/net/ipv6/route.c	2011-11-10 08:46:15.000000000 +0200
@@ -1230,9 +1230,18 @@
 	if (cfg->fc_metric == 0)
 		cfg->fc_metric = IP6_RT_PRIO_USER;
 
-	table = fib6_new_table(net, cfg->fc_table);
+	err = -ENOBUFS;
+	if (NULL != cfg->fc_nlinfo.nlh &&
+	    !(cfg->fc_nlinfo.nlh->nlmsg_flags&NLM_F_CREATE)) {
+		table = fib6_get_table(net, cfg->fc_table);
+		if (table == NULL) {
+			printk(KERN_WARNING "NLM_F_CREATE should be specified when creating new rt\n");
+			table = fib6_new_table(net, cfg->fc_table);
+		}
+	} else {
+		table = fib6_new_table(net, cfg->fc_table);
+	}
 	if (table == NULL) {
-		err = -ENOBUFS;
 		goto out;
 	}
 

Re: [PATCH RESUBMIT2 net-next 1/2] IPv6 routing, NLM_F_* flag support: warn if NEWROUTE lacks of both CREATE and REPLACE flags

[David Miller]

These changes still has problems.

These warning messages are poorly formatted.  Some of them don't even indicate
that ipv6 is involved by printing out an appropriate subsystem prefix.

And frankly, they are ugly, and not the kind of thing I want the networking
code spitting out.

Consolidate these messages into something that has good form and will indicate,
consistently, where the trouble is occuring.

And I still submit that perhaps these ugly messages are an indicate of how
bad an idea these changes are in the first place.  We've lived with the
current behavior for 15 years or more, nobody cared.  What changed?
What can't you do with the current setup?  And why can't you do it without
requiring operations that work currently to change semantics?

Re: [PATCH RESUBMIT2 net-next 1/2] IPv6 routing, NLM_F_* flag support: warn if NEWROUTE lacks of both CREATE and REPLACE flags

[Matti Vaittinen]

On Mon, 2011-11-14 at 02:06 -0500, ext David Miller wrote:
> These changes still has problems.
> 
> These warning messages are poorly formatted.  Some of them don't even indicate
> that ipv6 is involved by printing out an appropriate subsystem prefix.
> 
> And frankly, they are ugly, and not the kind of thing I want the networking
> code spitting out.
> 
> Consolidate these messages into something that has good form and will indicate,
> consistently, where the trouble is occuring.

Messages can be changed, that's no problem. I would like to hear suggestions 
because I personally see not much problems in current messages. 
(Of course printing out the subsystem is obvious improvement now that you 
mentioned it). 
It is hard for me to think something better that would still be short
warning telling what's wrong.

> And I still submit that perhaps these ugly messages are an indicate of how
> bad an idea these changes are in the first place.  We've lived with the
> current behavior for 15 years or more, nobody cared.  What changed?

Need for IPv6 is what changed for me. I've lived past 15 years purely with IPv4. 
I expect this change to be true for more users in the future - although I
cant say I expect IPv6 to be be suddenly utilized everywhere.

> What can't you do with the current setup?  And why can't you do it without
> requiring operations that work currently to change semantics?

Short answers: I cant just easily replace a route. And I cannot maintain
old way if I want to respect netlink specifications.

Longer story:

Why I started writing this patch is that I had unpleasant surprizes when I 
tried using netlink for IPv6 route manipulations first time. I noticed that 
IPv6 does not use netlink API as user (I) could expect. 

1. Replacing a route surprised me. I did not expect that request with no 
CREATE flag would create a new route. That's what happens. Even with
NLM_F_REPLACE

2. Also it was not possible to replace a route in single netlink message. 

What got me even more puzzled is, that no warning, no error, no nothing was
returned to me when I set NLM_F_REPLACE flag. I would have expected -ENOTSUP
if there's no support. Or -EINVAL. I had no idea that flag was not supported.
I addmitt it was propably stupid to expect IPv6 to work in same way as IPv4
does. However I used netlink for IPv4 too, so I assumed whatI tried with IPv6
 was correct. Finally I had to look at the kernel sources to find out what 
happens. I believe that prints (even ugly ones) are better than making 
averaǵe Joe like me to search for the answers from kernel sources.

Anyways, what really improves usability is support for REPLACE flag.
Instead of issuing REPLACE request user now needs to:

1. issue GET request to see if there is routes to given destination
2. explisitly delete the matching route
3. create new route and
4. keep on eye the changes done by other processes.

That is whole a lot more work for user, and that requires route manipulation
tools to do different handling for IPv4 and IPv6 routes.

I believe respecting all flags according to netlink specifications would be
what new users expect. Thats why my patch also handles NLM_F_CREATE and 
NLM_F_EXCL flags.

However, I think that in usability point of view the support for 
NLM_F_REPLACE  would be sufficient. Rest is more to fullfill the 
specification.

[siedenote: And if we further ponder the NLM_F_* flags that would
improve usability, then NLM_F_MATCH with GET requests is what I would
like. (Returning only routes matching values given in request). 
However that's not really supported by IPv4 either, so not having it in IPv6 
is not really a priority.]

Anyways, this is the discussion I hoped for before I wrote the patches. 
And in my first post to this list I asked if the support for these flags 
was left out in purpose. So please let me know, if you think these changes 
are unnecessary and wont be included in any case. Then I can just give up 
polluting your list with mails and patches, and stop wasting your and my 
time. However if you think these changes are worthy, then no problem, 
I can do required fixups to these patches. I just would like to know
what changes are acceptable / wanted.


-- 
Matti Vaittinen
+358 504863070
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Told a UDP joke the other night...
...but I'm not sure everyone got it...
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Re: [PATCH RESUBMIT2 net-next 1/2] IPv6 routing, NLM_F_* flag support: warn if NEWROUTE lacks of both CREATE and REPLACE flags

[David Miller]

From: Matti Vaittinen <matti.vaittinen@nsn.com>
Date: Mon, 14 Nov 2011 10:31:20 +0200

> Messages can be changed, that's no problem. I would like to hear suggestions 
> because I personally see not much problems in current messages. 

You don't even say that the route request is for an ipv6 route.

How in the world can we track down the application if you don't
even say what protocol the route request is for?

Re: [PATCH RESUBMIT2 net-next 1/2] IPv6 routing, NLM_F_* flag support: warn if NEWROUTE lacks of both CREATE and REPLACE flags

[Matti Vaittinen]

On Mon, 2011-11-14 at 03:36 -0500, ext David Miller wrote:
> From: Matti Vaittinen <matti.vaittinen@nsn.com>
> Date: Mon, 14 Nov 2011 10:31:20 +0200
> 
> > Messages can be changed, that's no problem. I would like to hear suggestions 
> > because I personally see not much problems in current messages. 
> 
> You don't even say that the route request is for an ipv6 route.

I'll add the IPv6 in prints, and also look through the ŕest of the prints 
in 2/2 patch && see if I can think of something to improve.

Also, this patch (1/2) has wrong subject and explanation - total brainfart
from my side, sorry. The check for NLM_F_REPLACE is not in this, but in 
the 2/2 patch. I'll correct subject and explanation too.

-- 
Matti Vaittinen
+358 504863070
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Told a UDP joke the other night...
...but I'm not sure everyone got it...
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~