| Message ID | 1321254611.23935.15.camel@hakki |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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; }
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> -- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html