Bug 8755 - "ip -6 route change " behaves like "ip -6 route add"
Summary: "ip -6 route change " behaves like "ip -6 route add"
Status: RESOLVED OBSOLETE
Alias: None
Product: Networking
Classification: Unclassified
Component: IPV6 (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Hideaki YOSHIFUJI
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-07-14 15:26 UTC by Simon Arlott
Modified: 2012-09-19 11:51 UTC (History)
3 users (show)

See Also:
Kernel Version: 2.6.20, 2.6.22
Subsystem:
Regression: No
Bisected commit-id:


Attachments
_untested_ patch to make ipv6 correctly honor NLM_F_EXCL and NLM_F_REPLACE flags (2.58 KB, patch)
2009-01-09 11:57 UTC, Neil Horman
Details | Diff
Another propsoed patch based on previous patch (1.94 KB, patch)
2009-01-27 07:50 UTC, Olivier Fourdan
Details | Diff

Description Simon Arlott 2007-07-14 15:26:19 UTC
The "ip -6 route change" command behaves as though "ip -6 route add" was used. The source of iproute2 shows that "change" uses the "NLM_F_REPLACE" netlink flag only.

ip utility, iproute2-ss070313

Changing an existing route:
# ip -6 r show 2002::/16
2002::/16 dev sit0  metric 1024  expires 4482618sec mtu 1480 advmss 7140 hoplimit 4294967295
# ip -6 r change 2002::/16 dev sit0 mtu 1280
RTNETLINK answers: File exists

Adding a route using "change":
# ip -6 r change 2002::/17 dev sit0 mtu 1280
# ip -6 r show 2002::/17
2002::/17 dev sit0  metric 1024  expires 21334368sec mtu 1280 advmss 1220 hoplimit 4294967295
Comment 1 Patrick McHardy 2007-07-16 12:21:07 UTC
Subject: Re: [Bugme-new]  New: "ip -6 route change " behaves like
 "ip -6 route add"

Simon Arlott wrote:
> On 15/07/07 16:07, Patrick McHardy wrote:
> 
>>>>Changing an existing route:
>>>># ip -6 r show 2002::/16
>>>>2002::/16 dev sit0  metric 1024  expires 4482618sec mtu 1480 advmss 7140
>>>>hoplimit 4294967295
>>>># ip -6 r change 2002::/16 dev sit0 mtu 1280
>>>>RTNETLINK answers: File exists
> 
> ^ This is clearly a bug, since I'm trying to change an existing route
> and it gives an error as if it tried to add it.


Agreed.

>>>>Adding a route using "change":
>>>># ip -6 r change 2002::/17 dev sit0 mtu 1280
>>>># ip -6 r show 2002::/17
>>>>2002::/17 dev sit0  metric 1024  expires 21334368sec mtu 1280 advmss 1220
>>>>hoplimit 4294967295
> 
> ^ This shouldn't be possible!


That one will probably be impossible to fix since IPv6 has never checked
for NLM_F_EXCL, doing to now might break things.

>>There is a difference between "add" and "change". "add" will only add a
>>new address if there isn't already one with the same identity, "change"
>>will allow to change attributes of an existing address, like flags,
>>lifetime, ..., or behave similar to "add" in case the address doesn't
>>already exist.
>>
>>What kind of behaviour are you expecting?
> 
> 
> That "change" would actually change not simply add. It won't let me
> change anything and in fact can only add instead.


The code looks like it would support it properly. Please add a
few printks to inet6_rtm_newaddr to find out what goes wrong.

> Compare it to ipv4 where "change" never adds - "replace" is "change, or
> add". (Also, "replace" doesn't work for v6 either).


IPv4 doesn't check any netlink flags, so I don't think that is correct.
Comment 2 Patrick McHardy 2007-07-16 12:37:44 UTC
Subject: Re: [Bugme-new]  New: "ip -6 route change " behaves like
 "ip -6 route add"

Andrew Morton wrote:
> On Sat, 14 Jul 2007 15:21:21 -0700 (PDT) bugme-daemon@bugzilla.kernel.org
> wrote:
> 
>>The "ip -6 route change" command behaves as though "ip -6 route add" was
>>used.
>>The source of iproute2 shows that "change" uses the "NLM_F_REPLACE" netlink
>>flag only.
>>
>>ip utility, iproute2-ss070313
>>
>>Changing an existing route:
>># ip -6 r show 2002::/16
>>2002::/16 dev sit0  metric 1024  expires 4482618sec mtu 1480 advmss 7140
>>hoplimit 4294967295
>># ip -6 r change 2002::/16 dev sit0 mtu 1280
>>RTNETLINK answers: File exists
>>
>>Adding a route using "change":
>># ip -6 r change 2002::/17 dev sit0 mtu 1280
>># ip -6 r show 2002::/17
>>2002::/17 dev sit0  metric 1024  expires 21334368sec mtu 1280 advmss 1220
>>hoplimit 4294967295


There is a difference between "add" and "change". "add" will only add a
new address if there isn't already one with the same identity, "change"
will allow to change attributes of an existing address, like flags,
lifetime, ..., or behave similar to "add" in case the address doesn't
already exist.

What kind of behaviour are you expecting?
Comment 3 Anonymous Emailer 2007-07-19 11:35:27 UTC
Reply-To: simon@fire.lp0.eu

On 16/07/07 14:01, Patrick McHardy wrote:
> Simon Arlott wrote:
>>>>>Changing an existing route:
>>>>># ip -6 r show 2002::/16
>>>>>2002::/16 dev sit0  metric 1024  expires 4482618sec mtu 1480 advmss 7140
>>>>>hoplimit 4294967295
>>>>># ip -6 r change 2002::/16 dev sit0 mtu 1280
>>>>>RTNETLINK answers: File exists

> The code looks like it would support it properly. Please add a
> few printks to inet6_rtm_newaddr to find out what goes wrong.

Erm. It's routing configuration that's broken not address configuration
(ip doesn't even . Looking at route.c through to ip6_fib.c, it doesn't
support replacing routes at all.

$ grep NLM_F_ ipv4/fib_*.c ipv4/route.c ipv6/fib6_* ipv6/ip6_fib.c
ipv6/route.c

ipv4/fib_frontend.c:            cfg->fc_nlflags = NLM_F_CREATE;
ipv4/fib_frontend.c:            .fc_nlflags = NLM_F_CREATE | NLM_F_APPEND,
ipv4/fib_hash.c:                if (cfg->fc_nlflags & NLM_F_EXCL)
ipv4/fib_hash.c:                if (cfg->fc_nlflags & NLM_F_REPLACE) {
ipv4/fib_hash.c:                                  &cfg->fc_nlinfo,
NLM_F_REPLACE);
ipv4/fib_hash.c:                if (!(cfg->fc_nlflags & NLM_F_APPEND))
ipv4/fib_hash.c:        if (!(cfg->fc_nlflags & NLM_F_CREATE))
ipv4/fib_hash.c:                                          NLM_F_MULTI) <
0) {
ipv4/fib_trie.c:                if (cfg->fc_nlflags & NLM_F_EXCL)
ipv4/fib_trie.c:                if (cfg->fc_nlflags & NLM_F_REPLACE) {
ipv4/fib_trie.c:                                tb->tb_id,
&cfg->fc_nlinfo, NLM_F_REPLACE);
ipv4/fib_trie.c:                if (!(cfg->fc_nlflags & NLM_F_APPEND))
ipv4/fib_trie.c:        if (!(cfg->fc_nlflags & NLM_F_CREATE))
ipv4/route.c:                                    1, NLM_F_MULTI) <= 0) {

ipv6/route.c:                prefix, NLM_F_MULTI);
Comment 4 Neil Horman 2009-01-09 11:57:03 UTC
Created attachment 19734 [details]
_untested_ patch to make ipv6 correctly honor NLM_F_EXCL and NLM_F_REPLACE flags

