netdev - Re: [PATCH net-next] ipv6: do not merge differe type and protocol routes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <01baf374-97c0-2a6f-db85-078488795bf9@kernel.org>
Date: Wed, 30 Aug 2023 12:57:28 -0600
From: David Ahern <dsahern@...nel.org>
To: nicolas.dichtel@...nd.com, Hangbin Liu <liuhangbin@...il.com>,
 netdev@...r.kernel.org
Cc: "David S . Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Ido Schimmel <idosch@...sch.org>,
 Thomas Haller <thaller@...hat.com>
Subject: Re: [PATCH net-next] ipv6: do not merge differe type and protocol
 routes

On 8/30/23 9:29 AM, Nicolas Dichtel wrote:
> Le 30/08/2023 à 08:15, Hangbin Liu a écrit :
>> Different with IPv4, IPv6 will auto merge the same metric routes into
>> multipath routes. But the different type and protocol routes are also
>> merged, which will lost user's configure info. e.g.
>>
>> + ip route add local 2001:db8:103::/64 via 2001:db8:101::10 dev dummy1 table 100
>> + ip route append unicast 2001:db8:103::/64 via 2001:db8:101::10 dev dummy2 table 100
>> + ip -6 route show table 100
>> local 2001:db8:103::/64 metric 1024 pref medium
>>         nexthop via 2001:db8:101::10 dev dummy1 weight 1
>>         nexthop via 2001:db8:101::10 dev dummy2 weight 1
>>
>> + ip route add 2001:db8:104::/64 via 2001:db8:101::10 dev dummy1 proto kernel table 200
>> + ip route append 2001:db8:104::/64 via 2001:db8:101::10 dev dummy2 proto bgp table 200
>> + ip -6 route show table 200
>> 2001:db8:104::/64 proto kernel metric 1024 pref medium
>>         nexthop via 2001:db8:101::10 dev dummy1 weight 1
>>         nexthop via 2001:db8:101::10 dev dummy2 weight 1
>>
>> So let's skip counting the different type and protocol routes as siblings.
>> After update, the different type/protocol routes will not be merged.
>>
>> + ip -6 route show table 100
>> local 2001:db8:103::/64 via 2001:db8:101::10 dev dummy1 metric 1024 pref medium
>> 2001:db8:103::/64 via 2001:db8:101::10 dev dummy2 metric 1024 pref medium
>>
>> + ip -6 route show table 200
>> 2001:db8:104::/64 via 2001:db8:101::10 dev dummy1 proto kernel metric 1024 pref medium
>> 2001:db8:104::/64 via 2001:db8:101::10 dev dummy2 proto bgp metric 1024 pref medium
> 
> This seems wrong. The goal of 'ip route append' is to add a next hop, not to
> create a new route. Ok, it adds a new route if no route exists, but it seems
> wrong to me to use it by default, instead of 'add', to make things work magically.

Legacy API; nothing can be done about that (ie., that append makes a new
route when none exists).

> 
> It seems more correct to return an error in these cases, but this will change
> the uapi and it may break existing setups.
> 
> Before this patch, both next hops could be used by the kernel. After it, one
> route will be ignored (the former or the last one?). This is confusing and also
> seems wrong.

Append should match all details of a route to add to an existing entry
and make it multipath. If there is a difference (especially the type -
protocol difference is arguable) in attributes, then they are different
routes.