I was poking about in this code today, and Patrick, I think your assertion that ipv4 doesn't check netlink flags is incorrect.  NLM_F_EXCL is checked in both fn_hash_insert and fn_trie_insert.  I think we probably need to do something simmilar in the ipv6 path.  This is completely untested, and theres probably a much more efficient way to do this, but off the top of my head I wrote this and I think it might be an option to solve the issue
Comment 5 Olivier Fourdan 2009-01-27 07:50:36 UTC
Created attachment 20009 [details]
Another propsoed patch based on previous patch

I built another patch based on Neil's patch, with some difference.

Instead of using ip6_route_del(), it adds a new function ip6_route_excl() that will search for the existing route, quite the same way ip6_route_del() does, and then either return -EEXIST if NLM_F_EXCL is set or call __ip6_del_rt() directly.

That function is called from inet6_rtm_newroute() only when either NLM_F_EXCL or NLM_F_REPLACE is set. 

I see two advantages to this approach:

 - It avoids searching twice for the route
 - It does not check for the metric like ip6_route_del() does
 
The last point is needed to be able to change the metric of an existing route (otherwise the metric is taken into account in the search and the route is added instead of being changed).

# ip -6 route add 2002::/16 dev eth0 metric 1024 mtu 1480
# ip -6 r show 2002::/16
2002::/16 dev eth0  metric 1024  mtu 1480 advmss 1420 hoplimit 4294967295
# ip -6 route change 2002::/16 dev eth0 metric 100
# ip -6 r show 2002::/16
2002::/16 dev eth0  metric 100  mtu 1492 advmss 1432 hoplimit 4294967295

Other example changing the mtu:

# ip -6 r show 2002::/16
# ip -6 route add 2002::/16 dev eth0 metric 1024 mtu 1480
# ip -6 r show 2002::/16
2002::/16 dev eth0  metric 1024  mtu 1480 advmss 1420 hoplimit 4294967295
# ip -6 route change 2002::/16 dev eth0 mtu 1280
# ip -6 r show 2002::/16
2002::/16 dev eth0  metric 1024  mtu 1280 advmss 1220 hoplimit 4294967295
Comment 6 Neil Horman 2009-01-27 12:46:24 UTC
ACK, I prefer ollivers solution to mine.

Note You need to log in before you can comment on or make changes to this bug.